Bug 58223

Summary: [GTK] Enable building GTK port with ENABLE_PLUGIN_PROCESS=1
Product: WebKit Reporter: Carlos Garcia Campos <cgarcia>
Component: WebKit2Assignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Normal CC: kbalazs, mrobinson, webkit-ews, webkit.review.bot
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: PC   
OS: Linux   
Bug Depends on: 55719, 60628, 60629    
Bug Blocks: 60546    
Attachments:
Description Flags
Patch
mrobinson: review+, mrobinson: commit-queue-
Patch
webkit-ews: commit-queue-
New patch mrobinson: review+

Description Carlos Garcia Campos 2011-04-11 03:32:46 PDT
GTK+ port doesn't currently build with ENABLE_PLUGIN_PROCESS=1
Comment 1 Carlos Garcia Campos 2011-04-11 03:37:54 PDT
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
Comment 2 WebKit Review Bot 2011-04-11 03:40:13 PDT
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.
Comment 3 Early Warning System Bot 2011-04-11 03:53:05 PDT
Attachment 88982 [details] did not build on qt:
Build output: http://queues.webkit.org/results/8379790
Comment 4 Martin Robinson 2011-04-11 09:09:11 PDT
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.
Comment 5 Carlos Garcia Campos 2011-04-11 09:17:02 PDT
(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!
Comment 6 Balazs Kelemen 2011-04-14 05:44:27 PDT
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?
Comment 7 Martin Robinson 2011-04-14 08:16:34 PDT
Yes! I'd love to share as much of the plugin code with the Qt port as possible.
Comment 8 Carlos Garcia Campos 2011-05-11 08:40:05 PDT
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 9 Early Warning System Bot 2011-05-11 08:54:02 PDT
Comment on attachment 93123 [details]
Patch

Attachment 93123 [details] did not pass qt-ews (qt):
Output: http://queues.webkit.org/results/8688348
Comment 10 Martin Robinson 2011-05-11 09:52:01 PDT
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.
Comment 11 Carlos Garcia Campos 2011-05-11 10:20:30 PDT
(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?
Comment 12 Martin Robinson 2011-05-11 10:21:48 PDT
(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.
Comment 13 Carlos Garcia Campos 2011-05-13 01:54:46 PDT
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 14 Martin Robinson 2011-05-13 08:34:42 PDT
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.
Comment 15 Carlos Garcia Campos 2011-05-16 01:05:25 PDT
Committed r86545: <http://trac.webkit.org/changeset/86545>