RESOLVED FIXED 105631
[GTK] Add support for loading web process extensions
https://bugs.webkit.org/show_bug.cgi?id=105631
Summary [GTK] Add support for loading web process extensions
Carlos Garcia Campos
Reported 2012-12-21 05:39:24 PST
Add support for loading plugins for the web process (web extensions) using injected bundle.
Attachments
Patch (56.74 KB, patch)
2012-12-21 05:57 PST, Carlos Garcia Campos
buildbot: commit-queue-
Updated patch (59.17 KB, patch)
2012-12-29 01:59 PST, Carlos Garcia Campos
sam: review-
New patch (65.29 KB, patch)
2013-01-02 06:47 PST, Carlos Garcia Campos
gustavo: review+
Patch for landing (65.21 KB, patch)
2013-01-10 02:49 PST, Carlos Garcia Campos
no flags
Carlos Garcia Campos
Comment 1 2012-12-21 05:57:07 PST
WebKit Review Bot
Comment 2 2012-12-21 06:00:11 PST
Thanks for the patch. If this patch contains new public API please make sure it follows the guidelines for new WebKit2 GTK+ API. See http://trac.webkit.org/wiki/WebKitGTK/AddingNewWebKit2API
Build Bot
Comment 3 2012-12-21 07:24:11 PST
Comment on attachment 180511 [details] Patch Attachment 180511 [details] did not pass mac-ews (mac): Output: http://queues.webkit.org/results/15454419 New failing tests: svg/as-image/img-preserveAspectRatio-support-2.html
Carlos Garcia Campos
Comment 4 2012-12-29 01:59:36 PST
Created attachment 180924 [details] Updated patch Removed the single header check added to the main web extension header by mistake.
Carlos Garcia Campos
Comment 5 2012-12-29 02:00:29 PST
See this ephy bug for an example of how this would be used by apps: https://bugzilla.gnome.org/show_bug.cgi?id=684437
Sam Weinig
Comment 6 2012-12-30 11:01:13 PST
I see this kind of uses the InjectedBundle, but also kind of doesn't (you have extra code in WebProcessSoup). Why can't this all be done on top of the InjectedBundle mechanism and the WebKit2 API?
Carlos Garcia Campos
Comment 7 2012-12-31 00:42:37 PST
(In reply to comment #6) > I see this kind of uses the InjectedBundle, but also kind of doesn't (you have extra code in WebProcessSoup). Why can't this all be done on top of the InjectedBundle mechanism and the WebKit2 API? This is just a first patch, we will use the injected bundle C API to implement things like bug #79918 and bug #99695. But instead of allowing apps to install their own injected bundle we use a builtin injected bundle, like qt does as well, to implement a web extensions system. This also allows us to expose the GObject DOM bindings API we use in WebKit1. And as we do for the UI process we expose our custom high level GTK+ API for the web process instead of the C API one.
Sam Weinig
Comment 8 2012-12-31 07:04:59 PST
(In reply to comment #7) > (In reply to comment #6) > > I see this kind of uses the InjectedBundle, but also kind of doesn't (you have extra code in WebProcessSoup). Why can't this all be done on top of the InjectedBundle mechanism and the WebKit2 API? > > This is just a first patch, we will use the injected bundle C API to implement things like bug #79918 and bug #99695. But instead of allowing apps to install their own injected bundle we use a builtin injected bundle, like qt does as well, to implement a web extensions system. This also allows us to expose the GObject DOM bindings API we use in WebKit1. And as we do for the UI process we expose our custom high level GTK+ API for the web process instead of the C API one. That doesn't exactly answer why you need to change WebProcessSoup?
Carlos Garcia Campos
Comment 9 2012-12-31 07:10:46 PST
(In reply to comment #8) > (In reply to comment #7) > > (In reply to comment #6) > > > I see this kind of uses the InjectedBundle, but also kind of doesn't (you have extra code in WebProcessSoup). Why can't this all be done on top of the InjectedBundle mechanism and the WebKit2 API? > > > > This is just a first patch, we will use the injected bundle C API to implement things like bug #79918 and bug #99695. But instead of allowing apps to install their own injected bundle we use a builtin injected bundle, like qt does as well, to implement a web extensions system. This also allows us to expose the GObject DOM bindings API we use in WebKit1. And as we do for the UI process we expose our custom high level GTK+ API for the web process instead of the C API one. > > That doesn't exactly answer why you need to change WebProcessSoup? Yes, to create the builtin injected bundle when the UI process hasn't passed a injected bundle path (typically WTR). This is what qt does too, see WebProcessQt.
Carlos Garcia Campos
Comment 10 2012-12-31 07:18:15 PST
(In reply to comment #9) > (In reply to comment #8) > > (In reply to comment #7) > > > (In reply to comment #6) > > > > I see this kind of uses the InjectedBundle, but also kind of doesn't (you have extra code in WebProcessSoup). Why can't this all be done on top of the InjectedBundle mechanism and the WebKit2 API? > > > > > > This is just a first patch, we will use the injected bundle C API to implement things like bug #79918 and bug #99695. But instead of allowing apps to install their own injected bundle we use a builtin injected bundle, like qt does as well, to implement a web extensions system. This also allows us to expose the GObject DOM bindings API we use in WebKit1. And as we do for the UI process we expose our custom high level GTK+ API for the web process instead of the C API one. > > > > That doesn't exactly answer why you need to change WebProcessSoup? > > Yes, to create the builtin injected bundle when the UI process hasn't passed a injected bundle path (typically WTR). This is what qt does too, see WebProcessQt. Of course we could do the same installing our own injected bundle lib, and that was indeed my initial idea, but then I looked at the bultin bundle used by Qt and I thought it was a good idea that simplifies the whole thing.
Sam Weinig
Comment 11 2012-12-31 11:36:17 PST
(In reply to comment #10) > (In reply to comment #9) > > (In reply to comment #8) > > > (In reply to comment #7) > > > > (In reply to comment #6) > > > > > I see this kind of uses the InjectedBundle, but also kind of doesn't (you have extra code in WebProcessSoup). Why can't this all be done on top of the InjectedBundle mechanism and the WebKit2 API? > > > > > > > > This is just a first patch, we will use the injected bundle C API to implement things like bug #79918 and bug #99695. But instead of allowing apps to install their own injected bundle we use a builtin injected bundle, like qt does as well, to implement a web extensions system. This also allows us to expose the GObject DOM bindings API we use in WebKit1. And as we do for the UI process we expose our custom high level GTK+ API for the web process instead of the C API one. > > > > > > That doesn't exactly answer why you need to change WebProcessSoup? > > > > Yes, to create the builtin injected bundle when the UI process hasn't passed a injected bundle path (typically WTR). This is what qt does too, see WebProcessQt. > > Of course we could do the same installing our own injected bundle lib, and that was indeed my initial idea, but then I looked at the bultin bundle used by Qt and I thought it was a good idea that simplifies the whole thing. I'm not clear on how it simplifies things, it certainly makes things harder on those of us working on core functionality by not using the abstractions we have in place. Given that this can be done without modifying core code, please do it that way.
Carlos Garcia Campos
Comment 12 2013-01-02 06:47:45 PST
Created attachment 181029 [details] New patch Use an injected bundle lib instead of the builtin bundle.
Gustavo Noronha (kov)
Comment 13 2013-01-09 05:06:36 PST
Comment on attachment 181029 [details] New patch View in context: https://bugs.webkit.org/attachment.cgi?id=181029&action=review Looks good to me, would be good to have a facility to let applications specify a private modules path so that we don't end up with a lot of modules that won't be used loaded unnecessarily, or with applications relying on others' modules. Other than that the API looks great to me, too. > Source/WebKit2/WebProcess/InjectedBundle/API/gtk/WebKitWebExtension.cpp:117 > + * > + * Get the web page of the given @page_id. > + * > + * Returns: the #WebKitWebPage for the given @page_id, or %NULL if the > + * identifier doesn't correspond to an exsiting web page. Be useful to state the WebExtension owns the object that is returned and annotate transfer none.
Carlos Garcia Campos
Comment 14 2013-01-10 02:49:58 PST
Created attachment 182101 [details] Patch for landing
WebKit Review Bot
Comment 15 2013-01-10 03:29:13 PST
Comment on attachment 182101 [details] Patch for landing Clearing flags on attachment: 182101 Committed r139305: <http://trac.webkit.org/changeset/139305>
WebKit Review Bot
Comment 16 2013-01-10 03:29:19 PST
All reviewed patches have been landed. Closing bug.
Note You need to log in before you can comment on or make changes to this bug.