Bug 1141041 - [e10s] Opening the "View" > "Page Style" menu in remote browser causes unsafe CPOW usage warnings

So the problem is that we're using a CPOW to populate the menupopup via a "syncHandler" defined in tab-content.js.

Let's look at solution 2.

In order for solution 2 to be possible, I need to do the following:

There are some stylesheet event notifications, but they're only fired when styleSheetChangeEventsEnabled is set to true on the document. That property, styleSheetChangeEventsEnabled, was added in https://bugzilla.mozilla.org/show_bug.cgi?id=839103 .

This puts us in a bit of a pickle, because if there is indeed a performance regression when firing these events, which necessitated only flipping it on for the documents that DevTools cares about… well, then, I guess we can't just go and flip it on for every document.

Baseline: https://treeherder.mozilla.org/#/jobs?repo=try&revision=48206c424625

Always fire events: https://treeherder.mozilla.org/#/jobs?repo=try&revision=f80b9f6eea40

Comparison: https://treeherder.mozilla.org/perf.html#/compare?originalProject=try&originalRevision=48206c424625&newProject=try&newRevision=f80b9f6eea40

Bad idea: Poll the content process for the stylesheets. This wouldn't be amazing for performance, since we'd be scheduling some DOM operations every poll, and this would scale linearly per tab.

Another part of Gecko must be informed when a stylesheet is added, because we update the style and repaint, so something must know about it. Can we hook into that?

Is it possible that the StyleSheet events are being fired before DOMContentLoaded is finished? If so, we might be able to reduce the overhead by not firing those events until after the page has been loaded, as StyleSheet events are really only interesting after the page load.

It appears as if we're firing StyleSheetAdded events while the DOM is still loading, which might be hampering performance.

So let's see if there's a performance regression on Dromaeo and Tp5 when always firing the events, and if so, let's try to only fire StyleSheetAdded when the DOM has finished loading and see if that addresses it.

Always fire events: https://treeherder.mozilla.org/#/jobs?repo=try&revision=f80b9f6eea40

Always fire events AFTER DOMContentLoaded: https://treeherder.mozilla.org/#/jobs?repo=try&revision=a55327f2ab56

Comparison between Always fire events and Always fire events AFTER DOMContentLoaded: https://treeherder.mozilla.org/perf.html#/compare?originalProject=try&originalRevision=f80b9f6eea40&newProject=try&newRevision=a55327f2ab56

Comparison between Baseline and Always fire events AFTER DOMContentLoaded: https://treeherder.mozilla.org/perf.html#/compare?originalProject=try&originalRevision=48206c424625&newProject=try&newRevision=a55327f2ab56

If so, then the solution is pretty straight-forward: As laid out in (2), just scan the top-level document and then subframes recursively for appropriate stylesheets when the DOM has finished loading, and then send those up to the parent. Any time StyleSheetAdded or StyleSheetRemoved, update the list and send it up to the parent (alternatively, instead of updating the list, just do the removal / addition to the cache).

When the parent needs to open the Page Style menu popup, just read synchronously from the cache. Bobs your uncle!

Other considerations:

This is a really niche feature, and other browsers like Chromium and Safari don't seem to support it. Do we really want to burn a whole lot of engineering time trying to make it work without CPOWs? It might be worth measuring how often this item is actually used. It looks like we collect clicks on Page Style menu and children for Linux and Windows. Might be worth investigating how often it's used.

Actually. Let's think about this for a second.

This StyleSheetAdded event thing… and the performance regression bits. I think what we've got here is the intersection of two niche functions of the browser / web. Like, Page Style is (imo) niche. Dynamically added stylesheets are (imo) also kinda niche. The intersection of the two is therefore super niche.

I suspect I can get away with making Page Style work with just the stylesheets that are on the page after page load, and then file follow-ups for the other cases.

Gijs pointed me at DOMLinkAdded, DOMLinkRemoved!

These are enabled by default!

Let's try this route.

When a tab has finished loading, scan all documents for their stylesheets, and send the list up to the parent for it to cache

Is DOMLinkAdded fired for every <link> node when the DOM is first being loaded, or is it fired only after the DOM has finished loading?

Hypothesis: DOMLinkAdded is fired every time, even during DOM load

Test: Add a breakpoint in the Browser Content Toolbox for the DOMLinkAdded event, and see if we hit it by just loading a page.

Result: Yes, I do get DOMLinkAdded fired in ContentLinkHandler during DOMContentLoaded for every <link> node!

I don't appear, however, to be getting DOMLinkAdded events for stylesheets in subframes...

I've got a local test folder that I can use called css-test .

Hypothesis: DOMLinkAdded is never fired for stylesheets.

Test: … purely from observation.

NOT hasNonEmptyAttribute = HasEmptyAttribute

Filed https://bugzilla.mozilla.org/show_bug.cgi?id=1210074 for not firing DOMLinkAdded, DOMLinkRemoved, DOMLinkChanged for stylesheets! Let's see what happens.

Bah… I don’t think I want this solution anymore. I’m worried about firing so many events, and how this will affect pageload performance.

Alright. Time to bite the bullet. Let’s go with the async menupopup thing.

Let’s make it so that when we send a message down to the child, we hold a weak reference to the browser’s permanentKey. When a response comes back up, make sure the popup is open, and the target’s permanentKey matches the weakref, and then do the population.

GAHHHH…. a problem. The menuitems are being properly inserted, but the menu glue stuff that we do for OS X doesn’t appear to be working… like, it looks like it’s not prepared for the case where the items in the menu will be populated after the menu has opened. Boooo...

*sigh*.

Okay, so let’s try just sending up the cache on pageshow, and not worrying about dynamically added alternative stylesheets.

The one wrinkle is that I might be breaking addon-compat for consumers of getAllStyleSheets()… so flagging.

Ahh, failing tests with my patch:

548 INFO TEST-UNEXPECTED-FAIL | browser/base/content/test/general/browser_page_style_menu.js | all valid links found - Got 16, expected 0

Ah, patch was causing test failure because we were reacting to the load event, which happens before pageshow, which is the event that we use to populate the menu. Switching the test to wait for pageshow fixes it. \o/

Account for the possibility that the stylesheet URI is a massive dataURI...

Consider closing bug 213289!