1622500 - Implement cookieStoreId parameter for tabs.create

Status: RESOLVED FIXED

(GeckoView :: Extensions, enhancement, P2)

Description

[Gabriel Luong [:gl] (ΦωΦ)]

This implements the cookieStoreId parameter for tabs.create() that will allow for creating container tabs.

Comment 2

[Emily Toop (:fluffyemily)]

Fenix request that work for this issue be completed in Q2 ahead of implementation of containers in Fenix in Q3.

Comment 3

[Gabriel Luong [:gl] (ΦωΦ)]

Attachment: Bug 1622500 - Implement cookieStoreId parameter for tabs.create.

Comment 4

[Gabriel Luong [:gl] (ΦωΦ)]

Still a wip.

Comment 5

[Emily Toop (:fluffyemily)]

We would like to see this done in Q2 for Fenix.

Comment 6

[Gabriel Luong [:gl] (ΦωΦ)]

Comment on attachment 9142152 [details]
Bug 1622500 - Implement cookieStoreId parameter for tabs.create.

I am still running into an error https://treeherder.mozilla.org/#/jobs?repo=try&revision=02d4a960cc5778679732e6967f9837938897721d&selectedTaskRun=DxuTEXuTQnOcR2E8qtbjHg-0 and wondering if you know where else I am missing the changes to get the mochitest to pass.

TEST-UNEXPECTED-FAIL | mobile/android/components/extensions/test/mochitest/test_ext_tabs_create.html | Expected value for tab.cookieStoreId - Expected: firefox-container-1, Actual: firefox-default

with the changes to the TestRunnerActivity.java to pass in the GeckoSessionSetting with the contextId when creating a GeckoSession. I also made a similar change to GeckoViewActivity.java, which I don't know if we want to keep or not.

I added a test in WebExtensionTest.java in the meantime which gives me a bit more confidence.

Comment 7

[[ex-Mozilla] Agi Sferro | :agi]

Responded on Phabricator, copying it here:

From what I can see you also need to update get cookieStoreId here: https://searchfox.org/mozilla-central/source/mobile/android/components/extensions/ext-utils.js#319-321

GeckoView uses a different attribute than Desktop, so the current implementation does not work. We set an attribute called geckoViewSessionContextId, so the implenetion of get cookieStoreId should read that instead, e.g. this.browser.getAttribute("geckoViewSessionContextId") should give you the right value, except...

There's an extra step, for safety we transform the id a bit, see here: https://searchfox.org/mozilla-central/rev/446160560bf32ebf4cb7c4e25d7386ee22667255/mobile/android/geckoview/src/main/java/org/mozilla/geckoview/StorageController.java#186-201 basically we append the string "gvctx" to the bytes hex representation of the string. To get the id that the extension is passed in we need to do the reverse transformation before returning it in the method above.

We can also chat about this on zoom/matrix if it makes it easier :)

Comment 8

[[ex-Mozilla] Agi Sferro | :agi]

I also made a similar change to GeckoViewActivity.java, which I don't know if we want to keep or not.

Yeah we need both, this one is for GeckoViewExample and the other one is for tests.

Comment 9

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/fd31c8d82b41
Implement cookieStoreId parameter for tabs.create. r=geckoview-reviewers,agi

Comment 10

[Cristian Brindusan [:cbrindusan]]

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