970280 - Test that an add-on can be enabled after disabling them

Status: RESOLVED INCOMPLETE

(Add-on SDK Graveyard :: General, defect)

Description

[Matteo Ferretti [:zer0] [:matteo]]

Enable an add-on after it was disable raises an exception that prevent the add-ons from be enabled again:

Bootstrap error: "prototype" is read-only
iced@resource://gre/modules/XPIProvider.jsm -> jar:file:///var/folders/tx/91xwh51562l5_4xcqjst9bvw0000gn/T/tmp_d9L9a.mozrunner/extensions/[e-mail]!/bootstrap.js -> resource://extensions.modules.my-test-at-jetpack.commonjs.path/toolkit/loader.js:112
Loader@resource://gre/modules/XPIProvider.jsm -> jar:file:///var/folders/tx/91xwh51562l5_4xcqjst9bvw0000gn/T/tmp_d9L9a.mozrunner/extensions/[e-mail]!/bootstrap.js -> resource://extensions.modules.my-test-at-jetpack.commonjs.path/toolkit/loader.js:132
@resource://gre/modules/XPIProvider.jsm -> jar:file:///var/folders/tx/91xwh51562l5_4xcqjst9bvw0000gn/T/tmp_d9L9a.mozrunner/extensions/[e-mail]!/bootstrap.js -> resource://extensions.modules.my-test-at-jetpack.commonjs.path/toolkit/loader.js:9
@resource://gre/modules/XPIProvider.jsm -> jar:file:///var/folders/tx/91xwh51562l5_4xcqjst9bvw0000gn/T/tmp_d9L9a.mozrunner/extensions/[e-mail]!/bootstrap.js -> resource://extensions.modules.my-test-at-jetpack.commonjs.path/toolkit/loader.js:26
loadSandbox@resource://gre/modules/XPIProvider.jsm -> jar:file:///var/folders/tx/91xwh51562l5_4xcqjst9bvw0000gn/T/tmp_d9L9a.mozrunner/extensions/[e-mail]!/bootstrap.js:278
@resource://gre/modules/XPIProvider.jsm -> jar:file:///var/folders/tx/91xwh51562l5_4xcqjst9bvw0000gn/T/tmp_d9L9a.mozrunner/extensions/[e-mail]!/bootstrap.js -> resource://extensions.modules.my-test-at-jetpack.commonjs.path/sdk/loader/cuddlefish.js:30
loadSandbox@resource://gre/modules/XPIProvider.jsm -> jar:file:///var/folders/tx/91xwh51562l5_4xcqjst9bvw0000gn/T/tmp_d9L9a.mozrunner/extensions/[e-mail]!/bootstrap.js:278
startup@resource://gre/modules/XPIProvider.jsm -> jar:file:///var/folders/tx/91xwh51562l5_4xcqjst9bvw0000gn/T/tmp_d9L9a.mozrunner/extensions/[e-mail]!/bootstrap.js:192
XPI_callBootstrapMethod@resource://gre/modules/XPIProvider.jsm:4312
XPI_updateAddonDisabledState@resource://gre/modules/XPIProvider.jsm:4432
AddonWrapper_userDisabledSetter@resource://gre/modules/XPIProvider.jsm:6672
set_userDisabled@chrome://mozapps/content/extensions/extensions.xml:1060
oncommand@about:addons:1

Step to reproduce:
- Install a jetpack add-on, or run `cfx run` with a simple add-on
- from `about:addons`, press disable button
- from `about:addons`, press enable button

The exception is raised and the add-on won't be enabled.

Note:
- The same happens even if the browser start with the jetpack add-on already disabled, and then we try to enable it.
- Restartless add-on that doesn't use SDK seems work just fine.

Comment 1

[murph]

Attachment: DontModifyFrozenFunctions.patch

I hit this same bug on Fennec. This fixes it.

There's probably a few places this could be changed, but this seemed reasonable to me.

Comment 2

[Dave Townsend [:mossop]]

Comment on attachment 8380813 [details] [diff] [review]
DontModifyFrozenFunctions.patch

Nit: We use two space indent. But I'll correct that when I apply it to github.

Comment 3

[Tomislav Jovanovic :zombie]

i think there should be a test for this, as it seems like a thing that could easily be break again by some other patch..

Comment 4

[[github robot]]

Commit pushed to master at https://github.com/mozilla/addon-sdk

https://github.com/mozilla/addon-sdk/commit/b62a63735667a653ece26f10b5351b7f0560179a
Bug 970280: Don't attempt to clear the prototype on frozen functions. r=Mossop

Comment 5

[Dave Townsend [:mossop]]

(In reply to Tomislav Jovanovic [:zombie] from comment #3)
> i think there should be a test for this, as it seems like a thing that could
> easily be break again by some other patch..

I agree but I wanted to get the fix in quickly. Any ideas how to do a test for this?

Comment 6

[Tomislav Jovanovic :zombie]

i think we can generate a simpe xpi manually, stick it in /test/fixtures, and try to load, disable, re-enable it via the AddonManager.

although Erik might have a better idea?

Comment 7

[Erik Vold [:erikvold] (please needinfo? me)]

(In reply to Tomislav Jovanovic [:zombie] from comment #6)
> i think we can generate a simpe xpi manually, stick it in /test/fixtures,
> and try to load, disable, re-enable it via the AddonManager.
> 
> although Erik might have a better idea?

Ya I was against using xpis at first, but we do this plenty in m-c and after we have native jetpacks it won't be as much of an issue going forward, plus we'll still have to use xpis to regression test the old school jetpacks.

Comment 8

[Dave Townsend [:mossop]]

Murph, Tomislav, does one of you want to put together a test here?

Comment 9

[murph]

I'll do it if Tomislav doesn't want to, but it might take me a day or two and some prodding people on IRC. I've only tried to run the existing tests once, and they didn't work on my setup yet.

Comment 10

[Tomislav Jovanovic :zombie]

Murph: the bug is yours. take good care of it..

Comment 11

[murph]

Matteo: Do you remember how you got this error? I've only reproduced it so far on mobile.

I tried with the nightly from 2/10 and a simple addon via cfx run and it doesn't reproduce. I'm not sure if you were using a different version/channel, or if your addon was doing something interesting?

Comment 12

[Matteo Ferretti [:zer0] [:matteo]]

Murph, you can reproduce the bug with the steps I mentioned in the first comment. It is always reproducible. Be sure that you do not have the fix in your repo, and run cfx with `-o` flag.

Comment 13

[Firefox Bug Husbandry Bot]

https://bugzilla.mozilla.org/show_bug.cgi?id=1399562