1742785 - Update more tests within dom/ and docshell/ to work with https-first enabled

Status: RESOLVED FIXED

(Core :: DOM: Security, task, P3)

Comment 1

[Christoph Kerschbaumer [:ckerschb}]

Attachment: Bug 1742785: Update more tests within dom/ and docshell/ to work with https-first enabled

Comment 2

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/dad850a374dd
Update more tests within dom/ and docshell/ to work with https-first enabled r=smaug

Comment 3

[Natalia Csoregi [:nataliaCs]]

Backed out for causing failures on browser_ProcessPriorityManager.js

• backout: https://hg.mozilla.org/integration/autoland/rev/941c2e91da75072a0721659f6dbf5a5ef5187ea4

• push: https://treeherder.mozilla.org/jobs?repo=autoland&revision=dad850a374dd4ec2c76733e47efe3ef1f3a28e3a&group_state=expanded

• failure log: https://treeherder.mozilla.org/logviewer?job_id=359135926&repo=autoland&lineNumber=7365

[task 2021-11-24T18:03:19.988Z] 18:03:19     INFO - TEST-PASS | dom/ipc/tests/browser_ProcessPriorityManager.js | Switching to a new tab should deprioritize the old one - "BACKGROUND" == "BACKGROUND" - 
[task 2021-11-24T18:03:19.989Z] 18:03:19     INFO - Buffered messages finished
[task 2021-11-24T18:03:19.990Z] 18:03:19     INFO - TEST-UNEXPECTED-FAIL | dom/ipc/tests/browser_ProcessPriorityManager.js | The same site should get loaded into the same process - 9 == 10 - JS frame :: chrome://mochitests/content/browser/dom/ipc/tests/browser_ProcessPriorityManager.js :: test_iframe_navigate/< :: line 369
[task 2021-11-24T18:03:19.990Z] 18:03:19     INFO - Stack trace:
[task 2021-11-24T18:03:19.990Z] 18:03:19     INFO - chrome://mochitests/content/browser/dom/ipc/tests/browser_ProcessPriorityManager.js:test_iframe_navigate/<:369
[task 2021-11-24T18:03:19.990Z] 18:03:19     INFO - TEST-PASS | dom/ipc/tests/browser_ProcessPriorityManager.js | Navigation should have switched processes - 11 != 10 - 

shown as high frequency: https://treeherder.mozilla.org/jobs?repo=autoland&group_state=expanded&revision=e4d4dd3872c727c49d93cbddf3448661615038b7&searchStr=browser-chrome%2Cwindows

Comment 4

[Andrew McCreight [:mccr8]]

Let me know if you need some help understanding what is going wrong here with browser_ProcessPriorityManager.js. The test only runs on Windows by default (because the actual priority changing only works there), but I think it should still be possible to run the test on other platforms. Possibly with some local edits or whatever.

Comment 5

[Christoph Kerschbaumer [:ckerschb}]

(In reply to Andrew McCreight [:mccr8] from comment #4)

Let me know if you need some help understanding what is going wrong here with browser_ProcessPriorityManager.js. The test only runs on Windows by default (because the actual priority changing only works there), but I think it should still be possible to run the test on other platforms. Possibly with some local edits or whatever.

Hey Andrew, thanks for offering your help - I guess my question is, why would changing the scheme from http to https has an effect here at all? Or has it to do with the fact that I replaced http://mochi.test:8888 with https://test1.example.com. Alternatively we could disable https-first for this test, but I also think that's semi optimal - do you have more insights for me?

Comment 6

[Andrew McCreight [:mccr8]]

The TLDR here is that you need to change iframeURI2 (test1.example.com) to something that isn't same-site with example.com, so that they will get Fission isolated from each other and put into different processes.

In more detail, the test loads iframeURI2 into one tab, then load a different page with an iframe (file_cross_frame.html) into another tab, then navigates the iframe on the latter page to the same URI as the first tab. The assertion checks that the iframe page ends up in the same process as the page in the first tab.

Your patch changed the domain of iframeURI2 from http://mochi.test:8888/ to https://test1.example.com/. Meanwhile, the domain that file_cross_frame.html is loaded into becomes https://example.com. These two domains are same-site, so they don't get Fission isolated. This issue ends up manifesting weirdly because dom.ipc.processCount.webIsolated is set to 4. This means that when the second example.com tab is loaded we put it into a new process, despite being same site with an existing tab. Then when we navigate the iframe of the second tab, the iframe stays in the process of the second tab (because it is same site with its top level page), instead of being swapped to that of the first tab, and the assertion fails.

There probably should be an additional check that the top level page in the second tab is in a different process than its iframe. That would have pointed more directly at the problem.

Comment 7

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/78307ccc4942
Update more tests within dom/ and docshell/ to work with https-first enabled r=smaug

Comment 8

[Cristina Cozmuta (:CrissCozmuta)]

https://hg.mozilla.org/mozilla-central/rev/78307ccc4942

Comment 9

[Christoph Kerschbaumer [:ckerschb}]

Aryx, based on Bug 1742943#c9 can you please back out that changeset again?

Comment 10

[Natalia Csoregi [:nataliaCs]]

Backed out for causing permafailures on browser_ProcessPriorityManager.js

• backout: https://hg.mozilla.org/integration/autoland/rev/58d2fbdb6b4a1a8aa6b5ea7aed4fd9a84651941e

• push: https://treeherder.mozilla.org/jobs?repo=autoland&revision=78307ccc494241275bba17446e2b989bb004d7f3

• permafail: https://treeherder.mozilla.org/jobs?repo=autoland&revision=7179e712e22bd0771ec9cd6242825efb2b6cb842&searchStr=windows%2C10%2Cx64%2C2004%2Cwebrender%2Cdebug%2Cmochitests%2Cwith%2Cfission%2Cenabled%2Ctest-windows10-64-2004-qr%2Fdebug-mochitest-browser-chrome-fis-e10s%2Cbc3

• failure log: https://treeherder.mozilla.org/logviewer?job_id=359635629&repo=autoland&lineNumber=63434

[task 2021-11-30T00:40:26.524Z] 00:40:26     INFO - TEST-PASS | dom/ipc/tests/browser_ProcessPriorityManager.js | Loading a new tab should make it prioritized - "FOREGROUND" == "FOREGROUND" - 
[task 2021-11-30T00:40:26.524Z] 00:40:26     INFO - Buffered messages finished
[task 2021-11-30T00:40:26.525Z] 00:40:26     INFO - TEST-UNEXPECTED-FAIL | dom/ipc/tests/browser_ProcessPriorityManager.js | Switching to a new tab should deprioritize the old one - "FOREGROUND" == "BACKGROUND" - JS frame :: chrome://mochitests/content/browser/dom/ipc/tests/browser_ProcessPriorityManager.js :: test_iframe_navigate/< :: line 346
[task 2021-11-30T00:40:26.525Z] 00:40:26     INFO - Stack trace:
[task 2021-11-30T00:40:26.525Z] 00:40:26     INFO - chrome://mochitests/content/browser/dom/ipc/tests/browser_ProcessPriorityManager.js:test_iframe_navigate/<:346
[task 2021-11-30T00:40:26.525Z] 00:40:26     INFO - resource://testing-common/BrowserTestUtils.jsm:withNewTab:148
[task 2021-11-30T00:40:26.526Z] 00:40:26     INFO - GECKO(6984) | [Child 4192: Main Thread]: I/DocShellAndDOMWindowLeak ++DOMWINDOW == 7 (24f40adf800) [pid = 4192] [serial = 7] [outer = 24f3e8f4e40]
[task 2021-11-30T00:40:26.526Z] 00:40:26     INFO - TEST-PASS | dom/ipc/tests/browser_ProcessPriorityManager.js | The same site should get loaded into the same process - 9 == 9 - 

Comment 11

[Iulian Moraru]

Backout merged to central: https://hg.mozilla.org/mozilla-central/rev/58d2fbdb6b4a

Comment 12

[Christoph Kerschbaumer [:ckerschb}]

(In reply to Iulian Moraru from comment #11)

Backout merged to central: https://hg.mozilla.org/mozilla-central/rev/58d2fbdb6b4a

Thank you!

Comment 13

[Christoph Kerschbaumer [:ckerschb}]

FWIW, I removed the changes to the test browser_ProcessPriorityManager.js from the patch attached here and will only land the other test updates within this Patch. Either we fix the test browser_ProcessPriorityManager.js with the changes within Bug 1742872 or I'll file a follow up bug.

Comment 14

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/ff2d8b1ba283
Update more tests within dom/ and docshell/ to work with https-first enabled r=smaug

Comment 15

[Atila Butkovits]

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