In Bug 1354602 we removed the containers enabled checks for the contextualIdentity APIs. At the time I think there was a thought that a user disabling containers would disable the addons, this could be an implementation avenue or we could let the addons themselves fix this by rejecting the promise. The second solution to reject the promises would fix containers when it becomes an optional pref also: Bug 1386673

I'm thinking we should go back on the decision not to have an enabled event also as if I wanted to check if containers have become disabled I would need to poll for rejections on query({}) for example which is super ugly. The problem then is we would have to maintain for a long time right? I can add a onStateChange or onEnabled event, if you are both happy with this.

Tracking this bug on containers: https://github.com/mozilla/testpilot-containers/issues/781

Try submission: https://treeherder.mozilla.org/#/jobs?repo=try&revision=9e0f65745917804bcbaae6ee33375142020bb0e8

Sorry, but I haven't been following the containers discussions (apart from one code review), so I don't have context to even understand what's being asked here. (If you can bother explaining in a way that someone not familiar with containers could follow, I would be happy to answer any questions I can understand.. ;)

So we previously resolved all the promise apis with false, which was pointed out to be bad rather than rejecting them purely on API design. So we were not sure if we would become on by default as a containers feature which is likely not to happen in the short term at least. There was a worry that we would add this "onStateChange" event as :robwu_nl mentioned perhaps we can just use https://developer.mozilla.org/en-US/Add-ons/WebExtensions/API/types/BrowserSetting/onChange for the browser.privacy.containers setting I made. I will file a different bug for that and make this just about the promise rejections here.

Comment on attachment 8903822 [details] Bug 1395659 - Rejecting contextual identity APIs when containers are disabled. https://reviewboard.mozilla.org/r/175582/#review181090 r=me for now, but I think we're agreed that the solution we really want is to prevent contextual identities from being disabled when an extension that requires them is active (which probably depends on bug 1342584). ::: toolkit/components/extensions/ext-contextualIdentities.js:33 (Diff revision 1) > +const apiDisabled = () => { > + return Promise.reject({ > + message: `Contextual identities are currently disabled`, > + }); > +}; Can we make this `checkAPIEnabled` and have it just throw an `ExtensionError` if the API is disabled?

:kmag I finally got try to behave: https://treeherder.mozilla.org/#/jobs?repo=try&revision=9b35be865a5e39779372bd1a346d474a3f1fb784 Could you take another look to make sure the changes are ok? - I added a missed async to a function that was missed in the previous bug - Fixed tests to send messages back and forth - Changed rejects to throws

lgtm, but I'd probably just make getContainerForCookieStoreId and getPublicIdentityFromId throw an ExtensionError for invalid IDs. Up to you, though.

> I'd probably just make getContainerForCookieStoreId and getPublicIdentityFromId throw an ExtensionError for invalid IDs. Up to you, though. Can do this as a follow up, this would mean changing the cookie code that touches those functions too. Merging this patch in autoland might make it conflict with Bug 1390738 please ni me if this happens. Bug 1390738 is more important to land first regardless of this here. Thanks

Pushed by ryanvm@gmail.com: https://hg.mozilla.org/integration/autoland/rev/cc8544557f9f Rejecting contextual identity APIs when containers are disabled. r=kmag

For the docs, is this a good account of the current state of things? * before Firefox 57, contextualIdentities methods could fail, and if they did, they signalled this by resolving the promise with "false". One reason the methods might fail is if contextual identities are disabled in the browser preferences (underlying setting: "privacy.userContext.enabled"). * from Firefox 57 onwards: * if contextualIdentities methods fail, they do so by rejecting the promise with an informative error message. * when an extension that has the "contextualIdentities" permission is installed or enabled, it switches on the underlying browser preference. * however, it's still possible that contextualIdentities methods might fail due to the contextual identities feature being disabled, because the user could go into about:config after installing the extension, and flip the "privacy.userContext.enabled" preference directly (I couldn't find a UI to do this in Firefox 57, although I could in Nightly). In this case, the contextualIdentities methods will reject their promises with an informative error: "Contextual identities are currently disabled".

I believe we used "null" for no identity and "false" for containers disabled. > I couldn't find a UI to do this in Firefox 57, although I could in Nightly Once installed the user should see it in about:preferences to remove the extension. All trains shouldn't be able to disable from about:preferences if they have an extension installed. Everything else is great, thank you!

OK, I've documented the overall in the main page: https://developer.mozilla.org/en-US/Add-ons/WebExtensions/API/contextualIdentities, and have updated all the methods to describe what they return in errors, and added compat notes to cover the old behavior. I'm going to mark this and the other related bugs, bug 1354602 and bug 1389265, as dev-doc-complete, but please let me know if you need anything else here.

Is manual testing required on this bug? If Yes, please provide some STR and the proper webextension(if required), if No set the “qe-verify-“ flag.

Not really needed, this isn't core DOM nor important to it working. I have verified it works anyway, it's a little bit of an edge case anyway.