Closed
Bug 1220911
Opened 9 years ago
Closed 9 years ago
Remove much of the special casing around experiment add-ons
Categories
(Toolkit :: Add-ons Manager, defect)
Toolkit
Add-ons Manager
Tracking
()
RESOLVED
FIXED
mozilla45
Tracking | Status | |
---|---|---|
firefox45 | --- | fixed |
People
(Reporter: mossop, Assigned: mossop)
References
Details
Attachments
(2 files)
There are a bunch of special code around the handling of experiments that I don't believe needs to be there and causes unexpected results like bug 1218266. The more these are different the more we'll accidentally forget about those differences when making changes so I'd like to largely remove these differences.
Assignee | ||
Comment 1•9 years ago
|
||
Bug 1220911: Switch test_experiment.js to task style. r?rhelmer
Before changing the handling of experiments make the tests a bit more readable
and use BootstrapMonitor to verify things.
Attachment #8688678 -
Flags: review?(rhelmer)
Assignee | ||
Comment 2•9 years ago
|
||
Bug 1220911: Remove most of the special casing around experiments. r?rhelmer
Experiments should differ from normal add-ons in a few ways:
* They can always be enabled regardless of compatibility info
* They default to disabled when installed
* They cannot be checked for updates
* They only stay enabled for the lifetime of the current process
* The UI doesn't give users the ability to enable/disable
This makes a few changes to keep these differences but remove much of the special casing code for experiments.
Being able to use regardless of compatibility was mostly fixed by bug 1220198 but I've also removed the redundant override in isCompatible.
Previously the "enabled until restart" feature worked with by not updating the DBAddonInternal object and instead using a hack to make the wrapper still seem enabled. This seems likely to break other code that relies on the state of the DBAddonInternal object so instead we update that as normal and simply don't persist the enabled state to disk.
Also switch the DBAddonInteral.prototype code to use some newer JS features.
I've removed the hack from addon.permissions which was hiding the enable/disable buttons in the UI and instead just hidden them in the UI stylesheet. This makes the API make sense and means callers can use addon.permissions to verify that enabling will work.
Attachment #8688679 -
Flags: review?(rhelmer)
Updated•9 years ago
|
Attachment #8688678 -
Flags: review?(rhelmer) → review+
Comment 3•9 years ago
|
||
Comment on attachment 8688678 [details]
MozReview Request: Bug 1220911: Switch test_experiment.js to task style. r?rhelmer
https://reviewboard.mozilla.org/r/25375/#review22971
Comment 4•9 years ago
|
||
Comment on attachment 8688679 [details]
MozReview Request: Bug 1220911: Remove most of the special casing around experiments. r?rhelmer
https://reviewboard.mozilla.org/r/25377/#review22973
Attachment #8688679 -
Flags: review?(rhelmer) → review+
Comment 6•9 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/58b4705d241e
https://hg.mozilla.org/mozilla-central/rev/1d6ba006a526
Status: NEW → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla45
![]() |
||
Comment 7•9 years ago
|
||
Dave, do I understand right that this is required for us not to shave the shutdown hang/crash issues with experiments that we've seen recently again when running an experiment? If so, we probably want to uplift this to any channel/train we want to run experiments on, which includes current aurora and beta.
Flags: needinfo?(dtownsend)
Assignee | ||
Comment 8•9 years ago
|
||
(In reply to Robert Kaiser (:kairo@mozilla.com) from comment #7)
> Dave, do I understand right that this is required for us not to shave the
> shutdown hang/crash issues with experiments that we've seen recently again
> when running an experiment? If so, we probably want to uplift this to any
> channel/train we want to run experiments on, which includes current aurora
> and beta.
No, that was bug 1220198
Flags: needinfo?(dtownsend)
![]() |
||
Comment 9•9 years ago
|
||
Ah, thanks, awesome.
You need to log in
before you can comment on or make changes to this bug.
Description
•