Draft
Conversation
This lock protects both current and pending document state. The old name suggested it only covered pending state.
There were repeated nil checks around document comparisons. A small matcher keeps those checks consistent.
Request lookup by document ID was scattered and easy to get wrong. Putting it behind one helper keeps that logic in one place.
Response lookup should follow the same path as request lookup. A dedicated helper keeps request/response access aligned.
The response could be read before network handling caught up. Waiting for lifecycle first removes that timing window.
WaitForNavigation had the same ordering problem as Goto. Applying the same ordering keeps both paths consistent.
During redirects, requests can land on current or pending documents. Rebinding by document ID keeps the association correct.
8 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What?
This PR fixes how navigation responses are resolved during Goto and WaitForNavigation.
Instead of reading the response directly from the navigation event (before network handling has caught up), it now waits for lifecycle events to complete, then looks up the response by document ID. It also fixes request binding during redirects, where a request could end up associated with the wrong document.
Why?
Improves stability, reduces the flaky behavior, and should reduce other flaky behavior in CI.
Before this, there was a timing window where the response was read as
nileven though the request had completed. This caused flaky results and was the reason WebVital tests had to be skipped on Windows.Note
This PR requires #5656 to be merged. Otherwise, it will fail.
Related PR(s)/Issue(s)
Depends on #5656
Closes #4371