Skip to content

webdriver: Allow Promise for synchronous script execution#41823

Merged
jdm merged 4 commits intoservo:mainfrom
yezhizhen:execute-script-improvement
Jan 10, 2026
Merged

webdriver: Allow Promise for synchronous script execution#41823
jdm merged 4 commits intoservo:mainfrom
yezhizhen:execute-script-improvement

Conversation

@yezhizhen
Copy link
Copy Markdown
Member

@yezhizhen yezhizhen commented Jan 10, 2026 •
edited
Loading

When a promise is given, it must be honoured.

Testing: /classic/execute_script/promise.py fully pass.

@yezhizhen yezhizhen requested a review from xiaochengh January 10, 2026 13:13
@servo-highfive servo-highfive added the S-awaiting-review There is new code that needs to be reviewed. label Jan 10, 2026
@yezhizhen
A promise given must be honoured
cb6972d 
Signed-off-by: Euclid Ye <yezhizhenjiakang@gmail.com>
@yezhizhen
Update wpt
b336add 
Signed-off-by: Euclid Ye <yezhizhenjiakang@gmail.com>
@yezhizhen yezhizhen force-pushed the execute-script-improvement branch from d2122d4 to b336add Compare January 10, 2026 13:14
@yezhizhen yezhizhen added the T-webdriver Do a try run of the WebDriver conformance tests label Jan 10, 2026
@github-actions github-actions bot removed the T-webdriver Do a try run of the WebDriver conformance tests label Jan 10, 2026
@github-actions
Copy link
Copy Markdown

github-actions bot commented Jan 10, 2026

🔨 Triggering try run (#20878861644) for Linux (WPT)

@github-actions
Copy link
Copy Markdown

github-actions bot commented Jan 10, 2026

Test results for linux-wpt from try job (#20878861644):

Flaky unexpected result (1)
  • OK /webdriver/tests/classic/perform_actions/wheel.py
    • ERROR [expected PASS] subtest: test_null_response_value

      teardown error:

    • ERROR [expected PASS] subtest: test_no_top_browsing_context

      setup error: ConnectionRefusedError: [Errno 111] Connection refused

    • ERROR [expected PASS] subtest: test_no_browsing_context

      setup error: ConnectionRefusedError: [Errno 111] Connection refused

    • ERROR [expected PASS] subtest: test_params_actions_origin_outside_viewport[element]

      setup error: ConnectionRefusedError: [Errno 111] Connection refused

    • ERROR [expected PASS] subtest: test_params_actions_origin_outside_viewport[viewport]

      setup error: ConnectionRefusedError: [Errno 111] Connection refused

    • ERROR [expected PASS] subtest: test_scroll_not_scrollable

      setup error: ConnectionRefusedError: [Errno 111] Connection refused

    • ERROR [expected PASS] subtest: test_scroll_scrollable_overflow

      setup error: ConnectionRefusedError: [Errno 111] Connection refused

    • ERROR [expected PASS] subtest: test_scroll_iframe

      setup error: ConnectionRefusedError: [Errno 111] Connection refused

    • ERROR [expected PASS] subtest: test_scroll_shadow_tree[outer-open]

      setup error: ConnectionRefusedError: [Errno 111] Connection refused

    • ERROR [expected PASS] subtest: test_scroll_shadow_tree[outer-closed]

      setup error: ConnectionRefusedError: [Errno 111] Connection refused

    • And 3 more unexpected results...

@github-actions
Copy link
Copy Markdown

github-actions bot commented Jan 10, 2026

✨ Try run (#20878861644) succeeded.

jdm
jdm reviewed Jan 10, 2026
View reviewed changes
components/webdriver_server/lib.rs Outdated
func_body,
r#"(async function() {{
try {{
let result = (function() {{
Copy link
Copy Markdown
Member

@jdm jdm Jan 10, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should this also be async? I notice one last await test is still failing.

Copy link
Copy Markdown
Member Author

@yezhizhen yezhizhen Jan 10, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeap!

Copy link
Copy Markdown
Member Author

@yezhizhen yezhizhen Jan 10, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more. 

pid:5072 [2026-01-10T12:55:58Z ERROR script::dom::bindings::error] Error at :5:19 await is only valid in async functions, async generators and modules

@yezhizhen yezhizhen marked this pull request as draft January 10, 2026 14:28
@yezhizhen
corner case
b9e27ae 
Signed-off-by: Euclid Ye <yezhizhenjiakang@gmail.com>
@yezhizhen
Fully passing
6de5e45 
Signed-off-by: Euclid Ye <yezhizhenjiakang@gmail.com>
@yezhizhen yezhizhen marked this pull request as ready for review January 10, 2026 14:30
@yezhizhen yezhizhen requested a review from jdm January 10, 2026 14:30
jdm
jdm approved these changes Jan 10, 2026
View reviewed changes
Copy link
Copy Markdown
Member

@jdm jdm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice!

@servo-highfive servo-highfive removed the S-awaiting-review There is new code that needs to be reviewed. label Jan 10, 2026
@jdm jdm added this pull request to the merge queue Jan 10, 2026
@servo-highfive servo-highfive added the S-awaiting-merge The PR is in the process of compiling and running tests on the automated CI. label Jan 10, 2026
Merged via the queue into servo:main with commit 0266a28 Jan 10, 2026
30 checks passed
@servo-highfive servo-highfive removed the S-awaiting-merge The PR is in the process of compiling and running tests on the automated CI. label Jan 10, 2026
@yezhizhen yezhizhen deleted the execute-script-improvement branch January 11, 2026 02:48
@yezhizhen yezhizhen mentioned this pull request Jan 11, 2026
Merged
github-merge-queue bot pushed a commit that referenced this pull request Jan 11, 2026
@yezhizhen
webdriver: Consolidates synchronous and asynchrounous script execution (
014fb96 
#41830)

Follow-up to #41823.
Now that we always execute JS with callback, we can safely remove the
obsolete sync script handler.

- Rename `ExecuteAsyncScript` to `ExecuteScriptWithCallback`
- Remove `ExecuteScript` related logic.
- Rewrite async callback wrapper to simplify things.

Testing: I expect no change in behaviour. See if WPT tests agree with
it.

---------

Signed-off-by: Euclid Ye <yezhizhenjiakang@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@jdm jdm jdm approved these changes

@xiaochengh xiaochengh Awaiting requested review from xiaochengh

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@yezhizhen @jdm @servo-highfive