Bug 58223 - [GTK] Enable building GTK port with ENABLE_PLUGIN_PROCESS=1
Summary: [GTK] Enable building GTK port with ENABLE_PLUGIN_PROCESS=1
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKit2 (show other bugs)
Version: 528+ (Nightly build)
Hardware: PC Linux
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on: 55719 60628 60629
Blocks: 60546
  Show dependency treegraph
 
Reported: 2011-04-11 03:32 PDT by Carlos Garcia Campos
Modified: 2011-05-16 01:05 PDT (History)
4 users (show)

See Also:


Attachments
Patch (15.20 KB, patch)
2011-04-11 03:37 PDT, Carlos Garcia Campos
mrobinson: review+
mrobinson: commit-queue-
Details | Formatted Diff | Diff
Patch (11.18 KB, patch)
2011-05-11 08:40 PDT, Carlos Garcia Campos
webkit-ews: commit-queue-
Details | Formatted Diff | Diff
New patch (13.58 KB, patch)
2011-05-13 01:54 PDT, Carlos Garcia Campos
mrobinson: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
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>