1930292 - Stop using non-zero chromemargin values.

Status: RESOLVED FIXED

(Core :: Widget: Win32, task)

Description

[Emilio Cobos Álvarez [:emilio]]

These were done in windows 7 to shrink the glass margin, but nowadays it should never be needed since windows 10 and 11 don't have such thing.

Comment 1

[Emilio Cobos Álvarez [:emilio]]

Attachment: Bug 1930292 - Stop setting non-zero chromemargin values. r=#win-reviewers

These only do something on Windows, and after windows 7 they are not
useful at all.

If this has no fallout I plan to simplify the code further in a
follow-up (we can basically make this a boolean attribute again or so).

Comment 2

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/1893713e1d17
Stop setting non-zero chromemargin values. r=win-reviewers,handyman

Comment 3

[Sandor Molnar[:smolnar]]

Backed out for causing bc failures @ browser_findbar_marks.js

Backout link: https://hg.mozilla.org/integration/autoland/rev/8cdd018168b025a36b6188eb9afed96e550c8cb0

Push with failures

Failure log -> TEST-UNEXPECTED-FAIL | toolkit/content/tests/browser/browser_findbar_marks.js

Comment 4

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/9d9fe1cd50ec
Stop setting non-zero chromemargin values. r=win-reviewers,handyman

Comment 5

[agoloman]

https://hg.mozilla.org/mozilla-central/rev/9d9fe1cd50ec

Comment 6

[tabmix.onemen]

On Windows 11, when the browser is in sizemode="normal" it has no border or shadow, unless i set:

#main-window[sizemode="normal"] {
  border-radius: 2px;
}

sorry, i've missed 1931747

Comment 7

[Atila Butkovits]

Backed out for causing Win11 visual regressions.

Backout link: https://hg.mozilla.org/integration/autoland/rev/406b3ccfa78117eb9c6fb5899633655f8493f9c6

Comment 8

[Emilio Cobos Álvarez [:emilio]]

Attachment: Bug 1930292 - Don't extend into the frame border area with chromemargin=0,0,0,0. r=#win-reviewers

Windows 10 and 11 don't have fat windows, so this is unnecessary and
breaks other effects like the accent-color border (if you have "Show
accent color on titlebar and window borders" enabled).

We need to make the custom resize margin thing we have for PiP not mess
with our non-client area, but that is trivial.

Comment 9

[Emilio Cobos Álvarez [:emilio]]

Attachment: Bug 1930292 - Turn chromemargin into a boolean attribute. r=#win-reviewers

There are probably other simplifications that can be done after this on the
windows side of things, but I tried to keep this minimal.

Comment 10

[Emilio Cobos Álvarez [:emilio]]

Attachment: Bug 1930292 - Use customtitlebar rather than tabsintitlebar in front-end code. r=dao!

This is probably also a better reflection of reality now that we can
have vertical tabs which would by definition not be in the titlebar.

Comment 11

[Emilio Cobos Álvarez [:emilio]]

Attachment: Bug 1930292 - Simplify chrome attribute handling. r=smaug!,dao!

Make them work like html boolean attributes consistently.

Nothing sets these to false afaict, we just remove them when needed
already.

Comment 12

[Phabricator Automation]

Comment on attachment 9439053 [details]
Bug 1930292 - Use customtitlebar rather than tabsintitlebar in front-end code. r=dao!

Revision D229767 was moved to bug 1932600. Setting attachment 9439053 [details] to obsolete.

Comment 13

[Phabricator Automation]

Comment on attachment 9439055 [details]
Bug 1930292 - Simplify chrome attribute handling. r=smaug!,dao!

Revision D229768 was moved to bug 1932600. Setting attachment 9439055 [details] to obsolete.

Comment 14

[Florin Bilt]

(In reply to Pulsebot from comment #4)

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/9d9fe1cd50ec
Stop setting non-zero chromemargin values. r=win-reviewers,handyman

Perfherder has detected a browsertime performance change from push 9d9fe1cd50ecbf6e8cf7eb4582bddb0c6bba76f3.

Improvements:

Ratio	Test	Platform	Options	Absolute values (old vs new)	Performance Profiles
10%	wikia SpeedIndex	windows11-64-shippable-qr	cold fission webrender	626.35 -> 565.31	Before/After
10%	wikia SpeedIndex	windows11-64-shippable-qr	fission warm webrender	325.78 -> 294.67	Before/After
9%	wikia SpeedIndex	windows11-64-shippable-qr	cold fission webrender	634.45 -> 577.43	Before/After
8%	wikia PerceptualSpeedIndex	windows11-64-shippable-qr	cold fission webrender	566.27 -> 520.92	Before/After

Details of the alert can be found in the alert summary, including links to graphs and comparisons for each of the affected tests.

If you need the profiling jobs you can trigger them yourself from treeherder job view or ask a sheriff to do that for you.

You can run these tests on try with ./mach try perf --alert 42749

For more information on performance sheriffing please see our FAQ.

Comment 15

[amarc]

Backout merged to central: https://hg.mozilla.org/mozilla-central/rev/406b3ccfa78117eb9c6fb5899633655f8493f9c6

Comment 16

[Florin Bilt]

(In reply to amarc from comment #15)

Backout merged to central: https://hg.mozilla.org/mozilla-central/rev/406b3ccfa78117eb9c6fb5899633655f8493f9c6

Perfherder has detected a browsertime performance change from push 6d4f2dcd1fae6ec131037263b96a6b181e39809e.

Improvements:

Ratio	Test	Platform	Options	Absolute values (old vs new)	Performance Profiles
10%	speedometer3 TodoMVC-Preact-Complex-DOM/DeletingAllItems/Sync	windows11-64-nightlyasrelease-qr	fission webrender	1.05 -> 0.95	Before/After

Details of the alert can be found in the alert summary, including links to graphs and comparisons for each of the affected tests.

If you need the profiling jobs you can trigger them yourself from treeherder job view or ask a sheriff to do that for you.

You can run these tests on try with ./mach try perf --alert 42763

For more information on performance sheriffing please see our FAQ.

Comment 17

[Phabricator Automation]

Comment on attachment 9439055 [details]
Bug 1930292 - Simplify chrome attribute handling. r=smaug!,dao!

Revision D229768 was moved to bug 1932600. Setting attachment 9439055 [details] to obsolete.

Comment 18

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/15a85ec739dd
Stop setting non-zero chromemargin values. r=win-reviewers,handyman,firefox-desktop-core-reviewers ,dao
https://hg.mozilla.org/integration/autoland/rev/7dd9f48216dd
Don't extend into the frame border area with chromemargin=0,0,0,0. r=win-reviewers,rkraesig
https://hg.mozilla.org/integration/autoland/rev/24a670dea977
Turn chromemargin into a boolean attribute. r=win-reviewers,desktop-theme-reviewers,pip-reviewers,firefox-desktop-core-reviewers ,dao,mconley,rkraesig

Comment 19

[Cristina Horotan [:chorotan]]

Backed out for causing chrome failures

Backout: https://hg.mozilla.org/integration/autoland/rev/361ea2ba675fd8d7154bcb42a019d972e1abd216

Failure push: https://treeherder.mozilla.org/jobs?repo=autoland&group_state=expanded&resultStatus=testfailed%2Cbusted%2Cexception%2Cretry%2Cusercancel&searchStr=os%2Cx%2C10.15%2Cwebrender%2Cdebug%2Cmochitests%2Cwithout%2Ce10s%2Cor%2Cfission%2Ctest-macosx1015-64-qr%2Fdebug-mochitest-chrome-1proc%2Cc2&revision=2418bfee53f18169c403faabdc7a0712ae702d25

Failure log: https://treeherder.mozilla.org/logviewer?job_id=484504974&repo=autoland&lineNumber=6068

Comment 20

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/3e6ecfd959f2
Stop setting non-zero chromemargin values. r=win-reviewers,handyman,firefox-desktop-core-reviewers ,dao
https://hg.mozilla.org/integration/autoland/rev/f7594a6cd1e6
Don't extend into the frame border area with chromemargin=0,0,0,0. r=win-reviewers,rkraesig
https://hg.mozilla.org/integration/autoland/rev/035b2effaabf
Turn chromemargin into a boolean attribute. r=win-reviewers,desktop-theme-reviewers,pip-reviewers,firefox-desktop-core-reviewers ,dao,mconley,rkraesig

Comment 21

[Atila Butkovits]

https://hg.mozilla.org/mozilla-central/rev/3e6ecfd959f2
https://hg.mozilla.org/mozilla-central/rev/f7594a6cd1e6
https://hg.mozilla.org/mozilla-central/rev/035b2effaabf

Comment 22

[Alex Finder]

(In reply to Atila Butkovits from comment #7)

Backed out for causing Win11 visual regressions.

Backout link: https://hg.mozilla.org/integration/autoland/rev/406b3ccfa78117eb9c6fb5899633655f8493f9c6

Perfherder has detected a browsertime performance change from push 406b3ccfa78117eb9c6fb5899633655f8493f9c6.

Regressions:

Ratio	Test	Platform	Options	Absolute values (old vs new)	Performance Profiles
45%	yahoo-mail powerUsage_gpu	windows11-64-shippable-qr	cold fission webrender	160.68 -> 233.54	Before/After
45%	wikipedia LastVisualChange	windows11-64-shippable-qr	bytecode-cached fission warm webrender	379.67 -> 550.69	Before/After
45%	yahoo-mail powerUsage_gpu	windows11-64-shippable-qr	fission warm webrender	155.78 -> 225.87	Before/After
42%	wikipedia LastVisualChange	windows11-64-shippable-qr	fission warm webrender	404.31 -> 572.84	Before/After
37%	wikipedia LastVisualChange	windows11-64-shippable-qr	cold fission webrender	748.21 -> 1,027.68	Before/After
36%	wikipedia LastVisualChange	windows11-64-shippable-qr	bytecode-cached cold fission webrender	747.31 -> 1,015.91	Before/After
26%	twitch ContentfulSpeedIndex	windows11-64-shippable-qr	fission warm webrender	361.03 -> 454.17	Before/After
20%	wikipedia loadtime	windows11-64-shippable-qr	cold fission webrender	537.64 -> 645.16	Before/After
19%	wikipedia largestContentfulPaint	windows11-64-shippable-qr	cold fission webrender	453.58 -> 539.50	Before/After
18%	wikipedia loadtime	windows11-64-shippable-qr	bytecode-cached cold fission webrender	537.63 -> 633.52	Before/After
...	...	...	...	...	...
5%	yahoo-mail PerceptualSpeedIndex	windows11-64-shippable-qr	fission warm webrender	146.55 -> 153.37	Before/After
4%	microsoft SpeedIndex	windows11-64-shippable-qr	fission warm webrender	407.85 -> 426.12	Before/After
4%	speedometer3 TodoMVC-Lit-Complex-DOM/DeletingAllItems/Async	windows11-64-nightlyasrelease-qr	fission webrender	2.99 -> 3.11	Before/After
4%	google-docs ContentfulSpeedIndex	windows11-64-shippable-qr	fission warm webrender	771.70 -> 798.98	Before/After
3%	google-docs SpeedIndex	windows11-64-shippable-qr	fission warm webrender	508.18 -> 523.23	Before/After

Improvements:

Ratio	Test	Platform	Options	Absolute values (old vs new)	Performance Profiles
94%	google-slides largestContentfulPaint	windows11-64-shippable-qr	fission warm webrender	1,056.95 -> 62.18	Before/After
88%	google-slides largestContentfulPaint	windows11-64-shippable-qr	bytecode-cached fission warm webrender	739.86 -> 87.83	Before/After
82%	imdb ContentfulSpeedIndex	windows11-64-shippable-qr	cold fission webrender	2,326.26 -> 415.18	Before/After
82%	imdb LastVisualChange	windows11-64-shippable-qr	fission warm webrender	6,203.77 -> 1,111.04	Before/After
80%	imdb PerceptualSpeedIndex	windows11-64-shippable-qr	fission warm webrender	953.96 -> 192.47	Before/After
...	...	...	...	...	...
3%	youtube SpeedIndex	windows11-64-shippable-qr	cold fission webrender	806.06 -> 781.31	Before/After

As author of one of the patches included in that push, we need your help to address this regression.
Details of the alert can be found in the alert summary, including links to graphs and comparisons for each of the affected tests.

If you need the profiling jobs you can trigger them yourself from treeherder job view or ask a sheriff to do that for you.

You can run these tests on try with ./mach try perf --alert 42779

For more information on performance sheriffing please see our FAQ.

Comment 23

[Acasandrei Beatrice (needinfo me)]

(In reply to Pulsebot from comment #18)

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/15a85ec739dd
Stop setting non-zero chromemargin values.
r=win-reviewers,handyman,firefox-desktop-core-reviewers ,dao
https://hg.mozilla.org/integration/autoland/rev/7dd9f48216dd
Don't extend into the frame border area with chromemargin=0,0,0,0.
r=win-reviewers,rkraesig
https://hg.mozilla.org/integration/autoland/rev/24a670dea977
Turn chromemargin into a boolean attribute.
r=win-reviewers,desktop-theme-reviewers,pip-reviewers,firefox-desktop-core-
reviewers ,dao,mconley,rkraesig

Perfherder has detected a browsertime performance change.

Improvements:

Ratio	Test	Platform	Options	Absolute values (old vs new)	Performance Profiles
8%	espn ContentfulSpeedIndex	windows11-64-shippable-qr	cold fission webrender	344.42 -> 315.90	Before/After
8%	espn ContentfulSpeedIndex	windows11-64-shippable-qr	cold fission webrender	341.97 -> 315.71	Before/After

Details of the alert can be found in the alert summary, including links to graphs and comparisons for each of the affected tests.

If you need the profiling jobs you can trigger them yourself from treeherder job view or ask a sheriff to do that for you.

You can run these tests on try with ./mach try perf --alert 42953

For more information on performance sheriffing please see our FAQ.