Merge r220741 - WebDriver: timeout when JavaScript alert is shown in …

…onload handler

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

Reviewed by Brian Burg.

When a JavaScript alert is shown in an onload handler, the alert prevents the load from finishing in case of
normal page load strategy, so navigation commands or any other command for which we wait for navigation to
complete end up timing out. There are two selenium tests covering this that are currently timing out:
testShouldHandleAlertOnPageLoad and testShouldHandleAlertOnPageLoadUsingGet. The spec says that in case of page
load timeout we should only fail with timeout error when there isn't an active alert dialog. If the next command
expects an alert it will just work, otherwise it will fail with UnexpectedAlertOpen error when trying to handle
user prompts.

9 Navigation.
https://w3c.github.io/webdriver/webdriver-spec.html#dfn-wait-for-navigation-to-complete

* UIProcess/Automation/WebAutomationSession.cpp:
(WebKit::WebAutomationSession::waitForNavigationToComplete): Do not wait for the timeout when the page is
loading and there's an active alert in case of normal page load strategy.
(WebKit::WebAutomationSession::respondToPendingPageNavigationCallbacksWithTimeout): Respond with timeout unless
the page is showing a JavaScript dialog.
(WebKit::WebAutomationSession::respondToPendingFrameNavigationCallbacksWithTimeout): Ditto.
(WebKit::WebAutomationSession::loadTimerFired): Use respondToPendingPageNavigationCallbacksWithTimeout() and
respondToPendingFrameNavigationCallbacksWithTimeout().
(WebKit::WebAutomationSession::willShowJavaScriptDialog): The page is about to show a JavaScript dialog, so
we wait until the next run loop iteration to give time for the client to show the dialog, then check if page is
loading and the dialog is still present. If that's the case we finish all normal strategy pending navigations.
* UIProcess/Automation/WebAutomationSession.h:
* UIProcess/WebPageProxy.cpp:
(WebKit::WebPageProxy::runJavaScriptAlert): If controlled by automation, notify the session.
(WebKit::WebPageProxy::runJavaScriptConfirm): Ditto.
(WebKit::WebPageProxy::runJavaScriptPrompt): Ditto.
(WebKit::WebPageProxy::runBeforeUnloadConfirmPanel): Ditto.

Source/WebKit/ChangeLog

@@ -1,3 +1,40 @@
+2017-08-15  Carlos Garcia Campos  <cgarcia@igalia.com>
+
+        WebDriver: timeout when JavaScript alert is shown in onload handler
+        https://bugs.webkit.org/show_bug.cgi?id=175315
+        <rdar://problem/33788294>
+
+        Reviewed by Brian Burg.
+
+        When a JavaScript alert is shown in an onload handler, the alert prevents the load from finishing in case of
+        normal page load strategy, so navigation commands or any other command for which we wait for navigation to
+        complete end up timing out. There are two selenium tests covering this that are currently timing out:
+        testShouldHandleAlertOnPageLoad and testShouldHandleAlertOnPageLoadUsingGet. The spec says that in case of page
+        load timeout we should only fail with timeout error when there isn't an active alert dialog. If the next command
+        expects an alert it will just work, otherwise it will fail with UnexpectedAlertOpen error when trying to handle
+        user prompts.
+
+        9 Navigation.
+        https://w3c.github.io/webdriver/webdriver-spec.html#dfn-wait-for-navigation-to-complete
+
+        * UIProcess/Automation/WebAutomationSession.cpp:
+        (WebKit::WebAutomationSession::waitForNavigationToComplete): Do not wait for the timeout when the page is
+        loading and there's an active alert in case of normal page load strategy.
+        (WebKit::WebAutomationSession::respondToPendingPageNavigationCallbacksWithTimeout): Respond with timeout unless
+        the page is showing a JavaScript dialog.
+        (WebKit::WebAutomationSession::respondToPendingFrameNavigationCallbacksWithTimeout): Ditto.
+        (WebKit::WebAutomationSession::loadTimerFired): Use respondToPendingPageNavigationCallbacksWithTimeout() and
+        respondToPendingFrameNavigationCallbacksWithTimeout().
+        (WebKit::WebAutomationSession::willShowJavaScriptDialog): The page is about to show a JavaScript dialog, so
+        we wait until the next run loop iteration to give time for the client to show the dialog, then check if page is
+        loading and the dialog is still present. If that's the case we finish all normal strategy pending navigations.
+        * UIProcess/Automation/WebAutomationSession.h:
+        * UIProcess/WebPageProxy.cpp:
+        (WebKit::WebPageProxy::runJavaScriptAlert): If controlled by automation, notify the session.
+        (WebKit::WebPageProxy::runJavaScriptConfirm): Ditto.
+        (WebKit::WebPageProxy::runJavaScriptPrompt): Ditto.
+        (WebKit::WebPageProxy::runBeforeUnloadConfirmPanel): Ditto.
+
 2017-08-14  Carlos Garcia Campos  <cgarcia@igalia.com>
 
         WebDriver: handle click events on option elements

Source/WebKit/UIProcess/Automation/WebAutomationSession.cpp

@@ -409,16 +409,36 @@ void WebAutomationSession::waitForNavigationToComplete(Inspector::ErrorString& e
         FAIL_WITH_PREDEFINED_ERROR_AND_DETAILS(InvalidParameter, "The parameter 'pageLoadStrategy' is invalid.");
     auto pageLoadTimeout = optionalPageLoadTimeout ? Seconds::fromMilliseconds(*optionalPageLoadTimeout) : defaultPageLoadTimeout;
 
+    // If page is loading and there's an active JavaScript dialog is probably because the
+    // dialog was started in an onload handler, so in case of normal page load strategy the
+    // load will not finish until the dialog is dismissed. Instead of waiting for the timeout,
+    // we return without waiting since we know it will timeout for sure. We want to check
+    // arguments first, though.
+    bool shouldTimeoutDueToUnexpectedAlert = pageLoadStrategy.value() == Inspector::Protocol::Automation::PageLoadStrategy::Normal
+        && page->pageLoadState().isLoading() && m_client->isShowingJavaScriptDialogOnPage(*this, *page);
+
     if (optionalFrameHandle && !optionalFrameHandle->isEmpty()) {
         std::optional<uint64_t> frameID = webFrameIDForHandle(*optionalFrameHandle);
         if (!frameID)
             FAIL_WITH_PREDEFINED_ERROR(FrameNotFound);
         WebFrameProxy* frame = page->process().webFrame(frameID.value());
         if (!frame)
             FAIL_WITH_PREDEFINED_ERROR(FrameNotFound);
-        waitForNavigationToCompleteOnFrame(*frame, pageLoadStrategy.value(), pageLoadTimeout, WTFMove(callback));
-    } else
-        waitForNavigationToCompleteOnPage(*page, pageLoadStrategy.value(), pageLoadTimeout, WTFMove(callback));
+        if (!shouldTimeoutDueToUnexpectedAlert)
+            waitForNavigationToCompleteOnFrame(*frame, pageLoadStrategy.value(), pageLoadTimeout, WTFMove(callback));
+    } else {
+        if (!shouldTimeoutDueToUnexpectedAlert)
+            waitForNavigationToCompleteOnPage(*page, pageLoadStrategy.value(), pageLoadTimeout, WTFMove(callback));
+    }
+
+    if (shouldTimeoutDueToUnexpectedAlert) {
+        // §9 Navigation.
+        // 7. If the previous step completed by the session page load timeout being reached and the browser does not
+        // have an active user prompt, return error with error code timeout.
+        // 8. Return success with data null.
+        // https://w3c.github.io/webdriver/webdriver-spec.html#dfn-wait-for-navigation-to-complete
+        callback->sendSuccess();
+    }
 }
 
 void WebAutomationSession::waitForNavigationToCompleteOnPage(WebPageProxy& page, Inspector::Protocol::Automation::PageLoadStrategy loadStrategy, Seconds timeout, Ref<Inspector::BackendDispatcher::CallbackBase>&& callback)
@@ -463,20 +483,61 @@ void WebAutomationSession::waitForNavigationToCompleteOnFrame(WebFrameProxy& fra
     }
 }
 
-static void respondToPendingNavigationCallbacksWithTimeout(HashMap<uint64_t, RefPtr<Inspector::BackendDispatcher::CallbackBase>>& map)
+void WebAutomationSession::respondToPendingPageNavigationCallbacksWithTimeout(HashMap<uint64_t, RefPtr<Inspector::BackendDispatcher::CallbackBase>>& map)
 {
+    Inspector::ErrorString timeoutError = STRING_FOR_PREDEFINED_ERROR_NAME(Timeout);
     for (auto id : map.keys()) {
+        auto page = WebProcessProxy::webPage(id);
         auto callback = map.take(id);
-        callback->sendFailure(STRING_FOR_PREDEFINED_ERROR_NAME(Timeout));
+        if (page && m_client->isShowingJavaScriptDialogOnPage(*this, *page))
+            callback->sendSuccess(InspectorObject::create());
+        else
+            callback->sendFailure(timeoutError);
+    }
+}
+
+static WebPageProxy* findPageForFrameID(const WebProcessPool& processPool, uint64_t frameID)
+{
+    for (auto& process : processPool.processes()) {
+        if (auto* frame = process->webFrame(frameID))
+            return frame->page();
+    }
+    return nullptr;
+}
+
+void WebAutomationSession::respondToPendingFrameNavigationCallbacksWithTimeout(HashMap<uint64_t, RefPtr<Inspector::BackendDispatcher::CallbackBase>>& map)
+{
+    Inspector::ErrorString timeoutError = STRING_FOR_PREDEFINED_ERROR_NAME(Timeout);
+    for (auto id : map.keys()) {
+        auto* page = findPageForFrameID(*m_processPool, id);
+        auto callback = map.take(id);
+        if (page && m_client->isShowingJavaScriptDialogOnPage(*this, *page))
+            callback->sendSuccess(InspectorObject::create());
+        else
+            callback->sendFailure(timeoutError);
     }
 }
 
 void WebAutomationSession::loadTimerFired()
 {
-    respondToPendingNavigationCallbacksWithTimeout(m_pendingNormalNavigationInBrowsingContextCallbacksPerFrame);
-    respondToPendingNavigationCallbacksWithTimeout(m_pendingEagerNavigationInBrowsingContextCallbacksPerFrame);
-    respondToPendingNavigationCallbacksWithTimeout(m_pendingNormalNavigationInBrowsingContextCallbacksPerPage);
-    respondToPendingNavigationCallbacksWithTimeout(m_pendingEagerNavigationInBrowsingContextCallbacksPerPage);
+    respondToPendingFrameNavigationCallbacksWithTimeout(m_pendingNormalNavigationInBrowsingContextCallbacksPerFrame);
+    respondToPendingFrameNavigationCallbacksWithTimeout(m_pendingEagerNavigationInBrowsingContextCallbacksPerFrame);
+    respondToPendingPageNavigationCallbacksWithTimeout(m_pendingNormalNavigationInBrowsingContextCallbacksPerPage);
+    respondToPendingPageNavigationCallbacksWithTimeout(m_pendingEagerNavigationInBrowsingContextCallbacksPerPage);
+}
+
+void WebAutomationSession::willShowJavaScriptDialog(WebPageProxy& page)
+{
+    // Wait until the next run loop iteration to give time for the client to show the dialog,
+    // then check if page is loading and the dialog is still present. The dialog will block the
+    // load in case of normal strategy, so we want to dispatch all pending navigation callbacks.
+    RunLoop::main().dispatch([this, protectedThis = makeRef(*this), page = makeRef(page)] {
+        if (!page->isValid() || !page->pageLoadState().isLoading() || !m_client || !m_client->isShowingJavaScriptDialogOnPage(*this, page))
+            return;
+
+        respondToPendingFrameNavigationCallbacksWithTimeout(m_pendingNormalNavigationInBrowsingContextCallbacksPerFrame);
+        respondToPendingPageNavigationCallbacksWithTimeout(m_pendingNormalNavigationInBrowsingContextCallbacksPerPage);
+    });
 }
 
 void WebAutomationSession::navigateBrowsingContext(Inspector::ErrorString& errorString, const String& handle, const String& url, const String* optionalPageLoadStrategyString, const int* optionalPageLoadTimeout, Ref<NavigateBrowsingContextCallback>&& callback)

Source/WebKit/UIProcess/Automation/WebAutomationSession.h

@@ -99,6 +99,7 @@ class WebAutomationSession final : public API::ObjectImpl<API::Object::Type::Aut
     void keyboardEventsFlushedForPage(const WebPageProxy&);
     void willClosePage(const WebPageProxy&);
     void handleRunOpenPanel(const WebPageProxy&, const WebFrameProxy&, const API::OpenPanelParameters&, WebOpenPanelResultListenerProxy&);
+    void willShowJavaScriptDialog(WebPageProxy&);
 
 #if ENABLE(REMOTE_INSPECTOR)
     // Inspector::RemoteAutomationTarget API
@@ -167,6 +168,8 @@ class WebAutomationSession final : public API::ObjectImpl<API::Object::Type::Aut
 
     void waitForNavigationToCompleteOnPage(WebPageProxy&, Inspector::Protocol::Automation::PageLoadStrategy, Seconds, Ref<Inspector::BackendDispatcher::CallbackBase>&&);
     void waitForNavigationToCompleteOnFrame(WebFrameProxy&, Inspector::Protocol::Automation::PageLoadStrategy, Seconds, Ref<Inspector::BackendDispatcher::CallbackBase>&&);
+    void respondToPendingPageNavigationCallbacksWithTimeout(HashMap<uint64_t, RefPtr<Inspector::BackendDispatcher::CallbackBase>>&);
+    void respondToPendingFrameNavigationCallbacksWithTimeout(HashMap<uint64_t, RefPtr<Inspector::BackendDispatcher::CallbackBase>>&);
     void loadTimerFired();
 
     // Implemented in generated WebAutomationSessionMessageReceiver.cpp.

Source/WebKit/UIProcess/WebPageProxy.cpp

@@ -3913,6 +3913,10 @@ void WebPageProxy::runJavaScriptAlert(uint64_t frameID, const SecurityOriginData
     // Since runJavaScriptAlert() can spin a nested run loop we need to turn off the responsiveness timer.
     m_process->responsivenessTimer().stop();
 
+    if (m_controlledByAutomation) {
+        if (auto* automationSession = process().processPool().automationSession())
+            automationSession->willShowJavaScriptDialog(*this);
+    }
     m_uiClient->runJavaScriptAlert(this, message, frame, securityOrigin, [reply = WTFMove(reply)] {
         reply->send();
     });
@@ -3926,6 +3930,11 @@ void WebPageProxy::runJavaScriptConfirm(uint64_t frameID, const SecurityOriginDa
     // Since runJavaScriptConfirm() can spin a nested run loop we need to turn off the responsiveness timer.
     m_process->responsivenessTimer().stop();
 
+    if (m_controlledByAutomation) {
+        if (auto* automationSession = process().processPool().automationSession())
+            automationSession->willShowJavaScriptDialog(*this);
+    }
+
     m_uiClient->runJavaScriptConfirm(this, message, frame, securityOrigin, [reply = WTFMove(reply)](bool result) {
         reply->send(result);
     });
@@ -3939,6 +3948,11 @@ void WebPageProxy::runJavaScriptPrompt(uint64_t frameID, const SecurityOriginDat
     // Since runJavaScriptPrompt() can spin a nested run loop we need to turn off the responsiveness timer.
     m_process->responsivenessTimer().stop();
 
+    if (m_controlledByAutomation) {
+        if (auto* automationSession = process().processPool().automationSession())
+            automationSession->willShowJavaScriptDialog(*this);
+    }
+
     m_uiClient->runJavaScriptPrompt(this, message, defaultValue, frame, securityOrigin, [reply](const String& result) { reply->send(result); });
 }
 
@@ -4098,6 +4112,11 @@ void WebPageProxy::runBeforeUnloadConfirmPanel(uint64_t frameID, const SecurityO
     // Since runBeforeUnloadConfirmPanel() can spin a nested run loop we need to turn off the responsiveness timer.
     m_process->responsivenessTimer().stop();
 
+    if (m_controlledByAutomation) {
+        if (auto* automationSession = process().processPool().automationSession())
+            automationSession->willShowJavaScriptDialog(*this);
+    }
+
     m_uiClient->runBeforeUnloadConfirmPanel(this, message, frame, securityOrigin, [reply](bool result) { reply->send(result); });
 }