Bug 58223 – [GTK] Enable building GTK port with ENABLE_PLUGIN_PROCESS=1

• Component: WebKit2
• Status: RESOLVED FIXED

[Carlos Garcia Campos]

GTK+ port doesn't currently build with ENABLE_PLUGIN_PROCESS=1

[Carlos Garcia Campos]

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

[WebKit Review Bot]

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.

[Early Warning System Bot]

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

[Martin Robinson]

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.

[Carlos Garcia Campos]

(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!

[Balazs Kelemen]

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?

[Martin Robinson]

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

[Carlos Garcia Campos]

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.

[Early Warning System Bot]

Comment on attachment 93123 [details]
Patch

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

[Martin Robinson]

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.

[Carlos Garcia Campos]

(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?

[Martin Robinson]

(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.

[Carlos Garcia Campos]

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.

[Martin Robinson]

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.

[Carlos Garcia Campos]

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