Bug 105631 – [GTK] Add support for loading web process extensions

• Component: WebKit2
• Status: RESOLVED FIXED

[Carlos Garcia Campos]

Add support for loading plugins for the web process (web extensions) using injected bundle.

[Carlos Garcia Campos]

Created attachment 180511 [details]
Patch

[WebKit Review Bot]

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 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]

Created attachment 180924 [details]
Updated patch

Removed the single header check added to the main web extension header by mistake.

[Carlos Garcia Campos]

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]

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]

(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]

(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]

(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]

(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]

(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]

Created attachment 181029 [details]
New patch

Use an injected bundle lib instead of the builtin bundle.

[Gustavo Noronha (kov)]

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]

Created attachment 182101 [details]
Patch for landing

[WebKit Review Bot]

Comment on attachment 182101 [details]
Patch for landing

Clearing flags on attachment: 182101

Committed r139305: <http://trac.webkit.org/changeset/139305>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.