1290617 - allow permanent as well as temporary add-ons via MockExtension

Status: RESOLVED FIXED

(WebExtensions :: Untriaged, defect, P2)

Description

[Robert Helmer [:rhelmer]]

Temporary add-ons are fine for most things, but don't work for a few cases such as testing updates.

Right now MockExtension can automatically install a temporary add-on, it should be able to install a permanent add-on as well.

Comment 1

[Robert Helmer [:rhelmer]]

Attachment: Bug 1290617 - allow non-temp mock webextensions

Review commit: https://reviewboard.mozilla.org/r/68098/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/68098/

Comment 2

[Robert Helmer [:rhelmer]]

I tested this by temporarily modifying the tests in `test_ext_simple.js` (needs signature check disabled and AddonManager started.)

Comment 3

[Andrew Swan [:aswan]]

Disabling the signature check will work fine on mozilla-central and various integration branches, but if folks start using that technique, tests will break when their code makes it to beta (where signing may not be disabled).

Comment 4

[Kris Maglione [:kmag]]

Comment on attachment 8776195 [details]
Bug 1290617 - allow non-temp mock webextensions

https://reviewboard.mozilla.org/r/68098/#review65252

::: toolkit/components/extensions/Extension.jsm:1252
(Diff revision 1)
>  
>    startup() {
> +    // Enable more extensive EM logging
> +    Services.prefs.setBoolPref("extensions.logging.enabled", true);
> +    // By default, don't cache add-ons in AddonRepository.jsm
> +    Services.prefs.setBoolPref("extensions.getAddons.cache.enabled", false);

We should make these pref changes in the appropriate head.js or test files, and make sure to revert them. This may be OK for xpcshell tests, but we shouldn't let pref changes like this stick around in mochitests.

::: toolkit/components/extensions/Extension.jsm:1307
(Diff revision 1)
>    Services.ppmm.broadcastAsyncMessage("Extension:FlushJarCache", {path: file.path});
>  
>    let fileURI = Services.io.newFileURI(file);
>    let jarURI = Services.io.newURI("jar:" + fileURI.spec + "!/", null, null);
>  
> +  // this may be "temporary" or "permanent".

Please capitalize.

::: toolkit/components/extensions/test/mochitest/test_chrome_ext_background_debug_global.html:38
(Diff revision 1)
>    window.testThing = "test!";
>    browser.test.notifyPass("background script ran");
>  }
>  
>  let extensionData = {
> -  useAddonManager: true,
> +  useAddonManager: "temporary",

There are a couple of tests in browser/components/extensions/test/browser that need this change too.

Comment 5

[Kris Maglione [:kmag]]

(In reply to Andrew Swan [:aswan] from comment #3)
> Disabling the signature check will work fine on mozilla-central and various
> integration branches, but if folks start using that technique, tests will
> break when their code makes it to beta (where signing may not be disabled).

It will still work in xpcshell tests where we've disabled signature checking. The sheriffs run periodic aurora and release channel simulation builds, so if it somehow makes it into a mochitest, we should catch it well before an actual merge.

Comment 6

[Andrew Swan [:aswan]]

(In reply to Kris Maglione [:kmag] from comment #5)
> (In reply to Andrew Swan [:aswan] from comment #3)
> > Disabling the signature check will work fine on mozilla-central and various
> > integration branches, but if folks start using that technique, tests will
> > break when their code makes it to beta (where signing may not be disabled).
> 
> It will still work in xpcshell tests where we've disabled signature
> checking. The sheriffs run periodic aurora and release channel simulation
> builds, so if it somehow makes it into a mochitest, we should catch it well
> before an actual merge.

Yes, I said it in a roundabout way but I meant to say "we shouldn't ever use the technique of disabling the signature check in mochitests".  And of course it is irrelevant for xpcshell as you point out.

Also, drive-by review comment, pushPrefEnv() it a nice way to apply temporary preference changes, you can undo them all with one line and the undo happens automatically at the end of a test.

Comment 7

[Robert Helmer [:rhelmer]]

(In reply to Andrew Swan [:aswan] from comment #6)
> (In reply to Kris Maglione [:kmag] from comment #5)
> > (In reply to Andrew Swan [:aswan] from comment #3)
> > > Disabling the signature check will work fine on mozilla-central and various
> > > integration branches, but if folks start using that technique, tests will
> > > break when their code makes it to beta (where signing may not be disabled).
> > 
> > It will still work in xpcshell tests where we've disabled signature
> > checking. The sheriffs run periodic aurora and release channel simulation
> > builds, so if it somehow makes it into a mochitest, we should catch it well
> > before an actual merge.
> 
> Yes, I said it in a roundabout way but I meant to say "we shouldn't ever use
> the technique of disabling the signature check in mochitests".  And of
> course it is irrelevant for xpcshell as you point out.


This is a good point - I only plan to use this feature in an xpcshell test, I'll make sure not to use `useAddonManager: "permanent"` in mochitests.

 
> Also, drive-by review comment, pushPrefEnv() it a nice way to apply
> temporary preference changes, you can undo them all with one line and the
> undo happens automatically at the end of a test.


That is only for SpecialPowers in mochitests?

Comment 8

[Robert Helmer [:rhelmer]]

https://reviewboard.mozilla.org/r/68098/#review65252

> We should make these pref changes in the appropriate head.js or test files, and make sure to revert them. This may be OK for xpcshell tests, but we shouldn't let pref changes like this stick around in mochitests.

OK I just removed these, the only really important one for this feature to work is disabling the add-on cache.

I'll just set it along with disabling signature etc. when I use it in the test for bug 1279012.

Comment 9

[Robert Helmer [:rhelmer]]

Comment on attachment 8776195 [details]
Bug 1290617 - allow non-temp mock webextensions

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/68098/diff/1-2/

Comment 10

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/ff654490bc1e
allow non-temp mock webextensions r=kmag

Comment 11

[Carsten Book [:Tomcat]]

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