1730867 - Use nsNativeBasicThemeAndroid to draw scrollbars instead of relying on mobile/scrollbars.css

Status: RESOLVED FIXED

(Core Graveyard :: Widget: Android, enhancement)

Description

[Nicolas Chevobbe [:nchevobbe]]

We had a discussion with Emilio on Element, first about DevTools Responsive Design Mode, where we insert a given stylesheet to simulate "floating" scrollbars (https://searchfox.org/mozilla-central/rev/a82dd14b6bc3d05a6a3facc126607e6d34cc2f53/devtools/server/actors/emulation/responsive.js#197,202,206,211)

Emilio pointed out that scrollbars in RDM looked suboptimal, as they're only translucent black and don't work well on website with black backgrounds, which is also the case in Firefox for Android, which made Emilio think:

a slightly more involved solution would be to teach nsNativeBasicThemeAndroid to draw scrollbars, and remove mobile/scrollbars.css... That'd improve scrollbars on android as well (allowing us to brighten scrollbars on dark websites like macOS does for example)

---

Let's see what can be done for Android here, and we might have a separate bug for RDM to use the same "scrollbar drawing" method.

Comment 1

[Emilio Cobos Álvarez [:emilio]]

Attachment: Bug 1730867 - Remove nsNativeThemeAndroid. r=mstange,#geckoview-reviewers

We now always use the non-native theme, and the Android native theme
never did much anyways, so let's remove it, no point in keeping
around dead code.

Comment 2

[Emilio Cobos Álvarez [:emilio]]

Attachment: Bug 1730867 - Use native scrollbars on Android. r=mstange

We only draw a scrollbar thumb, so it's easy enough (fair amount of
boilerplate, though).

For now I've done the trivial thing (keep painting a round thumb without
other fancy things), as that's what we were doing, but in the future we
should consider drawing a stroke around the thumb like cocoa does to add
more contrast.

This patch does change behavior in some cases. For example dark websites
now get a lighter, easier to distinguish scrollbar, thanks to the
scrollbar darkening code in nsNativeBasicTheme.cpp.

Depends on D125825

Comment 3

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/b93fd2876ca0
Remove nsNativeThemeAndroid. r=mstange

Comment 4

[Cristina Cozmuta (:CrissCozmuta)]

https://hg.mozilla.org/mozilla-central/rev/b93fd2876ca0

Comment 5

[Emilio Cobos Álvarez [:emilio]]

Attachment: Bug 1730867 - Update fuzziness in reftests. r=mstange

All these tests except the wpt one were already fuzzy due to scrollbars,
this patch just changes the fuzzy values.

There's a test which is passing now (was marked as a failure due to bug
1308702). Probably was fixed a while ago and the test was still failing
due to fuzziness.

Depends on D125826

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/894f76b91190
Use native scrollbars on Android. r=mstange
https://hg.mozilla.org/integration/autoland/rev/315dd0b76f3f
Update fuzziness in reftests. r=mstange

Comment 7

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/12f9678771c8
Tweak fuzz in a test.

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/6d0aac048440
Tweak fuzz in another test.

Comment 9

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/3a0215aefdf5
Minor tweak to fuzz ranges to account for swgl+debug.

Comment 10

[Pulsebot]

Backout by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/6b6eed16fcaf
Backed out 5 changesets for causing multiple reftest failures. CLOSED TREE

Comment 11

[Noemi Erli[:noemi_erli]]

Backed out 5 changesets (Bug 1730867) for causing multiple reftest failures. CLOSED TREE

Push with failures: https://treeherder.mozilla.org/jobs?repo=autoland&group_state=expanded&collapsedPushes=842382&resultStatus=testfailed%2Cbusted%2Cexception%2Cretry%2Cusercancel&revision=315dd0b76f3fc85b7023db41f93f39ded2fafa22&selectedTaskRun=Fi0i4ZU2RmueFQvT2S6EIQ.0

Log: https://treeherder.mozilla.org/logviewer?job_id=352896396&repo=autoland&lineNumber=5612
https://treeherder.mozilla.org/logviewer?job_id=352896273&repo=autoland&lineNumber=3915
https://treeherder.mozilla.org/logviewer?job_id=352887713&repo=autoland&lineNumber=19057
https://treeherder.mozilla.org/logviewer?job_id=352893181&repo=autoland&lineNumber=23382
Plus some perma tier2: https://treeherder.mozilla.org/logviewer?job_id=352892147&repo=autoland&lineNumber=2041

Backout: https://hg.mozilla.org/integration/autoland/rev/6b6eed16fcafa9dee9c7eb3c8b63d1447e29386f

Emilio, sorry for the mass backout but all the backfills pointed us to this bug and due to backout conflicts, we had to backout all pushes that involved this bug:
https://treeherder.mozilla.org/jobs?repo=autoland&group_state=expanded&collapsedPushes=842382&resultStatus=testfailed%2Cbusted%2Cexception%2Cretry%2Cusercancel&fromchange=f5d6ae6aea444f1498c552e6178b43de5b7e5291&searchStr=android%2C7.0%2Cx86-64%2Cwebrender%2Cdebug%2Creftests%2Cwith%2Csoftware%2Cwebrender%2Cenabled%2Ctest-android-em-7.0-x86_64-qr%2Fdebug-geckoview-reftest-swr-e10s%2Cr3&tochange=6b6eed16fcafa9dee9c7eb3c8b63d1447e29386f

https://treeherder.mozilla.org/jobs?repo=autoland&group_state=expanded&collapsedPushes=842382&selectedTaskRun=Fi0i4ZU2RmueFQvT2S6EIQ.0&resultStatus=testfailed%2Cbusted%2Cexception%2Cretry%2Cusercancel&fromchange=f5d6ae6aea444f1498c552e6178b43de5b7e5291&searchStr=android%2C7.0%2Cx86-64%2Cwebrender%2Cdebug%2Creftests%2Cwith%2Csoftware%2Cwebrender%2Cenabled%2Ctest-android-em-7.0-x86_64-qr%2Fdebug-geckoview-reftest-swr-e10s%2Cr1&tochange=6b6eed16fcafa9dee9c7eb3c8b63d1447e29386f

Comment 12

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/bbd4d5126a19
Use native scrollbars on Android. r=mstange
https://hg.mozilla.org/integration/autoland/rev/84abb450636b
Update fuzziness in reftests. r=mstange

Comment 13

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/aed4ce969bb6
Some more swgl fuzziness updates.

Comment 14

[Natalia Csoregi [:nataliaCs]]

Backed out for multiple reftest failures.

• backout: https://hg.mozilla.org/integration/autoland/rev/28f71bc407aea11658d64955b06d198d4c8a08d8

• push: https://treeherder.mozilla.org/jobs?repo=autoland&revision=84abb450636bd008c525528981585c24d17e6a0a&group_state=expanded

• failures:

  • REFTEST TEST-UNEXPECTED-FAIL | gfx/layers/apz/test/reftest/async-scrollbar-1-v.html == gfx/layers/apz/test/reftest/async-scrollbar-1-v-ref.html | image comparison, max difference: 4, number of differing pixels: 6 - with retrigger/backfill range
  • REFTEST TEST-UNEXPECTED-FAIL | layout/reftests/async-scrolling/offscreen-clipped-blendmode-1.html == layout/reftests/async-scrolling/offscreen-clipped-blendmode-ref.html | image comparison, max difference: 42, number of differing pixels: 40 with retrigger/backfill range

Comment 15

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/f30d951f39df
Use native scrollbars on Android. r=mstange
https://hg.mozilla.org/integration/autoland/rev/d2720681264c
Update fuzziness in reftests. r=mstange

Comment 16

[Cristian Tuns]

https://hg.mozilla.org/mozilla-central/rev/f30d951f39df
https://hg.mozilla.org/mozilla-central/rev/d2720681264c

Comment 17

[Alexandru Ionescu (needinfo me) [:alexandrui]]

== Change summary for alert #31575 (as of Thu, 30 Sep 2021 05:46:25 GMT) ==

Improvements:

Ratio	Test	Platform	Options	Absolute values (old vs new)
3%	facebook fnbpaint	android-hw-g5-7-0-arm7-shippable-qr	warm webrender	586.83 -> 569.54
2%	facebook SpeedIndex	android-hw-g5-7-0-arm7-shippable-qr	warm webrender	675.33 -> 661.75

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