Closed Bug 1423220 Opened 6 years ago Closed 6 years ago

Enable tab warming by default on Nightly

Categories

(Firefox :: Tabbed Browser, enhancement, P3)

enhancement

Tracking

()

RESOLVED FIXED
Firefox 61
Tracking Status
firefox61 --- fixed

People

(Reporter: mconley, Assigned: mconley)

References

(Regressed 1 open bug)

Details

(Whiteboard: [fxperf:p1])

Attachments

(2 files, 1 obsolete file)

This involves setting:

browser.tabs.remote.warmup.enabled

to true for Nightly builds.
Depends on: 1423208
Priority: -- → P3
Depends on: 1397426
Depends on: 1428604
Depends on: 1430160
Depends on: 1423200
Depends on: 1432509
No longer depends on: 1430160
Blocks: 1434651
Comment on attachment 8947168 [details]
Bug 1423220 - Enable tab warming by default for Nightly builds.

https://reviewboard.mozilla.org/r/216924/#review224822

Sorry for the delay, this trivial review request got lost in my backlog that I'm working through. Not much to review here, really.
Attachment #8947168 - Flags: review?(dao+bmo) → review+
Whiteboard: [fxperf:p1]
Assignee: nobody → mconley
Pushed by mconley@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/adf758d8cff9
Enable tab warming by default for Nightly builds. r=dao
Backout by csabou@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/5db3b0cf00bf
Backed out changeset adf758d8cff9 for frequent mochitest browser chrome failures on dom/base/test/browser_bug1303838.js
Ooof, that's a new one. Thanks, Investigating.
Flags: needinfo?(mconley)
We're running too long because on Linux, the mouse floats over the first tab, warming it up. The TabSwitchDone event, which this test waits for a bunch, only fires after warmed tabs are evicted, which is after a few seconds by default.

So I've modified this test to reduce the eviction time to 50 milliseconds. This allows the test to continue running with warming enabled, but not time out.
Pushed by mconley@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/664c633802d4
Enable tab warming by default for Nightly builds. r=dao
https://hg.mozilla.org/mozilla-central/rev/664c633802d4
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 61
Depends on: 1445213
Backed out for frequently failing mochitest dom/html/test/test_fullscreen-api-race.html (bug 1445330):

https://hg.mozilla.org/mozilla-central/rev/943930f3737f3b0850cc7e14c2fe23e4f5f5518b
Status: RESOLVED → REOPENED
Flags: needinfo?(mconley)
Resolution: FIXED → ---
Target Milestone: Firefox 61 → ---
19 runs of mochitest-2 running the test are green with my fix - I'm fairly confident that cinches it. Patch coming up.
Flags: needinfo?(mconley)
So the story here is that the DOM Fullscreen stuff tries to ensure that the DocShell is activated when going into fullscreen. If it's not activated, we throw an error here:

https://searchfox.org/mozilla-central/rev/6e96a3f1e44e286ddae5fdafab737709741d237a/dom/base/nsDocument.cpp#11160

The test that was failing opens tabs from content using window.open, and the async tab switcher (with warming enabled) was only activating the DocShell _after_ entering STATE_LOADED. I think that's fine for warming tabs, but for tabs that aren't being warmed, I think we can safely fall back to activating the DocShell inside STATE_LOADING.
Attachment #8947168 - Attachment is obsolete: true
Bah, sorry dao, MozReview is making me ask for your review again. :/
Comment on attachment 8959600 [details]
Bug 1423220 - Enable tab warming by default for Nightly builds.

https://reviewboard.mozilla.org/r/228408/#review234266
Attachment #8959600 - Flags: review?(dao+bmo) → review+
Comment on attachment 8959599 [details]
Bug 1423220 - Don't delay activating the DocShell for tabs that we're rendering by switching and not warming.

https://reviewboard.mozilla.org/r/228406/#review234272

Chatted in IRC about maybe checking if this is the requestedTab, rather than checking the warming set. I think it would only matter for pathological tests, though, so r+ for whichever direction you choose.
Attachment #8959599 - Flags: review?(dothayer) → review+
(In reply to Doug Thayer [:dthayer] from comment #19)

Yeah, I like the requestedTab approach better. Thanks!

Try build looks good: https://treeherder.mozilla.org/#/jobs?repo=try&revision=e689591dc95f0638e749c6ca38c8f16eb1555b45

Landing...
Pushed by mconley@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/8ff03a020b69
Don't delay activating the DocShell for tabs that we're rendering by switching and not warming. r=dthayer
https://hg.mozilla.org/integration/autoland/rev/beae66d4b661
Enable tab warming by default for Nightly builds. r=dao
https://hg.mozilla.org/mozilla-central/rev/8ff03a020b69
https://hg.mozilla.org/mozilla-central/rev/beae66d4b661
Status: REOPENED → RESOLVED
Closed: 6 years ago6 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 61
Depends on: 1447193
Regressions: 1512013
Regressions: 1656218
You need to log in before you can comment on or make changes to this bug.