- Revision
- 227674
- Author
- [email protected]
- Date
- 2018-01-26 05:20:11 -0800 (Fri, 26 Jan 2018)
Log Message
WebDriver: service hangs after a browser crash
https://bugs.webkit.org/show_bug.cgi?id=182170
Reviewed by Carlos Alberto Lopez Perez.
This is currently happening in the GTK+ debug bot. There's a test that makes the browser crash due to an assert,
hanging the whole process and preventing the rest of the tests from running. When the browser crashes, we
correctly handle the pending requests, by completing them with an error. However, if the client tries to send
another command we fail to send the message to the browser and the reply is never sent to the client. In the
case of the tests, delete session command is sent, but never gets a reply.
* Session.cpp:
(WebDriver::Session::isConnected const): Return whether the session is connected to the browser.
* Session.h:
* SessionHost.cpp:
(WebDriver::SessionHost::sendCommandToBackend): Pass the message ID to SessionHost::sendMessageToBackend().
* SessionHost.h:
* WebDriverService.cpp:
(WebDriver::WebDriverService::deleteSession): Ignore unknown errors if the session is no longer connected.
* glib/SessionHostGlib.cpp:
(WebDriver::SessionHost::sendMessageToBackend): Handle errors when sending the command by completing the request
with an error.
Modified Paths
Diff
Modified: trunk/Source/WebDriver/ChangeLog (227673 => 227674)
--- trunk/Source/WebDriver/ChangeLog 2018-01-26 10:46:38 UTC (rev 227673)
+++ trunk/Source/WebDriver/ChangeLog 2018-01-26 13:20:11 UTC (rev 227674)
@@ -1,5 +1,30 @@
2018-01-26 Carlos Garcia Campos <[email protected]>
+ WebDriver: service hangs after a browser crash
+ https://bugs.webkit.org/show_bug.cgi?id=182170
+
+ Reviewed by Carlos Alberto Lopez Perez.
+
+ This is currently happening in the GTK+ debug bot. There's a test that makes the browser crash due to an assert,
+ hanging the whole process and preventing the rest of the tests from running. When the browser crashes, we
+ correctly handle the pending requests, by completing them with an error. However, if the client tries to send
+ another command we fail to send the message to the browser and the reply is never sent to the client. In the
+ case of the tests, delete session command is sent, but never gets a reply.
+
+ * Session.cpp:
+ (WebDriver::Session::isConnected const): Return whether the session is connected to the browser.
+ * Session.h:
+ * SessionHost.cpp:
+ (WebDriver::SessionHost::sendCommandToBackend): Pass the message ID to SessionHost::sendMessageToBackend().
+ * SessionHost.h:
+ * WebDriverService.cpp:
+ (WebDriver::WebDriverService::deleteSession): Ignore unknown errors if the session is no longer connected.
+ * glib/SessionHostGlib.cpp:
+ (WebDriver::SessionHost::sendMessageToBackend): Handle errors when sending the command by completing the request
+ with an error.
+
+2018-01-26 Carlos Garcia Campos <[email protected]>
+
WebDriver: timeouts value and cookie expiry should be limited to max safe integer
https://bugs.webkit.org/show_bug.cgi?id=182167
Modified: trunk/Source/WebDriver/Session.cpp (227673 => 227674)
--- trunk/Source/WebDriver/Session.cpp 2018-01-26 10:46:38 UTC (rev 227673)
+++ trunk/Source/WebDriver/Session.cpp 2018-01-26 13:20:11 UTC (rev 227674)
@@ -69,6 +69,11 @@
return m_host->capabilities();
}
+bool Session::isConnected() const
+{
+ return m_host->isConnected();
+}
+
static std::optional<String> firstWindowHandleInResult(JSON::Value& result)
{
RefPtr<JSON::Array> handles;
Modified: trunk/Source/WebDriver/Session.h (227673 => 227674)
--- trunk/Source/WebDriver/Session.h 2018-01-26 10:46:38 UTC (rev 227673)
+++ trunk/Source/WebDriver/Session.h 2018-01-26 13:20:11 UTC (rev 227674)
@@ -48,6 +48,7 @@
const String& id() const;
const Capabilities& capabilities() const;
+ bool isConnected() const;
Seconds scriptTimeout() const { return m_scriptTimeout; }
Seconds pageLoadTimeout() const { return m_pageLoadTimeout; }
Seconds implicitWaitTimeout() const { return m_implicitWaitTimeout; }
Modified: trunk/Source/WebDriver/SessionHost.cpp (227673 => 227674)
--- trunk/Source/WebDriver/SessionHost.cpp 2018-01-26 10:46:38 UTC (rev 227673)
+++ trunk/Source/WebDriver/SessionHost.cpp 2018-01-26 13:20:11 UTC (rev 227674)
@@ -55,7 +55,7 @@
messageBuilder.append(parameters->toJSONString());
}
messageBuilder.append('}');
- sendMessageToBackend(messageBuilder.toString());
+ sendMessageToBackend(sequenceID, messageBuilder.toString());
return sequenceID;
}
Modified: trunk/Source/WebDriver/SessionHost.h (227673 => 227674)
--- trunk/Source/WebDriver/SessionHost.h 2018-01-26 10:46:38 UTC (rev 227673)
+++ trunk/Source/WebDriver/SessionHost.h 2018-01-26 13:20:11 UTC (rev 227674)
@@ -71,7 +71,7 @@
};
void inspectorDisconnected();
- void sendMessageToBackend(const String&);
+ void sendMessageToBackend(long, const String&);
void dispatchMessage(const String&);
#if USE(GLIB)
Modified: trunk/Source/WebDriver/WebDriverService.cpp (227673 => 227674)
--- trunk/Source/WebDriver/WebDriverService.cpp 2018-01-26 10:46:38 UTC (rev 227673)
+++ trunk/Source/WebDriver/WebDriverService.cpp 2018-01-26 13:20:11 UTC (rev 227674)
@@ -754,7 +754,11 @@
auto session = std::exchange(m_session, nullptr);
session->close([this, session, completionHandler = WTFMove(completionHandler)](CommandResult&& result) mutable {
- completionHandler(WTFMove(result));
+ // Ignore unknown errors when closing the session if the browser is closed.
+ if (result.isError() && result.errorCode() == CommandResult::ErrorCode::UnknownError && !session->isConnected())
+ completionHandler(CommandResult::success());
+ else
+ completionHandler(WTFMove(result));
});
}
Modified: trunk/Source/WebDriver/glib/SessionHostGlib.cpp (227673 => 227674)
--- trunk/Source/WebDriver/glib/SessionHostGlib.cpp 2018-01-26 10:46:38 UTC (rev 227673)
+++ trunk/Source/WebDriver/glib/SessionHostGlib.cpp 2018-01-26 13:20:11 UTC (rev 227674)
@@ -340,12 +340,18 @@
dispatchMessage(String::fromUTF8(message));
}
-void SessionHost::sendMessageToBackend(const String& message)
+struct MessageContext {
+ long messageID;
+ SessionHost* host;
+};
+
+void SessionHost::sendMessageToBackend(long messageID, const String& message)
{
ASSERT(m_dbusConnection);
ASSERT(m_connectionID);
ASSERT(m_target.id);
+ auto messageContext = std::make_unique<MessageContext>(MessageContext { messageID, this });
g_dbus_connection_call(m_dbusConnection.get(), nullptr,
INSPECTOR_DBUS_OBJECT_PATH,
INSPECTOR_DBUS_INTERFACE,
@@ -352,7 +358,20 @@
"SendMessageToBackend",
g_variant_new("(tts)", m_connectionID, m_target.id, message.utf8().data()),
nullptr, G_DBUS_CALL_FLAGS_NO_AUTO_START,
- -1, m_cancellable.get(), dbusConnectionCallAsyncReadyCallback, nullptr);
+ -1, m_cancellable.get(), [](GObject* source, GAsyncResult* result, gpointer userData) {
+ auto messageContext = std::unique_ptr<MessageContext>(static_cast<MessageContext*>(userData));
+ GUniqueOutPtr<GError> error;
+ GRefPtr<GVariant> resultVariant = adoptGRef(g_dbus_connection_call_finish(G_DBUS_CONNECTION(source), result, &error.outPtr()));
+ if (!resultVariant && !g_error_matches(error.get(), G_IO_ERROR, G_IO_ERROR_CANCELLED)) {
+ auto responseHandler = messageContext->host->m_commandRequests.take(messageContext->messageID);
+ if (responseHandler) {
+ auto errorObject = JSON::Object::create();
+ errorObject->setInteger(ASCIILiteral("code"), -32603);
+ errorObject->setString(ASCIILiteral("message"), String::fromUTF8(error->message));
+ responseHandler({ WTFMove(errorObject), true });
+ }
+ }
+ }, messageContext.release());
}
} // namespace WebDriver
