[GTK] WebDriver: test imported/w3c/webdriver/tests/sessions/new_session/response...
authorcarlosgc@webkit.org <carlosgc@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 22 Jan 2018 14:04:47 +0000 (14:04 +0000)
committercarlosgc@webkit.org <carlosgc@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 22 Jan 2018 14:04:47 +0000 (14:04 +0000)
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:

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@227290 268f45cc-cd09-0410-ab3c-d52691b4dbfc

Source/WebDriver/ChangeLog
Source/WebDriver/SessionHost.h
Source/WebDriver/WebDriverService.cpp
Source/WebDriver/glib/SessionHostGlib.cpp
WebDriverTests/ChangeLog
WebDriverTests/TestExpectations.json

index a31da53..c9e4727 100644 (file)
@@ -1,3 +1,26 @@
+2018-01-22  Carlos Garcia Campos  <cgarcia@igalia.com>
+
+        [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  <cgarcia@igalia.com>
 
         WebDriver: implement get timeouts command
index e2b8a9d..cbb0512 100644 (file)
@@ -53,8 +53,7 @@ public:
 
     const Capabilities& capabilities() const { return m_capabilities; }
 
-    enum class Succeeded { No, Yes };
-    void connectToBrowser(Function<void (Succeeded)>&&);
+    void connectToBrowser(Function<void (std::optional<String> error)>&&);
     void startAutomationSession(const String& sessionID, Function<void (std::optional<String>)>&&);
 
     struct CommandResponse {
@@ -77,10 +76,10 @@ private:
 #if USE(GLIB)
     static void dbusConnectionClosedCallback(SessionHost*);
     static const GDBusInterfaceVTable s_interfaceVTable;
-    void launchBrowser(Function<void (Succeeded)>&&);
+    void launchBrowser(Function<void (std::optional<String> error)>&&);
     void connectToBrowser(std::unique_ptr<ConnectToBrowserAsyncData>&&);
     std::optional<String> matchCapabilities(GVariant*);
-    void setupConnection(GRefPtr<GDBusConnection>&&, Function<void (Succeeded)>&&);
+    void setupConnection(GRefPtr<GDBusConnection>&&);
     void setTargetList(uint64_t connectionID, Vector<Target>&&);
     void sendMessageToFrontend(uint64_t connectionID, uint64_t targetID, const char* message);
 #endif
index 6553c70..96501fe 100644 (file)
@@ -611,9 +611,9 @@ void WebDriverService::newSession(RefPtr<JSON::Object>&& parameters, Function<vo
     parseCapabilities(*matchedCapabilities, capabilities);
     auto sessionHost = std::make_unique<SessionHost>(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<String> error) mutable {
+        if (error) {
+            completionHandler(CommandResult::fail(CommandResult::ErrorCode::SessionNotCreated, makeString("Failed to connect to browser: ", error.value())));
             return;
         }
 
index 58ef9e7..b2b414c 100644 (file)
@@ -98,7 +98,7 @@ const GDBusInterfaceVTable SessionHost::s_interfaceVTable = {
     { 0 }
 };
 
-void SessionHost::connectToBrowser(Function<void (Succeeded)>&& completionHandler)
+void SessionHost::connectToBrowser(Function<void (std::optional<String> error)>&& completionHandler)
 {
     launchBrowser(WTFMove(completionHandler));
 }
@@ -109,7 +109,7 @@ bool SessionHost::isConnected() const
 }
 
 struct ConnectToBrowserAsyncData {
-    ConnectToBrowserAsyncData(SessionHost* sessionHost, GUniquePtr<char>&& dbusAddress, GCancellable* cancellable, Function<void (SessionHost::Succeeded)>&& completionHandler)
+    ConnectToBrowserAsyncData(SessionHost* sessionHost, GUniquePtr<char>&& dbusAddress, GCancellable* cancellable, Function<void (std::optional<String> error)>&& completionHandler)
         : sessionHost(sessionHost)
         , dbusAddress(WTFMove(dbusAddress))
         , cancellable(cancellable)
@@ -120,7 +120,7 @@ struct ConnectToBrowserAsyncData {
     SessionHost* sessionHost;
     GUniquePtr<char> dbusAddress;
     GRefPtr<GCancellable> cancellable;
-    Function<void (SessionHost::Succeeded)> completionHandler;
+    Function<void (std::optional<String> 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<void (Succeeded)>&& completionHandler)
+void SessionHost::launchBrowser(Function<void (std::optional<String> error)>&& completionHandler)
 {
     m_cancellable = adoptGRef(g_cancellable_new());
     GRefPtr<GSubprocessLauncher> launcher = adoptGRef(g_subprocess_launcher_new(G_SUBPROCESS_FLAGS_NONE));
@@ -152,7 +152,13 @@ void SessionHost::launchBrowser(Function<void (Succeeded)>&& 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<GError> 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<GError> error;
         g_subprocess_wait_finish(G_SUBPROCESS(browser), result, &error.outPtr());
@@ -190,10 +196,11 @@ void SessionHost::connectToBrowser(std::unique_ptr<ConnectToBrowserAsyncData>&&
                         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<GDBusConnection>&& connection, Function<void (Succeeded)>&& completionHandler)
+void SessionHost::setupConnection(GRefPtr<GDBusConnection>&& connection)
 {
     ASSERT(!m_dbusConnection);
     ASSERT(connection);
@@ -225,8 +232,6 @@ void SessionHost::setupConnection(GRefPtr<GDBusConnection>&& 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<String> SessionHost::matchCapabilities(GVariant* capabilities)
index 5def7a1..339ae67 100644 (file)
@@ -1,3 +1,14 @@
+2018-01-22  Carlos Garcia Campos  <cgarcia@igalia.com>
+
+        [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-19  Carlos Garcia Campos  <cgarcia@igalia.com>
 
         Unreviewed GTK+ gardening. Skip imported/w3c/webdriver/tests/sessions/new_session/response.py.
index b1f82eb..24f399e 100644 (file)
         }
     },
     "imported/w3c/webdriver/tests/sessions/new_session/response.py": {
-        "expected": {"gtk": {"status": ["SKIP"], "bug": "webkit.org/b/181904"}},
         "subtests": {
             "test_resp_capabilites": {
                 "expected": {"all": {"status": ["FAIL"], "bug": "webkit.org/b/180408"}}