1381022 - Use non-UI message pump(s) in the content process

Status: RESOLVED FIXED

(Core :: DOM: Content Processes, enhancement, P5)

Description

[Alex Gaynor [:Alex_Gaynor]]

Currently in the content process, the main loop goes through MessagePumpForChildProcess which calls through to nsAppShell::ProcessNextNativeEvent.

ProcessNextNativeEvent makes many win32k syscalls, notably NtUserPeekMessage (via PeekMessage), NtUserValidateHandleSecure (via DispatchMessageWorker), and NtUserPostMessage (via PostMessage).

Per :jimm, it should be possible to use a non-UI message loop in content loop as:

17:17:35 <jimm> pretty sure that IO thread for only used in the ipdl background io thread

Comment 1

[Alex Gaynor [:Alex_Gaynor]]

In discussing with :billm, he's hopeful that the work on bug 1350432 will remove the nsAppShell code from the content process's main loop, which would solve this for us.

Comment 2

[Alex Gaynor [:Alex_Gaynor]]

I believe removing nsAppShell will also resolve the following stack:

20 - win32u!NtUserSystemParametersInfo
    win32u!NtUserSystemParametersInfo
    USER32!RealSystemParametersInfoW+0x82
    UxTheme!ClassicSystemParametersInfoW+0x29
    UxTheme!_InternalSystemParametersInfo+0x29
    UxTheme!ThemeSystemParametersInfoW+0xcb
    USER32!SystemParametersInfoW+0xa7
    xul!WinWakeLockListener::ResetScreenSaverTimeout+0x1b
    xul!WinWakeLockListener::Notify+0x71
    xul!nsTimerImpl::Fire+0x414

Comment 3

[Bill McCloskey [inactive unless it's an emergency] (:billm)]

I filed a separate bug for the appshell work I'm doing. I don't know if it will entirely resolve your issues. I think we might use UI message pumps elsewhere in the content process, but I could be wrong.

Comment 4

[Alex Gaynor [:Alex_Gaynor]]

I'm not sure if this should be it's own issue, or it's really just a part of this, but we also initialize the COM system as a part of content process startup:

    win32u!NtUserMessageCall
    USER32!RealDefWindowProcWorker+0x1ba
    USER32!RealDefWindowProcW+0x52
    UxTheme!_ThemeDefWindowProc+0x559
    UxTheme!ThemeDefWindowProcW+0x11
    USER32!DefWindowProcW+0x1b7
    combase!OleMainThreadWndProc+0x25
    USER32!UserCallWinProcCheckWow+0x280
    USER32!DispatchClientMessage+0x9c
    USER32!_fnINOUTNCCALCSIZE+0x3b
    ntdll!KiUserCallbackDispatcherContinue
    win32u!NtUserCreateWindowEx+0x14
    USER32!VerNtUserCreateWindowEx+0x212
    USER32!CreateWindowInternal+0x1b5
    USER32!CreateWindowExW+0x82
    combase!InitMainThreadWnd+0x56
    combase!ThreadFirstInitialize+0x136
    combase!_CoInitializeEx+0x1db
    combase!CoInitializeEx+0x36
    xul!mozilla::mscom::COMApartmentRegion<2>::{ctor}+0x11
    xul!mozilla::mscom::MainThreadRuntime::MainThreadRuntime+0x6a
    xul!mozilla::dom::ContentProcess::{ctor}+0x36
    xul!XRE_InitChildProcess+0x67c

Comment 5

[(No longer employed by Mozilla) Aaron Klotz]

Separate bug. Can be follow-up work once appshell lands. Please cc me.

Comment 6

[Alex Gaynor [:Alex_Gaynor]]

Thanks for the quick analysis! Will file!

Comment 7

[Alex Gaynor [:Alex_Gaynor]]

I believe at this point the only platform where we still pump native events in the content process is Linux. Is that right?

Comment 8

[Jim Mathies [:jimm]]

We're still initializing a UI main thread here, this results in various calls plus a bunch of COM stuff.

example stacks:

3 - win32u!NtUserQueryWindow
win32u!NtUserQueryWindow
user32!GetWindowThreadProcessId+0x83
xul!`anonymous
xul!mozilla::ipc::windows::InitUIThread+0x1b9
xul!XRE_InitChildProcess+0xa13
xul!mozilla::BootstrapImpl::XRE_InitChildProcess+0x2a
firefox!content_process_main+0xe4
firefox!NS_internal_main+0x117

3 - win32u!NtUserGetProcessWindowStation
win32u!NtUserGetProcessWindowStation
combase!CRpcResolver::GetThreadWinstaDesktop+0x6a
combase!<lambda_42f1a29483ec03d26359468a17427162>::operator()+0x85
combase!CRpcResolver::GetConnection+0x8c
combase!CoInitializeSecurity+0xb0
xul!mozilla::mscom::ProcessRuntime::InitializeSecurity+0xcfd
xul!mozilla::mscom::ProcessRuntime::InitInsideApartment+0x65
xul!mozilla::mscom::ProcessRuntime::ProcessRuntime+0x21a
xul!mozilla::mscom::ProcessRuntime::ProcessRuntime+0x24
xul!mozilla::dom::ContentProcess::ContentProcess+0x72
xul!XRE_InitChildProcess+0xb22

Comment 9

[Jim Mathies [:jimm]]

Attachment: test patch

This is built on top of bug 1381938. Looks like the event pump stuff in nsAppShell can just come out, although we may want to keep it around for a bit for testing.

Comment 10

[Jim Mathies [:jimm]]

Looks like we need some touch up work here as well -
https://searchfox.org/mozilla-central/source/ipc/mscom/ProcessRuntime.cpp#58

This code which runs really early can't detect 'mozilla.widget.disable-native-theme-for-content' though. It ends up calling CoInitializeEx which fails.

Comment 11

[(No longer employed by Mozilla) Aaron Klotz]

(In reply to Jim Mathies [:jimm] from comment #10)

Looks like we need some touch up work here as well -
https://searchfox.org/mozilla-central/source/ipc/mscom/ProcessRuntime.cpp#58

This code which runs really early can't detect 'mozilla.widget.disable-native-theme-for-content' though. It ends up calling CoInitializeEx which fails.

What's the problem here, exactly? That code is written to use the MTA when Win32k lockdown is turned on. Until that happens, it still uses the STA.

Comment 12

[Jim Mathies [:jimm]]

Attachment: test patch

com init area still needs to be sorted out.

Comment 13

[Jim Mathies [:jimm]]

(In reply to Aaron Klotz [:aklotz] from comment #11)

(In reply to Jim Mathies [:jimm] from comment #10)

Looks like we need some touch up work here as well -
https://searchfox.org/mozilla-central/source/ipc/mscom/ProcessRuntime.cpp#58

This code which runs really early can't detect 'mozilla.widget.disable-native-theme-for-content' though. It ends up calling CoInitializeEx which fails.

What's the problem here, exactly? That code is written to use the MTA when
Win32k lockdown is turned on. Until that happens, it still uses the STA.

with native events turned off the init call seemed to fail. I had some other stuff turned on so I'll check it again.

Comment 14

[Jim Mathies [:jimm]]

Attachment: wip

Comment 16

[Jim Mathies [:jimm]]

I still see this in stacks -

28 - win32u!NtUserCallOneParam
win32u!NtUserCallOneParam
xul!base::MessagePumpForUI::ProcessNextWindowsMessage
xul!mozilla::ipc::MessagePumpForNonMainUIThreads::DoRunLoop
xul!base::MessagePumpWin::RunWithDispatcher
xul!base::MessagePumpWin::Run
xul!MessageLoop::RunInternal
xul!MessageLoop::RunHandler

So I think we still need to touch up the type of chromium message pump we're using.

Comment 17

[Bob Owen (:bobowen)]

(In reply to Jim Mathies [:jimm] from comment #16)

I still see this in stacks -

28 - win32u!NtUserCallOneParam
win32u!NtUserCallOneParam
xul!base::MessagePumpForUI::ProcessNextWindowsMessage
xul!mozilla::ipc::MessagePumpForNonMainUIThreads::DoRunLoop
xul!base::MessagePumpWin::RunWithDispatcher
xul!base::MessagePumpWin::Run
xul!MessageLoop::RunInternal
xul!MessageLoop::RunHandler

So I think we still need to touch up the type of chromium message pump we're using.

I take it that's not on the main thread though?

Comment 18

[Jim Mathies [:jimm]]

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

(In reply to Jim Mathies [:jimm] from comment #16)

I still see this in stacks -

28 - win32u!NtUserCallOneParam
win32u!NtUserCallOneParam
xul!base::MessagePumpForUI::ProcessNextWindowsMessage
xul!mozilla::ipc::MessagePumpForNonMainUIThreads::DoRunLoop
xul!base::MessagePumpWin::RunWithDispatcher
xul!base::MessagePumpWin::Run
xul!MessageLoop::RunInternal
xul!MessageLoop::RunHandler

So I think we still need to touch up the type of chromium message pump we're using.

I take it that's not on the main thread though?

No apparently not. Look like it might be one of these two -

https://searchfox.org/mozilla-central/search?q=symbol:E_%3CT_MessageLoop%3A%3AType%3E_TYPE_MOZILLA_NONMAINUITHREAD&redirect=false

which doesn't look like much fun.

Comment 19

[Gian-Carlo Pascutto [:gcp]]

jya pointed out part of this may have been done during bug 1260828.

Comment 20

[Bob Owen (:bobowen)]

As far as I can tell this has now been resolved.
In the main loop we will stop pumping native events after bug 1701802 (when win32k lockdown is enabled).
Others uses should have been removed by work in bug 1260828, its only dependencies are for things in the GPU and GMP process.