Attachment 361400 Details for Bug 194370 – Patch

[patch] Patch

bug-194370-20190207104618.patch (text/plain), 8.42 KB, created by Michael Catanzaro on 2019-02-07 08:46:19 PST

(hide)

Description:

Filename:

MIME Type:

Creator: Michael Catanzaro

Created: 2019-02-07 08:46:19 PST

Size: 8.42 KB

patch

obsolete

>Subversion Revision: 241024
>diff --git a/Source/JavaScriptCore/ChangeLog b/Source/JavaScriptCore/ChangeLog
>index f4449dc425515ccf320f79b1e498b6166e489d66..9cf9465e356c94ba9a5091c8ed94db333737425e 100644
>--- a/Source/JavaScriptCore/ChangeLog
>+++ b/Source/JavaScriptCore/ChangeLog
>@@ -1,3 +1,19 @@
>+2019-02-07  Michael Catanzaro  <mcatanzaro@igalia.com>
>+
>+        [WPE][GTK] Unsafe g_unsetenv() use in WebProcessPool::platformInitialize
>+        https://bugs.webkit.org/show_bug.cgi?id=194370
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        Change a couple WTFLogAlways to use g_warning, for good measure. Of course this isn't
>+        necessary, but it will make errors more visible.
>+
>+        * inspector/remote/glib/RemoteInspectorGlib.cpp:
>+        (Inspector::RemoteInspector::start):
>+        (Inspector::dbusConnectionCallAsyncReadyCallback):
>+        * inspector/remote/glib/RemoteInspectorServer.cpp:
>+        (Inspector::RemoteInspectorServer::start):
>+
> 2019-02-06  Pablo Saavedra  <psaavedra@igalia.com>
> 
>         Build failure after r240431
>diff --git a/Source/WebKit/ChangeLog b/Source/WebKit/ChangeLog
>index e6bab68afd569566b456240507bfd96d6e6e397d..14edd295bef5c18bf01529b5894d716109fa66ba 100644
>--- a/Source/WebKit/ChangeLog
>+++ b/Source/WebKit/ChangeLog
>@@ -1,3 +1,28 @@
>+2019-02-07  Michael Catanzaro  <mcatanzaro@igalia.com>
>+
>+        [WPE][GTK] Unsafe g_unsetenv() use in WebProcessPool::platformInitialize
>+        https://bugs.webkit.org/show_bug.cgi?id=194370
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        It is incorrect to use g_unsetenv() here because it is MT-Unsafe. We know that it is
>+        impossible and unreasonable to expect the application has not started other threads at this
>+        point, and threads will be calling getenv(). WebKit itself has probably already started
>+        threads of its own.
>+
>+        Fortunately, the remote inspector in the web process is already prepared to deal with
>+        failure to connect to the inspector server, so we don't need to do anything except stop
>+        messing with the environment.
>+
>+        Note these files are copies of each other. I'll merge them together in a follow-up patch.
>+
>+        * UIProcess/gtk/WebProcessPoolGtk.cpp:
>+        (WebKit::initializeRemoteInspectorServer):
>+        (WebKit::WebProcessPool::platformInitialize):
>+        * UIProcess/wpe/WebProcessPoolWPE.cpp:
>+        (WebKit::initializeRemoteInspectorServer):
>+        (WebKit::WebProcessPool::platformInitialize):
>+
> 2019-02-06  Michael Catanzaro  <mcatanzaro@igalia.com>
> 
>         REGRESSION(r240785): [SOUP] Broke cookie persistent storage
>diff --git a/Source/JavaScriptCore/inspector/remote/glib/RemoteInspectorGlib.cpp b/Source/JavaScriptCore/inspector/remote/glib/RemoteInspectorGlib.cpp
>index b73455a40cc15d56353ff694fb65340182924c48..c075698bd9bf6e7048c0241cbff8327956272a83 100644
>--- a/Source/JavaScriptCore/inspector/remote/glib/RemoteInspectorGlib.cpp
>+++ b/Source/JavaScriptCore/inspector/remote/glib/RemoteInspectorGlib.cpp
>@@ -79,7 +79,7 @@ void RemoteInspector::start()
>             if (GRefPtr<GDBusConnection> connection = adoptGRef(g_dbus_connection_new_for_address_finish(result, &error.outPtr())))
>                 inspector->setupConnection(WTFMove(connection));
>             else if (!g_error_matches(error.get(), G_IO_ERROR, G_IO_ERROR_CANCELLED))
>-                WTFLogAlways("RemoteInspector failed to connect to inspector server at: %s: %s", g_getenv("WEBKIT_INSPECTOR_SERVER"), error->message);
>+                g_warning("RemoteInspector failed to connect to inspector server at: %s: %s", g_getenv("WEBKIT_INSPECTOR_SERVER"), error->message);
>     }, this);
> }
> 
>@@ -178,7 +178,7 @@ static void dbusConnectionCallAsyncReadyCallback(GObject* source, GAsyncResult*
>     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))
>-        WTFLogAlways("RemoteInspector failed to send DBus message: %s", error->message);
>+        g_warning("RemoteInspector failed to send DBus message: %s", error->message);
> }
> 
> TargetListing RemoteInspector::listingForInspectionTarget(const RemoteInspectionTarget& target) const
>diff --git a/Source/JavaScriptCore/inspector/remote/glib/RemoteInspectorServer.cpp b/Source/JavaScriptCore/inspector/remote/glib/RemoteInspectorServer.cpp
>index dc09c72bbca298ac860d164d2f6e4847f03f4b90..453e3a000b2116d66a0ca6821e0b6694706a3b6c 100644
>--- a/Source/JavaScriptCore/inspector/remote/glib/RemoteInspectorServer.cpp
>+++ b/Source/JavaScriptCore/inspector/remote/glib/RemoteInspectorServer.cpp
>@@ -197,7 +197,7 @@ bool RemoteInspectorServer::start(const char* address, unsigned port)
>     m_dbusServer = adoptGRef(g_dbus_server_new_sync(dbusAddress.get(), G_DBUS_SERVER_FLAGS_AUTHENTICATION_ALLOW_ANONYMOUS, uid.get(), nullptr, m_cancellable.get(), &error.outPtr()));
>     if (!m_dbusServer) {
>         if (!g_error_matches(error.get(), G_IO_ERROR, G_IO_ERROR_CANCELLED))
>-            WTFLogAlways("Failed to start remote inspector server on %s: %s\n", dbusAddress.get(), error->message);
>+            g_warning("Failed to start remote inspector server on %s: %s\n", dbusAddress.get(), error->message);
>         return false;
>     }
> 
>diff --git a/Source/WebKit/UIProcess/gtk/WebProcessPoolGtk.cpp b/Source/WebKit/UIProcess/gtk/WebProcessPoolGtk.cpp
>index 6b2357404e03ae3021ae880b63a8cec0ee86f0f6..77fa1bc0abf065c4770d6f1b452e664a5681a637 100644
>--- a/Source/WebKit/UIProcess/gtk/WebProcessPoolGtk.cpp
>+++ b/Source/WebKit/UIProcess/gtk/WebProcessPoolGtk.cpp
>@@ -44,36 +44,34 @@
> namespace WebKit {
> 
> #if ENABLE(REMOTE_INSPECTOR)
>-static bool initializeRemoteInspectorServer(const char* address)
>+static void initializeRemoteInspectorServer(const char* address)
> {
>     if (Inspector::RemoteInspectorServer::singleton().isRunning())
>-        return true;
>+        return;
> 
>     if (!address[0])
>-        return false;
>+        return;
> 
>     GUniquePtr<char> inspectorAddress(g_strdup(address));
>     char* portPtr = g_strrstr(inspectorAddress.get(), ":");
>     if (!portPtr)
>-        return false;
>+        return;
> 
>     *portPtr = '\0';
>     portPtr++;
>     guint64 port = g_ascii_strtoull(portPtr, nullptr, 10);
>     if (!port)
>-        return false;
>+        return;
> 
>-    return Inspector::RemoteInspectorServer::singleton().start(inspectorAddress.get(), port);
>+    Inspector::RemoteInspectorServer::singleton().start(inspectorAddress.get(), port);
> }
> #endif
> 
> void WebProcessPool::platformInitialize()
> {
> #if ENABLE(REMOTE_INSPECTOR)
>-    if (const char* address = g_getenv("WEBKIT_INSPECTOR_SERVER")) {
>-        if (!initializeRemoteInspectorServer(address))
>-            g_unsetenv("WEBKIT_INSPECTOR_SERVER");
>-    }
>+    if (const char* address = g_getenv("WEBKIT_INSPECTOR_SERVER"))
>+        initializeRemoteInspectorServer(address);
> #endif
> }
> 
>diff --git a/Source/WebKit/UIProcess/wpe/WebProcessPoolWPE.cpp b/Source/WebKit/UIProcess/wpe/WebProcessPoolWPE.cpp
>index f51017a8ca29ed07994d7d207c95714112a38bf5..1d9e2f83e1431f7fe5e8f0b841e9edd54b2bfb4b 100644
>--- a/Source/WebKit/UIProcess/wpe/WebProcessPoolWPE.cpp
>+++ b/Source/WebKit/UIProcess/wpe/WebProcessPoolWPE.cpp
>@@ -51,36 +51,34 @@
> namespace WebKit {
> 
> #if ENABLE(REMOTE_INSPECTOR)
>-static bool initializeRemoteInspectorServer(const char* address)
>+static void initializeRemoteInspectorServer(const char* address)
> {
>     if (Inspector::RemoteInspectorServer::singleton().isRunning())
>-        return true;
>+        return;
> 
>     if (!address[0])
>-        return false;
>+        return;
> 
>     GUniquePtr<char> inspectorAddress(g_strdup(address));
>     char* portPtr = g_strrstr(inspectorAddress.get(), ":");
>     if (!portPtr)
>-        return false;
>+        return;
> 
>     *portPtr = '\0';
>     portPtr++;
>     guint64 port = g_ascii_strtoull(portPtr, nullptr, 10);
>     if (!port)
>-        return false;
>+        return;
> 
>-    return Inspector::RemoteInspectorServer::singleton().start(inspectorAddress.get(), port);
>+    Inspector::RemoteInspectorServer::singleton().start(inspectorAddress.get(), port);
> }
> #endif
> 
> void WebProcessPool::platformInitialize()
> {
> #if ENABLE(REMOTE_INSPECTOR)
>-    if (const char* address = g_getenv("WEBKIT_INSPECTOR_SERVER")) {
>-        if (!initializeRemoteInspectorServer(address))
>-            g_unsetenv("WEBKIT_INSPECTOR_SERVER");
>-    }
>+    if (const char* address = g_getenv("WEBKIT_INSPECTOR_SERVER"))
>+        initializeRemoteInspectorServer(address);
> #endif
> }
>

Flags:

darin: review+
commit-queue: commit-queue-

Actions: View | Formatted Diff | Diff

Attachments on bug 194370: 361399 | 361400