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.