Hey Rob! I think in general a misunderstanding here is that GeckoView does not do any container management, so we can't really just use the values that gecko knows about. We also allow private browsing containers and that's why we didn't implement `firefox-default` and `firefox-private`. Those ids are already special and don't behave the same from normal container IDs, don't really give additional information and are not documented anywhere so I wouldn't expect extensions to use them in a meaningful way, but I'm happy to be proven wrong. (In reply to Rob Wu [:robwu] from comment #0) > Bug 1622500 introduces support for the "cookieStoreId" parameter, but skips some important checks that are present on desktop Firefox: https://searchfox.org/mozilla-central/rev/598e50d2c3cd81cd616654f16af811adceb08f9f/browser/components/extensions/parent/ext-tabs.js#671-678 > > The checks are in [getUserContextIdForCookieStoreId](https://searchfox.org/mozilla-central/rev/598e50d2c3cd81cd616654f16af811adceb08f9f/toolkit/components/extensions/parent/ext-tabs-base.js#2182-2229) and enforce several requirements: > - Extension must have "cookies" permission. good point, this was missed during code review, let's add a check. > - Container tabs must be enabled by preference. In general apps don't have. away to enable prefs, so this is not really useful for us. > - If "cookieStoreId" is "firefox-private", then it must match the private state of the window. This implicitly enforces the requirement that extensions must have access to that window; but since GV doesn't distinguish between windows and tabs yet, it must separately enforce the private browsing checks. GeckoView supports having private container tabs, if we did something like that we could not implement it. As a I understand it `firefox-private` is an implementation detail not documented anywhere so it shouldn't hurt that much? when we get to the full implementation of the API apps will be able to do this if they want to. > - Checks whether "cookieStoreId" is a valid (and existing) identifier. GeckoView doesn't keep a list of "valid" cookie store IDs, that's entirely to the app to decide. > > In bug 1622500, the `cookieStoreId` seems to be given by the GeckoView app (introduced in bug 1501108). This should not be done. The `cookieStoreId` in the extension API is an abstraction of three kinds of isolation: > - normal browsing "firefox-default" > - private browsing "firefox-private" > - container tabs "firefox-container-<internal container id>" This is a leaky abstraction though, isn't it? apps can't really create a "firefox-private" tab in a non-private window. Or a container window in a private window, so they need to be aware that `firefox-private` and `firefox-default` are and how they work differently than containers. > > The current version of the API lacks the necessary checks, and performs the mapping at the wrong level. The identifier for container tabs should be derived from Gecko platform information (OriginAttributes), NOT from GV-specific data. > we can't do that because the app never sees those identifiers, and the app is responsible for creating/modifying/deleting "contextual identies". We have a mapping though so we should be able to fix the extension APIs to understand us. > The "cookieStoreId" is used in other APIs, including cookies and webRequest. In order to have a consistent and functional API, [various places](https://searchfox.org/mozilla-central/search?q=userContextId&path=extensions%2F&case=true®exp=false) where "userContextId" is used should recognize "geckoViewSessionContextId" too. yep, good point we need to fix those APIs to use our context ID too.
Bug 1643688 Comment 1 Edit History
Note: The actual edited comment in the bug view page will always show the original commenter’s name and original timestamp.
Hey Rob! I think in general a misunderstanding here is that GeckoView does not do any container management, so we can't really just use the values that gecko knows about. We also allow private browsing containers and that's why we didn't implement `firefox-default` and `firefox-private`. Those ids are already special and don't behave the same from normal container IDs, don't really give additional information and are not documented anywhere so I wouldn't expect extensions to use them in a meaningful way, but I'm happy to be proven wrong. (In reply to Rob Wu [:robwu] from comment #0) > Bug 1622500 introduces support for the "cookieStoreId" parameter, but skips some important checks that are present on desktop Firefox: https://searchfox.org/mozilla-central/rev/598e50d2c3cd81cd616654f16af811adceb08f9f/browser/components/extensions/parent/ext-tabs.js#671-678 > > The checks are in [getUserContextIdForCookieStoreId](https://searchfox.org/mozilla-central/rev/598e50d2c3cd81cd616654f16af811adceb08f9f/toolkit/components/extensions/parent/ext-tabs-base.js#2182-2229) and enforce several requirements: > - Extension must have "cookies" permission. good point, this was missed during code review, let's add a check. > - Container tabs must be enabled by preference. In general apps don't have a way to enable prefs, so this is not really useful for us. > - If "cookieStoreId" is "firefox-private", then it must match the private state of the window. This implicitly enforces the requirement that extensions must have access to that window; but since GV doesn't distinguish between windows and tabs yet, it must separately enforce the private browsing checks. GeckoView supports having private container tabs, if we did something like that we could not implement it. As a I understand it `firefox-private` is an implementation detail not documented anywhere so it shouldn't hurt that much? when we get to the full implementation of the API apps will be able to do this if they want to. > - Checks whether "cookieStoreId" is a valid (and existing) identifier. GeckoView doesn't keep a list of "valid" cookie store IDs, that's entirely to the app to decide. > > In bug 1622500, the `cookieStoreId` seems to be given by the GeckoView app (introduced in bug 1501108). This should not be done. The `cookieStoreId` in the extension API is an abstraction of three kinds of isolation: > - normal browsing "firefox-default" > - private browsing "firefox-private" > - container tabs "firefox-container-<internal container id>" This is a leaky abstraction though, isn't it? apps can't really create a "firefox-private" tab in a non-private window. Or a container window in a private window, so they need to be aware that `firefox-private` and `firefox-default` are and how they work differently than containers. > > The current version of the API lacks the necessary checks, and performs the mapping at the wrong level. The identifier for container tabs should be derived from Gecko platform information (OriginAttributes), NOT from GV-specific data. > we can't do that because the app never sees those identifiers, and the app is responsible for creating/modifying/deleting "contextual identies". We have a mapping though so we should be able to fix the extension APIs to understand us. > The "cookieStoreId" is used in other APIs, including cookies and webRequest. In order to have a consistent and functional API, [various places](https://searchfox.org/mozilla-central/search?q=userContextId&path=extensions%2F&case=true®exp=false) where "userContextId" is used should recognize "geckoViewSessionContextId" too. yep, good point we need to fix those APIs to use our context ID too.