1598800 - Fix dom/tests/browser/browser_localStorage_e10s.js and dom/tests/browser/browser_localStorage_snapshotting_e10s.js for Fission

Status: NEW

(Core :: Storage: localStorage & sessionStorage, task, P3)

Description

[Chris Peterson [:cpeterson]]

Jan, Hsin-Yi's DOM test spreadsheet for Fission says you will investigate these browser_localStorage_e10s test failures this week (Nov 18). Do you have any updates?

Comment 1

[Jan Varga [:janv]]

We had to finish something for 71, I'll try to address this bug this week.

Comment 2

[Jan Varga [:janv]]

It's not very hard to fix browser_localStorage_e10s.js, I have patch for that.
I'm still working on fixing browser_localStorage_snapshotting_e10s.js

Comment 3

[Jan Varga [:janv]]

Actually, after more thinking about this, there's more work to be done. The goal of these tests is to verify that localStorage data accessed from different content processes for given origin is consistent. However, current fission makes it so that a new process is not created for the same origin in these tests, so the consistency is not tested. I asked around if it will be ever possible (with fission) to have different processes for the same origin and the answer is yes, so we need to figure out how to modify these tests to create different processes for the same origin.

Comment 4

[Jan Varga [:janv]]

I spoke with Nika, a new process can be created by setting these headers:
cross-origin-embedder-policy
cross-origin-opener-policy

Comment 5

[Eden Chuang[:edenchuang]]

I took a look at these tests.
As Jan mentioned in comment 3, the process creation assumption is not valid anymore after fission on.
And according to comment 4, the process creation in fission should be controlled through setting COOP and COEP.
So what I will do next is setting these tests with proper COOP and COEP to make sure the test pages can run in different processes. It is supposed to make these tests workable.

Comment 6

[Chris Peterson [:cpeterson]]

Eden, do you have any updates on a fix for this test bug? Fixing and re-enabling mochitests blocks enabling Fission in Nightly.

The browser_localStorage_e10s.js and browser_localStorage_snapshotting_e10s.js tests are still disabled for Fission:

https://searchfox.org/mozilla-central/rev/4166c15e2a99a23a9b38ad62c9fdfe8e5448b354/dom/tests/browser/browser.ini#62-63,66-70

Comment 7

[Chris Peterson [:cpeterson]]

The Fission team hopes to roll out Fission to some Nightly users in Q3, so it would be good if all tests are passing and enabled for Fission by the end of Q2.

Comment 8

[Eden Chuang[:edenchuang]]

After analyzing the test concept, I don't think these tests should be applied for fission, since these tests are for testing the localstorage consistency between same-origin tab but different processes tabs. However, the test scenario does not fit the fission assumption.

I remove the block of fission-mochitest, but I keep disabling them for fission. These tests might need to remove or rewrite when we no longer support non-fission Firefox.

Comment 9

[Chris Peterson [:cpeterson]]

Thanks for investigating.

Nika says we plan to support same-origin tabs in different processes someday. In the meantime, I'll track this bug for Fission Future (for work after we ship Fission MVP).

Comment 10

[Nika Layzell [:nika] (ni? for response)]

It actually is still possible to have multiple different processes which load same-origin tabs, thanks to the Cross-Origin-Opener-Policy and Cross-Origin-Embedder-Policy headers. When both of these headers are set together, and the pref is enabled, the document will load in the webCOOP+COEP=... process instead of the webIsolated=... process, but they will still be same-origin to one another, and share localstorage.

In addition, we do plan to eventually support having multiple content processes for a single site, even with fission enabled, though in the trivial case that isn't supported yet.

If adapting this test to work with fission right now is too difficult, though, I suppose we can postpone getting it to work with fission until a later date, so long as we're still confident it will work with webIsolated and webCOOP+COEP.

Comment 11

[Chris Peterson [:cpeterson]]

(In reply to Nika Layzell [:nika] (ni? for response) from comment #10)

If adapting this test to work with fission right now is too difficult, though, I suppose we can postpone getting it to work with fission until a later date, so long as we're still confident it will work with webIsolated and webCOOP+COEP.

Temporarily moving this bug from Fission Future to Nightly M6c until we confirm the test will work with webIsolated and webCOOP+COEP.

Comment 12

[Eden Chuang[:edenchuang]]

Yes, by providing COOP and COEP, we can load the same-origin document in webCOOP+COEP process. If I understand correctly, for each origin, there would be only one webCOOP+COEP process. However, the problem is the original test need to 4 and 5 processes at the same time. And it is hard to modify the test scenario from five into two processes.

I think we still need a test for LocalStorage consistency for fission, but it should be another test scenario. So I suggest to create a new test, not modify this one.

Comment 13

[Chris Peterson [:cpeterson]]

(In reply to Eden Chuang[:edenchuang] from comment #12)

I think we still need a test for LocalStorage consistency for fission, but it should be another test scenario. So I suggest to create a new test, not modify this one.

OK. I filed a new bug to create that test: bug 1641028

If adapting this test to work with fission right now is too difficult, though, I suppose we can postpone getting it to work with fission until a later date, so long as we're still confident it will work with webIsolated and webCOOP+COEP.

I'll move this bug back to the Fission Future list of bugs that don't block shipping Fission MVP.

... unless someone still needs to investigate the test to be "confident it will work with webIsolated and webCOOP+COEP"?