Attachment 343320 Details for Bug 186915 – Patch

[patch] Patch

wd-session-capabilities.diff (text/plain), 10.63 KB, created by Carlos Garcia Campos on 2018-06-22 05:28:16 PDT

(hide)

Description:

Filename:

MIME Type:

Creator: Carlos Garcia Campos

Created: 2018-06-22 05:28:16 PDT

Size: 10.63 KB

patch

obsolete

>diff --git a/Source/JavaScriptCore/ChangeLog b/Source/JavaScriptCore/ChangeLog
>index d27cc6974fe..e7e6ef3c6d6 100644
>--- a/Source/JavaScriptCore/ChangeLog
>+++ b/Source/JavaScriptCore/ChangeLog
>@@ -1,3 +1,15 @@
>+2018-06-22  Carlos Garcia Campos  <cgarcia@igalia.com>
>+
>+        [GTK] WebDriver: use a dictionary for session capabilities in StartAutomationSession message
>+        https://bugs.webkit.org/show_bug.cgi?id=186915
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        Update StartAutomationSession message handling to receive a dictionary of session capabilities.
>+
>+        * inspector/remote/glib/RemoteInspectorServer.cpp:
>+        (Inspector::processSessionCapabilities): Helper method to process the session capabilities.
>+
> 2018-06-21  Mark Lam  <mark.lam@apple.com>
> 
>         WebKit (JavaScriptCore) compilation error with Clang â¥ 6.
>diff --git a/Source/JavaScriptCore/inspector/remote/glib/RemoteInspectorServer.cpp b/Source/JavaScriptCore/inspector/remote/glib/RemoteInspectorServer.cpp
>index 4df3d0c323f..dc09c72bbca 100644
>--- a/Source/JavaScriptCore/inspector/remote/glib/RemoteInspectorServer.cpp
>+++ b/Source/JavaScriptCore/inspector/remote/glib/RemoteInspectorServer.cpp
>@@ -79,8 +79,7 @@ static const char introspectionXML[] =
>     "    </method>"
>     "    <method name='StartAutomationSession'>"
>     "      <arg type='s' name='sessionID' direction='in'/>"
>-    "      <arg type='b' name='acceptInsecureCertificates' direction='in'/>"
>-    "      <arg type='a(ss)' name='certificates' direction='in'/>"
>+    "      <arg type='a{sv}' name='capabilities' direction='in'/>"
>     "      <arg type='s' name='browserName' direction='out'/>"
>     "      <arg type='s' name='browserVersion' direction='out'/>"
>     "    </method>"
>@@ -88,6 +87,26 @@ static const char introspectionXML[] =
>     "</node>";
> }
> 
>+static RemoteInspector::Client::SessionCapabilities processSessionCapabilities(GVariant* sessionCapabilities)
>+{
>+    RemoteInspector::Client::SessionCapabilities capabilities;
>+
>+    gboolean acceptInsecureCerts;
>+    if (g_variant_lookup(sessionCapabilities, "acceptInsecureCerts", "b", &acceptInsecureCerts))
>+        capabilities.acceptInsecureCertificates = acceptInsecureCerts;
>+
>+    if (GRefPtr<GVariant> certificates = g_variant_lookup_value(sessionCapabilities, "certificates",  G_VARIANT_TYPE("a(ss)"))) {
>+        GVariantIter iter;
>+        auto childCount = g_variant_iter_init(&iter, certificates.get());
>+        capabilities.certificates.reserveCapacity(childCount);
>+        const char* host;
>+        const char* certificateFile;
>+        while (g_variant_iter_loop(&iter, "(&s&s)", &host, &certificateFile))
>+            capabilities.certificates.uncheckedAppend({ String::fromUTF8(host), String::fromUTF8(certificateFile) });
>+    }
>+
>+    return capabilities;
>+}
> const GDBusInterfaceVTable RemoteInspectorServer::s_interfaceVTable = {
>     // method_call
>     [] (GDBusConnection* connection, const gchar* /*sender*/, const gchar* /*objectPath*/, const gchar* /*interfaceName*/, const gchar* methodName, GVariant* parameters, GDBusMethodInvocation* invocation, gpointer userData) {
>@@ -124,16 +143,9 @@ const GDBusInterfaceVTable RemoteInspectorServer::s_interfaceVTable = {
>             g_dbus_method_invocation_return_value(invocation, nullptr);
>         } else if (!g_strcmp0(methodName, "StartAutomationSession")) {
>             const char* sessionID;
>-            gboolean acceptInsecureCertificates;
>-            GUniqueOutPtr<GVariantIter> certificates;
>-            g_variant_get(parameters, "(&sba(ss))", &sessionID, &acceptInsecureCertificates, &certificates.outPtr());
>-            RemoteInspector::Client::SessionCapabilities capabilities;
>-            capabilities.acceptInsecureCertificates = acceptInsecureCertificates;
>-            capabilities.certificates.reserveCapacity(g_variant_iter_n_children(certificates.get()));
>-            const char* host;
>-            const char* certificateFile;
>-            while (g_variant_iter_loop(certificates.get(), "(&s&s)", &host, &certificateFile))
>-                capabilities.certificates.uncheckedAppend({ String::fromUTF8(host), String::fromUTF8(certificateFile) });
>+            GRefPtr<GVariant> sessionCapabilities;
>+            g_variant_get(parameters, "(&s@a{sv})", &sessionID, &sessionCapabilities.outPtr());
>+            auto capabilities = processSessionCapabilities(sessionCapabilities.get());
>             inspectorServer->startAutomationSession(connection, sessionID, capabilities);
>             auto clientCapabilities = RemoteInspector::singleton().clientCapabilities();
>             g_dbus_method_invocation_return_value(invocation, g_variant_new("(ss)",
>diff --git a/Source/WebDriver/ChangeLog b/Source/WebDriver/ChangeLog
>index 3fb27dc398b..1a6eb4488c6 100644
>--- a/Source/WebDriver/ChangeLog
>+++ b/Source/WebDriver/ChangeLog
>@@ -1,3 +1,18 @@
>+2018-06-22  Carlos Garcia Campos  <cgarcia@igalia.com>
>+
>+        [GTK] WebDriver: use a dictionary for session capabilities in StartAutomationSession message
>+        https://bugs.webkit.org/show_bug.cgi?id=186915
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        Instead of growing the message signature every time we add a new capability, use a dictionary a{sv} and simply add
>+        new entries when new capabilities are added. This way we won't need to change the message signature anymore.
>+
>+        * SessionHost.h:
>+        * glib/SessionHostGlib.cpp:
>+        (WebDriver::SessionHost::buildSessionCapabilities const): Helper to build the session capabilities dictionary.
>+        (WebDriver::SessionHost::startAutomationSession):
>+
> 2018-06-21  Zan Dobersek  <zdobersek@igalia.com>
> 
>         [GTK] WebDriver: allow applying host-specific TLS certificates for automated sessions
>diff --git a/Source/WebDriver/SessionHost.h b/Source/WebDriver/SessionHost.h
>index 79ce9c1f906..8c476760684 100644
>--- a/Source/WebDriver/SessionHost.h
>+++ b/Source/WebDriver/SessionHost.h
>@@ -80,6 +80,7 @@ private:
>     void launchBrowser(Function<void (std::optional<String> error)>&&);
>     void connectToBrowser(std::unique_ptr<ConnectToBrowserAsyncData>&&);
>     bool matchCapabilities(GVariant*);
>+    GVariant* buildSessionCapabilities() const;
>     void setupConnection(GRefPtr<GDBusConnection>&&);
>     void setTargetList(uint64_t connectionID, Vector<Target>&&);
>     void sendMessageToFrontend(uint64_t connectionID, uint64_t targetID, const char* message);
>diff --git a/Source/WebDriver/glib/SessionHostGlib.cpp b/Source/WebDriver/glib/SessionHostGlib.cpp
>index b78e9e1d01b..42285c659ed 100644
>--- a/Source/WebDriver/glib/SessionHostGlib.cpp
>+++ b/Source/WebDriver/glib/SessionHostGlib.cpp
>@@ -261,25 +261,40 @@ bool SessionHost::matchCapabilities(GVariant* capabilities)
>     return didMatch;
> }
> 
>-void SessionHost::startAutomationSession(Function<void (bool, std::optional<String>)>&& completionHandler)
>+GVariant* SessionHost::buildSessionCapabilities() const
> {
>-    ASSERT(m_dbusConnection);
>-    ASSERT(!m_startSessionCompletionHandler);
>-    m_startSessionCompletionHandler = WTFMove(completionHandler);
>-    m_sessionID = createCanonicalUUIDString();
>+    if (!m_capabilities.acceptInsecureCerts && !m_capabilities.certificates)
>+        return nullptr;
>+
>     GVariantBuilder builder;
>-    g_variant_builder_init(&builder, G_VARIANT_TYPE("a(ss)"));
>+    g_variant_builder_init(&builder, G_VARIANT_TYPE("a{sv}"));
>+    if (m_capabilities.acceptInsecureCerts)
>+        g_variant_builder_add(&builder, "{sv}", "acceptInsecureCerts", g_variant_new_boolean(m_capabilities.acceptInsecureCerts.value()));
>+
>     if (m_capabilities.certificates) {
>+        GVariantBuilder arrayBuilder;
>+        g_variant_builder_init(&arrayBuilder, G_VARIANT_TYPE("a(ss)"));
>         for (auto& certificate : *m_capabilities.certificates) {
>-            g_variant_builder_add_value(&builder, g_variant_new("(ss)",
>+            g_variant_builder_add_value(&arrayBuilder, g_variant_new("(ss)",
>                 certificate.first.utf8().data(), certificate.second.utf8().data()));
>         }
>+        g_variant_builder_add(&builder, "{sv}", "certificates", g_variant_builder_end(&arrayBuilder));
>     }
>+
>+    return g_variant_builder_end(&builder);
>+}
>+
>+void SessionHost::startAutomationSession(Function<void (bool, std::optional<String>)>&& completionHandler)
>+{
>+    ASSERT(m_dbusConnection);
>+    ASSERT(!m_startSessionCompletionHandler);
>+    m_startSessionCompletionHandler = WTFMove(completionHandler);
>+    m_sessionID = createCanonicalUUIDString();
>     g_dbus_connection_call(m_dbusConnection.get(), nullptr,
>         INSPECTOR_DBUS_OBJECT_PATH,
>         INSPECTOR_DBUS_INTERFACE,
>         "StartAutomationSession",
>-        g_variant_new("(sba(ss))", m_sessionID.utf8().data(), m_capabilities.acceptInsecureCerts.value_or(false), &builder),
>+        g_variant_new("(s@a{sv})", m_sessionID.utf8().data(), buildSessionCapabilities()),
>         nullptr, G_DBUS_CALL_FLAGS_NO_AUTO_START,
>         -1, m_cancellable.get(), [](GObject* source, GAsyncResult* result, gpointer userData) {
>             GUniqueOutPtr<GError> error;
>diff --git a/Tools/ChangeLog b/Tools/ChangeLog
>index 3c87fc1987c..ebe000a8c3c 100644
>--- a/Tools/ChangeLog
>+++ b/Tools/ChangeLog
>@@ -1,3 +1,14 @@
>+2018-06-22  Carlos Garcia Campos  <cgarcia@igalia.com>
>+
>+        [GTK] WebDriver: use a dictionary for session capabilities in StartAutomationSession message
>+        https://bugs.webkit.org/show_bug.cgi?id=186915
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        Update /webkit/WebKitAutomationSession/request-session to use the new StartAutomationSession message signature.
>+
>+        * TestWebKitAPI/Tests/WebKitGLib/TestAutomationSession.cpp:
>+
> 2018-06-22  Carlos Garcia Campos  <cgarcia@igalia.com>
> 
>         Unreviewed. Fix GLib test /webkit/WebKitSecurityManager/file-xhr after r231000
>diff --git a/Tools/TestWebKitAPI/Tests/WebKitGLib/TestAutomationSession.cpp b/Tools/TestWebKitAPI/Tests/WebKitGLib/TestAutomationSession.cpp
>index 26c374e6d02..fb8a9ffa84a 100644
>--- a/Tools/TestWebKitAPI/Tests/WebKitGLib/TestAutomationSession.cpp
>+++ b/Tools/TestWebKitAPI/Tests/WebKitGLib/TestAutomationSession.cpp
>@@ -208,7 +208,7 @@ public:
>     {
>         auto signalID = g_signal_connect(m_webContext.get(), "automation-started", G_CALLBACK(automationStartedCallback), this);
>         g_dbus_connection_call(m_connection.get(), nullptr, "/org/webkit/Inspector", "org.webkit.Inspector",
>-            "StartAutomationSession", g_variant_new("(s)", sessionID), nullptr, G_DBUS_CALL_FLAGS_NO_AUTO_START, -1, nullptr,
>+            "StartAutomationSession", g_variant_new("(sa{sv})", sessionID, nullptr), nullptr, G_DBUS_CALL_FLAGS_NO_AUTO_START, -1, nullptr,
>             [](GObject* source, GAsyncResult* result, gpointer userData) {
>                 auto* test = static_cast<AutomationTest*>(userData);
>                 if (!test->m_session)

Flags:

zan: review+

Actions: View | Formatted Diff | Diff

Attachments on bug 186915: 343320