From f07be2bb3d04a0f676806244a7e4bff90836d5db Mon Sep 17 00:00:00 2001 From: Carlos Garcia Campos Date: Wed, 24 Jan 2018 10:02:33 +0000 Subject: [PATCH] Merge r227290 - [GTK] WebDriver: test imported/w3c/webdriver/tests/sessions/new_session/response.py is crashing in the bots https://bugs.webkit.org/show_bug.cgi?id=181904 Reviewed by Carlos Alberto Lopez Perez. Source/WebDriver: Handle the case of failing to launch the browser. The test is actually failing because it's sending wrong capabilities, the driver tries to fall back to the default driver, but since WebKit is not installed in the bots, it fails to find the MiniBrowser. The test needs to be fixed, but we shouldn't crash when the browser can't be spawned for whatever reason in any case. This patch handles that case and changes the boolean result of connectToBrowser to be an optional error string instead. This way we can provide more detailed error message when we reject the session creation because the browser failed to start. * SessionHost.h: * WebDriverService.cpp: (WebDriver::WebDriverService::newSession): * glib/SessionHostGlib.cpp: (WebDriver::SessionHost::connectToBrowser): (WebDriver::ConnectToBrowserAsyncData::ConnectToBrowserAsyncData): (WebDriver::SessionHost::launchBrowser): (WebDriver::SessionHost::setupConnection): WebDriverTests: Unskip imported/w3c/webdriver/tests/sessions/new_session/response.py. * TestExpectations.json: --- Source/WebDriver/ChangeLog | 23 +++++++++++++++++++++ Source/WebDriver/SessionHost.h | 7 +++---- Source/WebDriver/WebDriverService.cpp | 6 +++--- Source/WebDriver/glib/SessionHostGlib.cpp | 25 ++++++++++++++--------- WebDriverTests/ChangeLog | 11 ++++++++++ 5 files changed, 55 insertions(+), 17 deletions(-) diff --git a/Source/WebDriver/ChangeLog b/Source/WebDriver/ChangeLog index 55ef3c448c4a..13db5ae3be29 100644 --- a/Source/WebDriver/ChangeLog +++ b/Source/WebDriver/ChangeLog @@ -1,3 +1,26 @@ +2018-01-22 Carlos Garcia Campos + + [GTK] WebDriver: test imported/w3c/webdriver/tests/sessions/new_session/response.py is crashing in the bots + https://bugs.webkit.org/show_bug.cgi?id=181904 + + Reviewed by Carlos Alberto Lopez Perez. + + Handle the case of failing to launch the browser. The test is actually failing because it's sending wrong + capabilities, the driver tries to fall back to the default driver, but since WebKit is not installed in the + bots, it fails to find the MiniBrowser. The test needs to be fixed, but we shouldn't crash when the browser + can't be spawned for whatever reason in any case. This patch handles that case and changes the boolean result of + connectToBrowser to be an optional error string instead. This way we can provide more detailed error message + when we reject the session creation because the browser failed to start. + + * SessionHost.h: + * WebDriverService.cpp: + (WebDriver::WebDriverService::newSession): + * glib/SessionHostGlib.cpp: + (WebDriver::SessionHost::connectToBrowser): + (WebDriver::ConnectToBrowserAsyncData::ConnectToBrowserAsyncData): + (WebDriver::SessionHost::launchBrowser): + (WebDriver::SessionHost::setupConnection): + 2018-01-11 Carlos Garcia Campos WebDriver: implement get timeouts command diff --git a/Source/WebDriver/SessionHost.h b/Source/WebDriver/SessionHost.h index e2b8a9d0f432..cbb0512ed2a5 100644 --- a/Source/WebDriver/SessionHost.h +++ b/Source/WebDriver/SessionHost.h @@ -53,8 +53,7 @@ class SessionHost { const Capabilities& capabilities() const { return m_capabilities; } - enum class Succeeded { No, Yes }; - void connectToBrowser(Function&&); + void connectToBrowser(Function error)>&&); void startAutomationSession(const String& sessionID, Function)>&&); struct CommandResponse { @@ -77,10 +76,10 @@ class SessionHost { #if USE(GLIB) static void dbusConnectionClosedCallback(SessionHost*); static const GDBusInterfaceVTable s_interfaceVTable; - void launchBrowser(Function&&); + void launchBrowser(Function error)>&&); void connectToBrowser(std::unique_ptr&&); std::optional matchCapabilities(GVariant*); - void setupConnection(GRefPtr&&, Function&&); + void setupConnection(GRefPtr&&); void setTargetList(uint64_t connectionID, Vector&&); void sendMessageToFrontend(uint64_t connectionID, uint64_t targetID, const char* message); #endif diff --git a/Source/WebDriver/WebDriverService.cpp b/Source/WebDriver/WebDriverService.cpp index 6553c70b319f..96501fedc5c4 100644 --- a/Source/WebDriver/WebDriverService.cpp +++ b/Source/WebDriver/WebDriverService.cpp @@ -611,9 +611,9 @@ void WebDriverService::newSession(RefPtr&& parameters, Function(WTFMove(capabilities)); auto* sessionHostPtr = sessionHost.get(); - sessionHostPtr->connectToBrowser([this, sessionHost = WTFMove(sessionHost), completionHandler = WTFMove(completionHandler)](SessionHost::Succeeded succeeded) mutable { - if (succeeded == SessionHost::Succeeded::No) { - completionHandler(CommandResult::fail(CommandResult::ErrorCode::SessionNotCreated, String("Failed to connect to browser"))); + sessionHostPtr->connectToBrowser([this, sessionHost = WTFMove(sessionHost), completionHandler = WTFMove(completionHandler)](std::optional error) mutable { + if (error) { + completionHandler(CommandResult::fail(CommandResult::ErrorCode::SessionNotCreated, makeString("Failed to connect to browser: ", error.value()))); return; } diff --git a/Source/WebDriver/glib/SessionHostGlib.cpp b/Source/WebDriver/glib/SessionHostGlib.cpp index 58ef9e791767..b2b414c6eb05 100644 --- a/Source/WebDriver/glib/SessionHostGlib.cpp +++ b/Source/WebDriver/glib/SessionHostGlib.cpp @@ -98,7 +98,7 @@ const GDBusInterfaceVTable SessionHost::s_interfaceVTable = { { 0 } }; -void SessionHost::connectToBrowser(Function&& completionHandler) +void SessionHost::connectToBrowser(Function error)>&& completionHandler) { launchBrowser(WTFMove(completionHandler)); } @@ -109,7 +109,7 @@ bool SessionHost::isConnected() const } struct ConnectToBrowserAsyncData { - ConnectToBrowserAsyncData(SessionHost* sessionHost, GUniquePtr&& dbusAddress, GCancellable* cancellable, Function&& completionHandler) + ConnectToBrowserAsyncData(SessionHost* sessionHost, GUniquePtr&& dbusAddress, GCancellable* cancellable, Function error)>&& completionHandler) : sessionHost(sessionHost) , dbusAddress(WTFMove(dbusAddress)) , cancellable(cancellable) @@ -120,7 +120,7 @@ struct ConnectToBrowserAsyncData { SessionHost* sessionHost; GUniquePtr dbusAddress; GRefPtr cancellable; - Function completionHandler; + Function error)> completionHandler; }; static guint16 freePort() @@ -135,7 +135,7 @@ static guint16 freePort() return g_inet_socket_address_get_port(G_INET_SOCKET_ADDRESS(address.get())); } -void SessionHost::launchBrowser(Function&& completionHandler) +void SessionHost::launchBrowser(Function error)>&& completionHandler) { m_cancellable = adoptGRef(g_cancellable_new()); GRefPtr launcher = adoptGRef(g_subprocess_launcher_new(G_SUBPROCESS_FLAGS_NONE)); @@ -152,7 +152,13 @@ void SessionHost::launchBrowser(Function&& completionHandler) for (unsigned i = 0; i < browserArguments.size(); ++i) args.get()[i + 1] = g_strdup(browserArguments[i].utf8().data()); - m_browser = adoptGRef(g_subprocess_launcher_spawnv(launcher.get(), args.get(), nullptr)); + GUniqueOutPtr error; + m_browser = adoptGRef(g_subprocess_launcher_spawnv(launcher.get(), args.get(), &error.outPtr())); + if (error) { + completionHandler(String::fromUTF8(error->message)); + return; + } + g_subprocess_wait_async(m_browser.get(), m_cancellable.get(), [](GObject* browser, GAsyncResult* result, gpointer userData) { GUniqueOutPtr error; g_subprocess_wait_finish(G_SUBPROCESS(browser), result, &error.outPtr()); @@ -190,10 +196,11 @@ void SessionHost::connectToBrowser(std::unique_ptr&& return; } - data->completionHandler(Succeeded::No); + data->completionHandler(String::fromUTF8(error->message)); return; } - data->sessionHost->setupConnection(WTFMove(connection), WTFMove(data->completionHandler)); + data->sessionHost->setupConnection(WTFMove(connection)); + data->completionHandler(std::nullopt); }, data); }); } @@ -212,7 +219,7 @@ static void dbusConnectionCallAsyncReadyCallback(GObject* source, GAsyncResult* WTFLogAlways("RemoteInspectorServer failed to send DBus message: %s", error->message); } -void SessionHost::setupConnection(GRefPtr&& connection, Function&& completionHandler) +void SessionHost::setupConnection(GRefPtr&& connection) { ASSERT(!m_dbusConnection); ASSERT(connection); @@ -225,8 +232,6 @@ void SessionHost::setupConnection(GRefPtr&& connection, Functio introspectionData = g_dbus_node_info_new_for_xml(introspectionXML, nullptr); g_dbus_connection_register_object(m_dbusConnection.get(), REMOTE_INSPECTOR_CLIENT_OBJECT_PATH, introspectionData->interfaces[0], &s_interfaceVTable, this, nullptr, nullptr); - - completionHandler(Succeeded::Yes); } std::optional SessionHost::matchCapabilities(GVariant* capabilities) diff --git a/WebDriverTests/ChangeLog b/WebDriverTests/ChangeLog index 2178409e7b43..9ade8c2f5c27 100644 --- a/WebDriverTests/ChangeLog +++ b/WebDriverTests/ChangeLog @@ -1,3 +1,14 @@ +2018-01-22 Carlos Garcia Campos + + [GTK] WebDriver: test imported/w3c/webdriver/tests/sessions/new_session/response.py is crashing in the bots + https://bugs.webkit.org/show_bug.cgi?id=181904 + + Reviewed by Carlos Alberto Lopez Perez. + + Unskip imported/w3c/webdriver/tests/sessions/new_session/response.py. + + * TestExpectations.json: + 2018-01-18 Carlos Garcia Campos Unreviewed. Update Selenium WebDriver imported tests.