Closed Bug 1824236 Opened 2 years ago Closed 2 years ago

Stop using XUL layout for scrollbars.

Categories

(Core :: Layout: Scrolling and Overflow, task, P3)

task

Tracking

()

RESOLVED FIXED
113 Branch
Tracking Status
firefox113 --- fixed

People

(Reporter: emilio, Assigned: emilio)

References

(Blocks 1 open bug)

Details

(Keywords: perf-alert)

Attachments

(1 file)

No description provided.
Blocks: 1824254

This rewrites scrollbar layout to work with regular reflow rather than
box layout.

Overall it's about the same amount of code (mostly because
nsScrollbarFrame::Reflow is sorta hand-rolled), but it cleans up a bit
and it is progress towards removing XUL layout altogether, without
getting into much deeper refactoring.

The setup of abusing the available size in ReflowInput to make sizing
decisions on the children feels a bit funky, but I think scrollbars are
special enough that it probably doesn't matter much... Otherwise I
would've had to rewrite the thumb sizing code which is a bit gnarly.

This also blocks some other performance improvements and refactorings I
want to make in this code.

Blocks: 1824489

https://treeherder.mozilla.org/jobs?repo=try&revision=71649249479899d414b9acd4aa078a4a93888923

Gah, so Linux and macOS are green, but it seems I still need some work to do on Windows / Android, for some reason.

Priority: -- → P3

(It's fully green now fwiw)

Blocks: 1824667
Pushed by ealvarez@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/7da2469ac949 Stop using XUL layout for scrollbars. r=jwatt
Attachment #9324789 - Attachment description: Bug 1824236 - Stop using XUL layout for scrollbars. r=TYLin,dholbert,#layout-reviewers → Bug 1824236 - Stop using XUL layout for scrollbars. r=jwatt
Pushed by ealvarez@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/5ab63cda14e8 Stop using XUL layout for scrollbars. r=jwatt
Flags: needinfo?(emilio)
Pushed by emilio@crisal.io: https://hg.mozilla.org/integration/autoland/rev/897bab3fdfc6 Remove a crashtest using <slider> without a scrollbar.
Regressions: 1824900

== Change summary for alert #37912 (as of Fri, 31 Mar 2023 00:56:37 GMT) ==

Improvements:

Ratio Test Platform Options Absolute values (old vs new)
12% perf_reftest_singletons scrollbar-styles-1.html macosx1015-64-shippable-qr e10s fission stylo webrender 327.24 -> 287.24
12% perf_reftest_singletons scrollbar-styles-1.html linux1804-64-shippable-qr e10s fission stylo webrender 444.93 -> 392.41
10% perf_reftest_singletons scrollbar-styles-1.html macosx1015-64-shippable-qr e10s fission stylo webrender 330.34 -> 297.01
10% perf_reftest_singletons scrollbar-styles-1.html windows10-64-shippable-qr e10s fission stylo webrender 490.52 -> 442.27

For up to date results, see: https://treeherder.mozilla.org/perfherder/alerts?id=37912

(In reply to Sandor Molnar from comment #5)

Backed out for causing assertion failures in layout/generic/crashtests/369038-1.xhtml

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

Push with failures

Failure log -> Assertion failure: appearance == StyleAppearance::ScrollbarHorizontal || appearance == StyleAppearance::ScrollbarVertical, at /builds/worker/checkouts/gecko/layout/xul/nsScrollbarFrame.cpp:241

== Change summary for alert #37953 (as of Sat, 01 Apr 2023 06:57:26 GMT) ==

Regressions:

Ratio Test Platform Options Absolute values (old vs new)
0.12% installer size osx-aarch64-shippable aarch64 nightly 84,599,407.12 -> 84,702,245.17

For up to date results, see: https://treeherder.mozilla.org/perfherder/alerts?id=37953

Keywords: perf-alert
Regressions: 1828879
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: