Fix WaitForLoadState and add tests #628
Merged
Conversation
This file contains 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
inancgumus
approved these changes
Nov 3, 2022
ankur22
force-pushed
the
refactor/593-waitforloadstate
branch
from
2651bbf to
cfc2d95
Compare
ankur22
force-pushed
the
bug/622-waituntil-reload
branch
from
aa4e84c to
e05b8d6
Compare
ankur22
force-pushed
the
refactor/593-waitforloadstate
branch
from
cfc2d95 to
e8c762a
Compare
This test will ensure that when WaitForLoadState with waitUntil load is called, that it doesn't block on waiting for the lifecycle event if it has already fired, and so it uses the saved state of the frame to work out if the load lifecycle event has fired.
This test will ensure that when WaitForLoadState with waitUntil domcontentloaded is called, that it doesn't block for the lifecycle event if that event has already fired, and so using the frame's internal state to unblock quickly.
This test will ensure that when WaitForLoadState with waituntil networkidle is called, that it doesn't block for the lifecycle event if that event has already fired, and so uses the frame's internal state to unblock quickly.
This fixes an issue when we actually have to wait for a lifecycle event to be received from the browser since we've not received one yet and so we can't rely on the frame's state. The logic to wait for the lifecycle event has been copied from FrameManager.NavigateFrame.
This function is no longer being used. Also removing a todo comment which no longer is needed since these tests have been implemented in this PR.
ankur22
force-pushed
the
refactor/593-waitforloadstate
branch
from
e8c762a to
8573887
Compare
This was referenced Nov 8, 2022
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.
This PR was opened to help in part with issue #593.
This PR's aim is to do two things:
WaitForLoadStatewhen usingwaitUntiloptions.WaitForLoadStateneeds to wait for new events from the browser.#623 needs to be merged into
mainfirst before this PR.The tests all follow a similar setup to what is already present in
mainand in #623.The fix involved copying the
waitUntillogic used in FrameManager.NavigateFrame.