diff --git a/Source/WebDriver/ChangeLog b/Source/WebDriver/ChangeLog index a2f704f2ba96..bcfb084df75b 100644 --- a/Source/WebDriver/ChangeLog +++ b/Source/WebDriver/ChangeLog @@ -1,3 +1,40 @@ +2017-07-28 Carlos Garcia Campos + + WebDriver: fix return value of close window command + https://bugs.webkit.org/show_bug.cgi?id=174861 + + Reviewed by Brian Burg. + + We are currently returning null, but we should return the list of window handles, and try to close the session + if there aren't more window handles. + + 10.2 Close Window + https://w3c.github.io/webdriver/webdriver-spec.html#close-window + + 3. If there are no more open top-level browsing contexts, then try to close the session. + 4. Return the result of running the remote end steps for the Get Window Handles command. + + * Session.cpp: + (WebDriver::Session::closeAllToplevelBrowsingContexts): Helper function to close the given toplevel browsing + context and the next one if there are more. + (WebDriver::Session::close): Call closeAllToplevelBrowsingContexts() to delete all toplevel browsing contexts of + the session. + (WebDriver::Session::closeTopLevelBrowsingContext): Close the given toplevel browsing context and call + getWindowHandles() when done. + (WebDriver::Session::closeWindow): Call closeTopLevelBrowsingContext() passing the current toplevel browsing context. + (WebDriver::Session::getWindowHandles): Remove the early return, this command doesn't depend on a current + toplevel browsing context. + * Session.h: + * SessionHost.h: + * WebDriverService.cpp: + (WebDriver::WebDriverService::run): Disconnect the server when main loop quits. + (WebDriver::WebDriverService::deleteSession): Do not fail if the given session is not active. + (WebDriver::WebDriverService::closeWindow): Remove the session if the closed window was the last one. + * WebDriverService.h: Remove unused quit() method. + * glib/SessionHostGlib.cpp: + (WebDriver::SessionHost::isConnected): Return whether host is connected to a browser instance. + (WebDriver::SessionHost::dbusConnectionClosedCallback): Delete m_browser. + 2017-08-14 Carlos Garcia Campos WebDriver: handle click events on option elements diff --git a/Source/WebDriver/Session.cpp b/Source/WebDriver/Session.cpp index f74718a47e52..18be80090c80 100644 --- a/Source/WebDriver/Session.cpp +++ b/Source/WebDriver/Session.cpp @@ -57,6 +57,26 @@ const Capabilities& Session::capabilities() const return m_host->capabilities(); } +void Session::closeAllToplevelBrowsingContexts(const String& toplevelBrowsingContext, Function&& completionHandler) +{ + closeTopLevelBrowsingContext(toplevelBrowsingContext, [this, protectedThis = makeRef(*this), completionHandler = WTFMove(completionHandler)](CommandResult&& result) mutable { + if (result.isError()) { + completionHandler(WTFMove(result)); + return; + } + RefPtr handles; + if (result.result()->asArray(handles) && handles->length()) { + auto handleValue = handles->get(0); + String handle; + if (handleValue->asString(handle)) { + closeAllToplevelBrowsingContexts(handle, WTFMove(completionHandler)); + return; + } + } + completionHandler(CommandResult::success()); + }); +} + void Session::close(Function&& completionHandler) { if (!m_toplevelBrowsingContext) { @@ -64,16 +84,8 @@ void Session::close(Function&& completionHandler) return; } - RefPtr parameters = InspectorObject::create(); - parameters->setString(ASCIILiteral("handle"), m_toplevelBrowsingContext.value()); - m_host->sendCommandToBackend(ASCIILiteral("closeBrowsingContext"), WTFMove(parameters), [this, protectedThis = makeRef(*this), completionHandler = WTFMove(completionHandler)](SessionHost::CommandResponse&& response) { - if (response.isError) { - completionHandler(CommandResult::fail(WTFMove(response.responseObject))); - return; - } - switchToTopLevelBrowsingContext(std::nullopt); - completionHandler(CommandResult::success()); - }); + auto toplevelBrowsingContext = std::exchange(m_toplevelBrowsingContext, std::nullopt); + closeAllToplevelBrowsingContexts(toplevelBrowsingContext.value(), WTFMove(completionHandler)); } void Session::setTimeouts(const Timeouts& timeouts, Function&& completionHandler) @@ -426,6 +438,32 @@ void Session::getWindowHandle(Function&& completionHandl }); } +void Session::closeTopLevelBrowsingContext(const String& toplevelBrowsingContext, Function&& completionHandler) +{ + RefPtr parameters = InspectorObject::create(); + parameters->setString(ASCIILiteral("handle"), toplevelBrowsingContext); + m_host->sendCommandToBackend(ASCIILiteral("closeBrowsingContext"), WTFMove(parameters), [this, protectedThis = makeRef(*this), completionHandler = WTFMove(completionHandler)](SessionHost::CommandResponse&& response) mutable { + if (!m_host->isConnected()) { + // Closing the browsing context made the browser quit. + completionHandler(CommandResult::success(InspectorArray::create())); + return; + } + if (response.isError) { + completionHandler(CommandResult::fail(WTFMove(response.responseObject))); + return; + } + + getWindowHandles([this, completionHandler = WTFMove(completionHandler)](CommandResult&& result) { + if (!m_host->isConnected()) { + // Closing the browsing context made the browser quit. + completionHandler(CommandResult::success(InspectorArray::create())); + return; + } + completionHandler(WTFMove(result)); + }); + }); +} + void Session::closeWindow(Function&& completionHandler) { if (!m_toplevelBrowsingContext) { @@ -438,7 +476,8 @@ void Session::closeWindow(Function&& completionHandler) completionHandler(WTFMove(result)); return; } - close(WTFMove(completionHandler)); + auto toplevelBrowsingContext = std::exchange(m_toplevelBrowsingContext, std::nullopt); + closeTopLevelBrowsingContext(toplevelBrowsingContext.value(), WTFMove(completionHandler)); }); } @@ -458,11 +497,6 @@ void Session::switchToWindow(const String& windowHandle, Function&& completionHandler) { - if (!m_toplevelBrowsingContext) { - completionHandler(CommandResult::fail(CommandResult::ErrorCode::NoSuchWindow)); - return; - } - m_host->sendCommandToBackend(ASCIILiteral("getBrowsingContexts"), InspectorObject::create(), [this, protectedThis = makeRef(*this), completionHandler = WTFMove(completionHandler)](SessionHost::CommandResponse&& response) { if (response.isError || !response.responseObject) { completionHandler(CommandResult::fail(WTFMove(response.responseObject))); diff --git a/Source/WebDriver/Session.h b/Source/WebDriver/Session.h index c3255e2c007d..8b180614b2d9 100644 --- a/Source/WebDriver/Session.h +++ b/Source/WebDriver/Session.h @@ -104,6 +104,8 @@ class Session : public RefCounted { void switchToTopLevelBrowsingContext(std::optional); void switchToBrowsingContext(std::optional); + void closeTopLevelBrowsingContext(const String& toplevelBrowsingContext, Function&&); + void closeAllToplevelBrowsingContexts(const String& toplevelBrowsingContext, Function&&); std::optional pageLoadStrategyString() const; diff --git a/Source/WebDriver/SessionHost.cpp b/Source/WebDriver/SessionHost.cpp index 12478b9d0d56..d3fbc1af55aa 100644 --- a/Source/WebDriver/SessionHost.cpp +++ b/Source/WebDriver/SessionHost.cpp @@ -35,13 +35,13 @@ namespace WebDriver { void SessionHost::inspectorDisconnected() { - if (!m_closeMessageID) - return; - - // Closing the browsing context made the browser quit. - auto responseHandler = m_commandRequests.take(m_closeMessageID); - m_closeMessageID = 0; - responseHandler({ }); + // Browser closed or crashed, finish all pending commands with error. + Vector messages; + copyKeysToVector(m_commandRequests, messages); + for (auto messageID : messages) { + auto responseHandler = m_commandRequests.take(messageID); + responseHandler({ nullptr, true }); + } } long SessionHost::sendCommandToBackend(const String& command, RefPtr&& parameters, Function&& responseHandler) @@ -49,8 +49,6 @@ long SessionHost::sendCommandToBackend(const String& command, RefPtrgetInteger(ASCIILiteral("id"), sequenceID)) return; - if (m_closeMessageID == sequenceID) - m_closeMessageID = 0; - auto responseHandler = m_commandRequests.take(sequenceID); ASSERT(responseHandler); diff --git a/Source/WebDriver/SessionHost.h b/Source/WebDriver/SessionHost.h index 1b97fef8ce36..6b684181ba00 100644 --- a/Source/WebDriver/SessionHost.h +++ b/Source/WebDriver/SessionHost.h @@ -52,6 +52,8 @@ class SessionHost { } ~SessionHost(); + bool isConnected() const; + const Capabilities& capabilities() const { return m_capabilities; } enum class Succeeded { No, Yes }; @@ -92,7 +94,6 @@ class SessionHost { Target m_target; HashMap> m_commandRequests; - long m_closeMessageID { 0 }; #if USE(GLIB) Function)> m_startSessionCompletionHandler; diff --git a/Source/WebDriver/WebDriverService.cpp b/Source/WebDriver/WebDriverService.cpp index 58cddd766044..04f8e3e46f49 100644 --- a/Source/WebDriver/WebDriverService.cpp +++ b/Source/WebDriver/WebDriverService.cpp @@ -95,13 +95,9 @@ int WebDriverService::run(int argc, char** argv) RunLoop::run(); - return EXIT_SUCCESS; -} - -void WebDriverService::quit() -{ m_server.disconnect(); - RunLoop::main().stop(); + + return EXIT_SUCCESS; } const WebDriverService::Command WebDriverService::s_commands[] = { @@ -637,14 +633,15 @@ void WebDriverService::deleteSession(RefPtr&& parameters, Funct auto session = m_sessions.take(sessionID); if (!session) { - completionHandler(CommandResult::fail(CommandResult::ErrorCode::InvalidSessionID)); + completionHandler(CommandResult::success()); return; } - if (m_activeSession == session.get()) - m_activeSession = nullptr; - - session->close(WTFMove(completionHandler)); + session->close([this, session, completionHandler = WTFMove(completionHandler)](CommandResult&& result) mutable { + if (m_activeSession == session.get()) + m_activeSession = nullptr; + completionHandler(WTFMove(result)); + }); } void WebDriverService::setTimeouts(RefPtr&& parameters, Function&& completionHandler) @@ -838,8 +835,24 @@ void WebDriverService::closeWindow(RefPtr&& parameters, Functio { // ยง10.2 Close Window. // https://www.w3.org/TR/webdriver/#close-window - if (auto session = findSessionOrCompleteWithError(*parameters, completionHandler)) - session->closeWindow(WTFMove(completionHandler)); + auto session = findSessionOrCompleteWithError(*parameters, completionHandler); + if (!session) + return; + + session->closeWindow([this, session, completionHandler = WTFMove(completionHandler)](CommandResult&& result) mutable { + if (result.isError()) { + completionHandler(WTFMove(result)); + return; + } + + RefPtr handles; + if (result.result()->asArray(handles) && !handles->length()) { + m_sessions.remove(session->id()); + if (m_activeSession == session.get()) + m_activeSession = nullptr; + } + completionHandler(WTFMove(result)); + }); } void WebDriverService::switchToWindow(RefPtr&& parameters, Function&& completionHandler) diff --git a/Source/WebDriver/WebDriverService.h b/Source/WebDriver/WebDriverService.h index fbb66b65a52f..5c775efe7009 100644 --- a/Source/WebDriver/WebDriverService.h +++ b/Source/WebDriver/WebDriverService.h @@ -48,7 +48,6 @@ class WebDriverService final : public HTTPRequestHandler { ~WebDriverService() = default; int run(int argc, char** argv); - void quit(); static bool platformCompareBrowserVersions(const String&, const String&); diff --git a/Source/WebDriver/glib/SessionHostGlib.cpp b/Source/WebDriver/glib/SessionHostGlib.cpp index f3dd15b56ed6..58ef9e791767 100644 --- a/Source/WebDriver/glib/SessionHostGlib.cpp +++ b/Source/WebDriver/glib/SessionHostGlib.cpp @@ -103,6 +103,11 @@ void SessionHost::connectToBrowser(Function&& completionHandle launchBrowser(WTFMove(completionHandler)); } +bool SessionHost::isConnected() const +{ + return !!m_browser; +} + struct ConnectToBrowserAsyncData { ConnectToBrowserAsyncData(SessionHost* sessionHost, GUniquePtr&& dbusAddress, GCancellable* cancellable, Function&& completionHandler) : sessionHost(sessionHost) @@ -195,6 +200,7 @@ void SessionHost::connectToBrowser(std::unique_ptr&& void SessionHost::dbusConnectionClosedCallback(SessionHost* sessionHost) { + sessionHost->m_browser = nullptr; sessionHost->inspectorDisconnected(); }