UIScriptController: script with no async tasks fails if an earlier script registered a callback

• Component: Tools / Tests
• Status: RESOLVED FIXED

[Simon Fraser (smfr)]

If one UI script registers a callback:

        function getScrollDownUIScript(scrollX, scrollY)
        {
            return `(function() {
                uiController.didEndScrollingCallback = function() {
                    uiController.uiScriptComplete();
                };
                uiController.scrollToOffset(${scrollX}, ${scrollY}, function() {});
            })();`;
        }


then a later one just wants to return data:

        function getResultsUIScript()
        {
            return `(function() {
                var results = {
                    zoomScale : uiController.zoomScale,
                    visibleRect : uiController.contentVisibleRect,
                    scrollingTree : uiController.scrollingTreeAsText
                };

                return JSON.stringify(results);
            })();`;
        }

that later one never returns because UIScriptContext::runUIScript() thinks there's an outstanding task.

[Simon Fraser (smfr)]

Partial fix:

diff --git a/Tools/TestRunnerShared/UIScriptContext/UIScriptContext.cpp b/Tools/TestRunnerShared/UIScriptContext/UIScriptContext.cpp
index e2e7b0d1375c4def7e0592faf02c4d65fd9ed887..bb5c9e8becaea698f785a3e5bde20842e6549af7 100644
--- a/Tools/TestRunnerShared/UIScriptContext/UIScriptContext.cpp
+++ b/Tools/TestRunnerShared/UIScriptContext/UIScriptContext.cpp
@@ -117,6 +117,9 @@ unsigned UIScriptContext::registerCallback(JSValueRef taskCallback, CallbackType
     if (m_callbacks.contains(type))
         unregisterCallback(type);
 
+    if (JSValueIsUndefined(m_context.get(), taskCallback))
+        return 0;
+
     return prepareForAsyncTask(taskCallback, type);
 }

[Simon Fraser (smfr)]

Created attachment 294893 [details]
Patch

[Simon Fraser (smfr)]

https://trac.webkit.org/changeset/208770

[Ryan Haddad]

Reverted r208770 for reason:

The test added with this change is timing out on mac-wk1.

Committed r208795: <http://trac.webkit.org/changeset/208795>

[Ryan Haddad]

(In reply to comment #4)
> Reverted r208770 for reason:
> 
> The test added with this change is timing out on mac-wk1.
> 
> Committed r208795: <http://trac.webkit.org/changeset/208795>

The test added with this change was timing out on mac-wk1 and causing false positives for other patches. It also looks like the original change was landed before EWS had a chance to complete testing in the first place.

[Simon Fraser (smfr)]

The test should be skipped on mac WK1. I'll fix and re-land.

[Simon Fraser (smfr)]

Created attachment 294961 [details]
Patch

[WebKit Commit Bot]

Comment on attachment 294961 [details]
Patch

Clearing flags on attachment: 294961

Committed r208816: <http://trac.webkit.org/changeset/208816>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.