Changeset 174054 in webkit

Timestamp:

Sep 29, 2014 1:51:51 AM (10 years ago)

Author:

Carlos Garcia Campos

Message:

REGRESSION(r172919): WebKitPluginProcess fails to scan GTK+2 plugins after r172919.
​https://bugs.webkit.org/show_bug.cgi?id=137191

Reviewed by Philippe Normand.

In r172919 I moved the GTK+ symbols mix check earlier, before the
plugin is loaded and initialized. That made impossible to use the
GTK3 plugin process to scan gtk2 plugins, because we need to load
the plugin to get its metadata. But we don't need to initialize
the plugin to check if it requires GTK2, so we can do that check
in the UI process to decide which plugin process to use.

• Shared/Plugins/Netscape/x11/NetscapePluginModuleX11.cpp:

(WebKit::NetscapePluginModule::getPluginInfoForLoadedPlugin):
Remove the requires GTK2 check.
(WebKit::NetscapePluginModule::scanPlugin): Don't write
requires-gtk2 to stdout.

• UIProcess/Plugins/unix/PluginProcessProxyUnix.cpp:

(WebKit::pluginRequiresGtk2): Helper function to check if the
given plugin path requires GTK2.
(WebKit::PluginProcessProxy::scanPlugin): Check if the plugin path
requires GTK2 and use WebKitPluginProcess2 in such case, or return
early if GTK2 plugins are not enabled. Log error messages when
something fails scanning the plugin to make it easiert to debug
problems in the future.

Location:

trunk/Source/WebKit2

Files:

• ChangeLog (modified) (1 diff)
• Shared/Plugins/Netscape/x11/NetscapePluginModuleX11.cpp (modified) (2 diffs)
• UIProcess/Plugins/unix/PluginProcessProxyUnix.cpp (modified) (7 diffs)

trunk/Source/WebKit2/ChangeLog

@@ +1 @@
+2014-09-29  Carlos Garcia Campos  <cgarcia@igalia.com>
+
+        REGRESSION(r172919): WebKitPluginProcess fails to scan GTK+2 plugins after r172919.
+        https://bugs.webkit.org/show_bug.cgi?id=137191
+
+        Reviewed by Philippe Normand.
+
+        In r172919 I moved the GTK+ symbols mix check earlier, before the
+        plugin is loaded and initialized. That made impossible to use the
+        GTK3 plugin process to scan gtk2 plugins, because we need to load
+        the plugin to get its metadata. But we don't need to initialize
+        the plugin to check if it requires GTK2, so we can do that check
+        in the UI process to decide which plugin process to use.
+
+        * Shared/Plugins/Netscape/x11/NetscapePluginModuleX11.cpp:
+        (WebKit::NetscapePluginModule::getPluginInfoForLoadedPlugin):
+        Remove the requires GTK2 check.
+        (WebKit::NetscapePluginModule::scanPlugin): Don't write
+        requires-gtk2 to stdout.
+        * UIProcess/Plugins/unix/PluginProcessProxyUnix.cpp:
+        (WebKit::pluginRequiresGtk2): Helper function to check if the
+        given plugin path requires GTK2.
+        (WebKit::PluginProcessProxy::scanPlugin): Check if the plugin path
+        requires GTK2 and use WebKitPluginProcess2 in such case, or return
+        early if GTK2 plugins are not enabled. Log error messages when
+        something fails scanning the plugin to make it easiert to debug
+        problems in the future.
+
 2014-09-28  Myles C. Maxfield  <mmaxfield@apple.com>
 

trunk/Source/WebKit2/Shared/Plugins/Netscape/x11/NetscapePluginModuleX11.cpp

@@ -153 +153 @@
     metaData.mimeDescription = mimeDescription;
 
-#if PLATFORM(GTK)
-    metaData.requiresGtk2 = module->functionPointer<void (*)()>("gtk_progress_get_type");
-#endif
-
     return true;
 }
@@ -245 +241 @@
     writeLine(metaData.description);
     writeLine(metaData.mimeDescription);
-#if PLATFORM(GTK)
-    if (metaData.requiresGtk2)
-        writeLine("requires-gtk2");
-#endif
 
     fflush(stdout);

trunk/Source/WebKit2/UIProcess/Plugins/unix/PluginProcessProxyUnix.cpp

@@ -1 +1 @@
 /*
- * Copyright (C) 2011 Igalia S.L.
+ * Copyright (C) 2011, 2014 Igalia S.L.
  * Copyright (C) 2011 Apple Inc.
  * Copyright (C) 2012 Samsung Electronics
@@ -34 +34 @@
 #include "ProcessExecutablePath.h"
 #include <WebCore/FileSystem.h>
+#include <sys/wait.h>
 #include <wtf/text/CString.h>
 #include <wtf/text/WTFString.h>
+
 #if PLATFORM(GTK) || PLATFORM(EFL)
 #include <glib.h>
@@ -41 +43 @@
 #endif
 
-#include <sys/wait.h>
+#if PLATFORM(GTK)
+#include "Module.h"
+#endif
 
 using namespace WebCore;
@@ -66 +70 @@
 }
 
+#if PLATFORM(GTK)
+static bool pluginRequiresGtk2(const String& pluginPath)
+{
+    std::unique_ptr<Module> module = std::make_unique<Module>(pluginPath);
+    if (!module->load())
+        return false;
+    return module->functionPointer<gpointer>("gtk_object_get_type");
+}
+#endif
+
 #if PLUGIN_ARCHITECTURE(X11)
 bool PluginProcessProxy::scanPlugin(const String& pluginPath, RawPluginMetaData& result)
 {
 #if PLATFORM(GTK) || PLATFORM(EFL)
-    CString binaryPath = fileSystemRepresentation(executablePathOfPluginProcess());
+    String pluginProcessPath = executablePathOfPluginProcess();
+
+#if PLATFORM(GTK)
+    bool requiresGtk2 = pluginRequiresGtk2(pluginPath);
+    if (requiresGtk2)
+#if ENABLE(PLUGIN_PROCESS_GTK2)
+        pluginProcessPath.append('2');
+#else
+        return false;
+#endif
+#endif
+
+    CString binaryPath = fileSystemRepresentation(pluginProcessPath);
     CString pluginPathCString = fileSystemRepresentation(pluginPath);
     char* argv[4];
@@ -76 +102 @@
     argv[1] = const_cast<char*>("-scanPlugin");
     argv[2] = const_cast<char*>(pluginPathCString.data());
-    argv[3] = 0;
-
-    int status;
-    GUniqueOutPtr<char> stdOut;
+    argv[3] = nullptr;
 
     // If the disposition of SIGCLD signal is set to SIG_IGN (default)
@@ -95 +118 @@
 #endif
 
-    if (!g_spawn_sync(0, argv, 0, G_SPAWN_STDERR_TO_DEV_NULL, 0, 0, &stdOut.outPtr(), 0, &status, 0))
+    int status;
+    GUniqueOutPtr<char> stdOut;
+    GUniqueOutPtr<GError> error;
+    if (!g_spawn_sync(nullptr, argv, nullptr, G_SPAWN_STDERR_TO_DEV_NULL, nullptr, nullptr, &stdOut.outPtr(), nullptr, &status, &error.outPtr())) {
+        WTFLogAlways("Failed to launch %s: %s", argv[0], error->message);
         return false;
+    }
 
-    if (!WIFEXITED(status) || WEXITSTATUS(status) != EXIT_SUCCESS || !stdOut)
+    if (!WIFEXITED(status) || WEXITSTATUS(status) != EXIT_SUCCESS) {
+        WTFLogAlways("Error scanning plugin %s, %s returned %d exit status", argv[2], argv[0], status);
         return false;
+    }
 
-    String stdOutString = String::fromUTF8(stdOut.get());
+    if (!stdOut) {
+        WTFLogAlways("Error scanning plugin %s, %s didn't write any output to stdout", argv[2], argv[0]);
+        return false;
+    }
 
     Vector<String> lines;
-    stdOutString.split(UChar('\n'), true, lines);
+    String::fromUTF8(stdOut.get()).split(UChar('\n'), true, lines);
 
-    if (lines.size() < 3)
+    if (lines.size() < 3) {
+        WTFLogAlways("Error scanning plugin %s, too few lines of output provided", argv[2]);
         return false;
+    }
 
     result.name.swap(lines[0]);
@@ -113 +148 @@
     result.mimeDescription.swap(lines[2]);
 #if PLATFORM(GTK)
-    if (lines.size() > 3)
-        result.requiresGtk2 = lines[3] == "requires-gtk2";
+    result.requiresGtk2 = requiresGtk2;
 #endif
     return !result.mimeDescription.isEmpty();