- Revision
- 227290
- Author
- carlo...@webkit.org
- Date
- 2018-01-22 06:04:47 -0800 (Mon, 22 Jan 2018)
Log Message
[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:
Modified Paths
Diff
Modified: trunk/Source/WebDriver/ChangeLog (227289 => 227290)
--- trunk/Source/WebDriver/ChangeLog 2018-01-22 14:03:29 UTC (rev 227289)
+++ trunk/Source/WebDriver/ChangeLog 2018-01-22 14:04:47 UTC (rev 227290)
@@ -1,3 +1,26 @@
+2018-01-22 Carlos Garcia Campos <cgar...@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 <cgar...@igalia.com>
WebDriver: implement get timeouts command
Modified: trunk/Source/WebDriver/SessionHost.h (227289 => 227290)
--- trunk/Source/WebDriver/SessionHost.h 2018-01-22 14:03:29 UTC (rev 227289)
+++ trunk/Source/WebDriver/SessionHost.h 2018-01-22 14:04:47 UTC (rev 227290)
@@ -53,8 +53,7 @@
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 @@
#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
Modified: trunk/Source/WebDriver/WebDriverService.cpp (227289 => 227290)
--- trunk/Source/WebDriver/WebDriverService.cpp 2018-01-22 14:03:29 UTC (rev 227289)
+++ trunk/Source/WebDriver/WebDriverService.cpp 2018-01-22 14:04:47 UTC (rev 227290)
@@ -611,9 +611,9 @@
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;
}
Modified: trunk/Source/WebDriver/glib/SessionHostGlib.cpp (227289 => 227290)
--- trunk/Source/WebDriver/glib/SessionHostGlib.cpp 2018-01-22 14:03:29 UTC (rev 227289)
+++ trunk/Source/WebDriver/glib/SessionHostGlib.cpp 2018-01-22 14:04:47 UTC (rev 227290)
@@ -98,7 +98,7 @@
{ 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 @@
}
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 @@
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 @@
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 @@
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 @@
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 @@
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 @@
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)
Modified: trunk/WebDriverTests/ChangeLog (227289 => 227290)
--- trunk/WebDriverTests/ChangeLog 2018-01-22 14:03:29 UTC (rev 227289)
+++ trunk/WebDriverTests/ChangeLog 2018-01-22 14:04:47 UTC (rev 227290)
@@ -1,3 +1,14 @@
+2018-01-22 Carlos Garcia Campos <cgar...@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 <cgar...@igalia.com>
Unreviewed GTK+ gardening. Skip imported/w3c/webdriver/tests/sessions/new_session/response.py.
Modified: trunk/WebDriverTests/TestExpectations.json (227289 => 227290)
--- trunk/WebDriverTests/TestExpectations.json 2018-01-22 14:03:29 UTC (rev 227289)
+++ trunk/WebDriverTests/TestExpectations.json 2018-01-22 14:04:47 UTC (rev 227290)
@@ -353,7 +353,6 @@
}
},
"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"}}
