Make enable/disable/uninstall operations on AddonWrappers asynchronous

RESOLVED FIXED in Firefox 62

Status

()

enhancement
P3
normal
RESOLVED FIXED
Last year
10 months ago

People

(Reporter: kmag, Assigned: kmag)

Tracking

Trunk
mozilla62
Points:
---
Dependency tree / graph
Bug Flags:
qe-verify -

Firefox Tracking Flags

(firefox62 fixed)

Details

Attachments

(1 attachment)

Shutdown operations for add-ons often need to be asynchronous, so exposing things like the `userDisabled` setter as synchronous operations is essentially a lie. We need to make these operations explicitly asynchronous in order to support asynchronous startup/shutdown callbacks correctly.
Priority: -- → P3
Comment on attachment 8975304 [details]
Bug 1461146: Replace Addon.userDisabled setter with async enable()/disable() methods.

https://reviewboard.mozilla.org/r/243630/#review250848

I'm not clear on the intended scope here.  You've made these functions async but for extensions they don't actually wait for the change to be fully applied.
Please clarify the intent of this patch and plans for subsequent bugs/patches.
(In reply to Andrew Swan [:aswan] from comment #2)
> I'm not clear on the intended scope here.  You've made these functions async
> but for extensions they don't actually wait for the change to be fully
> applied.
> Please clarify the intent of this patch and plans for subsequent
> bugs/patches.

This is just an interface change, with updates to existing callers to make sure they can handle a small amount of asynchrony. It mostly just updates them to use the new API and make sure they don't expect the state to change instantly.

Bug 1461145 updates them to actually wait for bootstrap startup()/shutdown() functions to complete before resolving. Bug 1462855 builds on both patches to move install/uninstall operations to async IO, after the APIs and callers have been updated to properly deal with async startup/shutdown operations.

After those bugs land, there are probably some follow-ups we should do to remove hacks like promiseWebExtensionStartup, which it makes unnecessary. But that's merely nice to have, at this point.
Comment on attachment 8975304 [details]
Bug 1461146: Replace Addon.userDisabled setter with async enable()/disable() methods.

https://reviewboard.mozilla.org/r/243630/#review251460
Attachment #8975304 - Flags: review?(aswan) → review+
https://hg.mozilla.org/integration/mozilla-inbound/rev/a0d1b763f2d9bbaf8eef417cf599c17e1cb98dc1
Bug 1461146: Replace Addon.userDisabled setter with async enable()/disable() methods. r=aswan
Pushed by maglione.k@gmail.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/a0d1b763f2d9
Replace Addon.userDisabled setter with async enable()/disable() methods. r=aswan
https://hg.mozilla.org/mozilla-central/rev/a0d1b763f2d9
Status: NEW → RESOLVED
Closed: Last year
Resolution: --- → FIXED
Target Milestone: --- → mozilla62
See Also: → 1464682
Depends on: 1465413
Is manual testing required on this bug? If yes, please provide some STR and the proper extension(if required) or set the “qe-verify -“ flag.

Thanks!
Flags: needinfo?(kmaglione+bmo)
Depends on: 1467695
Duplicate of this bug: 771441
Flags: needinfo?(kmaglione+bmo) → qe-verify-
You need to log in before you can comment on or make changes to this bug.