Changeset 220741 in webkit


Ignore:
Timestamp:
Aug 15, 2017 12:17:38 AM (7 years ago)
Author:
Carlos Garcia Campos
Message:

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.

Location:
trunk/Source/WebKit
Files:
4 edited

Legend:

Unmodified
Added
Removed

  • trunk/Source/WebKit/ChangeLog

    r220740 r220741  
     12017-08-15  Carlos Garcia Campos  <cgarcia@igalia.com>
     2
     3        WebDriver: timeout when JavaScript alert is shown in onload handler
     4        https://bugs.webkit.org/show_bug.cgi?id=175315
     5        <rdar://problem/33788294>
     6
     7        Reviewed by Brian Burg.
     8
     9        When a JavaScript alert is shown in an onload handler, the alert prevents the load from finishing in case of
     10        normal page load strategy, so navigation commands or any other command for which we wait for navigation to
     11        complete end up timing out. There are two selenium tests covering this that are currently timing out:
     12        testShouldHandleAlertOnPageLoad and testShouldHandleAlertOnPageLoadUsingGet. The spec says that in case of page
     13        load timeout we should only fail with timeout error when there isn't an active alert dialog. If the next command
     14        expects an alert it will just work, otherwise it will fail with UnexpectedAlertOpen error when trying to handle
     15        user prompts.
     16
     17        9 Navigation.
     18        https://w3c.github.io/webdriver/webdriver-spec.html#dfn-wait-for-navigation-to-complete
     19
     20        * UIProcess/Automation/WebAutomationSession.cpp:
     21        (WebKit::WebAutomationSession::waitForNavigationToComplete): Do not wait for the timeout when the page is
     22        loading and there's an active alert in case of normal page load strategy.
     23        (WebKit::WebAutomationSession::respondToPendingPageNavigationCallbacksWithTimeout): Respond with timeout unless
     24        the page is showing a JavaScript dialog.
     25        (WebKit::WebAutomationSession::respondToPendingFrameNavigationCallbacksWithTimeout): Ditto.
     26        (WebKit::WebAutomationSession::loadTimerFired): Use respondToPendingPageNavigationCallbacksWithTimeout() and
     27        respondToPendingFrameNavigationCallbacksWithTimeout().
     28        (WebKit::WebAutomationSession::willShowJavaScriptDialog): The page is about to show a JavaScript dialog, so
     29        we wait until the next run loop iteration to give time for the client to show the dialog, then check if page is
     30        loading and the dialog is still present. If that's the case we finish all normal strategy pending navigations.
     31        * UIProcess/Automation/WebAutomationSession.h:
     32        * UIProcess/WebPageProxy.cpp:
     33        (WebKit::WebPageProxy::runJavaScriptAlert): If controlled by automation, notify the session.
     34        (WebKit::WebPageProxy::runJavaScriptConfirm): Ditto.
     35        (WebKit::WebPageProxy::runJavaScriptPrompt): Ditto.
     36        (WebKit::WebPageProxy::runBeforeUnloadConfirmPanel): Ditto.
     37
    1382017-08-14  Carlos Garcia Campos  <cgarcia@igalia.com>
    239

  • trunk/Source/WebKit/UIProcess/Automation/WebAutomationSession.cpp

    r220740 r220741  
    410410    auto pageLoadTimeout = optionalPageLoadTimeout ? Seconds::fromMilliseconds(*optionalPageLoadTimeout) : defaultPageLoadTimeout;
    411411
     412    // If page is loading and there's an active JavaScript dialog is probably because the
     413    // dialog was started in an onload handler, so in case of normal page load strategy the
     414    // load will not finish until the dialog is dismissed. Instead of waiting for the timeout,
     415    // we return without waiting since we know it will timeout for sure. We want to check
     416    // arguments first, though.
     417    bool shouldTimeoutDueToUnexpectedAlert = pageLoadStrategy.value() == Inspector::Protocol::Automation::PageLoadStrategy::Normal
     418        && page->pageLoadState().isLoading() && m_client->isShowingJavaScriptDialogOnPage(*this, *page);
     419
    412420    if (optionalFrameHandle && !optionalFrameHandle->isEmpty()) {
    413421        std::optional<uint64_t> frameID = webFrameIDForHandle(*optionalFrameHandle);
    … …  
    417425        if (!frame)
    418426            FAIL_WITH_PREDEFINED_ERROR(FrameNotFound);
    419         waitForNavigationToCompleteOnFrame(*frame, pageLoadStrategy.value(), pageLoadTimeout, WTFMove(callback));
    420     } else
    421         waitForNavigationToCompleteOnPage(*page, pageLoadStrategy.value(), pageLoadTimeout, WTFMove(callback));
     427        if (!shouldTimeoutDueToUnexpectedAlert)
     428            waitForNavigationToCompleteOnFrame(*frame, pageLoadStrategy.value(), pageLoadTimeout, WTFMove(callback));
     429    } else {
     430        if (!shouldTimeoutDueToUnexpectedAlert)
     431            waitForNavigationToCompleteOnPage(*page, pageLoadStrategy.value(), pageLoadTimeout, WTFMove(callback));
     432    }
     433
     434    if (shouldTimeoutDueToUnexpectedAlert) {
     435        // §9 Navigation.
     436        // 7. If the previous step completed by the session page load timeout being reached and the browser does not
     437        // have an active user prompt, return error with error code timeout.
     438        // 8. Return success with data null.
     439        // https://w3c.github.io/webdriver/webdriver-spec.html#dfn-wait-for-navigation-to-complete
     440        callback->sendSuccess();
     441    }
    422442}
    423443
    … …  
    464484}
    465485
    466 static void respondToPendingNavigationCallbacksWithTimeout(HashMap<uint64_t, RefPtr<Inspector::BackendDispatcher::CallbackBase>>& map)
    467 {
     486void WebAutomationSession::respondToPendingPageNavigationCallbacksWithTimeout(HashMap<uint64_t, RefPtr<Inspector::BackendDispatcher::CallbackBase>>& map)
     487{
     488    Inspector::ErrorString timeoutError = STRING_FOR_PREDEFINED_ERROR_NAME(Timeout);
    468489    for (auto id : map.keys()) {
     490        auto page = WebProcessProxy::webPage(id);
    469491        auto callback = map.take(id);
    470         callback->sendFailure(STRING_FOR_PREDEFINED_ERROR_NAME(Timeout));
     492        if (page && m_client->isShowingJavaScriptDialogOnPage(*this, *page))
     493            callback->sendSuccess(InspectorObject::create());
     494        else
     495            callback->sendFailure(timeoutError);
     496    }
     497}
     498
     499static WebPageProxy* findPageForFrameID(const WebProcessPool& processPool, uint64_t frameID)
     500{
     501    for (auto& process : processPool.processes()) {
     502        if (auto* frame = process->webFrame(frameID))
     503            return frame->page();
     504    }
     505    return nullptr;
     506}
     507
     508void WebAutomationSession::respondToPendingFrameNavigationCallbacksWithTimeout(HashMap<uint64_t, RefPtr<Inspector::BackendDispatcher::CallbackBase>>& map)
     509{
     510    Inspector::ErrorString timeoutError = STRING_FOR_PREDEFINED_ERROR_NAME(Timeout);
     511    for (auto id : map.keys()) {
     512        auto* page = findPageForFrameID(*m_processPool, id);
     513        auto callback = map.take(id);
     514        if (page && m_client->isShowingJavaScriptDialogOnPage(*this, *page))
     515            callback->sendSuccess(InspectorObject::create());
     516        else
     517            callback->sendFailure(timeoutError);
    471518    }
    472519}
    … …  
    474521void WebAutomationSession::loadTimerFired()
    475522{
    476     respondToPendingNavigationCallbacksWithTimeout(m_pendingNormalNavigationInBrowsingContextCallbacksPerFrame);
    477     respondToPendingNavigationCallbacksWithTimeout(m_pendingEagerNavigationInBrowsingContextCallbacksPerFrame);
    478     respondToPendingNavigationCallbacksWithTimeout(m_pendingNormalNavigationInBrowsingContextCallbacksPerPage);
    479     respondToPendingNavigationCallbacksWithTimeout(m_pendingEagerNavigationInBrowsingContextCallbacksPerPage);
     523    respondToPendingFrameNavigationCallbacksWithTimeout(m_pendingNormalNavigationInBrowsingContextCallbacksPerFrame);
     524    respondToPendingFrameNavigationCallbacksWithTimeout(m_pendingEagerNavigationInBrowsingContextCallbacksPerFrame);
     525    respondToPendingPageNavigationCallbacksWithTimeout(m_pendingNormalNavigationInBrowsingContextCallbacksPerPage);
     526    respondToPendingPageNavigationCallbacksWithTimeout(m_pendingEagerNavigationInBrowsingContextCallbacksPerPage);
     527}
     528
     529void WebAutomationSession::willShowJavaScriptDialog(WebPageProxy& page)
     530{
     531    // Wait until the next run loop iteration to give time for the client to show the dialog,
     532    // then check if page is loading and the dialog is still present. The dialog will block the
     533    // load in case of normal strategy, so we want to dispatch all pending navigation callbacks.
     534    RunLoop::main().dispatch([this, protectedThis = makeRef(*this), page = makeRef(page)] {
     535        if (!page->isValid() || !page->pageLoadState().isLoading() || !m_client || !m_client->isShowingJavaScriptDialogOnPage(*this, page))
     536            return;
     537
     538        respondToPendingFrameNavigationCallbacksWithTimeout(m_pendingNormalNavigationInBrowsingContextCallbacksPerFrame);
     539        respondToPendingPageNavigationCallbacksWithTimeout(m_pendingNormalNavigationInBrowsingContextCallbacksPerPage);
     540    });
    480541}
    481542

  • trunk/Source/WebKit/UIProcess/Automation/WebAutomationSession.h

    r220740 r220741  
    100100    void willClosePage(const WebPageProxy&);
    101101    void handleRunOpenPanel(const WebPageProxy&, const WebFrameProxy&, const API::OpenPanelParameters&, WebOpenPanelResultListenerProxy&);
     102    void willShowJavaScriptDialog(WebPageProxy&);
    102103
    103104#if ENABLE(REMOTE_INSPECTOR)
    … …  
    168169    void waitForNavigationToCompleteOnPage(WebPageProxy&, Inspector::Protocol::Automation::PageLoadStrategy, Seconds, Ref<Inspector::BackendDispatcher::CallbackBase>&&);
    169170    void waitForNavigationToCompleteOnFrame(WebFrameProxy&, Inspector::Protocol::Automation::PageLoadStrategy, Seconds, Ref<Inspector::BackendDispatcher::CallbackBase>&&);
     171    void respondToPendingPageNavigationCallbacksWithTimeout(HashMap<uint64_t, RefPtr<Inspector::BackendDispatcher::CallbackBase>>&);
     172    void respondToPendingFrameNavigationCallbacksWithTimeout(HashMap<uint64_t, RefPtr<Inspector::BackendDispatcher::CallbackBase>>&);
    170173    void loadTimerFired();
    171174

  • trunk/Source/WebKit/UIProcess/WebPageProxy.cpp

    r220443 r220741  
    39143914    m_process->responsivenessTimer().stop();
    39153915
     3916    if (m_controlledByAutomation) {
     3917        if (auto* automationSession = process().processPool().automationSession())
     3918            automationSession->willShowJavaScriptDialog(*this);
     3919    }
    39163920    m_uiClient->runJavaScriptAlert(this, message, frame, securityOrigin, [reply = WTFMove(reply)] {
    39173921        reply->send();
    … …  
    39273931    m_process->responsivenessTimer().stop();
    39283932
     3933    if (m_controlledByAutomation) {
     3934        if (auto* automationSession = process().processPool().automationSession())
     3935            automationSession->willShowJavaScriptDialog(*this);
     3936    }
     3937
    39293938    m_uiClient->runJavaScriptConfirm(this, message, frame, securityOrigin, [reply = WTFMove(reply)](bool result) {
    39303939        reply->send(result);
    … …  
    39393948    // Since runJavaScriptPrompt() can spin a nested run loop we need to turn off the responsiveness timer.
    39403949    m_process->responsivenessTimer().stop();
     3950
     3951    if (m_controlledByAutomation) {
     3952        if (auto* automationSession = process().processPool().automationSession())
     3953            automationSession->willShowJavaScriptDialog(*this);
     3954    }
    39413955
    39423956    m_uiClient->runJavaScriptPrompt(this, message, defaultValue, frame, securityOrigin, [reply](const String& result) { reply->send(result); });
    … …  
    40984112    // Since runBeforeUnloadConfirmPanel() can spin a nested run loop we need to turn off the responsiveness timer.
    40994113    m_process->responsivenessTimer().stop();
     4114
     4115    if (m_controlledByAutomation) {
     4116        if (auto* automationSession = process().processPool().automationSession())
     4117            automationSession->willShowJavaScriptDialog(*this);
     4118    }
    41004119
    41014120    m_uiClient->runBeforeUnloadConfirmPanel(this, message, frame, securityOrigin, [reply](bool result) { reply->send(result); });
Note: See TracChangeset for help on using the changeset viewer.