diff --git a/src/Playwright.Tests/PageWaitForUrlTests.cs b/src/Playwright.Tests/PageWaitForUrlTests.cs
index 9782e5180..f958fda93 100644
--- a/src/Playwright.Tests/PageWaitForUrlTests.cs
+++ b/src/Playwright.Tests/PageWaitForUrlTests.cs
@@ -153,4 +153,43 @@ 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 });
+ }
+ }
+
+ [PlaywrightTest]
+ public async Task ShouldNotHoldTheDispatchLockWhileMatchingTheUrl()
+ {
+ // The predicate is user code, so it must run with no client lock held. This one blocks
+ // on a call whose reply follows a frameAttached message, which the message pump
+ // handles under the dispatch lock.
+ await Page.GotoAsync(Server.EmptyPage);
+ var completed = false;
+ await Page.WaitForURLAsync(url =>
+ {
+ completed = Page.EvaluateAsync("() => { document.body.appendChild(document.createElement('iframe')); }").Wait(5000);
+ return true;
+ });
+ Assert.True(completed);
+ }
}
diff --git a/src/Playwright/Core/Frame.cs b/src/Playwright/Core/Frame.cs
index bdfbc3f60..09d87419e 100644
--- a/src/Playwright/Core/Frame.cs
+++ b/src/Playwright/Core/Frame.cs
@@ -33,6 +33,7 @@
using System.Text.Json;
using System.Text.Json.Serialization;
using System.Text.RegularExpressions;
+using System.Threading;
using System.Threading.Tasks;
using Microsoft.Playwright.Helpers;
using Microsoft.Playwright.Transport;
@@ -42,7 +43,14 @@ namespace Microsoft.Playwright.Core;
internal class Frame : ChannelOwner, IFrame
{
- private readonly List _loadStates = new();
+ // One slot per WaitUntilState. A completed task means the state is present, a pending one
+ // that it is not, and waiters await the current slot, so there is no check to interleave
+ // with a subscription. Only a completed slot is replaced on remove, so a waiter started
+ // before a navigation resolves when the new document reaches the state.
+ private readonly TaskCompletionSource[] _loadStateSlots = new TaskCompletionSource[Enum.GetValues(typeof(WaitUntilState)).Length];
+ // Incremented with every Url update, under the dispatch lock. WaitForURLAsync uses it to
+ // tell whether Url changed while it ran the matcher outside the lock.
+ private int _navigationCount;
internal readonly List _childFrames = new();
internal Frame(ChannelOwner parent, string guid, FrameInitializer initializer) : base(parent, guid)
@@ -50,7 +58,14 @@ internal Frame(ChannelOwner parent, string guid, FrameInitializer initializer) :
Url = initializer.Url;
Name = initializer.Name;
ParentFrame = initializer.ParentFrame;
- _loadStates = initializer.LoadStates;
+ for (var i = 0; i < _loadStateSlots.Length; i++)
+ {
+ _loadStateSlots[i] = NewLoadStateSlot();
+ }
+ foreach (var state in initializer.LoadStates)
+ {
+ _loadStateSlots[(int)state].TrySetResult(true);
+ }
Webmcp = new WebMCP(this);
}
@@ -114,13 +129,13 @@ internal void OnLoadState(WaitUntilState? add, WaitUntilState? remove)
{
if (add.HasValue)
{
- _loadStates.Add(add.Value);
+ _loadStateSlots[(int)add.Value].TrySetResult(true);
LoadState?.Invoke(this, add.Value);
}
- if (remove.HasValue)
+ if (remove.HasValue && _loadStateSlots[(int)remove.Value].Task.IsCompleted)
{
- _loadStates.Remove(remove.Value);
+ Volatile.Write(ref _loadStateSlots[(int)remove.Value], NewLoadStateSlot());
}
if (this.ParentFrame == null && add == WaitUntilState.Load && this.Page != null)
{
@@ -132,11 +147,25 @@ internal void OnLoadState(WaitUntilState? add, WaitUntilState? remove)
}
}
+ private static TaskCompletionSource NewLoadStateSlot()
+ => new(TaskCreationOptions.RunContinuationsAsynchronously);
+
+ private TaskCompletionSource LoadStateSlot(WaitUntilState state)
+ => Volatile.Read(ref _loadStateSlots[(int)state]);
+
internal void OnNavigated(FrameNavigatedEventArgs e)
{
- Url = e.Url;
- Name = e.Name;
- Navigated?.Invoke(this, e);
+ EventHandler? navigated;
+ lock (_connection.DispatchLock)
+ {
+ // WaitForURLAsync subscribes to Navigated under the same lock, after checking that
+ // the count has not moved, so a waiter either matched this Url or is in the snapshot.
+ Url = e.Url;
+ Name = e.Name;
+ _navigationCount++;
+ navigated = Navigated;
+ }
+ navigated?.Invoke(this, e);
if (string.IsNullOrEmpty(e.Error))
{
@@ -245,17 +274,15 @@ public async Task WaitForLoadStateAsync(LoadState? state = default, FrameWaitFor
{
waiter = SetupNavigationWaiter("frame.WaitForLoadStateAsync", options?.Timeout);
- if (_loadStates.Contains(loadState))
+ var loadStateTask = LoadStateSlot(loadState).Task;
+ if (loadStateTask.IsCompleted)
{
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).ConfigureAwait(false);
+ waiter.Log($" \"{state}\" event fired");
}
}
finally
@@ -333,16 +360,11 @@ await waiter.WaitForEventAsync(this, "LoadState", s =>
await waiter.WaitForPromiseAsync(Task.FromException