1806135 - Enable installation and uninstallation of addons in GeckoView

Status: ASSIGNED

(Remote Protocol :: Marionette, enhancement, P3)

Description

[Thomas Wisniewski [:twisniewski]]

While adding support for Android testing for Firefox's webcompat addon, I noticed that I could not issue a POST to moz/install/addon to install it on a Geckoview Example instance (which does not have it bundled).

It seems we have two assertions for Fennec which no longer make sense for Geckoview: https://searchfox.org/mozilla-central/source/remote/marionette/driver.sys.mjs#2847,2863

Could we remove those assertions? I am able to install the addon successfully without them.

Comment 1

[Henrik Skupin [:whimboo][⌚️UTC+2]]

Those mentioned assertions were in place for Fennec where we weren't able to install addons. With the move to GeckoView this restriction doesn't have to be kept in place anymore and could be removed.

When doing that we might also want to move our existing Marionette unit tests for installing and uninstalling addons to the Mozilla specific web-platform test folder at:

https://searchfox.org/mozilla-central/source/testing/web-platform/mozilla/tests/webdriver/

Thomas, how important is it for you to have the ability to install addons? Is any webcompat work blocked on it?

Comment 2

[Thomas Wisniewski [:twisniewski]]

Yes, my work in bug 1805826 is realistically blocked by this. Geckoview Example doesn't have system addons/features bundled with it, so I need to install the webcompat addon manually. Otherwise anyone adding tests will have to work with full Fenix builds, which is significant effort compared to patching this, from what I can see. I have no problem working on this patch, if it helps.

Comment 3

[Henrik Skupin [:whimboo][⌚️UTC+2]]

I see. So it would totally make sense to have that restriction removed then. If you could work on that patch we would kinda appreciate. I can help with anything that is needed to get the appropriate webdriver web-platform-tests written. It shouldn't be too hard.

Comment 4

[Thomas Wisniewski [:twisniewski]]

Attachment: Add support for installing/uninstalling addons (including enabling them in private browsing mode)

Actually, it turns out that I can also install the addon without this patch, by executing a script in the chrome context, so there's less pressure to get this done.

It's worth noting as well that there is another issue; any addon installed won't also get permission to run in private browsing mode. So we would probably want to either have an option for that as well, a new endpoint to grant an addon permissions, or something along those lines.

I'm attaching my proof-of-concept patch from before I realized that I could just install the addon executing a chrome context script, in case it helps (while we decide how to best handle all of this API-wise).

Comment 5

[Henrik Skupin [:whimboo][⌚️UTC+2]]

Thanks Thomas. We will discuss the private browsing approach in one of our next triage meetings. Thanks for providing the PoC.

Comment 6

[Thomas Wisniewski [:twisniewski]]

Oh, and one other thing to note is that right now, one must use abd (via ADBDeviceFactory in my case) to push the XPI file to the emulator/device, as the endpoint expects the file to be on the local device, not wherever GeckoDriver is actually running. Maybe it would be nice for the install endpoint to do that file-pushing itself?

Comment 7

[Henrik Skupin [:whimboo][⌚️UTC+2]]

Please note that the add-on installation command can also take a base64 encode string. By using that there is no need to copy the XPI to the device first. As such the creation of the tests under eg. /testing/web-platform/mozilla/tests/webdriver/install_addon should be simple.

Will you take a try?

Comment 8

[Thomas Wisniewski [:twisniewski]]

I don't mind, but would it not be best to test both cases? It's not difficult to transfer the file over using the device factory, even if only for the tests. That way we probably just need to agree on the details of how to enable addons in private browsing mode (a second API call, or a flag on the install call, etc). I don't actually know what the better approach is there, given the MV3 addon effort might end up making it necessary/useful to toggle other such permissions at some point.

Comment 9

[Henrik Skupin [:whimboo][⌚️UTC+2]]

We discussed the private browsing part in our triage meeting today. It's ok to get it added given that it is a vendor specific end-point. But how important is that feature to you? Is it crucial for your work? If yes and it needs to be also available through geckodriver then it will be a bit more work including Rust code changes. But I'm happy to help with whatever comes up.

(In reply to Thomas Wisniewski [:twisniewski] from comment #8)

I don't mind, but would it not be best to test both cases? It's not difficult to transfer the file over using the device factory, even if only for the tests.

Sadly we don't have that possibility for web-platform (wdspec) tests yet. Or maybe I miss something?

That way we probably just need to agree on the details of how to enable addons in private browsing mode (a second API call, or a flag on the install call, etc). I don't actually know what the better approach is there, given the MV3 addon effort might end up making it necessary/useful to toggle other such permissions at some point.

We actually want to re-use the existing API's and simply add a new flag to indicate the permissions. That way it will also be easy to extend for other possible flags in the future.

Comment 10

[Henrik Skupin [:whimboo][⌚️UTC+2]]

Tom, we discussed this bug again in our triage meeting and we actually propose to split the work up into three bugs:

1. Remove the asserts and add generic test cases to the mozilla specific wdspec directory for installing addons via a base64 encoded string (this bug).

2. Lets file a follow-up bug to support Private Browsing.

3. Lets file a follow-up bug to add tests for installing the addon from a path on the actual device.

Would that work for you? As you said above only 1) is blocking you at the moment. 2) will introduce quite a good amount of work including for geckodriver which we don't think should hold back landing 1).

Comment 11

[Thomas Wisniewski [:twisniewski]]

Sure, I'm not actually blocked on this anymore, so I'm alright with however we'd like to split up the work.

Comment 12

[Henrik Skupin [:whimboo][⌚️UTC+2]]

I've filed two follow-up bugs which are bug 1810718 and bug 1810722.

Comment 13

[Henrik Skupin [:whimboo][⌚️UTC+2]]

Hi Tom. I wanted to ask if you are still interested to provide the patch for a minimal solution. If you don't have the time please let us know. Thanks.

Comment 14

[Thomas Wisniewski [:twisniewski]]

I currently don't have the time to finish my attached patch, so if someone else wants to beat me to it, please feel free. (I'm hoping to get it done over the next few weeks, but it's a low-priority task for me).

Comment 16

[Thomas Wisniewski [:twisniewski]]

Please note that one thing that would be helpful here would be to be able to specify an addon as to be treated as though it's "built-in", even if it's not (see bug 1923468 comment 2 for context, but it would help folks to be able to use artifact builds for testing built-in addons, especially with the GeckoView Embedded browser). I will be figuring out the changes to my proposed patch above to do this ASAP, if someone doesn't beat me to it.

Comment 17

[Henrik Skupin [:whimboo][⌚️UTC+2]]

(In reply to Thomas Wisniewski [:twisniewski] from comment #16)

Please note that one thing that would be helpful here would be to be able to specify an addon as to be treated as though it's "built-in", even if it's not (see bug 1923468 comment 2 for context, but it would help folks to be able to use artifact builds for testing built-in addons, especially with the GeckoView Embedded browser). I will be figuring out the changes to my proposed patch above to do this ASAP, if someone doesn't beat me to it.

This would be a nice addition but I don't think it fits into this particular bug. Tom, would you mind filing a follow-up bug? Thanks.

Comment 18

[Alexandra Borovova [:Sasha]]

Attachment: Bug 1806135 - Allow installation and uninstallation of addons in GeckoView.

Comment 19

[Alexandra Borovova [:Sasha]]

Attachment: Bug 1806135 - Add gecko-specific tests for installation and uninstallation of addons.