diff --git a/src/Playwright.Tests/PageWaitForUrlTests.cs b/src/Playwright.Tests/PageWaitForUrlTests.cs index 9782e5180..1c2991e8c 100644 --- a/src/Playwright.Tests/PageWaitForUrlTests.cs +++ b/src/Playwright.Tests/PageWaitForUrlTests.cs @@ -153,4 +153,27 @@ await TaskUtils.WhenAll( frame.EvaluateAsync("url => window.location.href = url", Server.Prefix + "/grid.html")); ; } + + [PlaywrightTest] + public async Task ShouldNotMissNavigationCommittedRightAfterTheCall() + { + // https://github.com/microsoft/playwright-dotnet/issues/3364 + // Page A navigates to B a few milliseconds after it loads. The wait must see that + // navigation even when it commits right after WaitForURLAsync was called. + Server.SetRoute("/a.html", context => + { + context.Response.ContentType = "text/html"; + return context.Response.WriteAsync(""); + }); + Server.SetRoute("/b.html", context => + { + context.Response.ContentType = "text/html"; + return context.Response.WriteAsync("

b

"); + }); + for (int i = 0; i < 10; i++) + { + await Page.GotoAsync(Server.Prefix + "/a.html"); + await Page.WaitForURLAsync("**/b.html", new() { Timeout = 5000 }); + } + } } diff --git a/src/Playwright/Core/Frame.cs b/src/Playwright/Core/Frame.cs index b28b7e84b..b41928654 100644 --- a/src/Playwright/Core/Frame.cs +++ b/src/Playwright/Core/Frame.cs @@ -245,17 +245,22 @@ public async Task WaitForLoadStateAsync(LoadState? state = default, FrameWaitFor { waiter = SetupNavigationWaiter("frame.WaitForLoadStateAsync", options?.Timeout); + // Unlike the JS client, load states are updated on the transport thread, so subscribe + // before checking: an event raised between the check and the subscription would be lost + // (https://github.com/microsoft/playwright-dotnet/issues/3364). + var (loadStateTask, unsubscribe) = waiter.GetWaitForEventTask(this, "LoadState", s => + { + waiter.Log($" \"{s}\" event fired"); + return s == loadState; + }); if (_loadStates.Contains(loadState)) { + unsubscribe(); waiter.Log($" not waiting, \"{state}\" event already fired"); } else { - await waiter.WaitForEventAsync(this, "LoadState", s => - { - waiter.Log($" \"{s}\" event fired"); - return s == loadState; - }).ConfigureAwait(false); + await waiter.WaitForPromiseAsync(loadStateTask, unsubscribe).ConfigureAwait(false); } } finally @@ -297,14 +302,22 @@ await waiter.WaitForEventAsync(this, "LoadState", s => string? urlString, Func? urlFunc, Regex? urlRegex, - WaitUntilState? waitUntil) + WaitUntilState? waitUntil, + bool recheckUrl = false) { waitUntil ??= WaitUntilState.Load; string toUrl = !string.IsNullOrEmpty(urlString) ? $" to \"{urlString}\"" : string.Empty; waiter.Log($"waiting for navigation{toUrl} until \"{waitUntil}\""); - var navigatedEventTask = waiter.WaitForEventAsync( + var urlMatch = new URLMatch() + { + glob = urlString, + re = urlRegex, + func = urlFunc, + baseURL = (Page as Page)?.Context.BaseURL, + }; + var (navigatedEventTask, unsubscribe) = waiter.GetWaitForEventTask( this, "Navigated", e => @@ -316,36 +329,46 @@ await waiter.WaitForEventAsync(this, "LoadState", s => } waiter.Log($" navigated to \"{e.Url}\""); - return new URLMatch() - { - glob = urlString, - re = urlRegex, - func = urlFunc, - baseURL = (Page as Page)?.Context.BaseURL, - }.Match(e.Url); + return urlMatch.Match(e.Url); }); - var navigatedEvent = await navigatedEventTask.ConfigureAwait(false); - - if (navigatedEvent.Error != null) + FrameNavigatedEventArgs? navigatedEvent = null; + // Unlike the JS client, the frame navigates on the transport thread. WaitForURLAsync checked + // the URL before the subscription above existed, so check again now that a navigation can no + // longer go unnoticed (https://github.com/microsoft/playwright-dotnet/issues/3364). + if (recheckUrl && urlMatch.Match(Url)) { - var ex = new PlaywrightException(navigatedEvent.Error); - await waiter.WaitForPromiseAsync(Task.FromException(ex)).ConfigureAwait(false); + unsubscribe(); + } + else + { + navigatedEvent = await waiter.WaitForPromiseAsync(navigatedEventTask, unsubscribe).ConfigureAwait(false); + if (navigatedEvent.Error != null) + { + var ex = new PlaywrightException(navigatedEvent.Error); + await waiter.WaitForPromiseAsync(Task.FromException(ex)).ConfigureAwait(false); + } } - if (!_loadStates.Select(s => s.ToValueString()).Contains(waitUntil.Value.ToValueString())) + // Same as in WaitForLoadStateAsync: subscribe before checking the load state. + var (loadStateTask, unsubscribeLoadState) = waiter.GetWaitForEventTask( + this, + "LoadState", + e => + { + waiter.Log($" \"{e}\" event fired"); + return e.ToValueString() == waitUntil.Value.ToValueString(); + }); + if (_loadStates.Select(s => s.ToValueString()).Contains(waitUntil.Value.ToValueString())) { - await waiter.WaitForEventAsync( - this, - "LoadState", - e => - { - waiter.Log($" \"{e}\" event fired"); - return e.ToValueString() == waitUntil.Value.ToValueString(); - }).ConfigureAwait(false); + unsubscribeLoadState(); + } + else + { + await waiter.WaitForPromiseAsync(loadStateTask, unsubscribeLoadState).ConfigureAwait(false); } - var request = navigatedEvent.NewDocument?.Request; + var request = navigatedEvent?.NewDocument?.Request; return request != null ? await waiter.WaitForPromiseAsync(request.FinalRequest.ResponseAsync()).ConfigureAwait(false) @@ -1049,7 +1072,7 @@ await SendMessageToServerAsync( } } - private Task WaitForURLAsync(string? urlString, Regex? urlRegex, Func? urlFunc, FrameWaitForURLOptions? options = default) + private async Task WaitForURLAsync(string? urlString, Regex? urlRegex, Func? urlFunc, FrameWaitForURLOptions? options = default) { if (new URLMatch() { @@ -1059,18 +1082,15 @@ private Task WaitForURLAsync(string? urlString, Regex? urlRegex, Func