Closed Bug 1610888 Opened 5 years ago Closed 5 years ago

Disable DocumentChannel and dependent features for Firefox 73

Categories

(Core :: Networking, task, P1)

task

Tracking

()

VERIFIED FIXED
Tracking Status
firefox72 --- unaffected
firefox73 blocking verified
firefox74 --- unaffected

People

(Reporter: RyanVM, Assigned: RyanVM)

References

(Blocks 2 open bugs)

Details

Attachments

(1 file)

Due to stability issues, it's been decided to hold back DocumentChannel from the Fx73 release. This has downstream impacts on other features such as SW-e10s.

We need to determine which prefs will need disabling and get a patch running on Try to ensure there is no CI fallout from doing so. Ideally, we would like to get this patch landed for tomorrow's 73.0b9 build so there is time to look for any fallout ahead of the 73 RC build on 3-Feb.

@ jya or Matt: can one of you please disable DocumentChannel in 73 Beta? Do we need to also disable frameloader rebuilding (bug 1583614)? Do dependent features (like Service Workers e10s and frameloader rebuilding) need to be pref'd off atomically in the same changeset we pref off DocumentChannel?

We don't want to disable DocumentChannel in Nightly (74) because we'd like to try to ship 74.

@ Jens: can you please notify your Service Worker e10s engineer that we will be disabling DocumentChannel in 73 Beta?

Flags: needinfo?(matt.woodrow)
Flags: needinfo?(jyavenard)
Flags: needinfo?(jstutte)

(In reply to Matt Woodrow (:mattwoodrow) from comment #2)

https://treeherder.mozilla.org/#/jobs?repo=try&revision=91e538f370919048eb7b44f6785dac2b6aea98b6

This had some test failures related to various tests which had annotations adjusted to account for DocumentChannel and/or SW-e10s being enabled. Also, the harness was still operating as if SW-e10s was enabled even though it wasn't, causing some test annotations to not properly work. Below is a Try push which addresses those issues. We should just fold the patch into Matt's pref flip patch when it's ready to land.
https://treeherder.mozilla.org/#/jobs?repo=try&group_state=expanded&revision=f6b423b7ff4bf172a852c192e6f59c7e2c6d61f8

There's only one remaining issue, a leak in the dom/serviceworkers tests which both ASAN and debug builds are seeing:
https://treeherder.mozilla.org/logviewer.html#?job_id=286086685&repo=try
https://treeherder.mozilla.org/logviewer.html#?job_id=286086464&repo=try

The leaks look similar to bug 1604047 except those apparently went away on their own. NI Perry for updates on that as he was the one investigating last I knew.

Flags: needinfo?(perry)
Flags: needinfo?(jstutte)
Assignee: nobody → ryanvm
Status: NEW → ASSIGNED

Comment on attachment 9122829 [details]
Bug 1610888 - Disable DocumentChannel and SW-e10s.

Beta/Release Uplift Approval Request

  • User impact if declined: High frequency DocumentChannel crash on beta, as well as some lower frequency service worker crashes.
  • 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): We haven't been shipping this code with these prefs disabled on Nightly for a while, so it's possible that things have regressed in a way that don't show up in tests.

I think at this point it's worth the risk, since we know the current state has bad crash issues.

  • String changes made/needed: None
Flags: needinfo?(matt.woodrow)
Attachment #9122829 - Flags: approval-mozilla-beta?

Comment on attachment 9122829 [details]
Bug 1610888 - Disable DocumentChannel and SW-e10s.

Approved for 73.0b9. Thanks for helping get this rounded into shape, Matt and Perry!

Also, can we please get a follow-up bug filed for the leaks we wallpapered over for now?

Flags: needinfo?(perry)
Flags: needinfo?(jyavenard)
Attachment #9122829 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
Status: ASSIGNED → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED

We should have QA verify that these are disabled as expected in 73.0b9.

Flags: qe-verify+

There's no way to ignore a debug leak in any kind of limited way. I don't want to let through main process leaks in across all mochitests. Maybe you can figure out what test is causing the leak.

dom/serviceworkers/test/test_https_origin_after_redirect.html is the leaking test.

See Also: → 1611588
QA Whiteboard: [qa-triaged]

Hello,
Confirming this issue as verified fixed with 73.0b10(20200125104833) . Verified using Windows 10x64, macOS 10.15 and Ubuntu 18.04.

browser.tabs.documentchannel, dom.serviceWorkers.parent_intercept preff's are turned off by default.

Status: RESOLVED → VERIFIED
Flags: qe-verify+

== Change summary for alert #24808 (as of Mon, 27 Jan 2020 13:53:23 GMT) ==

Improvements:

18% raptor-tp6-yandex-firefox-cold fcp windows10-64-shippable-qr opt 375.67 -> 309.33

For up to date results, see: https://treeherder.mozilla.org/perf.html#/alerts?id=24808

Regressions: 1612009
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: