<!DOCTYPE html PUBLIC "-//W3C//DTD XHTML 1.1//EN"
"http://www.w3.org/TR/xhtml11/DTD/xhtml11.dtd">
<html xmlns="http://www.w3.org/1999/xhtml">
<head><meta http-equiv="content-type" content="text/html; charset=utf-8" />
<title>[174054] trunk/Source/WebKit2</title>
</head>
<body>

<style type="text/css"><!--
#msg dl.meta { border: 1px #006 solid; background: #369; padding: 6px; color: #fff; }
#msg dl.meta dt { float: left; width: 6em; font-weight: bold; }
#msg dt:after { content:':';}
#msg dl, #msg dt, #msg ul, #msg li, #header, #footer, #logmsg { font-family: verdana,arial,helvetica,sans-serif; font-size: 10pt;  }
#msg dl a { font-weight: bold}
#msg dl a:link    { color:#fc3; }
#msg dl a:active  { color:#ff0; }
#msg dl a:visited { color:#cc6; }
h3 { font-family: verdana,arial,helvetica,sans-serif; font-size: 10pt; font-weight: bold; }
#msg pre { overflow: auto; background: #ffc; border: 1px #fa0 solid; padding: 6px; }
#logmsg { background: #ffc; border: 1px #fa0 solid; padding: 1em 1em 0 1em; }
#logmsg p, #logmsg pre, #logmsg blockquote { margin: 0 0 1em 0; }
#logmsg p, #logmsg li, #logmsg dt, #logmsg dd { line-height: 14pt; }
#logmsg h1, #logmsg h2, #logmsg h3, #logmsg h4, #logmsg h5, #logmsg h6 { margin: .5em 0; }
#logmsg h1:first-child, #logmsg h2:first-child, #logmsg h3:first-child, #logmsg h4:first-child, #logmsg h5:first-child, #logmsg h6:first-child { margin-top: 0; }
#logmsg ul, #logmsg ol { padding: 0; list-style-position: inside; margin: 0 0 0 1em; }
#logmsg ul { text-indent: -1em; padding-left: 1em; }#logmsg ol { text-indent: -1.5em; padding-left: 1.5em; }
#logmsg > ul, #logmsg > ol { margin: 0 0 1em 0; }
#logmsg pre { background: #eee; padding: 1em; }
#logmsg blockquote { border: 1px solid #fa0; border-left-width: 10px; padding: 1em 1em 0 1em; background: white;}
#logmsg dl { margin: 0; }
#logmsg dt { font-weight: bold; }
#logmsg dd { margin: 0; padding: 0 0 0.5em 0; }
#logmsg dd:before { content:'\00bb';}
#logmsg table { border-spacing: 0px; border-collapse: collapse; border-top: 4px solid #fa0; border-bottom: 1px solid #fa0; background: #fff; }
#logmsg table th { text-align: left; font-weight: normal; padding: 0.2em 0.5em; border-top: 1px dotted #fa0; }
#logmsg table td { text-align: right; border-top: 1px dotted #fa0; padding: 0.2em 0.5em; }
#logmsg table thead th { text-align: center; border-bottom: 1px solid #fa0; }
#logmsg table th.Corner { text-align: left; }
#logmsg hr { border: none 0; border-top: 2px dashed #fa0; height: 1px; }
#header, #footer { color: #fff; background: #636; border: 1px #300 solid; padding: 6px; }
#patch { width: 100%; }
#patch h4 {font-family: verdana,arial,helvetica,sans-serif;font-size:10pt;padding:8px;background:#369;color:#fff;margin:0;}
#patch .propset h4, #patch .binary h4 {margin:0;}
#patch pre {padding:0;line-height:1.2em;margin:0;}
#patch .diff {width:100%;background:#eee;padding: 0 0 10px 0;overflow:auto;}
#patch .propset .diff, #patch .binary .diff  {padding:10px 0;}
#patch span {display:block;padding:0 10px;}
#patch .modfile, #patch .addfile, #patch .delfile, #patch .propset, #patch .binary, #patch .copfile {border:1px solid #ccc;margin:10px 0;}
#patch ins {background:#dfd;text-decoration:none;display:block;padding:0 10px;}
#patch del {background:#fdd;text-decoration:none;display:block;padding:0 10px;}
#patch .lines, .info {color:#888;background:#fff;}
--></style>
<div id="msg">
<dl class="meta">
<dt>Revision</dt> <dd><a href="http://trac.webkit.org/projects/webkit/changeset/174054">174054</a></dd>
<dt>Author</dt> <dd>carlosgc@webkit.org</dd>
<dt>Date</dt> <dd>2014-09-29 01:51:51 -0700 (Mon, 29 Sep 2014)</dd>
</dl>

<h3>Log Message</h3>
<pre>REGRESSION(<a href="http://trac.webkit.org/projects/webkit/changeset/172919">r172919</a>): WebKitPluginProcess fails to scan GTK+2 plugins after <a href="http://trac.webkit.org/projects/webkit/changeset/172919">r172919</a>.
https://bugs.webkit.org/show_bug.cgi?id=137191

Reviewed by Philippe Normand.

In <a href="http://trac.webkit.org/projects/webkit/changeset/172919">r172919</a> 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.</pre>

<h3>Modified Paths</h3>
<ul>
<li><a href="#trunkSourceWebKit2ChangeLog">trunk/Source/WebKit2/ChangeLog</a></li>
<li><a href="#trunkSourceWebKit2SharedPluginsNetscapex11NetscapePluginModuleX11cpp">trunk/Source/WebKit2/Shared/Plugins/Netscape/x11/NetscapePluginModuleX11.cpp</a></li>
<li><a href="#trunkSourceWebKit2UIProcessPluginsunixPluginProcessProxyUnixcpp">trunk/Source/WebKit2/UIProcess/Plugins/unix/PluginProcessProxyUnix.cpp</a></li>
</ul>

</div>
<div id="patch">
<h3>Diff</h3>
<a id="trunkSourceWebKit2ChangeLog"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebKit2/ChangeLog (174053 => 174054)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/WebKit2/ChangeLog        2014-09-29 07:15:14 UTC (rev 174053)
+++ trunk/Source/WebKit2/ChangeLog        2014-09-29 08:51:51 UTC (rev 174054)
</span><span class="lines">@@ -1,3 +1,31 @@
</span><ins>+2014-09-29  Carlos Garcia Campos  &lt;cgarcia@igalia.com&gt;
+
+        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.
+
</ins><span class="cx"> 2014-09-28  Myles C. Maxfield  &lt;mmaxfield@apple.com&gt;
</span><span class="cx"> 
</span><span class="cx">         Replace wkGetGlyphsForCharacters() with CGFontGetGlyphsForUnichars()
</span></span></pre></div>
<a id="trunkSourceWebKit2SharedPluginsNetscapex11NetscapePluginModuleX11cpp"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebKit2/Shared/Plugins/Netscape/x11/NetscapePluginModuleX11.cpp (174053 => 174054)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/WebKit2/Shared/Plugins/Netscape/x11/NetscapePluginModuleX11.cpp        2014-09-29 07:15:14 UTC (rev 174053)
+++ trunk/Source/WebKit2/Shared/Plugins/Netscape/x11/NetscapePluginModuleX11.cpp        2014-09-29 08:51:51 UTC (rev 174054)
</span><span class="lines">@@ -152,10 +152,6 @@
</span><span class="cx"> 
</span><span class="cx">     metaData.mimeDescription = mimeDescription;
</span><span class="cx"> 
</span><del>-#if PLATFORM(GTK)
-    metaData.requiresGtk2 = module-&gt;functionPointer&lt;void (*)()&gt;(&quot;gtk_progress_get_type&quot;);
-#endif
-
</del><span class="cx">     return true;
</span><span class="cx"> }
</span><span class="cx"> 
</span><span class="lines">@@ -244,10 +240,6 @@
</span><span class="cx">     writeLine(metaData.name);
</span><span class="cx">     writeLine(metaData.description);
</span><span class="cx">     writeLine(metaData.mimeDescription);
</span><del>-#if PLATFORM(GTK)
-    if (metaData.requiresGtk2)
-        writeLine(&quot;requires-gtk2&quot;);
-#endif
</del><span class="cx"> 
</span><span class="cx">     fflush(stdout);
</span><span class="cx"> 
</span></span></pre></div>
<a id="trunkSourceWebKit2UIProcessPluginsunixPluginProcessProxyUnixcpp"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebKit2/UIProcess/Plugins/unix/PluginProcessProxyUnix.cpp (174053 => 174054)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/WebKit2/UIProcess/Plugins/unix/PluginProcessProxyUnix.cpp        2014-09-29 07:15:14 UTC (rev 174053)
+++ trunk/Source/WebKit2/UIProcess/Plugins/unix/PluginProcessProxyUnix.cpp        2014-09-29 08:51:51 UTC (rev 174054)
</span><span class="lines">@@ -1,5 +1,5 @@
</span><span class="cx"> /*
</span><del>- * Copyright (C) 2011 Igalia S.L.
</del><ins>+ * Copyright (C) 2011, 2014 Igalia S.L.
</ins><span class="cx">  * Copyright (C) 2011 Apple Inc.
</span><span class="cx">  * Copyright (C) 2012 Samsung Electronics
</span><span class="cx">  *
</span><span class="lines">@@ -33,14 +33,18 @@
</span><span class="cx"> #include &quot;PluginProcessCreationParameters.h&quot;
</span><span class="cx"> #include &quot;ProcessExecutablePath.h&quot;
</span><span class="cx"> #include &lt;WebCore/FileSystem.h&gt;
</span><ins>+#include &lt;sys/wait.h&gt;
</ins><span class="cx"> #include &lt;wtf/text/CString.h&gt;
</span><span class="cx"> #include &lt;wtf/text/WTFString.h&gt;
</span><ins>+
</ins><span class="cx"> #if PLATFORM(GTK) || PLATFORM(EFL)
</span><span class="cx"> #include &lt;glib.h&gt;
</span><span class="cx"> #include &lt;wtf/gobject/GUniquePtr.h&gt;
</span><span class="cx"> #endif
</span><span class="cx"> 
</span><del>-#include &lt;sys/wait.h&gt;
</del><ins>+#if PLATFORM(GTK)
+#include &quot;Module.h&quot;
+#endif
</ins><span class="cx"> 
</span><span class="cx"> using namespace WebCore;
</span><span class="cx"> 
</span><span class="lines">@@ -65,21 +69,40 @@
</span><span class="cx"> {
</span><span class="cx"> }
</span><span class="cx"> 
</span><ins>+#if PLATFORM(GTK)
+static bool pluginRequiresGtk2(const String&amp; pluginPath)
+{
+    std::unique_ptr&lt;Module&gt; module = std::make_unique&lt;Module&gt;(pluginPath);
+    if (!module-&gt;load())
+        return false;
+    return module-&gt;functionPointer&lt;gpointer&gt;(&quot;gtk_object_get_type&quot;);
+}
+#endif
+
</ins><span class="cx"> #if PLUGIN_ARCHITECTURE(X11)
</span><span class="cx"> bool PluginProcessProxy::scanPlugin(const String&amp; pluginPath, RawPluginMetaData&amp; result)
</span><span class="cx"> {
</span><span class="cx"> #if PLATFORM(GTK) || PLATFORM(EFL)
</span><del>-    CString binaryPath = fileSystemRepresentation(executablePathOfPluginProcess());
</del><ins>+    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);
</ins><span class="cx">     CString pluginPathCString = fileSystemRepresentation(pluginPath);
</span><span class="cx">     char* argv[4];
</span><span class="cx">     argv[0] = const_cast&lt;char*&gt;(binaryPath.data());
</span><span class="cx">     argv[1] = const_cast&lt;char*&gt;(&quot;-scanPlugin&quot;);
</span><span class="cx">     argv[2] = const_cast&lt;char*&gt;(pluginPathCString.data());
</span><del>-    argv[3] = 0;
</del><ins>+    argv[3] = nullptr;
</ins><span class="cx"> 
</span><del>-    int status;
-    GUniqueOutPtr&lt;char&gt; stdOut;
-
</del><span class="cx">     // If the disposition of SIGCLD signal is set to SIG_IGN (default)
</span><span class="cx">     // then the signal will be ignored and g_spawn_sync() will not be
</span><span class="cx">     // able to return the status.
</span><span class="lines">@@ -94,26 +117,37 @@
</span><span class="cx">     }
</span><span class="cx"> #endif
</span><span class="cx"> 
</span><del>-    if (!g_spawn_sync(0, argv, 0, G_SPAWN_STDERR_TO_DEV_NULL, 0, 0, &amp;stdOut.outPtr(), 0, &amp;status, 0))
</del><ins>+    int status;
+    GUniqueOutPtr&lt;char&gt; stdOut;
+    GUniqueOutPtr&lt;GError&gt; error;
+    if (!g_spawn_sync(nullptr, argv, nullptr, G_SPAWN_STDERR_TO_DEV_NULL, nullptr, nullptr, &amp;stdOut.outPtr(), nullptr, &amp;status, &amp;error.outPtr())) {
+        WTFLogAlways(&quot;Failed to launch %s: %s&quot;, argv[0], error-&gt;message);
</ins><span class="cx">         return false;
</span><ins>+    }
</ins><span class="cx"> 
</span><del>-    if (!WIFEXITED(status) || WEXITSTATUS(status) != EXIT_SUCCESS || !stdOut)
</del><ins>+    if (!WIFEXITED(status) || WEXITSTATUS(status) != EXIT_SUCCESS) {
+        WTFLogAlways(&quot;Error scanning plugin %s, %s returned %d exit status&quot;, argv[2], argv[0], status);
</ins><span class="cx">         return false;
</span><ins>+    }
</ins><span class="cx"> 
</span><del>-    String stdOutString = String::fromUTF8(stdOut.get());
</del><ins>+    if (!stdOut) {
+        WTFLogAlways(&quot;Error scanning plugin %s, %s didn't write any output to stdout&quot;, argv[2], argv[0]);
+        return false;
+    }
</ins><span class="cx"> 
</span><span class="cx">     Vector&lt;String&gt; lines;
</span><del>-    stdOutString.split(UChar('\n'), true, lines);
</del><ins>+    String::fromUTF8(stdOut.get()).split(UChar('\n'), true, lines);
</ins><span class="cx"> 
</span><del>-    if (lines.size() &lt; 3)
</del><ins>+    if (lines.size() &lt; 3) {
+        WTFLogAlways(&quot;Error scanning plugin %s, too few lines of output provided&quot;, argv[2]);
</ins><span class="cx">         return false;
</span><ins>+    }
</ins><span class="cx"> 
</span><span class="cx">     result.name.swap(lines[0]);
</span><span class="cx">     result.description.swap(lines[1]);
</span><span class="cx">     result.mimeDescription.swap(lines[2]);
</span><span class="cx"> #if PLATFORM(GTK)
</span><del>-    if (lines.size() &gt; 3)
-        result.requiresGtk2 = lines[3] == &quot;requires-gtk2&quot;;
</del><ins>+    result.requiresGtk2 = requiresGtk2;
</ins><span class="cx"> #endif
</span><span class="cx">     return !result.mimeDescription.isEmpty();
</span><span class="cx"> #else // PLATFORM(GTK) || PLATFORM(EFL)
</span></span></pre>
</div>
</div>

</body>
</html>