Bug 58223

Created attachment 88982 [details] Patch At the moment it simply adds the dummy files needed to build GTK+ port with plugin process enabled, it doesn't build the plugin process bin. It depends on patch attached to bug #55719

Attachment 88982 [details] did not pass style-queue: Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WebKit2/ChangeLog', u'Source/WebKit..." exit_code: 1 Source/WebKit2/PluginProcess/gtk/PluginProcessMainGtk.cpp:35: Alphabetical sorting problem. [build/include_order] [4] Total errors found: 1 in 9 files If any of these errors are false positives, please file a bug against check-webkit-style.

Attachment 88982 [details] did not build on qt: Build output: http://queues.webkit.org/results/8379790

Comment on attachment 88982 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=88982&action=review Nice. Not sure why the Qt build is broken with this patch, but double-check that it's unrelated before landing. >> Source/WebKit2/PluginProcess/gtk/PluginProcessMainGtk.cpp:35 >> +#include <WebCore/ResourceHandle.h> > > Alphabetical sorting problem. [build/include_order] [4] Please fix this one before landing.

(In reply to comment #4) > (From update of attachment 88982 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=88982&action=review > > Nice. Not sure why the Qt build is broken with this patch, but double-check that it's unrelated before landing. Because it depends on patch attached to bug #55719 that contains code common to GTK and Qt ports. > >> Source/WebKit2/PluginProcess/gtk/PluginProcessMainGtk.cpp:35 > >> +#include <WebCore/ResourceHandle.h> > > > > Alphabetical sorting problem. [build/include_order] [4] > > Please fix this one before landing. Sure. I'll land it as soon as bug #55719 is fixed. Thanks!

We don't need that amount of specific Qt and GTK files. My plan is to land the implementation for Qt first with no qt-specific code (bug 57617). Then we can move the files that are not really Qt specific to a unix directory and start using them on GTK. Does it sounds good to you?

Yes! I'd love to share as much of the plugin code with the Qt port as possible.

Created attachment 93123 [details] Patch It adds the stubs needed to build with ENABLE_PLUGIN_PROCESS=1. The macro is hardcoded in config.h for now, I'm not sure if we want to add a --enable-plugin-process configure option.

Comment on attachment 93123 [details] Patch Attachment 93123 [details] did not pass qt-ews (qt): Output: http://queues.webkit.org/results/8688348

Comment on attachment 93123 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=93123&action=review Is the ENABLE_PLUGIN_PROCESS macro used by all ports or just GTK+? > Source/WebKit2/config.h:65 > +#define ENABLE_PLUGIN_PROCESS 1 > + If the macro is used by other ports, this changes their default build.

(In reply to comment #10) > (From update of attachment 93123 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=93123&action=review > > Is the ENABLE_PLUGIN_PROCESS macro used by all ports or just GTK+? All ports. > > Source/WebKit2/config.h:65 > > +#define ENABLE_PLUGIN_PROCESS 1 > > + > > If the macro is used by other ports, this changes their default build. do we add a configure option then?

(In reply to comment #11) > > If the macro is used by other ports, this changes their default build. > do we add a configure option then? If we are sure that we always want it enabled, we can simply make sure to add the define in the makefile.

Created attachment 93416 [details] New patch Instead of hardcoding ENABLE_PLUGIN_PROCESS=1 in config.h, this patch adds a new configure option. Plugin process is enabled by default when building wk2.

Comment on attachment 93416 [details] New patch View in context: https://bugs.webkit.org/attachment.cgi?id=93416&action=review > configure.ac:1054 > +# build the plugin process only when building webkit2 Please use capital letter and a period here.

Committed r86545: <http://trac.webkit.org/changeset/86545>