+++ This bug was initially created as a clone of Bug #624422 +++ For the gtk2 plugin support in a gtk3 build we need to build both versions simultaneously.

IMHO we may also rename widget/gtk2 to widget/gtk in this bug. widget/gtk2 contains gtk2 and gtk3 code so it makes sense to move it to widget/gtk and put gtk2 plugin code to widget/gtk/gtk2 subdir.

(In reply to Martin Stránský from comment #1) > IMHO we may also rename widget/gtk2 to widget/gtk in this bug. Sounds good.

Build config update for gtk2->gtk directory rename

Try run for those two patches: https://tbpl.mozilla.org/?tree=Try&rev=d7374e389802

Comment on attachment 807151 [details] [diff] [review] build config update Note: The gtk2_override.h file is used for building gtk2 builds in gtk3 environment, it undefines gtk3 build flags. We can't do that on gcc command line because it's reverted by preprocessor. I'll attach gtk2_override.h in other patches.

Comment on attachment 807185 [details] [diff] [review] widget/gtk/gtk2 subdir patch Creates gtk2 subdirs under widget directory. It covers widget/gtk, widget/shared. The shared module is linked to widget instead to xpwidgets on gtk, otherwise we have to build xpwidgets for both gtk2 and gtk3 too.

Comment on attachment 807145 [details] [diff] [review] move files from gtk2 to gtk Please change the commit message to say this is moving widget/gtk2 to widget/gtk, and include the necessary build changes (without MOZ_ENABLE_GTK2_PLUGIN changes) so that this revision will build and run tests.

Comment on attachment 807151 [details] [diff] [review] build config update Please move the gtk2_override.h changes to the subdir patch.

Comment on attachment 807185 [details] [diff] [review] widget/gtk/gtk2 subdir patch I'll pass this to a build config peer. My suggestion to just build a standard GTK2 libxul in bug 624422 was assuming that such a build could use the existing buildmconfig, by doing two passes for example. If this is going to be in one pass, given the GTK2 build is just for the plugin, can we just not build widget for the plugin library? Does XPCOM give us enough separation to just not have the widget services/constructors? That would be simpler than adding build info for things that are not used.

(In reply to Karl Tomlinson (:karlt) from comment #11) > I'll pass this to a build config peer. > > My suggestion to just build a standard GTK2 libxul in bug 624422 was > assuming that such a build could use the existing buildmconfig, by doing two > passes for example. > > If this is going to be in one pass, given the GTK2 build is just for the > plugin, can we just not build widget for the plugin library? Does XPCOM > give us enough separation to just not have the widget services/constructors? > That would be simpler than adding build info for things that are not used. My recent solution divides the code to two parts - the one which links gtk libraries (mark is as "gtk code" and the one without gtk parts ("the rest"). So the approach it do build "gtk code" twice (for gtk2 and gtk3) and link with "the rest" to two libraries, libxul.so (for gtk3) and libxul_gtk2.so (for gtk2). Pros is the the "gtk code" is relatively small (widget/gtk, widget/shared, dom/plugins) so the build time is almost the same as a normal build. Cons is the you need extra directories for the gtk2 parts to allow them to build simultaneously. I can try to remove the widget module from libxul_gtk2.so library but it may take more time. I'd like to do such optimization later, when the whole solution works.

Gtk2 rename in one patch without gtk2_override.h Try: https://tbpl.mozilla.org/?tree=Try&rev=62594d75c865

Comment on attachment 807699 [details] [diff] [review] gtk2 rename patch Please land this patch after Bug 914607.

More patches are needed, sorry.

(In reply to Karl Tomlinson (:karlt) from comment #11) > My suggestion to just build a standard GTK2 libxul in bug 624422 was > assuming that such a build could use the existing buildmconfig, by doing two > passes for example. > > If this is going to be in one pass, given the GTK2 build is just for the > plugin, can we just not build widget for the plugin library? Does XPCOM > give us enough separation to just not have the widget services/constructors? > That would be simpler than adding build info for things that are not used. See my comment at https://bugzilla.mozilla.org/show_bug.cgi?id=624422#c15 - it's tricky to do such separation and even if we do so, the result would be difficult to maintain. It's not just widget/toolkit directory, the widget specific code is nested to other parts. More specifically, those directories use gtk code and needs to be build in gtk2 and gtk3 variants: dom/plugins/base dom/plugins/ipc gfx/gl gfx/thebes toolkit/components/remote widget/gtk widget/shared

Comment 11 was not about removing gtk use from every part of the code, but questioning whether some parts of the code were not necessary at all and so wouldn't need to be included at all in the "libxul_gtk2" for the plugin. I can imagine that gfx/thebes and gfx/gl would be hard to drop, and the plugin will need dom/plugins, but I would expect toolkit/components/remote to be easy to remove. I don't know about widget, so will leave this to your judgement. I've passed review to the build peers. Ted, I picked you because you commented in bug 624422, but feel free to redirect if you like.

Comment on attachment 807185 [details] [diff] [review] widget/gtk/gtk2 subdir patch Sorry, I'm not finding the time to review these patches. I'll punt to glandium who is probably more suited anyway.

(In reply to Karl Tomlinson (:karlt) from comment #19) > I can imagine that gfx/thebes and gfx/gl would be hard to drop, and the > plugin will need dom/plugins, but I would expect toolkit/components/remote > to be easy to remove. toolkit/components/remote is used by /toolkit/xre/nsAppRunner.cpp, which is executed by plugin-container...so removing remote leads to two-pass build of /toolkit/xre/ or nsAppRunner.cpp at least...or to nsAppRunnerGtk2.cpp file.

Comment on attachment 819736 [details] [diff] [review] patch to widget/shared Ahh, it's already included in the widget patch.

(In reply to Martin Stránský from comment #21) > (In reply to Karl Tomlinson (:karlt) from comment #19) > > I can imagine that gfx/thebes and gfx/gl would be hard to drop, and the > > plugin will need dom/plugins, but I would expect toolkit/components/remote > > to be easy to remove. > > toolkit/components/remote is used by /toolkit/xre/nsAppRunner.cpp, which is > executed by plugin-container...so removing remote leads to two-pass build of > /toolkit/xre/ or nsAppRunner.cpp at least...or to nsAppRunnerGtk2.cpp file. Isn't the only interface nsIRemoteService.idl? nsAppRunner looks like it tries to handle failure of do_GetService("@mozilla.org/toolkit/remote-service;1").

An updated one, with config changes.

Comment on attachment 820177 [details] [diff] [review] patch to widget, v.2 Review of attachment 820177 [details] [diff] [review]: ----------------------------------------------------------------- Same reason as bug 929408.

Closing as per bug 929408. The one-pass build seems to be too difficult to maintain.