1697607 - Consider enabling widget.remote-look-and-feel everywhere.

Status: RESOLVED FIXED

(Core :: Widget, task)

Description

[Emilio Cobos Álvarez (:emilio)]

Doing this allows us to:

• Remove ad-hoc LookAndFeelCaches
• Add a cache for LookAndFeel{Float,Int} in nsXPLookAndFeel, and then remove all the ad-hoc caching from nsLookAndFeel implementations.

That should simplify the setup quite a lot. Is there any reason why this wouldn't be possible?

https://treeherder.mozilla.org/#/jobs?repo=try&revision=0fc8e8316cc6e0de20b88b1e2f52212cbc5ab2d1

Comment 1

[Markus Stange [:mstange]]

I would love to see this.

Comment 2

[Emilio Cobos Álvarez (:emilio)]

I tested this on macOS and Android, and stuff seems to work just fine (as in, color changes mirrored properly etc)

Comment 3

[Emilio Cobos Álvarez (:emilio)]

(ni? so I don't forget to check the try run results later today / tomorrow).

Comment 4

[Chris Martin [:cmartin]]

Definitely want to see this happen -- Will try to help in any way I can. Windows nsLookAndFeel is a bit of a mess.

Comment 5

[Emilio Cobos Álvarez (:emilio)]

Attachment: Bug 1697607 - Enable widget.remote-look-and-feel everywhere. r=mstange!,cmartin!,#geckoview-reviewers!

(And add a missing include while at it)

The biggest concern here is potential startup time penalty (and it
shouldn't be much anyways, if at all). In exchange, we avoid doing a lot
of this work in content processes.

Let's keep an eye on for regressions, but this sticking allows us to
simplify a lot of the lookandfeel code in follow-ups.

Comment 6

[Emilio Cobos Álvarez (:emilio)]

Looks good, and I manually tested dynamic changes on android and macOS, and they work great.

Comment 7

[Chris Martin [:cmartin]]

(In reply to Emilio Cobos Álvarez (:emilio) from comment #6)

Looks good, and I manually tested dynamic changes on android and macOS, and they work great.

I'm just doing a manual check of widget/windows/nsLookAndFeel.cpp to ensure we aren't missing any invalidation paths (since there may be direct Win32 calls that are now being replaced with cached values). If everything looks good, I'd say ship it and I'll craft a commit to remove all the caching and content process logic.

Comment 8

[Emilio Cobos Álvarez (:emilio)]

(In reply to Chris Martin [:cmartin] from comment #7)

I'm just doing a manual check of widget/windows/nsLookAndFeel.cpp to ensure we aren't missing any invalidation paths (since there may be direct Win32 calls that are now being replaced with cached values). If everything looks good, I'd say ship it and I'll craft a commit to remove all the caching and content process logic.

I wanted to wait to add the extra caches and cleaning up once we've enabled this for at least a few days, fwiw

Comment 9

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/d77fccbf6d2a
Enable widget.remote-look-and-feel everywhere. r=cmartin,geckoview-reviewers,agi

Comment 10

[Cristian Brindusan [:cbrindusan]]

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