UI-side scripts in WebKitTestRunner should wait until event handling …

…completes before finishing

https://bugs.webkit.org/show_bug.cgi?id=151101
<rdar://problem/23428601>

Reviewed by Simon Fraser.

Tools:

WebKitTestRunner may still crash in the scenario where a marker event is dequeued and handled
after uiScriptComplete has been called. This patch teaches the UI script execution context to
defer script completion until all non-persistent tasks (currently tap, double tap and hardware
keyboard) have been handled, so marker events will no longer bleed through tests.

We accomplish this by changing the behavior of uiScriptComplete. When calling uiScriptComplete,
we store that a request to complete the UI-side script for the current parent callback has been
made. Subsequently, when a callback (either persistent or non-persistent) finishes invoking and
a request has been made to complete the UI script (this request may have been made when running
a previous callback) we check if all the non-persistent callbacks that have the same parent
callback as the current one have finished. If so, we complete the callback immediately;
otherwise, we wait until the in-flight non-persistent callbacks finish execution to complete the
UI script.

This patch also refactors some logic in UIScriptContext. It introduces a new convention for
assigning IDs to callbacks: IDs 1000 and above are treated as non-persistent callbacks, whereas
IDs between 0 and 999 are persistent task callbacks. This is similar to the existing convention
for assigning IDs in the 100s range to parent callbacks, and allows us to easily differentiate
between callbacks that are persistent and non-persistent, as well as determine when an existing
persistent callback must be unregistered before a new callback function can be set.

* WebKitTestRunner/UIScriptContext/UIScriptContext.cpp:
(isPersistentCallbackID):
(UIScriptContext::runUIScript):
(UIScriptContext::nextTaskCallbackID):
(UIScriptContext::prepareForAsyncTask):
(UIScriptContext::asyncTaskComplete):
(UIScriptContext::registerCallback):
(UIScriptContext::fireCallback):
(UIScriptContext::requestUIScriptCompletion):
(UIScriptContext::tryToCompleteUIScriptForCurrentParentCallback):
(UIScriptContext::currentParentCallbackHasOutstandingAsyncTasks):
(UIScriptContext::uiScriptComplete): Deleted.
* WebKitTestRunner/UIScriptContext/UIScriptContext.h:
(WTR::UIScriptContext::currentParentCallbackIsPendingCompletion):
* WebKitTestRunner/UIScriptContext/UIScriptController.cpp:
(WTR::UIScriptController::setWillBeginZoomingCallback):
(WTR::UIScriptController::willBeginZoomingCallback):
(WTR::UIScriptController::setDidEndZoomingCallback):
(WTR::UIScriptController::didEndZoomingCallback):
(WTR::UIScriptController::setDidShowKeyboardCallback):
(WTR::UIScriptController::didShowKeyboardCallback):
(WTR::UIScriptController::setDidHideKeyboardCallback):
(WTR::UIScriptController::didHideKeyboardCallback):
(WTR::UIScriptController::uiScriptComplete):
* WebKitTestRunner/UIScriptContext/UIScriptController.h:
* WebKitTestRunner/ios/UIScriptControllerIOS.mm:
(WTR::UIScriptController::doAsyncTask):
(WTR::UIScriptController::zoomToScale):
(WTR::UIScriptController::singleTapAtPoint):
(WTR::UIScriptController::doubleTapAtPoint):
(WTR::UIScriptController::typeCharacterUsingHardwareKeyboard):
(WTR::UIScriptController::platformSetWillBeginZoomingCallback):
(WTR::UIScriptController::platformSetDidEndZoomingCallback):
(WTR::UIScriptController::platformSetDidShowKeyboardCallback):
(WTR::UIScriptController::platformSetDidHideKeyboardCallback):
(WTR::UIScriptController::platformClearAllCallbacks): Deleted.
* WebKitTestRunner/mac/UIScriptControllerMac.mm:
(WTR::UIScriptController::doAsyncTask):

LayoutTests:

Ensures that all tests using UIScriptController properly complete the UI script from within
a completion callback.

* fast/events/ios/clicking-document-should-not-trigger-focus.html:
* fast/events/ios/input-value-after-oninput.html:
* fast/events/ios/single-tap-generates-click.html:

Canonical link: https://commits.webkit.org/169333@main
git-svn-id: https://svn.webkit.org/repository/webkit/trunk@192314 268f45cc-cd09-0410-ab3c-d52691b4dbfc

LayoutTests/ChangeLog

@@ -1,3 +1,18 @@
+2015-11-10  Wenson Hsieh  <wenson_hsieh@apple.com>
+
+        UI-side scripts in WebKitTestRunner should wait until event handling completes before finishing
+        https://bugs.webkit.org/show_bug.cgi?id=151101
+        <rdar://problem/23428601>
+
+        Reviewed by Simon Fraser.
+
+        Ensures that all tests using UIScriptController properly complete the UI script from within
+        a completion callback.
+
+        * fast/events/ios/clicking-document-should-not-trigger-focus.html:
+        * fast/events/ios/input-value-after-oninput.html:
+        * fast/events/ios/single-tap-generates-click.html:
+
 2015-11-10  Brady Eidson  <beidson@apple.com>
 
         Modern IDB: Make indexes actually index.

LayoutTests/fast/events/ios/clicking-document-should-not-trigger-focus.html

@@ -5,10 +5,14 @@
     <head>
         <script id="ui-script" type="text/plain">
             (function() {
-                uiController.singleTapAtPoint(100, 300);
+                uiController.singleTapAtPoint(100, 300, function() {
+                    uiController.uiScriptComplete("");
+                });
             })();
         </script>
         <script>
+            var uiScriptHasCompleted = false;
+            var clickHasBeenHandled = false;
             if (window.testRunner) {
                 testRunner.dumpAsText();
                 testRunner.waitUntilDone();
@@ -24,11 +28,17 @@
                 document.addEventListener("click", function() {
                     document.body.appendChild(document.createTextNode("The currently focused element is of type " + document.activeElement.tagName));
                     document.body.appendChild(document.createElement("br"));
-                    if (window.testRunner)
+                    clickHasBeenHandled = true;
+                    if (window.testRunner && uiScriptHasCompleted)
                         testRunner.notifyDone();
                 });
-                if (window.testRunner && testRunner.runUIScript)
-                    testRunner.runUIScript(getUIScript(), function() { });
+                if (window.testRunner && testRunner.runUIScript) {
+                    testRunner.runUIScript(getUIScript(), function() {
+                        uiScriptHasCompleted = true;
+                        if (clickHasBeenHandled)
+                            testRunner.notifyDone();
+                    });
+                }
             }
         </script>
     </head>

LayoutTests/fast/events/ios/input-value-after-oninput.html

@@ -4,15 +4,18 @@
     <meta name="viewport" content="initial-scale=1.0">
     <script id="ui-script" type="text/plain">
         (function() {
-            uiController.singleTapAtPoint(50, 25, function() {
-                uiController.didShowKeyboardCallback = function() {
-                    uiController.typeCharacterUsingHardwareKeyboard("a", function() { });
-                }
-            });
+            uiController.didShowKeyboardCallback = function() {
+                uiController.typeCharacterUsingHardwareKeyboard("a", function() {
+                    uiController.uiScriptComplete();
+                });
+            }
+            uiController.singleTapAtPoint(50, 25, function() {});
         })();
     </script>
 
     <script>
+        var uiScriptHasCompleted = false;
+        var oninputHasBeenHandled = false;
         if (window.testRunner) {
             testRunner.dumpAsText();
             testRunner.waitUntilDone();
@@ -25,15 +28,21 @@
 
         function handleValueChanged(value) {
             document.getElementById("console").textContent = "Successfully handled oninput, value is now \"" + value.toLowerCase() + "\"";
-            testRunner.notifyDone();
+            oninputHasBeenHandled = true;
+            if (uiScriptHasCompleted)
+                testRunner.notifyDone();
         }
 
         function runTest()
         {
             if (!window.testRunner || !testRunner.runUIScript)
                 return;
 
-            testRunner.runUIScript(getUIScript(), function(result) { });
+            testRunner.runUIScript(getUIScript(), function() {
+                uiScriptHasCompleted = true;
+                if (oninputHasBeenHandled)
+                    testRunner.notifyDone();
+            });
         }
     </script>
 </head>

LayoutTests/fast/events/ios/single-tap-generates-click.html

@@ -5,10 +5,14 @@
     <meta name="viewport" content="width=device-width">
     <script id="ui-script" type="text/plain">
         (function() {
-            uiController.singleTapAtPoint(50, 50);
+            uiController.singleTapAtPoint(50, 50, function() {
+                uiController.uiScriptComplete();
+            });
         })();
     </script>
     <script>
+        var uiScriptHasCompleted = false;
+        var boxHasBeenClicked = false;
         if (window.testRunner) {
             testRunner.dumpAsText();
             testRunner.waitUntilDone();
@@ -23,15 +27,18 @@
         {
             if (testRunner.runUIScript) {
                 testRunner.runUIScript(getUIScript(), function() {
-                    // Just wait for the tap.
+                    uiScriptHasCompleted = true;
+                    if (boxHasBeenClicked)
+                        testRunner.notifyDone();
                 });
             }
         }
 
         function boxClicked(event)
         {
             document.getElementById('target').textContent = 'PASS: received click event at ' + event.clientX + ' ' + event.clientY;
-            if (window.testRunner)
+            boxHasBeenClicked = true;
+            if (uiScriptHasCompleted && window.testRunner)
                 testRunner.notifyDone();
         }
     </script>

Tools/ChangeLog

@@ -1,3 +1,71 @@
+2015-11-10  Wenson Hsieh  <wenson_hsieh@apple.com>
+
+        UI-side scripts in WebKitTestRunner should wait until event handling completes before finishing
+        https://bugs.webkit.org/show_bug.cgi?id=151101
+        <rdar://problem/23428601>
+
+        Reviewed by Simon Fraser.
+
+        WebKitTestRunner may still crash in the scenario where a marker event is dequeued and handled
+        after uiScriptComplete has been called. This patch teaches the UI script execution context to
+        defer script completion until all non-persistent tasks (currently tap, double tap and hardware
+        keyboard) have been handled, so marker events will no longer bleed through tests.
+
+        We accomplish this by changing the behavior of uiScriptComplete. When calling uiScriptComplete,
+        we store that a request to complete the UI-side script for the current parent callback has been
+        made. Subsequently, when a callback (either persistent or non-persistent) finishes invoking and
+        a request has been made to complete the UI script (this request may have been made when running
+        a previous callback) we check if all the non-persistent callbacks that have the same parent
+        callback as the current one have finished. If so, we complete the callback immediately;
+        otherwise, we wait until the in-flight non-persistent callbacks finish execution to complete the
+        UI script.
+
+        This patch also refactors some logic in UIScriptContext. It introduces a new convention for
+        assigning IDs to callbacks: IDs 1000 and above are treated as non-persistent callbacks, whereas
+        IDs between 0 and 999 are persistent task callbacks. This is similar to the existing convention
+        for assigning IDs in the 100s range to parent callbacks, and allows us to easily differentiate
+        between callbacks that are persistent and non-persistent, as well as determine when an existing
+        persistent callback must be unregistered before a new callback function can be set.
+
+        * WebKitTestRunner/UIScriptContext/UIScriptContext.cpp:
+        (isPersistentCallbackID):
+        (UIScriptContext::runUIScript):
+        (UIScriptContext::nextTaskCallbackID):
+        (UIScriptContext::prepareForAsyncTask):
+        (UIScriptContext::asyncTaskComplete):
+        (UIScriptContext::registerCallback):
+        (UIScriptContext::fireCallback):
+        (UIScriptContext::requestUIScriptCompletion):
+        (UIScriptContext::tryToCompleteUIScriptForCurrentParentCallback):
+        (UIScriptContext::currentParentCallbackHasOutstandingAsyncTasks):
+        (UIScriptContext::uiScriptComplete): Deleted.
+        * WebKitTestRunner/UIScriptContext/UIScriptContext.h:
+        (WTR::UIScriptContext::currentParentCallbackIsPendingCompletion):
+        * WebKitTestRunner/UIScriptContext/UIScriptController.cpp:
+        (WTR::UIScriptController::setWillBeginZoomingCallback):
+        (WTR::UIScriptController::willBeginZoomingCallback):
+        (WTR::UIScriptController::setDidEndZoomingCallback):
+        (WTR::UIScriptController::didEndZoomingCallback):
+        (WTR::UIScriptController::setDidShowKeyboardCallback):
+        (WTR::UIScriptController::didShowKeyboardCallback):
+        (WTR::UIScriptController::setDidHideKeyboardCallback):
+        (WTR::UIScriptController::didHideKeyboardCallback):
+        (WTR::UIScriptController::uiScriptComplete):
+        * WebKitTestRunner/UIScriptContext/UIScriptController.h:
+        * WebKitTestRunner/ios/UIScriptControllerIOS.mm:
+        (WTR::UIScriptController::doAsyncTask):
+        (WTR::UIScriptController::zoomToScale):
+        (WTR::UIScriptController::singleTapAtPoint):
+        (WTR::UIScriptController::doubleTapAtPoint):
+        (WTR::UIScriptController::typeCharacterUsingHardwareKeyboard):
+        (WTR::UIScriptController::platformSetWillBeginZoomingCallback):
+        (WTR::UIScriptController::platformSetDidEndZoomingCallback):
+        (WTR::UIScriptController::platformSetDidShowKeyboardCallback):
+        (WTR::UIScriptController::platformSetDidHideKeyboardCallback):
+        (WTR::UIScriptController::platformClearAllCallbacks): Deleted.
+        * WebKitTestRunner/mac/UIScriptControllerMac.mm:
+        (WTR::UIScriptController::doAsyncTask):
+
 2015-11-10  Daniel Bates  <dabates@apple.com>
 
         Teach Makefile to build LayoutTestRelay when building for iOS Simulator

Tools/WebKitTestRunner/UIScriptContext/UIScriptContext.cpp

@@ -36,6 +36,11 @@
 
 using namespace WTR;
 
+static inline bool isPersistentCallbackID(unsigned callbackID)
+{
+    return callbackID < firstNonPersistentCallbackID;
+}
+
 UIScriptContext::UIScriptContext(UIScriptContextDelegate& delegate)
     : m_context(Adopt, JSGlobalContextCreate(nullptr))
     , m_delegate(delegate)
@@ -59,20 +64,22 @@ void UIScriptContext::runUIScript(WKStringRef script, unsigned scriptCallbackID)
 
     if (!hasOutstandingAsyncTasks()) {
         JSValueRef stringifyException = nullptr;
-        JSRetainPtr<JSStringRef> resultString(Adopt, JSValueToStringCopy(m_context.get(), result, &stringifyException));
-        uiScriptComplete(resultString.get());
-        m_currentScriptCallbackID = 0;
+        requestUIScriptCompletion(JSValueToStringCopy(m_context.get(), result, &stringifyException));
+        tryToCompleteUIScriptForCurrentParentCallback();
     }
 }
 
-unsigned UIScriptContext::nextTaskCallbackID()
+unsigned UIScriptContext::nextTaskCallbackID(CallbackType type)
 {
-    return ++m_nextTaskCallbackID;
+    if (type == CallbackTypeNonPersistent)
+        return ++m_nextTaskCallbackID + firstNonPersistentCallbackID;
+
+    return type;
 }
 
-unsigned UIScriptContext::prepareForAsyncTask(JSValueRef callback)
+unsigned UIScriptContext::prepareForAsyncTask(JSValueRef callback, CallbackType type)
 {
-    unsigned callbackID = nextTaskCallbackID();
+    unsigned callbackID = nextTaskCallbackID(type);
 
     JSValueProtect(m_context.get(), callback);
     Task task;
@@ -99,12 +106,16 @@ void UIScriptContext::asyncTaskComplete(unsigned callbackID)
     JSObjectCallAsFunction(m_context.get(), callbackObject, JSContextGetGlobalObject(m_context.get()), 0, nullptr, &exception);
     JSValueUnprotect(m_context.get(), task.callback);
 
+    tryToCompleteUIScriptForCurrentParentCallback();
     m_currentScriptCallbackID = 0;
 }
 
-unsigned UIScriptContext::registerCallback(JSValueRef taskCallback)
+unsigned UIScriptContext::registerCallback(JSValueRef taskCallback, CallbackType type)
 {
-    return prepareForAsyncTask(taskCallback);
+    if (m_callbacks.contains(type))
+        unregisterCallback(type);
+
+    return prepareForAsyncTask(taskCallback, type);
 }
 
 void UIScriptContext::unregisterCallback(unsigned callbackID)
@@ -133,14 +144,31 @@ void UIScriptContext::fireCallback(unsigned callbackID)
     exception = nullptr;
     JSObjectCallAsFunction(m_context.get(), callbackObject, JSContextGetGlobalObject(m_context.get()), 0, nullptr, &exception);
 
+    tryToCompleteUIScriptForCurrentParentCallback();
     m_currentScriptCallbackID = 0;
 }
 
-void UIScriptContext::uiScriptComplete(JSStringRef result)
+void UIScriptContext::requestUIScriptCompletion(JSStringRef result)
 {
+    ASSERT(m_currentScriptCallbackID);
+    if (currentParentCallbackIsPendingCompletion())
+        return;
+
+    // This request for the UI script to complete is not fulfilled until the last non-persistent task for the parent callback is finished.
+    m_uiScriptResultsPendingCompletion.add(m_currentScriptCallbackID, result ? JSStringRetain(result) : nullptr);
+}
+
+void UIScriptContext::tryToCompleteUIScriptForCurrentParentCallback()
+{
+    if (!currentParentCallbackIsPendingCompletion() || currentParentCallbackHasOutstandingAsyncTasks())
+        return;
+
+    JSStringRef result = m_uiScriptResultsPendingCompletion.take(m_currentScriptCallbackID);
     WKRetainPtr<WKStringRef> uiScriptResult = adoptWK(WKStringCreateWithJSString(result));
     m_delegate.uiScriptDidComplete(uiScriptResult.get(), m_currentScriptCallbackID);
     m_currentScriptCallbackID = 0;
+    if (result)
+        JSStringRelease(result);
 }
 
 JSObjectRef UIScriptContext::objectFromRect(const WKRect& rect) const
@@ -155,3 +183,15 @@ JSObjectRef UIScriptContext::objectFromRect(const WKRect& rect) const
     return object;
 }
 
+bool UIScriptContext::currentParentCallbackHasOutstandingAsyncTasks() const
+{
+    for (auto entry : m_callbacks) {
+        unsigned callbackID = entry.key;
+        Task task = entry.value;
+        if (task.parentScriptCallbackID == m_currentScriptCallbackID && !isPersistentCallbackID(callbackID))
+            return true;
+    }
+
+    return false;
+}
+

Tools/WebKitTestRunner/UIScriptContext/UIScriptContext.h

@@ -39,38 +39,52 @@ class UIScriptContextDelegate {
     virtual void uiScriptDidComplete(WKStringRef result, unsigned callbackID) = 0;
 };
 
+const unsigned firstNonPersistentCallbackID = 1000;
+
+typedef enum  {
+    CallbackTypeWillBeginZooming = 0,
+    CallbackTypeDidEndZooming,
+    CallbackTypeDidShowKeyboard,
+    CallbackTypeDidHideKeyboard,
+    CallbackTypeNonPersistent = firstNonPersistentCallbackID
+} CallbackType;
+
 class UIScriptContext {
 public:
 
     UIScriptContext(UIScriptContextDelegate&);
 
     void runUIScript(WKStringRef script, unsigned scriptCallbackID);
-    void uiScriptComplete(JSStringRef);
+    void requestUIScriptCompletion(JSStringRef);
 
     // For one-shot tasks callbacks.
-    unsigned prepareForAsyncTask(JSValueRef taskCallback);
+    unsigned prepareForAsyncTask(JSValueRef taskCallback, CallbackType);
     void asyncTaskComplete(unsigned taskCallbackID);
 
     // For persistent callbacks.
-    unsigned registerCallback(JSValueRef taskCallback);
+    unsigned registerCallback(JSValueRef taskCallback, CallbackType);
     JSValueRef callbackWithID(unsigned callbackID);
     void unregisterCallback(unsigned callbackID);
     void fireCallback(unsigned callbackID);
 
-    unsigned nextTaskCallbackID();
+    unsigned nextTaskCallbackID(CallbackType);
 
     JSObjectRef objectFromRect(const WKRect&) const;
 
 private:
     JSRetainPtr<JSGlobalContextRef> m_context;
 
     bool hasOutstandingAsyncTasks() const { return !m_callbacks.isEmpty(); }
+    bool currentParentCallbackIsPendingCompletion() const { return m_uiScriptResultsPendingCompletion.contains(m_currentScriptCallbackID); }
+    bool currentParentCallbackHasOutstandingAsyncTasks() const;
+    void tryToCompleteUIScriptForCurrentParentCallback();
 
     struct Task {
         unsigned parentScriptCallbackID { 0 };
         JSValueRef callback { nullptr };
     };
     HashMap<unsigned, Task> m_callbacks;
+    HashMap<unsigned, JSStringRef> m_uiScriptResultsPendingCompletion;
 
     UIScriptContextDelegate& m_delegate;
     RefPtr<UIScriptController> m_controller;