Page.goto should properly handle historyAPI in beforeunload (#647)

* Page.goto should properly handle historyAPI in beforeunload

* Feature complete

* Code Factor

Author: kblok

lib/PuppeteerSharp.Tests/PageTests/GotoTests.cs

@@ -34,6 +34,18 @@ public async Task ShouldNavigateToEmptyPageWithDOMContentLoaded()
             Assert.Null(response.SecurityDetails);
         }
 
+        [Fact]
+        public async Task ShouldWorkWhenPageCallsHistoryAPIInBeforeunload()
+        {
+            await Page.GoToAsync(TestConstants.EmptyPage);
+            await Page.EvaluateFunctionAsync(@"() =>
+            {
+                window.addEventListener('beforeunload', () => history.replaceState(null, 'initial', window.location.href), false);
+            }");
+            var response = await Page.GoToAsync(TestConstants.ServerUrl + "/grid.html");
+            Assert.Equal(HttpStatusCode.OK, response.Status);
+        }
+
         [Fact]
         public async Task ShouldFailWhenServerReturns204()
         {

lib/PuppeteerSharp/Frame.cs

@@ -613,7 +613,7 @@ internal void Detach()
         {
             while (WaitTasks.Count > 0)
             {
-                WaitTasks[0].Termiante(new Exception("waitForFunction failed: frame got detached."));
+                WaitTasks[0].Terminate(new Exception("waitForFunction failed: frame got detached."));
             }
             Detached = true;
             if (ParentFrame != null)

lib/PuppeteerSharp/Messaging/PageNavigateResponse.cs

@@ -0,0 +1,14 @@
+using System;
+using Newtonsoft.Json;
+
+namespace PuppeteerSharp.Messaging
+{
+    internal class PageNavigateResponse
+    {
+        [JsonProperty("errorText")]
+        internal string ErrorText { get; set; }
+
+        [JsonProperty("loaderId")]
+        internal string LoaderId { get; set; }
+    }
+}

lib/PuppeteerSharp/NavigatorWatcher.cs

@@ -52,29 +52,21 @@ public NavigatorWatcher(FrameManager frameManager, Frame mainFrame, int timeout,
             frameManager.LifecycleEvent += CheckLifecycleComplete;
             frameManager.FrameNavigatedWithinDocument += NavigatedWithinDocument;
             frameManager.FrameDetached += CheckLifecycleComplete;
-            LifeCycleCompleteTaskWrapper = new TaskCompletionSource<bool>();
-
-            NavigationTask = Task.WhenAny(new[]
-            {
-                TaskHelper.CreateTimeoutTask(_timeout),
-                LifeCycleCompleteTask
-            }).ContinueWith((task) =>
-            {
-                CleanUp();
-                return task.GetAwaiter().GetResult();
-            });
+            SameDocumentNavigationTaskWrapper = new TaskCompletionSource<bool>();
+            NewDocumentNavigationTaskWrapper = new TaskCompletionSource<bool>();
+            TimeoutTask = TaskHelper.CreateTimeoutTask(timeout);
         }
 
         #region Properties
         public Task<Task> NavigationTask { get; internal set; }
-        public Task<bool> LifeCycleCompleteTask => LifeCycleCompleteTaskWrapper.Task;
-        public TaskCompletionSource<bool> LifeCycleCompleteTaskWrapper { get; }
+        public Task<bool> SameDocumentNavigationTask => SameDocumentNavigationTaskWrapper.Task;
+        public TaskCompletionSource<bool> SameDocumentNavigationTaskWrapper { get; }
+        public Task<bool> NewDocumentNavigationTask => NewDocumentNavigationTaskWrapper.Task;
+        public TaskCompletionSource<bool> NewDocumentNavigationTaskWrapper { get; }
+        public Task TimeoutTask { get; }
 
         #endregion
 
-        #region Public methods
-        public void Cancel() => CleanUp();
-        #endregion
         #region Private methods
 
         private void CheckLifecycleComplete(object sender, FrameEventArgs e)
@@ -88,8 +80,15 @@ private void CheckLifecycleComplete(object sender, FrameEventArgs e)
             {
                 return;
             }
-
-            LifeCycleCompleteTaskWrapper.TrySetResult(true);
+
+            if (_hasSameDocumentNavigation)
+            {
+                SameDocumentNavigationTaskWrapper.TrySetResult(true);
+            }
+            if (_frame.LoaderId != _initialLoaderId)
+            {
+                NewDocumentNavigationTaskWrapper.TrySetResult(true);
+            }
         }
 
         private void NavigatedWithinDocument(object sender, FrameEventArgs e)

lib/PuppeteerSharp/Page.cs

@@ -42,6 +42,7 @@ public class Page : IDisposable
         private readonly Dictionary<string, Delegate> _pageBindings;
         private readonly Dictionary<string, Worker> _workers;
         private readonly ILogger _logger;
+        private bool _ensureNewDocumentNavigation;
 
         private static readonly Dictionary<string, decimal> _unitToPixels = new Dictionary<string, decimal> {
             {"px", 1},
@@ -716,7 +717,7 @@ void createRequestEventListener(object sender, RequestEventArgs e)
             var navigateTask = Navigate(Client, url, referrer);
 
             await Task.WhenAny(
-                watcher.NavigationTask,
+                watcher.TimeoutTask,
                 navigateTask).ConfigureAwait(false);
 
             AggregateException exception = null;
@@ -725,21 +726,19 @@ await Task.WhenAny(
             {
                 exception = navigateTask.Exception;
             }
-            else if (watcher.NavigationTask.IsCompleted &&
-                watcher.NavigationTask.Result.IsFaulted)
+            else
             {
-                exception = watcher.NavigationTask.Result?.Exception;
-            }
+                await Task.WhenAny(
+                    watcher.TimeoutTask,
+                    _ensureNewDocumentNavigation ? watcher.NewDocumentNavigationTask : watcher.SameDocumentNavigationTask
+                ).ConfigureAwait(false);
 
-            if (exception == null)
-            {
-                await Task.WhenAll(
-                    watcher.NavigationTask,
-                    navigateTask).ConfigureAwait(false);
-                exception = navigateTask.Exception ?? watcher.NavigationTask.Result.Exception;
+                if (watcher.TimeoutTask.IsFaulted)
+                {
+                    exception = watcher.TimeoutTask.Exception;
+                }
             }
 
-            watcher.Cancel();
             _networkManager.Request -= createRequestEventListener;
 
             if (exception != null)
@@ -1408,11 +1407,15 @@ public async Task<Response> WaitForNavigationAsync(NavigationOptions options = n
 
             _networkManager.Response += createResponseEventListener;
 
-            await watcher.NavigationTask.ConfigureAwait(false);
+            var raceTask = await Task.WhenAny(
+                watcher.NewDocumentNavigationTask,
+                watcher.SameDocumentNavigationTask,
+                watcher.TimeoutTask
+            ).ConfigureAwait(false);
 
             _networkManager.Response -= createResponseEventListener;
 
-            var exception = watcher.NavigationTask.Exception;
+            var exception = raceTask.Exception;
             if (exception != null)
             {
                 throw new NavigationException(exception.Message, exception);
@@ -2025,15 +2028,17 @@ await Task.WhenAll(Frames.Select(frame => frame.EvaluateExpressionAsync(expressi
 
         private async Task Navigate(CDPSession client, string url, string referrer)
         {
-            dynamic response = await client.SendAsync("Page.navigate", new
+            var response = await client.SendAsync<PageNavigateResponse>("Page.navigate", new
             {
                 url,
                 referrer = referrer ?? string.Empty
             }).ConfigureAwait(false);
 
-            if (response.errorText != null)
+            _ensureNewDocumentNavigation = !string.IsNullOrEmpty(response.LoaderId);
+
+            if (!string.IsNullOrEmpty(response.ErrorText))
             {
-                throw new NavigationException(response.errorText.ToString(), url);
+                throw new NavigationException(response.ErrorText, url);
             }
         }
 

lib/PuppeteerSharp/WaitTask.cs

@@ -144,7 +144,7 @@ internal WaitTask(
             if (timeout > 0)
             {
                 _timeoutTimer = System.Threading.Tasks.Task.Delay(timeout, _cts.Token).ContinueWith(_
-                    => Termiante(new WaitTaskTimeoutException(timeout, title)));
+                    => Terminate(new WaitTaskTimeoutException(timeout, title)));
             }
 
             _taskCompletion = new TaskCompletionSource<JSHandle>();
@@ -213,7 +213,7 @@ await _frame.EvaluateFunctionAsync<bool>("s => !s", success)
             Cleanup();
         }
 
-        internal void Termiante(Exception exception)
+        internal void Terminate(Exception exception)
         {
             _terminated = true;
             _taskCompletion.TrySetException(exception);