Bug 179915

Summary: [WPE] Install files needed for WebKitWebExtensions
Product: WebKit Reporter: Adrian Perez <aperez>
Component: WPE WebKitAssignee: Michael Catanzaro <mcatanzaro>
Status: RESOLVED FIXED    
Severity: Normal CC: bugs-noreply, commit-queue, mcatanzaro, zan
Priority: P2    
Version: Other   
Hardware: Unspecified   
OS: Unspecified   
See Also: https://github.com/Igalia/dinghy/pull/13
Bug Depends on: 180608    
Bug Blocks: 178894    
Attachments:
Description Flags
Patch
none
Patch
none
Patch
none
Patch none

Description Adrian Perez 2017-11-21 05:01:13 PST
Now that WPE shares most of the GLib API bits with the GTK+ port, it also has
the ability of load WebProcess plug-ins which use the WebKitWebExtension API:

  https://webkitgtk.org/reference/webkit2gtk/stable/WebKitWebExtension.html

Currently, this API is unusable in the WPE because the following are missing:

  - A “wpe-webkit-webextension.pc” (or similarly named) file for pkg-config.
  - API headers (“WebKitWebExtension.h” et al.)
Comment 1 Michael Catanzaro 2017-11-23 11:02:56 PST
*** Bug 179956 has been marked as a duplicate of this bug. ***
Comment 2 Michael Catanzaro 2018-01-07 18:16:33 PST
I told Adrian a while ago that I would do this (it should be very easy), but after working on bug #173915, I'm no longer sure if we should do it at all. The problem is that there is really not all that much you can do with the web process API if the entire DOM API is unavailable. Almost everything we do in Epiphany's web extension depends on the DOM API. The only remaining API I can think of that's really useful and doesn't is WebKitWebPage::send-request. Maybe that in itself is useful enough to justify exposing the web process API, but I don't think so.

Also, whatever we expose now will be redundant with a future JavaScript API. For example, say you want to use WebKitWebPage::form-controls-associated or the new WebKitWebPage::will-submit-form. Both of those depend on the DOM API, but they aren't themselves part of the DOM API. That means our JS API will need to contain bindings for the entire web process API (WebKitWebPage et. al.), not just the DOM API, in order to be a real replacement for the current WebKitGTK+ API.

So I'm not sure what we should do. We should discuss with Carlos Garcia.
Comment 3 Michael Catanzaro 2018-02-15 10:47:45 PST
(In reply to Michael Catanzaro from comment #2)
> So I'm not sure what we should do. We should discuss with Carlos Garcia.

I'm going to WONTFIX this for now. We can decide at any point in the future that we want to expose this API, or not.
Comment 4 Michael Catanzaro 2018-02-19 06:40:00 PST
(In reply to Michael Catanzaro from comment #3)
> I'm going to WONTFIX this for now. We can decide at any point in the future
> that we want to expose this API, or not.

We decided this should be exposed.
Comment 5 Michael Catanzaro 2018-04-06 08:53:28 PDT
This is done in trunk, but as part of a huge JSC GObject API commit, so it will require a bit of effort to extricate the relevant bits and backport to 2.20.
Comment 6 Michael Catanzaro 2018-04-06 11:25:06 PDT
Actually, thinking this through more, we don't really need it in 2.20: there is only one useful function (send-request) without the DOM or JSC APIs. So I think it's good enough to have this in trunk.
Comment 7 Michael Catanzaro 2018-04-06 17:53:57 PDT
(In reply to Adrian Perez from comment #0)
> Currently, this API is unusable in the WPE because the following are missing:
> 
>   - A “wpe-webkit-webextension.pc” (or similarly named) file for pkg-config.
>   - API headers (“WebKitWebExtension.h” et al.)

I was mistaken; all of this is still missing. Reopening.
Comment 8 Michael Catanzaro 2018-04-07 11:04:35 PDT
The injected bundle itself is not installed, so more than development files is needed here.
Comment 9 Michael Catanzaro 2018-04-07 15:21:17 PDT
Created attachment 337433 [details]
Patch
Comment 10 Zan Dobersek 2018-04-11 23:10:37 PDT
Comment on attachment 337433 [details]
Patch

I think this needs updating to reflect changes in bug #184376.
Comment 11 Michael Catanzaro 2018-04-12 08:21:25 PDT
Yup, and also to add API versioning
Comment 12 Michael Catanzaro 2018-04-12 08:52:24 PDT
Created attachment 337801 [details]
Patch
Comment 13 Michael Catanzaro 2018-04-12 08:53:34 PDT
Created attachment 337802 [details]
Patch
Comment 14 Zan Dobersek 2018-04-12 10:24:53 PDT
Comment on attachment 337802 [details]
Patch

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

> Source/WebKit/PlatformWPE.cmake:336
> +        DESTINATION "${CMAKE_INSTALL_INCLUDEDIR}/wpe-${WPE_API_VERSION}/WPE/wpe"

Did we not drop the extra uppercase WPE component in this path? Is the path even correct?
Comment 15 Michael Catanzaro 2018-04-12 11:05:53 PDT
It's not correct, good catch
Comment 16 Michael Catanzaro 2018-04-12 11:11:21 PDT
Comment on attachment 337802 [details]
Patch

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

>> Source/WebKit/PlatformWPE.cmake:336
>> +        DESTINATION "${CMAKE_INSTALL_INCLUDEDIR}/wpe-${WPE_API_VERSION}/WPE/wpe"
> 
> Did we not drop the extra uppercase WPE component in this path? Is the path even correct?

Ah, here's the problem, the include path commit had not yet gone in when I rebased this patch today, and I only accounted for that in the pkg-config file, but not here.
Comment 17 Michael Catanzaro 2018-04-12 11:12:28 PDT
Created attachment 337809 [details]
Patch
Comment 18 Michael Catanzaro 2018-04-12 11:18:19 PDT
BTW, I am still thinking that we can just have this in trunk and not backport it to 2.20. It's not really needed there.
Comment 19 WebKit Commit Bot 2018-04-15 09:32:32 PDT
Comment on attachment 337809 [details]
Patch

Clearing flags on attachment: 337809

Committed r230661: <https://trac.webkit.org/changeset/230661>
Comment 20 WebKit Commit Bot 2018-04-15 09:32:34 PDT
All reviewed patches have been landed.  Closing bug.