1526340 - Places' tree.xml destructor triggers a sync style flush during shutdown

Status: RESOLVED FIXED

(Core :: XUL, defect, P5)

Description

[Florian Quèze [:florian]]

See this profile https://perfht.ml/2DkYBh7 that blames the XULTreeElement.view getter: https://searchfox.org/mozilla-central/rev/490ab7f9b84570573a49d7fa018673ce0d5ddf22/browser/components/places/content/tree.xml#32,55

Given this code has changed recently, I suspect this of being a regression from bug 1482389.

The profile shows a second style flush in the radio.xml destructor, due to this code: https://searchfox.org/mozilla-central/rev/490ab7f9b84570573a49d7fa018673ce0d5ddf22/toolkit/content/widgets/radio.xml#34,61

Comment 1

[Brian Grinstead [:bgrins]]

How do you cause this to reproduce? Is the destructor running due to a window going away or due to the tree being hidden or removed from the DOM?

I ask because we are in the process of converting this XBL to a Custom Element in https://bugzilla.mozilla.org/show_bug.cgi?id=1523957, and I'm not sure when it's important for this destructor logic to run (or at least what cases we should be testing for).

Comment 2

[Brian Grinstead [:bgrins]]

Oh now I see "during shutdown" in the bug title. I'm guessing we don't really need to run this logic if we are in the process of shutting down. If that's the case, then Bug 1523957 will fix it.

Comment 3

[Florian Quèze [:florian]]

(In reply to Brian Grinstead [:bgrins] from comment #2)

If that's the case, then Bug 1523957 will fix it.

Would be nice. I was just looking at a shutdown profile, and doing CSS work on a window that's already closed seemed pointless enough to be worth filing a bug.

Comment 4

[Florian Quèze [:florian]]

It was a shutdown profile, but I expect closing a browser window to have the same behavior.

Comment 5

[Brian Grinstead [:bgrins]]

I believe this should be fixed now. How do I take a profile during shutdown to confirm?

Comment 6

[Florian Quèze [:florian]]

Here's a shutdown profile http://bit.ly/2EeGEkH captured on today's nightly, zoomed on the unload event of the browser window. Indeed we no longer have any style flush there.

I still see 2 unfortunate things in this profile:

• we initialize SearchOneOffs (covered by bug 1526374 already)
• https://searchfox.org/mozilla-central/rev/dc0adc07db3df9431a0876156f50c65d580010cb/browser/base/content/browser-sidebar.js#108 triggers the browser-sidebar.js _title lazy getter.

And now about your direct question, to profile shutdown, from a terminal, cd into a directory that you can write to (eg. your home directory), then launch Firefox with the MOZ_PROFILER_SHUTDOWN="name-of-file" environment variable set.

Quit Firefox After the shutdown is over, you'll find a name-of-file file in the current directory.

Load https://profiler.firefox.com in a Firefox tab and drop this file onto it.

The profile currently won't have symbols, if you are using an official build (eg. a nightly), you can fix this (force symbolication) by using https://deploy-preview-1522--perf-html.netlify.com/ instead of https://profiler.firefox.com

Comment 7

[Brian Grinstead [:bgrins]]

Thanks Florian! It sounds like this can be closed now after Bug 1523957.