Bug 204151 – Automation: evaluateJavaScriptFunction should use Promises

• Component: WebKit2
• Status: RESOLVED FIXED

[Carlos Garcia Campos]

As documented in the spec. This is causing several test failures:

imported/w3c/webdriver/tests/execute_async_script/promise.py::test_promise_resolve
imported/w3c/webdriver/tests/execute_async_script/promise.py::test_promise_resolve_delayed
imported/w3c/webdriver/tests/execute_async_script/promise.py::test_promise_all_resolve
imported/w3c/webdriver/tests/execute_async_script/promise.py::test_await_promise_resolve
imported/w3c/webdriver/tests/execute_async_script/promise.py::test_promise_resolve_timeout
imported/w3c/webdriver/tests/execute_async_script/promise.py::test_promise_reject
imported/w3c/webdriver/tests/execute_async_script/promise.py::test_promise_reject_delayed
imported/w3c/webdriver/tests/execute_async_script/promise.py::test_promise_all_reject
imported/w3c/webdriver/tests/execute_async_script/promise.py::test_promise_reject_timeout

[Carlos Garcia Campos]

Created attachment 384893 [details]
Patch

[Carlos Garcia Campos]

Created attachment 386336 [details]
Rebased partch

It won't apply yet though, but it's rebased after r253883

[Blaze Burg]

Does this apply now?

[Carlos Garcia Campos]

(In reply to Brian Burg from comment #3)
> Does this apply now?

I don't think so, it depends on bug #204880. Even if this applied, it would make imported/w3c/webdriver/tests/back/user_prompts.py and imported/w3c/webdriver/tests/forward/user_prompts.py regress without the patch for bug #204880, so better fix that one first.

[Carlos Garcia Campos]

Created attachment 387089 [details]
Rebased patch

[Blaze Burg]

Comment on attachment 387089 [details]
Rebased patch

View in context: https://bugs.webkit.org/attachment.cgi?id=387089&action=review

r=me, nice work!

> Source/WebKit/WebProcess/Automation/WebAutomationSessionProxy.cpp:198
> +    } else if (JSValueIsObject(context, arguments[2])) {

I like this change.

> Source/WebKit/WebProcess/Automation/WebAutomationSessionProxy.js:102
> +        // finishes resolved, so we need to start a new one here to wait for the second promise to be reolved or the timeout.

Nit: resolved

So in other words, `resultPromise` could resolve to a Promise that is not settled (because we passed the `resolve` for the inner racing promise), so it has to be waited. It's a little tragic to have to do this, but I guess I can't think of an alternative if we can't assume anything about the callback's argument type.

[Carlos Garcia Campos]

Committed r254329: <https://trac.webkit.org/changeset/254329>

[Radar WebKit Bug Importer]

<rdar://problem/58472883>