mxschmitt / playwright-go / 34021920463

78%

main: 78%

Build Type

Pull #641

github

Committed by myrjola

Commit Message

fix: subscribe before re-checking in WaitForLoadState and WaitForURL so a state recorded in between is not lost

Frame.WaitForLoadState read the frame's load states, then built its
waiter (a timer goroutine and four listener registrations), and only then
subscribed to "loadstate". Events arrive on the connection's single
dispatch goroutine, which can run in that gap; a "loadstate" it delivered
there was emitted to no listener and is never replayed, so the wait slept
out its whole timeout with the state it wanted already recorded.
WaitForURL had the same shape around "navigated": it checked the URL, then
called ExpectNavigation, which subscribes several registrations later.

Upstream's client/frame.ts waitForLoadState builds the waiter first, checks
_loadStates, and calls waiter.dispose() when the state is already there;
JavaScript's run-to-completion makes that check atomic with the
subscription. This brings the Go port to the same shape: attach the
listener, re-check the recorded state, and dispose of the waiter (new
waiter.dispose(), after upstream's) rather than wait for an event that has
already been delivered. onLoadState and onFrameNavigated both update their
state before emitting, so a re-check after subscribing cannot miss an
event that fired before it.

Measured with a browser (six pages clicking between two pages whose
stylesheet and script make `load` trail the commit by a few ms):
page.WaitForURL after a click timed out 6 times in 1,800 with the page
already at the wanted URL and `load` delivered 0.2-0.4 ms after the call;
with this change 0 in 1,800 under heavy load. The two new unit tests aim
the event at the gap with no browser and fail within a few rounds on the
previous code at any GOMAXPROCS (they raise it to 2 if needed).

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>

Pull Request Pull Request #641: fix: flaky navigation timeouts caused by a lost wakeup in WaitForLoadState and WaitForURL

Coverage Stats

39 of 46 new or added lines in 2 files covered. (84.78%)

15 existing lines in 9 files now uncovered.

7926 of 10152 relevant lines covered (78.07%)

14089.7 hits per line