1633718 - ThemeSupportsWidget is extremely slow when WindowBlinds is installed

Status: VERIFIED FIXED

(Core :: Widget: Win32, defect)

Description

[wOxxOm]

Attachment: test.html

User Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/81.0.4044.122 Safari/537.36 Kinza/6.2.2

Steps to reproduce:

1. Install WindowBlinds
2. Open the attached test.html which shows 100 textarea elements.

Actual results:

contents is shown in 9 seconds

Expected results:

contents is shown instantly (100ms)

Comment 1

[wOxxOm]

Attachment: performance-profile.json.gz

Online version: https://perfht.ml/2zutPnI

Comment 2

[wOxxOm]

Bisected to https://hg.mozilla.org/integration/autoland/rev/6d4fe74b51ca6a2158941a263a121c4776d22053
"Block wbload.dll as it causes GPU process crashes"

Performance profile shows that 99% of the time is spent in trying to load the blocked(?) WindowBlinds dll.

Comment 3

[wOxxOm]

The workaround is to set widget.disable-native-theme-for-content to true in about:config. This will prevent Firefox from querying WindowBlinds dll and the test case will take just ~100ms to complete on my machine, same as Chrome.

Comment 4

[wOxxOm]

Quoting #c70 from bug 190147:

The code seems that it should be cached, though: https://searchfox.org/mozilla-central/rev/158bac3df3a1890da55bdb6ffdaf9a7ffc0bfb0a/widget/windows/nsUXThemeData.cpp#55

I guess somehow OpenThemeData returns null every time in your case and thus saves no cache.

This may be theoretically explained by the fact that wbload.dll is blocked (see comment 2 above).

Comment 5

[Darkspirit]

Thanks for narrowing this down!

Comment 6

[Kartikaya Gupta (email:[e-mail])]

:aklotz, you're the resident expert on DLL blocking. Is this kind of long delay expected here?

Comment 7

[(No longer employed by Mozilla) Aaron Klotz]

I'm not working on DLL blocking anymore.

Comment 8

[Toshihito Kikuchi [:toshi]]

Confirmed repro. Let me see what we can do..

• Launcher enabled + wbload.dll blocked: 3200~3400ms
• Launcher disabled + wbload.dll blocked: 300~400ms
• Launcher + wbload.dll NOT blocked: less than 100ms

Comment 9

[Markus Stange [:mstange]]

I think it would make sense to change sThemes into a tristate value: Uninitialized, InitializationFailed, InitializationSucceeded(HANDLE), so that nsUXThemeData::GetTheme() can stop querying themes that have failed before.

And maybe it makes sense to speed up patched_LdrLoadDll so that it fails faster for blocked DLLs.

Comment 10

[Toshihito Kikuchi [:toshi]]

Thank you for your suggestion, Markus!

I learned wbload.dll of WindowsBlinds is injected via the undocumented API user32!RegisterUserApiHook. In general, blocking a module injected via RegisterUserApiHook or SetWindowsHookExW is not a good idea because win32k tries to load a module every time at hook points such as OpenThemeData or other user32 functions regardless of the previous loading results.

I tried implementing Markus's suggestion, not repeating OpenThemeData once failed, by using Maybe<HANDLE>. With this, rendering time got much better, about 300msec, but it's still slower than with the default theme. I see the process still tries to load wbload.dll through USER32!DrawFrameControl, that should be the reason.

In the short term, the change above will be a good mitigation (3000msec -> 300msec is not so bad). As a long term solution, I'm curious about feasibility of the following items.

1. Any workaround for wbload's crash (bug 1544435) instead of blocking?
2. Can we limit versions of wbload.dll to block?
3. Can we limit a process type where we block wbload.dll? (According to bug 1544435, we want to block it in GPU process)
4. Is there any way to block only a hook function, allowing a module to be loaded as bug 1603974 did?

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

And maybe it makes sense to speed up patched_LdrLoadDll so that it fails faster for blocked DLLs.

I'm not sure we can improve patched_LdrLoadDll or patched_NtMapViewOfSection. Even if we can, I don't expect much because we cannot skip heavy tasks like file I/O or creating a section object.

Comment 11

[Toshihito Kikuchi [:toshi]]

Attachment: Bug 1633718 - Make sure not to call OpenThemeData if it failed before. r=mstange

As bug 1544435, we blocked wbload.dll from being loaded in content processes.
Because that modules is injected via user32!RegisterUserApiHook and Windows
keeps trying to load hooking modules even if the previous attempt has failed,
blocking wbload.dll caused repetitive loading attempts in the tab process,
resulting in bad rendering performance.

This patch is to mitigate that performance issue by not calling OpenThemeData,
which is one of the entrypoints triggering user api hook, if the previous call
has failed. With this patch, performance is still slower than with the default
theme. We will seek out a long-term solution to solve the issue.

Comment 12

[(No longer employed by Mozilla) Aaron Klotz]

(In reply to Toshihito Kikuchi [:toshi] from comment #11)

Created attachment 9144837 [details]
We will seek out a long-term solution to solve the issue.

The best long-term solution is probably just turning on Win32k lockdown.

Comment 13

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/5fb91d34f34e
Make sure not to call OpenThemeData if it failed before.  r=mstange

Comment 14

[Cristian Brindusan [:cbrindusan]]

https://hg.mozilla.org/mozilla-central/rev/5fb91d34f34e

Comment 15

[wOxxOm]

The best long-term solution is probably just turning on Win32k lockdown.

Enabling security.sandbox.content.win32k-disable crashes the tabs for me so maybe system call filtering in Windows 10 could be used (assuming Firefox doesn't use it already) until the lockdown code is polished, it was mentioned in the win32k article for Chrome.

Comment 16

[Anca Soncutean, Desktop QA]

I’ve reproduced this issue with Fx 77.0a1 (2020-04-28).
The issue is verified fixed with 77.0b9 and Fx 78.0a1 (2020-05-24) on Windows 10 x64.