Bug 60539 - [GTK] Split libWebCore into two libWebCore and libWebCoreGtk
Summary: [GTK] Split libWebCore into two libWebCore and libWebCoreGtk
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: Gtk
Depends on:
Blocks: 60546
  Show dependency treegraph
 
Reported: 2011-05-10 01:40 PDT by Carlos Garcia Campos
Modified: 2011-06-20 09:40 PDT (History)
4 users (show)

See Also:


Attachments
Patch (17.11 KB, patch)
2011-05-10 01:47 PDT, Carlos Garcia Campos
no flags Details | Formatted Diff | Diff
Updated patch (18.37 KB, patch)
2011-06-03 06:03 PDT, Carlos Garcia Campos
gustavo: commit-queue-
Details | Formatted Diff | Diff
Patch updated to apply on current git master (19.11 KB, patch)
2011-06-20 04:05 PDT, Carlos Garcia Campos
xan.lopez: 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-05-10 01:40:52 PDT
The motivation is to build the plugin process with gtk2 even when --with-gtk=3.0 has been passed to configure. The idea is the following:

 - libWebCore contains all the webcore files except the ones actually using gtk (there are some files in gtk dirs that don't use gtk at all)
 - libWebCoreGtk contains only the files using gtk
 - Both libs are built always using the gtk passed to the configure script
 - webkit1, webkit2 and webprocess link directly to both libraries
 - pluginprocess will link to libWebCore, and only when gtk2 has been passed to configure, it will also link to libWebCoreGtk. When building with gtk3, plugin process will use libWebCoreGtk sources as part of its sources, so that they will be built again using gtk2.

This will allow us to use the flash plugin when building webkit with gtk3.
Comment 1 Carlos Garcia Campos 2011-05-10 01:47:12 PDT
Created attachment 92930 [details]
Patch
Comment 2 Martin Robinson 2011-05-10 08:41:46 PDT
Comment on attachment 92930 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=92930&action=review

This looks fine to me, but I'd like to get Xan and Kov to look at this one as well.

> Source/WebCore/GNUmakefile.am:762
> +libWebCoreGtk_la_CPPFLAGS = \
> +	-DBUILDING_WEBKIT \
> +	$(global_cppflags) \

Would it make sense to remove GTK_CLFAGS here?
Comment 3 Carlos Garcia Campos 2011-05-30 01:01:22 PDT
(In reply to comment #2)
> (From update of attachment 92930 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=92930&action=review
> 
> This looks fine to me, but I'd like to get Xan and Kov to look at this one as well.
> 
> > Source/WebCore/GNUmakefile.am:762
> > +libWebCoreGtk_la_CPPFLAGS = \
> > +	-DBUILDING_WEBKIT \
> > +	$(global_cppflags) \
> 
> Would it make sense to remove GTK_CLFAGS here?

global_cppflags contains some flags for GTK (deprecation guards, etc.), but I think it doesn't contain GTK_CFLAGS. Anyway, as long as it doesn't link to GTK, it's not a problem.
Comment 4 Carlos Garcia Campos 2011-06-03 06:03:20 PDT
Created attachment 95903 [details]
Updated patch

Patch updated to current git master. I also moved FileChooserGtk.cpp from webcoregtk to webcore since it doesn't actually use gtk and all freetype files except FontPlatformDataFreeType.cpp which is the only one using gtk
Comment 5 Gustavo Noronha (kov) 2011-06-03 06:07:33 PDT
Comment on attachment 95903 [details]
Updated patch

Attachment 95903 [details] did not pass gtk-ews (gtk):
Output: http://queues.webkit.org/results/8763088
Comment 6 Xan Lopez 2011-06-03 07:09:43 PDT
Comment on attachment 95903 [details]
Updated patch

Cannot find anything weird here. At some point it would be interesting to add a comment explaining why we have the libraries we have here, otherwise I suppose it's not easy to figure out our makefiles.
Comment 7 Carlos Garcia Campos 2011-06-03 09:52:25 PDT
The patch works for me on a clean build, I think the bot doesn't rebuild libWebCore, it only builds libWebCoreGtk so that all symbols in libWebCoreGtk are already in libWebCore.
Comment 8 Carlos Garcia Campos 2011-06-20 04:05:55 PDT
Created attachment 97777 [details]
Patch updated to apply on current git master

I've also added a comment explaining why we need this split, as suggested by Xan.
Comment 9 Xan Lopez 2011-06-20 09:10:25 PDT
Comment on attachment 97777 [details]
Patch updated to apply on current git master

Looks good to me.
Comment 10 Carlos Garcia Campos 2011-06-20 09:40:26 PDT
Committed r89251: <http://trac.webkit.org/changeset/89251>