Cherry-pick 5.0625 webdriver-related ladybird PRs #25389
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
github-actions
bot
added
the
👀 pr-needs-review
(cherry picked from commit b20a1d0fcddb999de372a49d213bec01bda24c10)
(cherry picked from commit fbff0ca6a88ba52ad8a1f7ad89705ef12570af66; amended to change a `page_load_timeout_fired || page_load_timeout_fired || ...` that gcc's -Wlogical-op complainted about to `page_load_timeout_fired || ...`, see also discussion on LadybirdBrowser/ladybird#1021)
(cherry picked from commit 1b2f35c3affc4b2779c13452c2b1ec38bbf144b9)
The timers added previously in SerenityOS#1021 had no effect, as they were never started. (cherry picked from commit 5b09430c5efeb839242fb9db30f777cc446957d9)
Previously, the conversion assumed that the supplied timeout was in seconds rather than milliseconds. (cherry picked from commit b688b5d9d4cfd8f781dd05d522d9533cd6796aca)
This change updates `ExecuteScript::execute_script()` and `ExecuteScript::execute_script()` to bring their behavior in line with each other and the current specification text. Instances of the variable `timeout` have also been renamed to `timeout_ms`, for clarity. (cherry picked from commit 107549dc86eb0a02bb6468ccfa03475114474187)
Spinning the HTML event loop allows microtasks to run (i.e. Promise completions). (cherry picked from commit 6f31a19c5f0b7bcae89037ceb65c474b20482ca2)
We currently spin the platform event loop while awaiting scripts to complete. This causes WebContent to hang if another component is also spinning the event loop. The particular example that instigated this patch was the navigable's navigation loop (which spins until the fetch process is complete), triggered by a form submission to an iframe. So instead of spinning, we now return immediately from the script executors, after setting up listeners for either the script's promise to be resolved or for a timeout. The HTTP request to WebDriver must finish synchronously though, so now the WebDriver process spins its event loop until WebContent signals that the script completed. This should be ok - the WebDriver process isn't expected to be doing anything else in the meantime. Also, as a consequence of these changes, we now actually handle time outs. We were previously creating the timeout timer, but not starting it. (cherry picked from commit c2cf65adac78912883996153fb608dafe389b6e0)
github-actions
bot
removed
the
👀 pr-needs-review
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.
LadybirdBrowser/ladybird#676 7th commit
LadybirdBrowser/ladybird#1021
LadybirdBrowser/ladybird#1030
LadybirdBrowser/ladybird#1209
LadybirdBrowser/ladybird#1234
LadybirdBrowser/ladybird#1389
Not quite sure about if we want LadybirdBrowser/ladybird#676. At least while we're cherry-picking, it helps reduce conflicts, so I'll pick the ones that do help – in this case, the 7th commit. Maybe we'll actually want all of it. And if not, it's easy to undo once we no longer cherry-pick.