1923468 - No longer able to install a privileged add-on in WebDriver on Android for testing the webcompat add-on

Status: RESOLVED FIXED

(Toolkit :: Add-ons Manager, defect)

Description

[Thomas Wisniewski [:twisniewski]]

I'm not sure which bugzilla component to file this under, but at some point in the past few months, our automated testing framework for the webcompat addon has stopping working with Android artifact builds, apparently due to experimental APIs no longer being acceptable on those builds. This causes a lot of pointless friction at a time when we have a goal to speed up our ability to ship interventions, not slow it down.

For some background, our WebDriver-based testing harness essentially starts up a local artifact build of the GeckoView Embedded browser, installs it on the Android emulator, and then starts it up in order to run the tests.

To do so, it must first upload the webcompat addon as an XPI to the emulated device using ADB, since GVE doesn't have any built-in addons: https://searchfox.org/mozilla-central/source/testing/webcompat/mach_commands.py#216

It then installs that XPI using some basic marionette chrome JS:
https://searchfox.org/mozilla-central/source/testing/webcompat/fixtures.py#180

Of course, the expected about:config settings for disabling signing and enabling experimental APIs are set in advance here:
https://searchfox.org/mozilla-central/source/testing/webcompat/fixtures.py#112-114

But this approach no longer works. It appears that the artifact builds now block the addon's installation in this code, which no longer bothers to read the experimental API pref, as this if statement fails:
https://searchfox.org/mozilla-central/source/toolkit/mozapps/extensions/internal/AddonSettings.sys.mjs#103-115

It's false since the addon XPI is not built-in (it's a file on the phone, but not bundled with the GVE artifact build), and the app-constant checks for signing and nightly etc fail, and Cu.isInAutomation is false as well.

Something makes me highly doubt that we want to tweak that if-statement. But I have found a work-around where I pass in a new, second "isPrivileged" parameter to AddonManager.installTemporaryAddon in my script here: https://searchfox.org/mozilla-central/source/testing/webcompat/fixtures.py#194

I can then pass it around in the addon framework code, making sure the addon is forced to be treated as privileged. I'll attach the patch which I've been working with, in the hopes that it's an acceptable work-around. Rob, do you think that will work?

Comment 1

[Thomas Wisniewski [:twisniewski]]

Attachment: WIP: Bug 1923468 - allow installing temporary addons as privileged in chrome JS, to ease testing them on GVE artifact builds

Comment 2

[Rob Wu [:robwu]]

This is more a testing issue with webcompat than an issue with the extension framework, but since the relevant expertise is with the extensions team, I will temporarily move it there. Once the questions have been resolved, this would probably fit better at Testing::General or even the component of webcompat.

In any case, the proposed solution does not look right to me, because it introduces additional complexity to security sensitive code, to work around a test issue. There are a number of other options to consider, such as ensuring that the build has MOZ_REQUIRE_SIGNING=0.

In any case, when I take a closer look at your proposed patch (thanks for that, as it allows me to offer concrete advice more easily!), it stands out to me that your test runner is basically running arbitrary code with chrome privileges. That opens the door for methods that are not part of the usual obvious interface.

The ordinary webcompat add-on is a privileged add-on because it is built-in. In the patch you are attempting to fake the "privileged" flag, but there is also another option: faking being built-in. There are existing examples of that in unit tests: https://searchfox.org/mozilla-central/rev/7a85a111b5f42cdc07f438e36f9597c4c6dc1d48/toolkit/components/extensions/test/xpcshell/test_ext_webRequest_startup.js#344-357

Basically, set up a resource:-protocol substitution that points to your extension, then call the internal AddonManager.installBuiltIn method.

Comment 3

[Thomas Wisniewski [:twisniewski]]

Thanks, Rob. Your resource:/protocol suggestion does seem like the better fix here, and seems to work fine in a quick test. So I'm going to make a new patch which just updates the code in our testing framework to do that, instead of altering any other code.

Comment 4

[Thomas Wisniewski [:twisniewski]]

Attachment: Bug 1923468 - install the webcompat addon as a built-in addon in our testing framework, rather than just a temporary one, so it works again; r?ksenia

Comment 5

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/f9d511c019e2
install the webcompat addon as a built-in addon in our testing framework, rather than just a temporary one, so it works again; r=ksenia

Comment 6

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

https://hg.mozilla.org/mozilla-central/rev/f9d511c019e2