1957813 - tabs onUpdated is called unnecessarily during form.submit() (was: UBlock samples appear on an testcase submitting N forms even when uBlock is disabled on that page)

Status: NEW

(WebExtensions :: General, defect, P3)

Description

[Mayank Bansal]

Attachment: pswword-management.html

Split off from https://bugzilla.mozilla.org/show_bug.cgi?id=1957692#c4

(In reply to :Gijs (he/him) from comment #4)

Not really, there's no jank markers in the parent. I suspect (but am not sure how to verify) that each form submission is generating a separate JSWindowActor IPC message, and processing those in the parent fills up the event queue which is going to lead to the unresponsivenes, but each of the messages themselves processes quickly so there's no hang marker. Florian, is there something on file to notice this more clearly (and/or am I just missing something) ?
There is some noise in the profile in the parent due to uBO - can you re-profile without it to compare apples to apples?

---

STR

1. Install uBlock origin. Enable filterlists if not already enabled.
2. Open attached testcase.
3. Disable ublock on the testcase. Reload the page.
4. Enter N=100 (or 500)
5. Click generate. WAIT for the forms to get generated
6. Start the Gecko profiler
7. Click on "Submit all" button of the testcase
8. Once the browser is usable again, stop profiling.

Profile for N=100: https://share.firefox.dev/3XIWOwx

Looks like there are samples of ublock in the parent process, even when they shouldnt because ublock is disabled.
(Maybe the page navigates due to POST and it taks a finite time for uBlock to calculate that the POST page should have uBlock disabled?)

Something to improve?

Comment 1

[Mayank Bansal]

Attachment: about:support

Comment 2

[Mayank Bansal]

Attachment: my-ublock-backup_2025-04-02_10.21.39.txt

Comment 3

[[e-mail]]

That's expected, uBO declares content scripts for <all_urls> in its manifest, so these content scripts are injected declaratively on all webpages. These content scripts quit immediately when uBO is disabled on a specific site (as confirmed by your profiling results).

Comment 4

[:Gijs (he/him)]

I think the thing that surprised me about this situation is less the content scripts / content process (didn't look at that too much), and more the badge updates in the parent process, which appear to be happening multiple times:

https://share.firefox.dev/41TKAne (edited; I think the "stack chart" view makes this the most obvious)

I don't really understand why that is (maybe navigation as a result of the form? But there should only be 1 "real" navigation in the end... plus Mayank said he'd disabled uBlock?), though tbh it's also not super-concerning in terms of performance impact.

Comment 5

[[e-mail]]

The toolbar icon is updated with a tabs.onUpdated listener[1], and through requestIdleCallback in order to coallesce potentially multiple calls to actually update the icon into fewer browserAction.setIcon calls. Maybe requestIdleCallback is not enough and calls to update the icon should also be based on a minimal delay?

---

[1] I have this comment in the code concerning onUpdated listener: "It may happen the URL in the tab changes, while the page's document stays the same (for instance, Google Maps). Without this listener, the extension icon won't be properly refreshed."

Comment 6

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

The severity field is not set for this bug.
:mstange, could you have a look please?

For more information, please visit BugBot documentation.

Comment 7

[Markus Stange [:mstange]]

Here are the callers of MessageChannel::Send in the content process: https://share.firefox.dev/4iqhIYB

I can see this call stack for PBrowserChild::SendOnStateChange:

mozilla::dom::PBrowserChild::SendOnStateChange(mozilla::dom::WebProgressData const&, mozilla::dom::RequestData const&, unsigned int const&, nsresult const&, mozilla::Maybe<mozilla::dom::WebProgressStateChangeData> const&) [ipc/ipdl/PBrowserChild.cpp]
mozilla::dom::BrowserChild::OnStateChange(nsIWebProgress*, nsIRequest*, unsigned int, nsresult) [dom/ipc/BrowserChild.cpp]
nsBrowserStatusFilter::OnStateChange(nsIWebProgress*, nsIRequest*, unsigned int, nsresult) [toolkit/components/statusfilter/nsBrowserStatusFilter.cpp]
nsDocLoader::DoFireOnStateChange(nsIWebProgress* const, nsIRequest* const, int&, const nsresult) [uriloader/base/nsDocLoader.cpp]
nsDocLoader::doStopDocumentLoad(nsIRequest*, nsresult) [uriloader/base/nsDocLoader.cpp]
nsDocLoader::DocLoaderIsEmpty(bool, mozilla::Maybe<nsresult> const&) [uriloader/base/nsDocLoader.cpp]
nsDocLoader::OnStopRequest(nsIRequest*, nsresult) [uriloader/base/nsDocLoader.cpp]
nsDocShell::OnStopRequest(nsIRequest*, nsresult) [docshell/base/nsDocShell.cpp]
mozilla::net::nsLoadGroup::NotifyRemovalObservers(nsIRequest*, nsresult) [netwerk/base/nsLoadGroup.cpp]
mozilla::net::nsLoadGroup::Cancel(nsresult) [netwerk/base/nsLoadGroup.cpp]
nsDocLoader::Stop() [uriloader/base/nsDocLoader.cpp]
nsDocShell::Stop() [docshell/base/nsDocShell.h]
nsDocShell::Stop(unsigned int) [docshell/base/nsDocShell.cpp]
nsDocShell::InternalLoad(nsDocShellLoadState*, mozilla::Maybe<unsigned int>) [docshell/base/nsDocShell.cpp]
nsDocShell::OnLinkClickSync(nsIContent*, nsDocShellLoadState*, bool, nsIPrincipal*) [docshell/base/nsDocShell.cpp]
mozilla::dom::HTMLFormElement::SubmitSubmission(mozilla::dom::HTMLFormSubmission*) [dom/html/HTMLFormElement.cpp]
mozilla::dom::HTMLFormElement::DoSubmit(mozilla::dom::Event*) [dom/html/HTMLFormElement.cpp]
mozilla::dom::HTMLFormElement::Submit(mozilla::ErrorResult&) [dom/html/HTMLFormElement.cpp]
mozilla::dom::HTMLFormElement_Binding::submit(JSContext*, JS::Handle<JSObject *>, void*, JSJitMethodCallArgs const&) [dom/bindings/HTMLFormElementBinding.cpp]
mozilla::dom::binding_detail::GenericMethod<mozilla::dom::binding_detail::NormalThisPolicy,mozilla::dom::binding_detail::ThrowExceptions>(JSContext*, unsigned int, JS::Value*) [dom/bindings/BindingUtils.cpp]
HTMLFormElement.submit

And this call stack for PExtensionsChild::SendStateChange:

mozilla::extensions::PExtensionsChild::SendStateChange(mozilla::dom::MaybeDiscarded<mozilla::dom::BrowsingContext> const&, nsIURI*, nsresult const&, unsigned int const&) [ipc/ipdl/PExtensionsChild.cpp]
mozilla::extensions::WebNavigationContent::OnStateChange(nsIWebProgress*, nsIRequest*, unsigned int, nsresult) [toolkit/components/extensions/webrequest/WebNavigationContent.cpp]
nsDocLoader::DoFireOnStateChange(nsIWebProgress* const, nsIRequest* const, int&, const nsresult) [uriloader/base/nsDocLoader.cpp]
nsDocLoader::FireOnStateChange(nsIWebProgress*, nsIRequest*, int, nsresult) [uriloader/base/nsDocLoader.cpp]
nsDocLoader::doStartURLLoad(nsIRequest*, int) [uriloader/base/nsDocLoader.cpp]
nsDocLoader::OnStartRequest(nsIRequest*) [uriloader/base/nsDocLoader.cpp]
nsDocShell::OnStartRequest(nsIRequest*) [docshell/base/nsDocShell.cpp]
mozilla::net::nsLoadGroup::AddRequest(nsIRequest*, nsISupports*) [netwerk/base/nsLoadGroup.cpp]
mozilla::net::DocumentChannelChild::AsyncOpen(nsIStreamListener*) [netwerk/ipc/DocumentChannelChild.cpp]
nsURILoader::OpenURI(nsIChannel*, unsigned int, nsIInterfaceRequestor*) [uriloader/base/nsURILoader.cpp]
nsDocShell::OpenInitializedChannel(nsIChannel*, nsIURILoader*, unsigned int) [docshell/base/nsDocShell.cpp]
nsDocShell::DoURILoad(nsDocShellLoadState*, mozilla::Maybe<unsigned int>, nsIRequest**) [docshell/base/nsDocShell.cpp]
nsDocShell::InternalLoad(nsDocShellLoadState*, mozilla::Maybe<unsigned int>) [docshell/base/nsDocShell.cpp]
nsDocShell::OnLinkClickSync(nsIContent*, nsDocShellLoadState*, bool, nsIPrincipal*) [docshell/base/nsDocShell.cpp]
mozilla::dom::HTMLFormElement::SubmitSubmission(mozilla::dom::HTMLFormSubmission*) [dom/html/HTMLFormElement.cpp]
mozilla::dom::HTMLFormElement::DoSubmit(mozilla::dom::Event*) [dom/html/HTMLFormElement.cpp]
mozilla::dom::HTMLFormElement::Submit(mozilla::ErrorResult&) [dom/html/HTMLFormElement.cpp]
mozilla::dom::HTMLFormElement_Binding::submit(JSContext*, JS::Handle<JSObject *>, void*, JSJitMethodCallArgs const&) [dom/bindings/HTMLFormElementBinding.cpp]
mozilla::dom::binding_detail::GenericMethod<mozilla::dom::binding_detail::NormalThisPolicy,mozilla::dom::binding_detail::ThrowExceptions>(JSContext*, unsigned int, JS::Value*) [dom/bindings/BindingUtils.cpp]
HTMLFormElement.submit

And in the parent process I can see a call to fireForTab here:

fireForTab [chrome://browser/content/parent/ext-tabs.js:382:24]
statusListener [chrome://browser/content/parent/ext-tabs.js:480:28]
onStateChange [chrome://extensions/content/parent/ext-tabs-base.js:1439:16]
js::RunScript
callListeners [chrome://browser/content/tabbrowser/tabbrowser.js:972:29]
_callProgressListeners [chrome://browser/content/tabbrowser/tabbrowser.js:963:27]
_callProgressListeners [chrome://browser/content/tabbrowser/tabbrowser.js:7594:27]
onStateChange [chrome://browser/content/tabbrowser/tabbrowser.js:7685:18]
js::RunScript
JS_CallFunctionValue(JSContext*, JS::Handle<JSObject *>, JS::Handle<JS::Value>, JS::HandleValueArray const&, JS::MutableHandle<JS::Value>) [js/src/vm/CallAndConstruct.cpp]
nsXPCWrappedJS::CallMethod(unsigned short, nsXPTMethodInfo const*, nsXPTCMiniVariant*) [js/xpconnect/src/XPCWrappedJSClass.cpp]
XPCWrappedJS method call
PrepareAndDispatch(nsXPTCStubBase*, unsigned int, unsigned long long*, unsigned long long*, double*) [xpcom/reflect/xptcall/md/win32/xptcstubs_x86_64.cpp]
SharedStub() [/builds/worker/workspace/obj-build/toolkit/library/build/Z:/builds/worker/checkouts/gecko/xpcom/reflect/xptcall/md/win32/xptcstubs_asm_x86_64.asm]
nsBrowserStatusFilter::OnStateChange(nsIWebProgress*, nsIRequest*, unsigned int, nsresult) [toolkit/components/statusfilter/nsBrowserStatusFilter.cpp]
mozilla::dom::BrowsingContextWebProgress::OnStateChange::<lambda_37>::operator()(nsIWebProgressListener*) const [docshell/base/BrowsingContextWebProgress.cpp]
std::invoke(mozilla::dom::BrowsingContextWebProgress::OnStateChange::<lambda_37>&, nsIWebProgressListener*&&) [/builds/worker/fetches/vs/VC/Tools/MSVC/14.39.33519/include/type_traits]
std::_Func_impl_no_alloc<`lambda at /builds/worker/checkouts/gecko/docshell/base/BrowsingContextWebProgress.cpp:257:7',void,nsIWebProgressListener *>::_Do_call(nsIWebProgressListener*&&) [/builds/worker/fetches/vs/VC/Tools/MSVC/14.39.33519/include/functional]
std::_Func_class<void,nsIWebProgressListener *>::operator()(nsIWebProgressListener*) const [/builds/worker/fetches/vs/VC/Tools/MSVC/14.39.33519/include/functional]
mozilla::dom::BrowsingContextWebProgress::UpdateAndNotifyListeners(unsigned int, std::function<void (nsIWebProgressListener *)> const&) [docshell/base/BrowsingContextWebProgress.cpp]
mozilla::dom::BrowsingContextWebProgress::OnStateChange(nsIWebProgress*, nsIRequest*, unsigned int, nsresult) [docshell/base/BrowsingContextWebProgress.cpp]
mozilla::dom::BrowserParent::RecvOnStateChange(mozilla::dom::WebProgressData const&, mozilla::dom::RequestData const&, const unsigned int, const nsresult, mozilla::Maybe<mozilla::dom::WebProgressStateChangeData> const&) [dom/ipc/BrowserParent.cpp]
mozilla::dom::PBrowserParent::OnMessageReceived(IPC::Message const&) [ipc/ipdl/PBrowserParent.cpp]
PBrowser::Msg_OnStateChange
mozilla::dom::PContentParent::OnMessageReceived(IPC::Message const&) [ipc/ipdl/PContentParent.cpp]

So if this is still considered a valid bug, I think a more concrete description would be "tabs onUpdated is called unnecessarily during form.submit()"

Comment 8

[:Gijs (he/him)]

Thanks Markus! Let's pass this to the webext folks, then.