Optimize calls to nsUXThemeData::InitTitlebarInfo(). See bug 1366874.

Quoting from that bug: Finally we have 88ms spent in nsUXThemeData::InitTitlebarInfo() calling GetSystemMetrics. This seems to be to get data that will be overwritten very soon when the first actual browser window will be opened, with a call to nsUXThemeData::UpdateTitlebarInfo (that spends another 12ms in NtUserGetWindowCompositionAttribute). Could we initialize this placeholder data lazily so that we don't pay this cost if the data isn't going to be used?

We get these dimensions when hitting the cases such as NS_THEME_WINDOW_BUTTON_MINIMIZE in nsNativeThemeWin::GetMinimumWidgetSize due to the use of |-moz-appearance: -moz-window-button-minimize;| etc. in browser/themes/windows/browser.css. Additionally we get the button box size (the button box wraps the buttons) when painting when nsLayoutUtils::PaintFrame calls nsWindow::UpdateOpaqueRegion. The IsWin8OrLater() check and early return in nsUXThemeData::UpdateTitlebarInfo implies that we only sometimes need values for the buttons since that check makes us return early just after setting the button box dimensions, but before the button dimensions are set further down that function. That indeed seems to be the case on my pretty much vanilla Win10 machine where the only dimensions that were set during window construction were the button box dimensions, and the only dimensions that seemed to be read were also the button box dimensions. It was therefore necessary to separated the initialization of the button and button box sizes so that fetching the button box size didn't require initializing the other sizes (and therefore making the slow GetSystemMetrics calls).

https://treeherder.mozilla.org/#/jobs?repo=try&revision=60a236db5253d24223b8617bff2bd38d500b88ab We get 99% of the way through our crashtests before we fail the MOZ_ASSERT_UNREACHABLE assert running: https://dxr.mozilla.org/mozilla-central/source/widget/crashtests/1128214.html Basically the issue is that arbitrary content is allowed to use |-moz-appearance:-moz-window-button-close;| etc. regardless of whether the Windows configurations would normally require us to fetch those metrics for our UI elements or not. It looks like I'll need to change the MOZ_ASSERT_UNREACHABLE to an NS_WARNING which is unfortunate. :/

(In reply to Jonathan Watt [:jwatt] from comment #4) > arbitrary content is allowed to use > |-moz-appearance:-moz-window-button-close;| Is this intentional or a bug? I can't think of a legitimate reason for a web page to use this, and it seems like it would help fingerprinters.

I believe it's a limitation of our style system. We have the ability to restrict certain properties to chrome, but not property values as far as I know.

We don't have a simple way to do this (by adding a flag somewhere), but we could give -moz-appearance its own nsCSSParser parsing function. In there we can check ChromeRulesEnabled() when parsing certain values, which would restrict style sheets in content documents from settings those values.

Thanks, Cameron. Definitely something for a separate bug then.

Just FYI, we will have to refresh metrics on DPI change when we make use of per-DPI metrics that Creators Update introduced.

Best to file a bug for that since that wouldn't be part of this bug.

Comment on attachment 8878877 [details] [diff] [review] patch with MOZ_ASSERT_UNREACHABLE switched to NS_WARNING Review of attachment 8878877 [details] [diff] [review]: ----------------------------------------------------------------- lgtm. I tested this on win7/classic and basic - no issues. great!

Pushed by jwatt@jwatt.org: https://hg.mozilla.org/integration/mozilla-inbound/rev/71444e81ac24 Avoid slow and unnecessary GetSystemMetrics calls for Window's command buttons during startup. r=jimm

sorry had to back this out in https://treeherder.mozilla.org/#/jobs?repo=mozilla-inbound&revision=71444e81ac24f74179aed226fcbd90f463460c9b&filter-classifiedState=unclassified&filter-resultStatus=testfailed&filter-resultStatus=busted&filter-resultStatus=exception since one of the changes caused crashes like https://treeherder.mozilla.org/logviewer.html#?job_id=108844802&repo=mozilla-inbound

Backout by cbook@mozilla.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/1adb9bb710f5 Backed out changeset 71444e81ac24

Juggling patches between machines I somehow managed to land the old version of the patch with the MOZ_ASSERT_UNREACHABLE in it. That fails running the headless tests in addition to the crashtest that I noted in comment 4, but we don't yet run the headless tests (MnH) on Try runs it seems. I'll land again with the new version of the patch.

Pushed by jwatt@jwatt.org: https://hg.mozilla.org/integration/mozilla-inbound/rev/147ad58662c7 Avoid slow and unnecessary GetSystemMetrics calls for Window's command buttons during startup. r=jimm