1309610 - Add test to verify that the default context of the cookies API is incognito for scripts in incognito tabs

Status: RESOLVED FIXED

(WebExtensions :: General, defect, P3)

Description

[Rob Wu [:robwu]]

ext-cookies.js uses the context.incognito property to get the default storeId if it is not specified. This feature has no test coverage and does not work (i.e. the context is always non-private, even if the caller is in incognito mode). In the past context.incognito *was* defined, but it was removed as a part of https://hg.mozilla.org/mozilla-central/rev/598895fae31d because the property was unused.

The only situation where this is relevant is when extension tabs are opened in a private window.
Specifically, for the cookies API, the following contexts are NOT relevant:
- content scripts (does not support chrome.cookies API)
- background pages ("split incognito mode" is not supported, so background pages are always non-private).
- extension subframes in non-extension tabs (same as content scripts - chrome.cookies not available).

Suggested resolution:
- Add an "incognito" property to BaseContext, initialized to null.
- Let BaseContext's setContentWindow set the incognito property if it was null, using PrivateBrowsingUtils.
- Update the implementation of inIncognitoContext in ext-c-context.js use context.incognito.
- Send the "incognito" property through ChildAPIManager to ParentAPIManager and assign it to ProxyContext (similar to how it was done before in the patch linked above).
- Add tests to verify that the default context is correctly inherit when called from an incognito extension context.


This bug is up for grabs, if you need help I'm happy to assist. Otherwise I may eventually implement the above myself after finishing bug 1287007.

Comment 1

[Rob Wu [:robwu]]

This incognito property can also be useful for bug 1309620.

Comment 2

[Rob Wu [:robwu]]

Note: Matt is implementing context.incognito in bug 1247435.
The current patch does not contain regression tests for this bug, but I can pick that up if needed.

Comment 3

[Jonathan Watt [:jwatt]]

(In reply to Rob Wu [:robwu] from comment #2)
> Note: Matt is implementing context.incognito in bug 1247435.

That bug is now fixed. Does this bug just cover writing tests at this point? (Maybe fix the bug Summary if so?)

Comment 4

[Jonathan Watt [:jwatt]]

Also https://developer.mozilla.org/en-US/Add-ons/WebExtensions does not mention 'incognito' under "Manifest keys". Can someone who knows what they're doing add that, even if it just says "only incognito: spanning" is accepted". (And maybe add other missing keys?)

Comment 5

[Rob Wu [:robwu]]

Bug 1309637(In reply to Jonathan Watt [:jwatt] from comment #3)
> (In reply to Rob Wu [:robwu] from comment #2)
> > Note: Matt is implementing context.incognito in bug 1247435.
> 
> That bug is now fixed. Does this bug just cover writing tests at this point?
> (Maybe fix the bug Summary if so?)

Done.

The tests cannot be written yet because incognito cookies in tests disappear due to bug 1309637.

(In reply to Jonathan Watt [:jwatt] from comment #4)
> Also https://developer.mozilla.org/en-US/Add-ons/WebExtensions does not
> mention 'incognito' under "Manifest keys". Can someone who knows what
> they're doing add that, even if it just says "only incognito: spanning" is
> accepted". (And maybe add other missing keys?)

I think that it would be good to make the documentation update part of bug 1280027.

Comment 6

[Julien Cristau [:jcristau]]

Mass wontfix for bugs affecting firefox 52.

Comment 7

[Shane Caraveo (:mixedpuppy)]

I suspect we have what we need for this in D11699 from bug 1345474.