Closed
Bug 1384588
Opened 7 years ago
Closed 7 years ago
The observer for net:current-toplevel-outer-content-windowid is needlessly weak
Categories
(Core :: Networking, defect)
Core
Networking
Tracking
()
RESOLVED
DUPLICATE
of bug 1366822
People
(Reporter: ehsan.akhgari, Unassigned)
References
(Blocks 1 open bug)
Details
See https://searchfox.org/mozilla-central/rev/8a61c71153a79cda2e1ae7d477564347c607cc5f/netwerk/protocol/http/nsHttpHandler.cpp#526.
I'm not sure why that is, is there a good reason Kershaw? The QI() for the weak reference shows up in profiles when pages call .focus() on an element under a call stack like this:
nsWeakReference::QueryReferent(nsID const&, void**)
nsQueryReferent::operator()(nsID const&, void**) const
nsCOMPtr_base::assign_from_helper(nsCOMPtr_helper const&, nsID const&)
nsObserverList::FillObserverArray(nsCOMArray<nsIObserver>&)
nsObserverList::NotifyObservers(nsISupports*, char const*, char16_t const*)
nsObserverService::NotifyObservers(nsISupports*, char const*, char16_t const*)
nsObserverService::NotifyObservers net:current-toplevel-outer-content-windowid
NS_NotifyCurrentTopLevelOuterContentWindowId(unsigned long)
nsFocusManager::Focus(nsPIDOMWindowOuter*, nsIContent*, unsigned int, bool, bool, bool, bool, nsIContent*)
nsFocusManager::SetFocusInner(nsIContent*, int, bool, bool)
nsFocusManager::SetFocus(nsIDOMElement*, unsigned int)
mozilla::dom::Element::Focus(mozilla::ErrorResult&)
mozilla::dom::HTMLElementBinding::focus(JSContext*, JS::Handle<JSObject*>, nsGenericHTMLElement*, JSJitMethodCallArgs const&)
mozilla::dom::GenericBindingMethod(JSContext*, unsigned int, JS::Value*)
js::InternalCallOrConstruct(JSContext*, JS::CallArgs const&, js::MaybeConstruct)
js::Call(JSContext*, JS::Handle<JS::Value>, JS::Handle<JS::Value>, js::AnyInvokeArgs const&, JS::MutableHandle<JS::Value>)
js::Wrapper::call(JSContext*, JS::Handle<JSObject*>, JS::CallArgs const&) const
js::CrossCompartmentWrapper::call(JSContext*, JS::Handle<JSObject*>, JS::CallArgs const&) const
js::Proxy::call(JSContext*, JS::Handle<JSObject*>, JS::CallArgs const&)
js::proxy_Call(JSContext*, unsigned int, JS::Value*)
js::InternalCallOrConstruct(JSContext*, JS::CallArgs const&, js::MaybeConstruct)
Interpret(JSContext*, js::RunState&)
js::RunScript(JSContext*, js::RunState&)
prepare/<
js::RunScript
js::InternalCallOrConstruct(JSContext*, JS::CallArgs const&, js::MaybeConstruct)
js::jit::DoCallFallback(JSContext*, js::jit::BaselineFrame*, js::jit::ICCall_Fallback*, unsigned int, JS::Value*, JS::MutableHandle<JS::Value>)
0x38ea1c81e106
0x7f16ffaff1ff
SimplePromise.prototype.resolve
0x38ea1c88f710
0x7f17000cf4cf
SimplePromise.prototype.then
0x38ea1c8128a9
EnterBaseline(JSContext*, js::jit::EnterJitData&)
js::jit::EnterBaselineMethod(JSContext*, js::RunState&)
Interpret(JSContext*, js::RunState&)
js::RunScript(JSContext*, js::RunState&)
prepare
BenchmarkState.prototype.prepareCurrentSuite/frame.onload
js::RunScript
js::InternalCallOrConstruct(JSContext*, JS::CallArgs const&, js::MaybeConstruct)
js::Call(JSContext*, JS::Handle<JS::Value>, JS::Handle<JS::Value>, js::AnyInvokeArgs const&, JS::MutableHandle<JS::Value>)
JS::Call(JSContext*, JS::Handle<JS::Value>, JS::Handle<JS::Value>, JS::HandleValueArray const&, JS::MutableHandle<JS::Value>)
mozilla::dom::EventHandlerNonNull::Call(JSContext*, JS::Handle<JS::Value>, mozilla::dom::Event&, JS::MutableHandle<JS::Value>, mozilla::ErrorResult&)
mozilla::JSEventHandler::HandleEvent(nsIDOMEvent*)
mozilla::EventListenerManager::HandleEventSubType(mozilla::EventListenerManager::Listener*, nsIDOMEvent*, mozilla::dom::EventTarget*)
mozilla::EventListenerManager::HandleEventInternal(nsPresContext*, mozilla::WidgetEvent*, nsIDOMEvent**, mozilla::dom::EventTarget*, nsEventStatus*)
EventListenerManager::HandleEventInternal load
mozilla::EventTargetChainItem::HandleEventTargetChain(nsTArray<mozilla::EventTargetChainItem>&, mozilla::EventChainPostVisitor&, mozilla::EventDispatchingCallback*, mozilla::ELMCreationDetector&)
mozilla::EventDispatcher::Dispatch(nsISupports*, nsPresContext*, mozilla::WidgetEvent*, nsIDOMEvent*, nsEventStatus*, mozilla::EventDispatchingCallback*, nsTArray<mozilla::dom::EventTarget*>*)
EventDispatcher::Dispatch
nsGlobalWindow::PostHandleEvent(mozilla::EventChainPostVisitor&)
mozilla::EventTargetChainItem::HandleEventTargetChain(nsTArray<mozilla::EventTargetChainItem>&, mozilla::EventChainPostVisitor&, mozilla::EventDispatchingCallback*, mozilla::ELMCreationDetector&)
mozilla::EventTargetChainItem::HandleEventTargetChain(nsTArray<mozilla::EventTargetChainItem>&, mozilla::EventChainPostVisitor&, mozilla::EventDispatchingCallback*, mozilla::ELMCreationDetector&)
mozilla::EventDispatcher::Dispatch(nsISupports*, nsPresContext*, mozilla::WidgetEvent*, nsIDOMEvent*, nsEventStatus*, mozilla::EventDispatchingCallback*, nsTArray<mozilla::dom::EventTarget*>*)
EventDispatcher::Dispatch
nsDocumentViewer::LoadComplete(nsresult)
nsDocShell::EndPageLoad(nsIWebProgress*, nsIChannel*, nsresult)
nsDocShell::OnStateChange(nsIWebProgress*, nsIRequest*, unsigned int, nsresult)
non-virtual thunk to nsDocShell::OnStateChange(nsIWebProgress*, nsIRequest*, unsigned int, nsresult)
nsDocLoader::DoFireOnStateChange(nsIWebProgress*, nsIRequest*, int&, nsresult)
nsDocLoader::doStopDocumentLoad(nsIRequest*, nsresult)
nsDocLoader::DocLoaderIsEmpty(bool)
nsDocLoader::OnStopRequest(nsIRequest*, nsISupports*, nsresult)
non-virtual thunk to nsDocLoader::OnStopRequest(nsIRequest*, nsISupports*, nsresult)
mozilla::net::nsLoadGroup::RemoveRequest(nsIRequest*, nsISupports*, nsresult)
nsDocument::UnblockOnload(bool)
nsBindingManager::DoProcessAttachedQueue()
mozilla::detail::RunnableMethodImpl<nsBindingManager*, void (nsBindingManager::*)(), true, (mozilla::RunnableKind)0>::Run()
mozilla::SchedulerGroup::Runnable::Run()
nsThread::ProcessNextEvent(bool, bool*)
NS_ProcessNextEvent(nsIThread*, bool)
mozilla::ipc::MessagePump::Run(base::MessagePump::Delegate*)
MessageLoop::Run()
nsBaseAppShell::Run()
XRE_RunAppShell()
MessageLoop::Run()
XRE_InitChildProcess(int, char**, XREChildData const*)
XRE_InitChildProcess
(root)
Since there are other observers registered here which are actively being managed it seems we can also actively unregister this one when we're about to away and not pay this QI cost needlessly?
Flags: needinfo?(kechang)
Reporter | ||
Updated•7 years ago
|
Blocks: Speedometer_V2
Comment 1•7 years ago
|
||
NS_NotifyCurrentTopLevelOuterContentWindowId() and its caller in nsFocusManager are going to be removed in bug 1366822 [1], so the observer code in nsHttpHandler [2] will not be called every time when an element is focused.
FWIW, "net:current-toplevel-outer-content-windowid" is used for notifying necko the current top-level content window id.
In bug 1366822, we want to use front end code to send the notification to necko. After bug 1366822 is landed, the code in [2] will be also happened only in parent process.
So, the performance issue in this bug will be gone soon. I think we can close this bug. What do you think, Ehsan?
[1] https://bug1366822.bmoattachments.org/attachment.cgi?id=8884205
[2] http://searchfox.org/mozilla-central/rev/ad093e98f42338effe2e2513e26c3a311dd96422/netwerk/protocol/http/nsHttpHandler.cpp#2372
Flags: needinfo?(kechang) → needinfo?(ehsan)
Reporter | ||
Comment 2•7 years ago
|
||
(In reply to Kershaw Chang [:kershaw] from comment #1)
> NS_NotifyCurrentTopLevelOuterContentWindowId() and its caller in
> nsFocusManager are going to be removed in bug 1366822 [1], so the observer
> code in nsHttpHandler [2] will not be called every time when an element is
> focused.
Great!
> FWIW, "net:current-toplevel-outer-content-windowid" is used for notifying
> necko the current top-level content window id.
> In bug 1366822, we want to use front end code to send the notification to
> necko. After bug 1366822 is landed, the code in [2] will be also happened
> only in parent process.
>
> So, the performance issue in this bug will be gone soon. I think we can
> close this bug. What do you think, Ehsan?
Yes, indeed bug 1366822 will take care of the issue here. If this indeed needs to stay a weak reference after that, please feel free to close this bug. Thanks for the quick response!
Flags: needinfo?(ehsan)
Comment 3•7 years ago
|
||
Dup to bug 1366822 based on comment #2.
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → DUPLICATE
You need to log in
before you can comment on or make changes to this bug.
Description
•