- Revision
- 220741
- Author
- [email protected]
- Date
- 2017-08-15 00:17:38 -0700 (Tue, 15 Aug 2017)
Log 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.
Modified Paths
Diff
Modified: trunk/Source/WebKit/ChangeLog (220740 => 220741)
--- trunk/Source/WebKit/ChangeLog 2017-08-15 07:03:13 UTC (rev 220740)
+++ trunk/Source/WebKit/ChangeLog 2017-08-15 07:17:38 UTC (rev 220741)
@@ -1,3 +1,40 @@
+2017-08-15 Carlos Garcia Campos <[email protected]>
+
+ 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 <[email protected]>
WebDriver: handle click events on option elements
Modified: trunk/Source/WebKit/UIProcess/Automation/WebAutomationSession.cpp (220740 => 220741)
--- trunk/Source/WebKit/UIProcess/Automation/WebAutomationSession.cpp 2017-08-15 07:03:13 UTC (rev 220740)
+++ trunk/Source/WebKit/UIProcess/Automation/WebAutomationSession.cpp 2017-08-15 07:17:38 UTC (rev 220741)
@@ -409,6 +409,14 @@
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)
@@ -416,9 +424,21 @@
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,22 +483,63 @@
}
}
-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)
{
WebPageProxy* page = webPageProxyForHandle(handle);
Modified: trunk/Source/WebKit/UIProcess/Automation/WebAutomationSession.h (220740 => 220741)
--- trunk/Source/WebKit/UIProcess/Automation/WebAutomationSession.h 2017-08-15 07:03:13 UTC (rev 220740)
+++ trunk/Source/WebKit/UIProcess/Automation/WebAutomationSession.h 2017-08-15 07:17:38 UTC (rev 220741)
@@ -99,6 +99,7 @@
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 @@
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.
Modified: trunk/Source/WebKit/UIProcess/WebPageProxy.cpp (220740 => 220741)
--- trunk/Source/WebKit/UIProcess/WebPageProxy.cpp 2017-08-15 07:03:13 UTC (rev 220740)
+++ trunk/Source/WebKit/UIProcess/WebPageProxy.cpp 2017-08-15 07:17:38 UTC (rev 220741)
@@ -3913,6 +3913,10 @@
// 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 @@
// 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 @@
// 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 @@
// 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); });
}
