REGRESSION(r172919): WebKitPluginProcess fails to scan GTK+2 plugins after r172919.
authorcarlosgc@webkit.org <carlosgc@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 29 Sep 2014 08:51:51 +0000 (08:51 +0000)
committercarlosgc@webkit.org <carlosgc@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 29 Sep 2014 08:51:51 +0000 (08:51 +0000)
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.

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@174054 268f45cc-cd09-0410-ab3c-d52691b4dbfc

Source/WebKit2/ChangeLog
Source/WebKit2/Shared/Plugins/Netscape/x11/NetscapePluginModuleX11.cpp
Source/WebKit2/UIProcess/Plugins/unix/PluginProcessProxyUnix.cpp

index f254666..ca6bcd9 100644 (file)
@@ -1,3 +1,31 @@
+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>
 
         Replace wkGetGlyphsForCharacters() with CGFontGetGlyphsForUnichars()
index 859122b..eced05b 100644 (file)
@@ -152,10 +152,6 @@ bool NetscapePluginModule::getPluginInfoForLoadedPlugin(RawPluginMetaData& metaD
 
     metaData.mimeDescription = mimeDescription;
 
-#if PLATFORM(GTK)
-    metaData.requiresGtk2 = module->functionPointer<void (*)()>("gtk_progress_get_type");
-#endif
-
     return true;
 }
 
@@ -244,10 +240,6 @@ bool NetscapePluginModule::scanPlugin(const String& pluginPath)
     writeLine(metaData.name);
     writeLine(metaData.description);
     writeLine(metaData.mimeDescription);
-#if PLATFORM(GTK)
-    if (metaData.requiresGtk2)
-        writeLine("requires-gtk2");
-#endif
 
     fflush(stdout);
 
index 538dd79..70d873d 100644 (file)
@@ -1,5 +1,5 @@
 /*
- * Copyright (C) 2011 Igalia S.L.
+ * Copyright (C) 2011, 2014 Igalia S.L.
  * Copyright (C) 2011 Apple Inc.
  * Copyright (C) 2012 Samsung Electronics
  *
 #include "PluginProcessCreationParameters.h"
 #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>
 #include <wtf/gobject/GUniquePtr.h>
 #endif
 
-#include <sys/wait.h>
+#if PLATFORM(GTK)
+#include "Module.h"
+#endif
 
 using namespace WebCore;
 
@@ -65,20 +69,39 @@ void PluginProcessProxy::platformInitializePluginProcess(PluginProcessCreationPa
 {
 }
 
+#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];
     argv[0] = const_cast<char*>(binaryPath.data());
     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)
     // then the signal will be ignored and g_spawn_sync() will not be
@@ -94,26 +117,37 @@ bool PluginProcessProxy::scanPlugin(const String& pluginPath, RawPluginMetaData&
     }
 #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]);
     result.description.swap(lines[1]);
     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();
 #else // PLATFORM(GTK) || PLATFORM(EFL)