1382118 - Crash in nsPrintEngine::AfterNetworkPrint

Status: RESOLVED FIXED

(Core :: Print Preview, defect)

Description

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

This bug was filed from the Socorro interface and is 
report bp-4ce5d06d-3e57-49ba-80d4-838fb0170718.
=============================================================
> nsPrintEngine::AfterNetworkPrint(bool)
> nsPrintEngine::OnStateChange(nsIWebProgress*, nsIRequest*, unsigned int, nsresult)
> nsDocLoader::DoFireOnStateChange(nsIWebProgress* const, nsIRequest* const, int&, nsresult)
> nsDocLoader::FireOnStateChange(nsIWebProgress*, nsIRequest*, int, nsresult)
> nsDocLoader::doStopURLLoad(nsIRequest*, nsresult)
> nsDocLoader::OnStopRequest(nsIRequest*, nsISupports*, nsresult)
> mozilla::net::nsLoadGroup::RemoveRequest(nsIRequest*, nsISupports*, nsresult)
> mozilla::net::nsLoadGroup::Cancel(nsresult)
> nsDocLoader::Stop()
> nsDocShell::Stop(unsigned int)
> nsDocShell::Destroy()
> nsFrameLoader::DestroyDocShell()
> nsFrameLoaderDestroyRunnable::Run()
> nsDocument::MaybeInitializeFinalizeFrameLoaders()
> nsDocument::EndUpdate(unsigned int)
> mozilla::dom::XULDocument::EndUpdate(unsigned int)
> mozAutoDocUpdate::~mozAutoDocUpdate()
> nsINode::doRemoveChildAt(unsigned int, bool, nsIContent*, nsAttrAndChildArray&)
> mozilla::dom::FragmentOrElement::RemoveChildAt(unsigned int, bool)
> nsXULElement::RemoveChildAt(unsigned int, bool)
> nsINode::Remove()
> mozilla::dom::ElementBinding::remove
> js::InternalCallOrConstruct(JSContext*, JS::CallArgs const&, js::MaybeConstruct)
> InternalCall
> Interpret
> js::RunScript(JSContext*, js::RunState&)
> js::InternalCallOrConstruct(JSContext*, JS::CallArgs const&, js::MaybeConstruct)
> InternalCall
> Interpret
> js::RunScript(JSContext*, js::RunState&)
> js::InternalCallOrConstruct(JSContext*, JS::CallArgs const&, js::MaybeConstruct)
> InternalCall
> Interpret
> js::RunScript(JSContext*, js::RunState&)
> 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>)
> JS_NewObjectWithGivenProto(JSContext*, JSClass const*, JS::Handle<JSObject*>)
> mozilla::dom::EventHandlerNonNull::Call(JSContext*, JS::Handle<JS::Value>, mozilla::dom::Event&, JS::MutableHandle<JS::Value>, mozilla::ErrorResult&)
> mozilla::dom::EventHandlerNonNull::Call<nsISupports*>(nsISupports* const&, mozilla::dom::Event&, JS::MutableHandle<JS::Value>, mozilla::ErrorResult&, char const*, mozilla::dom::CallbackObject::ExceptionHandling, JSCompartment*)
> `anonymous namespace'::MaybeScheduleMemoryPressureEvent
> nsGlobalWindow::QueryInterface(nsID const&, void**)
> 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*)
> js::GetProperty(JSContext*, JS::Handle<JSObject*>, JS::Handle<JS::Value>, JS::Handle<jsid>, JS::MutableHandle<JS::Value>)
> 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*>*)
> mozilla::EventDispatcher::DispatchDOMEvent(nsISupports*, mozilla::WidgetEvent*, nsIDOMEvent*, nsPresContext*, nsEventStatus*)
> nsGlobalWindow::DispatchDOMEvent(mozilla::WidgetEvent*, nsIDOMEvent*, nsPresContext*, nsEventStatus*)
> nsWebShellWindow::RequestWindowClose(nsIWidget*)

Even during loading document for printing or print preview, the document may be destroyed because of closing window.

According to the crash address, nsPrintEngine::AfterNetworkPrint() may be called even after mPrt is nullptr.

> nsresult
> nsPrintEngine::AfterNetworkPrint(bool aHandleError)
> {
>   nsCOMPtr<nsIWebProgress> webProgress = do_QueryInterface(mPrt->mPrintObject->mDocShell);

Comment 1

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=db7370497915cd8dd1e14caff3d4e42e706323cb

Comment 2

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

Attachment: Bug 1382118 - Make nsPrintEngine::AfterNetworkPrint() do nothing if mPtr is nullptr

According to the crash reports, nsPrintEngine::AfterNetworkPrint() may be called after nsPrintEngine::Destroy() is called.  In this case, the method should do nothing because it's already been destroyed.

Review commit: https://reviewboard.mozilla.org/r/160086/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/160086/

Comment 3

[Daniel Holbert [:dholbert]]

Comment on attachment 8889032 [details]
Bug 1382118 - Make nsPrintEngine::AfterNetworkPrint() do nothing if mPtr is nullptr

https://reviewboard.mozilla.org/r/160086/#review165824

r=me

Comment 4

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/aebaa2a545ba
Make nsPrintEngine::AfterNetworkPrint() do nothing if mPtr is nullptr r=dholbert

Comment 5

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=a7da1ee180a35c0487a84c6583e3aec78385085a

Comment 6

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/aebaa2a545ba

Comment 7

[Ryan VanderMeulen [:RyanVM]]

Looks like a simple patch, but I'm not convinced that the crash volume is high enough to warrant consideration for uplifting. What do you think?

Comment 8

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

Yeah, I don't think that it's worthwhile to uplift it.