Web Automation: evaluateJavaScriptFunction should start the callback timeout after the function is applied

• Component: WebKit2
• Status: RESOLVED FIXED

[Carlos Garcia Campos]

This is causing selenium test testShouldNotTimeoutIfScriptCallsbackInsideAZeroTimeout to fail, because JavaScriptTimeout error is generated unexpectedly. When no script timeout is specified, 0 is used by default, which means we do a setTimeout with 0 and then the script does another setTimeout with 0, but ours is dispatched before and reportTimeoutError is called. We should start our timeout after applying the function, and only if the result hasn't been reported yet.

FAIL test/selenium/webdriver/common/executing_async_javascript_tests.py::testShouldNotTimeoutIfScriptCallsbackInsideAZeroTimeout[WebKitGTK]

========================================================================================== FAILURES ==========================================================================================
_____________________________________________________________ testShouldNotTimeoutIfScriptCallsbackInsideAZeroTimeout[WebKitGTK] _____________________________________________________________

driver = <selenium.webdriver.webkitgtk.webdriver.WebDriver (session="0ebfd066-81e2-4b84-9012-23eced4eed4c")>, pages = <conftest.Pages object at 0x7f5b9e19bfd0>

    def testShouldNotTimeoutIfScriptCallsbackInsideAZeroTimeout(driver, pages):
        pages.load("ajaxy_page.html")
        driver.execute_async_script(
            """var callback = arguments[arguments.length - 1];
>           window.setTimeout(function() { callback(123); }, 0)""")

E       TimeoutException: Message:

selenium/webdriver/remote/errorhandler.py:193: TimeoutException

[Carlos Garcia Campos]

Created attachment 315235 [details]
Patch

[BJ Burg]

Comment on attachment 315235 [details]
Patch

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

r=me

For some reason, this test passes in safaridriver without the patch. I agree that this is a valid fix though.

> Source/WebKit2/WebProcess/Automation/WebAutomationSessionProxy.js:54
> +        let reportResult = (result) => { clearTimeout(timeoutIdentifier); resultCallback(frameID, callbackID, this._jsonStringify(result), false); resultReported = true; };

This is getting unwieldy, can you add newlines to the function?

The clearTimeout won't do anything if the function being evaluated calls the callback/completion handler synchronously. I guess that's okay.

[Carlos Garcia Campos]

(In reply to Brian Burg from comment #2)
> Comment on attachment 315235 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=315235&action=review
> 
> r=me
> 
> For some reason, this test passes in safaridriver without the patch. I agree
> that this is a valid fix though.

Could it be that in mac Timers implementation two timers scheduled in the same run loop cycle with the same time interval (0) could be dispatched in any order?

> > Source/WebKit2/WebProcess/Automation/WebAutomationSessionProxy.js:54
> > +        let reportResult = (result) => { clearTimeout(timeoutIdentifier); resultCallback(frameID, callbackID, this._jsonStringify(result), false); resultReported = true; };
> 
> This is getting unwieldy, can you add newlines to the function?

Sure.

> The clearTimeout won't do anything if the function being evaluated calls the
> callback/completion handler synchronously. I guess that's okay.

I'm assuming clearTimeout(0) does nothing, but maybe it's better to simply check it before calling it.

[Carlos Garcia Campos]

Committed r219442: <http://trac.webkit.org/changeset/219442>

[BJ Burg]

(In reply to Carlos Garcia Campos from comment #3)
> (In reply to Brian Burg from comment #2)
> > Comment on attachment 315235 [details]
> > Patch
> > 
> > View in context:
> > https://bugs.webkit.org/attachment.cgi?id=315235&action=review
> > 
> > r=me
> > 
> > For some reason, this test passes in safaridriver without the patch. I agree
> > that this is a valid fix though.
> 
> Could it be that in mac Timers implementation two timers scheduled in the
> same run loop cycle with the same time interval (0) could be dispatched in
> any order?

No, DOMTimers are scheduled FIFO.