1529762 - Page Style (alternative style sheets) menu population can delay about:home from painting

Status: RESOLVED FIXED

(Firefox :: Menus, defect, P3)

Description

[Mike Conley (:mconley) (:⚙️)]

Here's a profile:

http://bit.ly/2Xitk7C

Here's the relevant bit:

http://bit.ly/2XfOJ1n

We're... getting a pageshow off of an SVG? And that's causing PageStyleChild.jsm to load at a pretty bad time when it's clear that the disk is pretty busy.

Do we ever need PageStyleChild.jsm for our internal pages? I doubt it. We barely need for normal pages. Does the matches syntax allow us to blacklist about: pages?

Comment 1

[Mike Conley (:mconley) (:⚙️)]

Even better, just have the actor match https://* and http://*.

Comment 2

[:Gijs (out for now; he/him)]

Backlog for the menus component, but that shouldn't affect the fxperf prioritization, which should probably be higher if this is trivial to fix.

Comment 3

[:Felipe Gomes (needinfo for replies!)]

Interesting.. In addition to the suggestion here, it looks like PageStyleChild.jsm gathers the info synchronously with the pageshow event [1], but it really should send it off an idle callback, or at least a spin of the event loop.

[1] https://searchfox.org/mozilla-central/rev/b10ae6b7a50d176a813900cbe9dc18c85acd604b/browser/actors/PageStyleChild.jsm#56

There's one catch with using matches: on the actors, which is that it implies using allFrames=true. It's not gonna break anything since the handleEvent in PageStyleChild filters non-toplevel events, but it'll run more often. I don't think this will be a problem though.. but perhaps the idle callback suggestion is enough to fix the problem?

Comment 4

[Mike Conley (:mconley) (:⚙️)]

Attachment: Bug 1529762 - PageStyle child actors should only instantiate for web pages. r?felipe

This avoids instantiating them for internal pages, like about:home.

Comment 5

[Mike Conley (:mconley) (:⚙️)]

Attachment: Bug 1529762 - Send PageStyle information up from pageshow events only after idle. r?felipe

Depends on D22520

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/459979fd1d25
PageStyle child actors should only instantiate for web pages. r=Felipe
https://hg.mozilla.org/integration/autoland/rev/f49189976e12
Send PageStyle information up from pageshow events only after idle. r=Felipe

Comment 7

[Noemi Erli[:noemi_erli]]

Backed out 2 changesets (Bug 1529762) for failures in browser_page_style.js CLOSED TREE

Push with failures: https://treeherder.mozilla.org/#/jobs?repo=autoland&resultStatus=testfailed%2Cbusted%2Cexception&classifiedState=unclassified&revision=f49189976e12e2f01ff80a0f8fb2389a3136aecb&selectedJob=232512182

Failure log: https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=232512182&repo=autoland&lineNumber=8130

Backout: https://hg.mozilla.org/integration/autoland/rev/101887de976288348feae904140b343a1ee2eec4

Comment 8

[Mike Conley (:mconley) (:⚙️)]

Attachment: Bug 1529762 - Update browser_page_style.js test to use a normal web page rather than a data URI. r?gl

The PageStyleChild now only attaches to normal web pages.

Depends on D22521

Comment 9

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/cd0a4ead024e
PageStyle child actors should only instantiate for web pages. r=Felipe
https://hg.mozilla.org/integration/autoland/rev/92aa678ac36b
Send PageStyle information up from pageshow events only after idle. r=Felipe
https://hg.mozilla.org/integration/autoland/rev/8ac8ed5ac772
Update browser_page_style.js test to use a normal web page rather than a data URI. r=gl

Comment 10

[Dorel Luca [:dluca]]

https://hg.mozilla.org/mozilla-central/rev/cd0a4ead024e
https://hg.mozilla.org/mozilla-central/rev/92aa678ac36b
https://hg.mozilla.org/mozilla-central/rev/8ac8ed5ac772

Comment 11

[Florin Strugariu [:Bebe]]

== Change summary for alert #19832 (as of Fri, 08 Mar 2019 22:12:40 GMT) ==

Improvements:

0% Base Content JS osx-10-10 opt stylo 4,013,984.00 -> 4,000,452.00

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

Comment 12

[Daniel Holbert [:dholbert]]

[adding Alternative Style Sheets to bug summary for searchability purposes, since this is about https://developer.mozilla.org/en-US/docs/Web/CSS/Alternative_style_sheets ]