Kris Maglione [:kmag] Assignee
	Description • 6 years ago

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.

	Andrew Swan [:aswan]
	Comment 2 • 6 years ago
mozreview-review

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.

	Kris Maglione [:kmag] Assignee
	Comment 3 • 6 years ago

(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.

	Andrew Swan [:aswan]
	Comment 4 • 6 years ago
mozreview-review

Comment on attachment 8975304 [details]
Bug 1461146: Replace Addon.userDisabled setter with async enable()/disable() methods.

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

	Kris Maglione [:kmag] Assignee
	Comment 5 • 6 years ago

https://hg.mozilla.org/integration/mozilla-inbound/rev/a0d1b763f2d9bbaf8eef417cf599c17e1cb98dc1
Bug 1461146: Replace Addon.userDisabled setter with async enable()/disable() methods. r=aswan

	Pulsebot
	Comment 6 • 6 years ago

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

	Cosmin Sabou [:CosminS]
	Comment 7 • 6 years ago
bugherder

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

	CosminB
	Comment 8 • 6 years ago

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!