1958327 - On Android, launching the WebExtension process blocks the parent process for 80ms

Status: ASSIGNED

(Core :: DOM: Content Processes, enhancement)

Description

[Markus Stange [:mstange]]

Profile: https://share.firefox.dev/42cssDY

When chrome://extensions/content/dummy.xhtml loads in the parent process and the custom element for the <browser> is initialized, we synchronously launch the WebExtension process.

We need to make this non-blocking so that the parent process main thread can process other runnables, most importantly the network runnables to get applink URL request data.

Is there a way to tell the <browser> to use asynchronous child process launch? Or do we have to make sure we preallocate the WebExtension process early enough?

Comment 1

[Rob Wu [:robwu]]

FWIW, from the WebExtensions side, the caller expects the load to be async already: https://searchfox.org/mozilla-central/rev/7014a821bb4689deb04f647a153a6db10e580a00/toolkit/components/extensions/ExtensionParent.sys.mjs#1445-1453
So there is not a strict requirement to synchronously block on the creation of the process.

Comment 2

[Markus Stange [:mstange]]

This is the stack where the parent is blocking:

mozilla::ipc::GeckoChildProcessHost::WaitForProcessHandle() [ipc/glue/GeckoChildProcessHost.cpp]
mozilla::dom::ContentParent::WaitForLaunchSync(mozilla::hal::ProcessPriority) [dom/ipc/ContentParent.cpp]
mozilla::dom::ContentParent::GetNewOrUsedBrowserProcess(nsTSubstring<char> const&, mozilla::dom::BrowsingContextGroup*, mozilla::hal::ProcessPriority, bool, unsigned long) [dom/ipc/ContentParent.cpp]
mozilla::dom::ContentParent::CreateBrowser(mozilla::dom::TabContext const&, mozilla::dom::Element*, nsTSubstring<char> const&, mozilla::dom::BrowsingContext*, mozilla::dom::ContentParent*) [dom/ipc/ContentParent.cpp]
nsFrameLoader::TryRemoteBrowserInternal() [dom/base/nsFrameLoader.cpp]
nsFrameLoader::TryRemoteBrowser() [dom/base/nsFrameLoader.cpp]
nsFrameLoader::TryRemoteBrowser:Create
nsFrameLoader::EnsureRemoteBrowser() [dom/base/nsFrameLoader.cpp]
nsFrameLoader::ShowRemoteFrame(nsSubDocumentFrame*) [dom/base/nsFrameLoader.cpp]
nsFrameLoader::ShowRemoteFrame
nsFrameLoader::Show(nsSubDocumentFrame*) [dom/base/nsFrameLoader.cpp]
nsSubDocumentFrame::ShowViewer() [layout/generic/nsSubDocumentFrame.cpp]
AsyncFrameInit::Run() [layout/generic/nsSubDocumentFrame.cpp]
AsyncFrameInit::Run
nsContentUtils::RemoveScriptBlocker() [dom/base/nsContentUtils.cpp]
nsAutoScriptBlocker::~nsAutoScriptBlocker() [dom/base/nsContentUtils.h]
mozilla::PresShell::DoFlushPendingNotifications(mozilla::ChangesToFlush) [layout/base/PresShell.cpp]
PresShell::DoFlushPendingNotifications Style
mozilla::PresShell::FlushPendingNotifications(mozilla::ChangesToFlush) [layout/base/PresShell.h]
mozilla::dom::Document::FlushPendingNotifications(mozilla::ChangesToFlush) [dom/base/Document.cpp]
nsFrameLoader::TryRemoteBrowserInternal() [dom/base/nsFrameLoader.cpp]
nsFrameLoader::TryRemoteBrowser() [dom/base/nsFrameLoader.cpp]
nsFrameLoader::EnsureRemoteBrowser() [dom/base/nsFrameLoader.cpp]
nsFrameLoader::ReallyStartLoadingInternal() [dom/base/nsFrameLoader.cpp]
nsFrameLoader::ReallyStartLoadingInternal
nsFrameLoader::ReallyStartLoading() [dom/base/nsFrameLoader.cpp]
mozilla::dom::Document::MaybeInitializeFinalizeFrameLoaders() [dom/base/Document.cpp]
RunnableMethod::Run
nsContentUtils::RemoveScriptBlocker() [dom/base/nsContentUtils.cpp]
mozilla::dom::Document::EndUpdate() [dom/base/Document.cpp]
mozAutoDocUpdate::~mozAutoDocUpdate() [dom/base/mozAutoDocUpdate.h]
nsINode::ReplaceOrInsertBefore(bool, nsINode*, nsINode*, mozilla::ErrorResult&) [dom/base/nsINode.cpp]
nsINode::InsertBefore(nsINode&, nsINode*, mozilla::ErrorResult&) [dom/base/nsINode.h]
nsINode::AppendChild(nsINode&, mozilla::ErrorResult&) [dom/base/nsINode.h]
mozilla::dom::Node_Binding::appendChild(JSContext*, JS::Handle<JSObject*>, void*, JSJitMethodCallArgs const&) [dom/bindings/NodeBinding.cpp]
Node.appendChild
createBrowserElement [resource://gre/modules/ExtensionParent.sys.mjs:1471:29]

Nika, do you know what the right way is to avoid this blocking?

Comment 3

[Olli Pettay [:smaug][[e-mail]]]

For the content process part I'd expect that we want to ensure there is a content process around when the browser element is bound to the DOM.
Since if it is not, we haven't preallocated one early enough

Comment 4

[Markus Stange [:mstange]]

PreallocatedProcessManager currently only allocates processes on idle. We'll probably have to fix that anyway in bug 1924849.

Comment 5

[Nika Layzell [:nika] (ni? for response)]

Creating a xul:browser directly with a given remoteType currently always causes a blocking start of the new content process. Prior to fission basically all content processes started synchronously in this way, though we do start during navigations async with Fission. This is also the situation on desktop where we'll block during initial content process startup for e.g. the initial about:newtab page or the extension process.

There were some ideas a while back to reduce the blocking in a situation like that, but it's difficult to do with how we currently require startup to work. Doing changes there would probably require us to eliminate code which requires the pid of the child process synchronously from ContentParent, which would be a fairly significant project.

Given that there is only a single extension process, you could theoretically explicitly kick off async startup of the extension content process early using ensureHeadlessContentProcess (https://searchfox.org/mozilla-central/rev/b42dbdf31bc27acaf3dbcb9d069c55ddfa2cd34e/dom/chrome-webidl/ChromeUtils.webidl#697-711). Perhaps that's worth doing on Android?

Comment 6

[Jens Stutte [:jstutte]]

Can you help us with triage? I would actually not see a defect here, but an enhancement?

Comment 7

[Markus Stange [:mstange]]

You're right, I've changed it to enhancement.

Comment 8

[Markus Stange [:mstange]]

Attachment: Bug 1958327 - Preallocate a WebExtension process at startup. r=nika

Also preallocate a regular (tab) content process first, so that we don't
end up stealing the Java-side preallocated content process.

This avoids calling ContentParent::WaitForLaunchAsync, so
it avoids installing the promise resolution handler that calls
LaunchSubprocessResolve too early.

LaunchSubprocessResolve will still be called once the frameloader calls
ContentParent::CreateBrowser, but this happens late enough so that the
trouble spots are avoided - at least given the current timing on the machines
that I was testing on. Specifically, when ContentParent::CreateBrowser is
called, on macOS the InitFontList thread is more likely to have finished, and
on Android the GPU process is more likely to have advanced far enough that
the sync IPC call for GPUProcessManager::EnsureGPUReady is very short.

Comment 9

[Markus Stange [:mstange]]

macOS profiles:
before: https://share.firefox.dev/3LY11cq (15ms blocking for WebExtension process startup)
after: https://share.firefox.dev/3WVjWXU (0ms blocking for WebExtension process startup)

Comment 10

[Rob Wu [:robwu]]

For visibility, here is an analysis showing 100+ ms impact on startup: https://bugzilla.mozilla.org/show_bug.cgi?id=2004778#c8