1689884 - Scrolling on Amazon Prime Music playlist is very slow

Status: RESOLVED FIXED

(Core :: Widget: Win32, defect)

Description

[Mayank Bansal]

1. Have a amazon prime music account
2. Go to https://music.amazon.in/playlists/B07XQHS3PF?refMarker=dm_wcp_af_r&ref_=&
3. Scroll down using the mouse. The highlighted row takes a long time to focus on the new row of song.

Not sure which component this belongs in, as I haven't seen this function before in profile.
Could it be that we are continuously checking if the device is a tablet or not?

https://share.firefox.dev/3j3fQYG

Comment 1

[Mayank Bansal]

Attachment: about:support

Comment 2

[Markus Stange [:mstange]]

(In reply to Mayank Bansal from comment #0)

Could it be that we are continuously checking if the device is a tablet or not?

Indeed, a lot of time is spent under WinUtils::GetPrimaryPointerCapabilities, called from nsLookAndFeel::NativeGetInt.
This sounds like something that we shouldn't be querying repeatedly, and also it seems like the content process isn't the right place to run this code anyway.

Is there an effort ongoing to have all LookAndFeel settings queried in the parent process, and only re-queried (and re-synced to the content processes) when they change?

Comment 3

[Bob Owen (:bobowen)]

(In reply to Markus Stange [:mstange] from comment #2)

(In reply to Mayank Bansal from comment #0)

Could it be that we are continuously checking if the device is a tablet or not?

Indeed, a lot of time is spent under WinUtils::GetPrimaryPointerCapabilities, called from nsLookAndFeel::NativeGetInt.
This sounds like something that we shouldn't be querying repeatedly, and also it seems like the content process isn't the right place to run this code anyway.

Is there an effort ongoing to have all LookAndFeel settings queried in the parent process, and only re-queried (and re-synced to the content processes) when they change?

I know that a lot of LookAndFeel setting retrieval has been moved to the parent.
From bug 1400317 comment 28 it looks like the remaining things (possibly including this) were blocked on non-native theming.
Not sure if that means they go away with non-native theming or that they can now be worked on.
cmartin - do you know the situation here?

Comment 4

[Chris Martin [:cmartin]]

(In reply to Bob Owen (:bobowen) from comment #3)

I know that a lot of LookAndFeel setting retrieval has been moved to the parent.
From bug 1400317 comment 28 it looks like the remaining things (possibly including this) were blocked on non-native theming.
Not sure if that means they go away with non-native theming or that they can now be worked on.
cmartin - do you know the situation here?

The unfortunate situation is that things related to Win32 themeing, widget metrics, etc are splayed out between nsUXThemeData, nsLookAndFeel, and WinUtils, and over time things have ended up somewhat-arbitrarily in one of those places. That also means that some of them go through the nsLookAndFeel channels, but some don't and just directly call Win32 APIs with no cacheing.

A while ago I remoted a bunch of APIs specifically in the nsUXThemeData part, but couldn't finish the job because it needed non-native themeing. I started remoting some of the things in nsLookAndFeel, but I was informed that I might want to hold off because there were plans to re-architect all the stuff in nsLookAndFeel to be remote, and it indeed looks like :heycam landed that a couple months ago. Unfortunately, it looks like it's only tested/enabled so far on GTK (here), and I doubt that it's as trivial as flipping the switch on Windows because of the issues above.

That's all a long way to say: In the short term, somebody will probably need to add to the patchwork of Win32 remoting for this specific API, but in the long run this entire thing needs some work done so we can wrangle all of this and take advantage of the work that's already been done for LookAndFeel remoting.

I may get some time to get this API remoted in the new few weeks. I'll take the bug for now, and if someone else gets a chance to look at it first just be sure to let me know so we don't double-up on work.

Comment 5

[Chris Martin [:cmartin]]

Attachment: Bug 1689884 - Cache pointer information needed by media queries

Currently, every single usage of the @media (pointer) CSS query causes a
substantial amount of processing to figure out whether the device has a
pointer installed, and whether or not it's fine or coarse.

This value should probably just be cached after the first time it's called.
Furthermore, it probably shouldn't be called in content at all because it's a
win32k call.

This changes the variable to be both read in the parent process and cached
in the child.

Comment 6

[Chris Martin [:cmartin]]

Attachment: Bug 1689884 - Inform PresShell when tablet-mode changes occur

Currently, PresShell is not informed when a system setting involving
Windows 10 Tablet Mode, System Dock Mode, or "Convertible Slate Mode" occur.

As this can change the result of @media (pointer) queries, layout should know
about this so it can react accordingly.

Comment 7

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/416c387ad136
Inform PresShell when tablet-mode changes occur r=emilio
https://hg.mozilla.org/integration/autoland/rev/64b2735a8fe6
Cache pointer information needed by media queries r=jld

Comment 8

[Alexandru Michis [:malexandru]]

Backed out 2 changesets (bug 1689884) for causing xpcshell assertion failures in ServiceWorkerManager.cpp (Bug 1697972).
Backout link: https://hg.mozilla.org/integration/autoland/rev/58a6c1a4be2c6abbb57d15b7b1aaac52e9249b0b
Push with failures, failure log.
Initially, Bug 1697972 was made for this issue, so I'm closing it as fixed by backout.

Comment 9

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/7661ff480e39
Inform PresShell when tablet-mode changes occur r=emilio
https://hg.mozilla.org/integration/autoland/rev/64771c2078b2
Cache pointer information needed by media queries r=jld

Comment 10

[Alexandru Michis [:malexandru]]

Re-landed changesets 416c387ad136 and 64b2735a8fe6 since the failures still appeared even after backing out.
Sorry for the inconvenience!

Re-land links:

• https://hg.mozilla.org/integration/autoland/rev/7661ff480e39
• https://hg.mozilla.org/integration/autoland/rev/64771c2078b2

Comment 11

[Cristian Brindusan [:cbrindusan]]

https://hg.mozilla.org/mozilla-central/rev/7661ff480e39
https://hg.mozilla.org/mozilla-central/rev/64771c2078b2

Comment 12

[BugBot [:suhaib / :marco/ :calixte]]

The patch landed in nightly and beta is affected.
:cmartin, is this bug important enough to require an uplift?
If not please set status_beta to wontfix.

For more information, please visit auto_nag documentation.

Comment 13

[Chris Martin [:cmartin]]

This change is at-least a little risky (chance of causing regressions on multiple media queries), and the reward is fairly minor (performance improvement on few specific sites like Amazon Music that use a lot of the not-very-popular @media(pointer) queries.)

It doesn't seem worthwhile to me for this to be fixed in Beta. Anyone facing this issue will probably be fine to wait until the end of April for Firefox 88 to go through the entire testing pipeline.