1554217 - Disable HTTPResponseProcessSelection on Beta

Status: RESOLVED FIXED

(Core :: DOM: Navigation, task, P1)

Comment 1

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

Attachment: Bug 1554217 - Disable HTTPResponseProcessSelection on Beta,

Comment 2

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

Comment on attachment 9067373 [details]
Bug 1554217 - Disable HTTPResponseProcessSelection on Beta,

Beta/Release Uplift Approval Request

• User impact if declined: A feature with potential known issues will be enabled in 68.
• Is this code covered by automated tests?: Yes
• Has the fix been verified in Nightly?: No
• Needs manual test from QE?: No
• If yes, steps to reproduce:
• List of other uplifts needed: None
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): This patch just disables a feature which was also disabled in 67. It is disabled outside of nightly right now due to some known edge-case issues around it.
• String changes made/needed: None

Comment 3

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

Just requested approval despite not having r+ yet, as it's just a pref flip, so the review may not be needed.

Comment 4

[Julien Cristau [:jcristau]]

Shouldn't we just put the pref behind #ifdef NIGHTLY_BUILD and uplift that, just in case the issues aren't fixed in time for 69? Or is that not going to happen? :)

Comment 5

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

Sure, I'd also be fine with that.

Comment 6

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

The patch has been updated to use a compile guard to as suggested in comment 4.

Comment 7

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/1d376f00f17e
Disable HTTPResponseProcessSelection on Beta, r=farre

Comment 8

[Neha Kochar [:neha]]

Looks like bug 1551601 is going in 68 so we shouldn't need to disable this pref now in 68.

Comment 9

[Oana Pop-Rus]

https://hg.mozilla.org/mozilla-central/rev/1d376f00f17e

Comment 10

[Neha Kochar [:neha]]

oh never mind, we still need to disable the pref in 68 because we don't yet have a fix for bug 1552012, which is happening only when we enable httpResponseProcessSelection by default.

Comment 11

[Julien Cristau [:jcristau]]

Comment on attachment 9067373 [details]
Bug 1554217 - Disable HTTPResponseProcessSelection on Beta,

turn a pref back off on beta to avoid regressions. approved for 68.0b7

Comment 12

[Alexandru Michis [:malexandru]]

https://hg.mozilla.org/releases/mozilla-beta/rev/607ce840d797

Comment 13

[Cosmin Sabou [:CosminS]]

Backed out for browser chrome failures
Push with failures: https://treeherder.mozilla.org/#/jobs?repo=mozilla-beta&resultStatus=testfailed%2Cbusted%2Cexception&fromchange=72e634f9360acbc01148c18bad4fbbb4defb3238&tochange=670f4fbbfd591ac82dfa8a7632aec3974dad5a2b&selectedJob=249432120

Failure log: https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=249432120&repo=mozilla-beta&lineNumber=17774

Backout link: https://hg.mozilla.org/releases/mozilla-beta/rev/4d86da68f2f1aba9484a8b3253e0a694c9185052

18:18:31 INFO - TEST-PASS | dom/serviceworkers/test/browser_navigation_process_swap.js | file:///Users/cltbld/tasks/task_1559325648/build/tests/mochitest/browser/dom/serviceworkers/test/empty.html should load in a file process - "file" == "file" -
18:18:31 INFO - Dynamically creating file:///Users/cltbld/tasks/task_1559325648/build/tests/mochitest/browser/dom/serviceworkers/test/empty.html's link
18:18:31 INFO - Console message: [JavaScript Error: "The character encoding of the HTML document was not declared. The document will render with garbled text in some browser configurations if the document contains characters from outside the US-ASCII range. The character encoding of the page must be declared in the document or in the transfer protocol." {file: "file:///Users/cltbld/tasks/task_1559325648/build/tests/mochitest/browser/dom/serviceworkers/test/empty.html" line: 0}]
18:18:31 INFO - Starting navigation
18:18:31 INFO - Waiting for location to change to http://example.com/browser/dom/serviceworkers/test/empty_with_utils.html
18:18:31 INFO - Buffered messages logged at 18:18:31
18:18:31 INFO - Waiting for the browser to stop
18:18:31 INFO - Console message: [JavaScript Error: "RemoteWebProgress failed to call onProgressChange: [Exception... "JavaScript component does not have a method named: "onProgressChange"'JavaScript component does not have a method named: "onProgressChange"' when calling method: [nsIWebProgressListener::onProgressChange]" nsresult: "0x80570030 (NS_ERROR_XPC_JSOBJECT_HAS_NO_FUNCTION_NAMED)" location: "JS frame :: resource://gre/modules/RemoteWebProgress.jsm :: _callProgressListeners :: line 103" data: no]
18:18:31 INFO - " {file: "resource://gre/modules/RemoteWebProgress.jsm" line: 105}]
18:18:31 INFO - _callProgressListeners@resource://gre/modules/RemoteWebProgress.jsm:105:14
18:18:31 INFO - onProgressChange@resource://gre/modules/RemoteWebProgress.jsm:120:10
18:18:31 INFO -
18:18:31 INFO - Buffered messages finished
18:18:31 INFO - TEST-UNEXPECTED-FAIL | dom/serviceworkers/test/browser_navigation_process_swap.js | http://example.com/browser/dom/serviceworkers/test/empty_with_utils.html should load in a web-content process - "file" == "web" - JS frame :: chrome://mochitests/content/browser/dom/serviceworkers/test/browser_navigation_process_swap.js :: runTest :: line 106
18:18:31 INFO - Stack trace:
18:18:31 INFO - chrome://mochitests/content/browser/dom/serviceworkers/test/browser_navigation_process_swap.js:runTest:106
18:18:31 INFO - Loading initial page to unregister all Service Workers
18:18:31 INFO - GECKO(2247) | --DOMWINDOW == 8 (0x12b884000) [pid = 2251] [serial = 10] [outer = 0x0] [url = about:blank]
18:18:31 INFO - GECKO(2247) | --DOMWINDOW == 7 (0x12c0a3c00) [pid = 2251] [serial = 13] [outer = 0x0] [url = about:blank]
18:18:31 INFO - GECKO(2247) | --DOCSHELL 0x12b89d000 == 1 [pid = 2251] [id = {d9e0e988-fdf1-de43-9839-61e39c127908}] [url = about:blank]
18:18:31 INFO - GECKO(2247) | --DOCSHELL 0x12b8a5800 == 0 [pid = 2251] [id = {f242c452-95e5-914d-af31-1a30def1ee16}] [url = http://mochi.test:8888/browser/dom/serviceworkers/test/empty_with_utils.html]
18:18:31 INFO - GECKO(2247) | ++DOCSHELL 0x12b8a6000 == 1 [pid = 2251] [id = {c1a0e9c6-ea8c-e84d-9909-95d9ae995ec0}]
18:18:31 INFO - GECKO(2247) | ++DOMWINDOW == 8 (0x12b825200) [pid = 2251] [serial = 18] [outer = 0x0]
18:18:31 INFO - GECKO(2247) | ++DOMWINDOW == 9 (0x12b887000) [pid = 2251] [serial = 19] [outer = 0x12b825200]
18:18:31 INFO - GECKO(2247) | [Child 2251, Main Thread] WARNING: NS_ENSURE_TRUE(!mHasOrHasHadOwnerWindow || mOwnerWindow) failed: file /builds/worker/workspace/build/src/dom/events/DOMEventTargetHelper.cpp, line 318
18:18:31 INFO - GECKO(2247) | --DOMWINDOW == 8 (0x12b825020) [pid = 2251] [serial = 9] [outer = 0x0] [url = http://mochi.test:8888/browser/dom/serviceworkers/test/download_canceled/page_download_canceled.html]
18:18:31 INFO - GECKO(2247) | ###!!! [Child][MessageChannel] Error: (msgtype=0x350117,name=PContent::Msg_StoreUserInteractionAsPermission) Closed channel: cannot send/recv
18:18:31 INFO - GECKO(2247) | [Child 2258, Main Thread] WARNING: MsgDropped in ContentChild: file /builds/worker/workspace/build/src/dom/ipc/ContentChild.cpp, line 2319
18:18:31 INFO - GECKO(2247) | [Child 2258, Main Thread] WARNING: nsAppShell::Exit() called redundantly: file /builds/worker/workspace/build/src/widget/cocoa/nsAppShell.mm, line 725
18:18:31 INFO - GECKO(2247) | ++DOMWINDOW == 9 (0x12c0a6800) [pid = 2251] [serial = 20] [outer = 0x12b825200]
18:18:31 INFO - GECKO(2247) | --DOCSHELL 0x111e44800 == 0 [pid = 2255] [id = {0e415122-8308-3947-9f1b-c631cae6a16b}] [url = about:blank]
18:18:31 INFO - GECKO(2247) | --DOCSHELL 0x118d44800 == 0 [pid = 2258] [id = {36d8b16f-51e1-c547-a92e-d1845affb62c}] [url = http://example.com/browser/dom/serviceworkers/test/empty_with_utils.html]
18:18:31 INFO - Unregistering all Service Workers

Comment 14

[Neha Kochar [:neha]]

Adding Valentin as he might have ideas for this failure too.

Comment 15

[Valentin Gosu [:valentin] (he/him)]

(In reply to Neha Kochar [:neha] from comment #14)

Adding Valentin as he might have ideas for this failure too.

The test depends on the pref.
This change should fix it:

diff --git a/dom/serviceworkers/test/browser_navigation_process_swap.js b/dom/serviceworkers/test/browser_navigation_process_swap.js
--- a/dom/serviceworkers/test/browser_navigation_process_swap.js
+++ b/dom/serviceworkers/test/browser_navigation_process_swap.js
@@ -49,6 +49,7 @@ async function runTest() {
       ['dom.serviceWorkers.exemptFromPerDomainMax', true],
       ['dom.serviceWorkers.testing.enabled', true],
       ['devtools.console.stdout.content', true],
+      ['browser.tabs.remote.useHTTPResponseProcessSelection', true],
     ],
   });

Comment 16

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

That bug was also seen in the uplift emulator for the landed one: bug 1555382. We might also need to uplift the pref flip for bug 1555688.

Comment 17

[Alexandru Michis [:malexandru]]

Could you request an uplift approval also for Bug 1555688?
Thank you.

Comment 18

[Julien Cristau [:jcristau]]

It's test-only, doesn't need approval.

Comment 19

[Alexandru Michis [:malexandru]]

https://hg.mozilla.org/releases/mozilla-beta/rev/0d8fa616dda8