760074 - Shouldn't be calling InstantiatePluginInstance in an inactive document

Status: RESOLVED FIXED

(Core Graveyard :: Plug-ins, defect)

Description

[Benjamin Smedberg]

I got this while reloading the testcase for bug 759788 a lot. From the stack, tt appears that a tooltip is being shown on a document which has already been navigated-away-from, which is causing us to sync-start a plugin instance on a dead document.

 	xul.dll!NS_DebugBreak_P(aSeverity=0x00000001, aStr=0x57f71b98, aExpr=0x57f71b90, aFile=0x57f71b20, aLine=0x00000292)  Line 374	C++
>	xul.dll!nsObjectLoadingContent::InstantiatePluginInstance(aMimeType=0x0cc28530, aURI=0x0cc285f0)  Line 658	C++
 	xul.dll!nsObjectLoadingContent::SyncStartPluginInstance()  Line 2107	C++
 	xul.dll!nsHTMLPluginObjElementSH::GetPluginInstanceIfSafe(wrapper=0x07fb2460, obj=0x07b9b380, _result=0x0040b154)  Line 9590	C++
 	xul.dll!nsHTMLPluginObjElementSH::SetupProtoChain(wrapper=0x07fb2460, cx=0x0878d778, obj=0x07b9b380)  Line 9655	C++
 	xul.dll!nsHTMLPluginObjElementSH::PostCreate(wrapper=0x07fb2460, cx=0x0878d778, obj=0x07b9b380)  Line 9755	C++
 	xul.dll!FinishCreate(ccx={...}, Scope=0x0d39ebc0, Interface=0x0a752048, cache=0x0a5ea944, inWrapper=0x07fb2460, resultWrapper=0x0040b664)  Line 716	C++
 	xul.dll!XPCWrappedNative::GetNewOrUsed(ccx={...}, helper={...}, Scope=0x0d39ebc0, Interface=0x0a752048, resultWrapper=0x0040b664)  Line 662	C++
 	xul.dll!XPCWrappedNative::GetNewOrUsed(ccx={...}, helper={...}, Scope=0x087b2860, Interface=0x0a752048, resultWrapper=0x0040b664)  Line 549	C++
 	xul.dll!XPCConvert::NativeInterface2JSObject(lccx={...}, d=0x0040ba5c, dest=0x00000000, aHelper={...}, iid=0x0040ba30, Interface=0x00000000, allowNativeWrapper=true, pErr=0x0040ba04)  Line 957	C++
 	xul.dll!XPCConvert::NativeData2JS(lccx={...}, d=0x0040ba5c, s=0x0040bab8, type={...}, iid=0x0040ba30, pErr=0x0040ba04)  Line 324	C++
 	xul.dll!XPCConvert::NativeData2JS(ccx={...}, d=0x0040ba5c, s=0x0040bab8, type={...}, iid=0x0040ba30, pErr=0x0040ba04)  Line 3213	C++
 	xul.dll!CallMethodHelper::GatherAndConvertResults()  Line 2593	C++
 	xul.dll!CallMethodHelper::Call()  Line 2404	C++
 	xul.dll!XPCWrappedNative::CallMethod(ccx={...}, mode=CALL_GETTER)  Line 2356	C++
 	xul.dll!XPCWrappedNative::GetAttribute(ccx={...})  Line 2717	C++
 	xul.dll!XPC_WN_GetterSetter(cx=0x0878d778, argc=0x00000000, vp=0x072600c0)  Line 1548	C++
 	mozjs.dll!js::CallJSNative(cx=0x0878d778, native=0x55baac49, args={...})  Line 395	C++
 	mozjs.dll!js::InvokeKernel(cx=0x0878d778, args={...}, construct=NO_CONSTRUCT)  Line 310	C++
 	mozjs.dll!js::Invoke(cx=0x0878d778, args={...}, construct=NO_CONSTRUCT)  Line 125	C++
 	mozjs.dll!js::Invoke(cx=0x0878d778, thisv={...}, fval={...}, argc=0x00000000, argv=0x00000000, rval=0x0040c5fc)  Line 358	C++
 	mozjs.dll!js::InvokeGetterOrSetter(cx=0x0878d778, obj=0x08ca37e0, fval={...}, argc=0x00000000, argv=0x00000000, rval=0x0040c5fc)  Line 432	C++
 	mozjs.dll!js::Shape::get(cx=0x0878d778, receiver={...}, obj=0x08ca37e0, pobj=0x08ca37a0, vp=0x0040c5fc)  Line 274	C++
 	mozjs.dll!js_NativeGetInline(cx=0x0878d778, receiver=0x08ca37e0, obj=0x08ca37e0, pobj=0x08ca37a0, shape=0x0b8ee598, getHow=0x00000001, vp=0x0040c5fc)  Line 4938	C++
 	mozjs.dll!js_GetPropertyHelperInline(cx=0x0878d778, obj={...}, receiver={...}, id_={...}, getHow=0x00000001, vp=0x0040c5fc)  Line 5087	C++
 	mozjs.dll!js::GetPropertyHelper(cx=0x0878d778, obj={...}, id={...}, getHow=0x00000001, vp=0x0040c5fc)  Line 5096	C++
 	mozjs.dll!js::GetPropertyOperation(cx=0x0878d778, pc=0x0c8e4b73, lval={...}, vp=0x0040c5fc)  Line 230	C++
 	mozjs.dll!js::Interpret(cx=0x0878d778, entryFrame=0x07260068, interpMode=JSINTERP_NORMAL)  Line 2407	C++
 	mozjs.dll!js::RunScript(cx=0x0878d778, script=0x0d84a028, fp=0x07260068)  Line 266	C++
 	mozjs.dll!js::InvokeKernel(cx=0x0878d778, args={...}, construct=NO_CONSTRUCT)  Line 326	C++
 	mozjs.dll!js::Invoke(cx=0x0878d778, args={...}, construct=NO_CONSTRUCT)  Line 125	C++
 	mozjs.dll!js::Invoke(cx=0x0878d778, thisv={...}, fval={...}, argc=0x00000001, argv=0x0040cae4, rval=0x0040cc2c)  Line 358	C++
 	mozjs.dll!JS_CallFunctionValue(cx=0x0878d778, obj=0x08ca3aa0, fval={...}, argc=0x00000001, argv=0x0040cae4, rval=0x0040cc2c)  Line 5496	C++
 	xul.dll!nsJSContext::CallEventHandler(aTarget=0x09ff98e8, aScope=0x08ca2040, aHandler=0x0d84c080, aargv=0x0685d4b8, arv=0x0040ce58)  Line 1898	C++
 	xul.dll!nsJSEventListener::HandleEvent(aEvent=0x07f7cfc0)  Line 191	C++
 	xul.dll!nsEventListenerManager::HandleEventSubType(aListenerStruct=0x086f17b0, aListener=0x09ff9a08, aDOMEvent=0x07f7cfc0, aCurrentTarget=0x09ff98e8, aPhaseFlags=0x00000006, aPusher=0x0040d034)  Line 809	C++
 	xul.dll!nsEventListenerManager::HandleEventInternal(aPresContext=0x088b7700, aEvent=0x0040d13c, aDOMEvent=0x0040d024, aCurrentTarget=0x09ff98e8, aFlags=0x00000006, aEventStatus=0x0040d028, aPusher=0x0040d034)  Line 868	C++
 	xul.dll!nsEventListenerManager::HandleEvent(aPresContext=0x088b7700, aEvent=0x0040d13c, aDOMEvent=0x0040d024, aCurrentTarget=0x09ff98e8, aFlags=0x00000006, aEventStatus=0x0040d028, aPusher=0x0040d034)  Line 138	C++
 	xul.dll!nsEventTargetChainItem::HandleEvent(aVisitor={...}, aFlags=0x00000006, aMayHaveNewListenerManagers=false, aPusher=0x0040d034)  Line 186	C++
 	xul.dll!nsEventTargetChainItem::HandleEventTargetChain(aVisitor={...}, aFlags=0x00000006, aCallback=0x00000000, aMayHaveNewListenerManagers=false, aPusher=0x0040d034)  Line 319	C++
 	xul.dll!nsEventDispatcher::Dispatch(aTarget=0x09ff98e8, aPresContext=0x088b7700, aEvent=0x0040d13c, aDOMEvent=0x00000000, aEventStatus=0x0040d138, aCallback=0x00000000, aTargets=0x00000000)  Line 643	C++
 	xul.dll!nsXULPopupManager::FirePopupShowingEvent(aPopup=0x09ff98e8, aIsContextMenu=false, aSelectFirstItem=false)  Line 1156	C++
 	xul.dll!nsXULPopupManager::ShowTooltipAtScreen(aPopup=0x09ff98e8, aTriggerContent=0x0a5ea940, aXPos=0x00000419, aYPos=0x00000108)  Line 635	C++
 	xul.dll!nsXULTooltipListener::LaunchTooltip()  Line 516	C++
 	xul.dll!nsXULTooltipListener::ShowTooltip()  Line 410	C++
 	xul.dll!nsXULTooltipListener::sTooltipCallback(aTimer=0x0a4f94b8, aListener=0x0a3b1e50)  Line 708	C++
 	xul.dll!nsTimerImpl::Fire()  Line 473	C++
 	xul.dll!nsTimerEvent::Run()  Line 558	C++
 	xul.dll!nsThread::ProcessNextEvent(mayWait=true, result=0x0040d48f)  Line 624	C++
 	xul.dll!NS_ProcessNextEvent_P(thread=0x004233a8, mayWait=true)  Line 213	C++
 	xul.dll!mozilla::ipc::MessagePump::Run(aDelegate=0x00421330)  Line 113	C++
 	xul.dll!MessageLoop::RunInternal()  Line 209	C++
 	xul.dll!MessageLoop::RunHandler()  Line 202	C++
 	xul.dll!MessageLoop::Run()  Line 176	C++
 	xul.dll!nsBaseAppShell::Run()  Line 165	C++
 	xul.dll!nsAppShell::Run()  Line 232	C++
 	xul.dll!nsAppStartup::Run()  Line 256	C++
 	xul.dll!XREMain::XRE_mainRun()  Line 3786	C++
 	xul.dll!XREMain::XRE_main(argc=0x00000004, argv=0x00abe538, aAppData=0x00d5c864)  Line 3863	C++

Comment 1

[Josh Aas]

Will the patch in bug 757262 fix this?

Comment 2

[Benjamin Smedberg]

No, this is after that patch landed, it's one indication of the fundamental problem which caused that bug. I'm not sure why it's marked security-sensitive, though.

Comment 5

[Mats Palmgren (inactive)]

Ah, it's the assertion roc added in bug 757262 , gotcha. :-)

Comment 6

[Robert O'Callahan (:roc) (email my personal email if necessary)]

This doesn't need to be security-sensitive.

So I guess the fix in bug 757262 is actually helping here because we won't instantiate the plugin in the inactive document (and we shouldn't!).

Comment 7

[Robert O'Callahan (:roc) (email my personal email if necessary)]

I wonder what the script is that's running here. It must be a chrome script since the popupshowing event is not exposed to Web content (right?). I guess that script is calling nsPopupBoxObject::GetTriggerNode to see what triggered the tooltip and touching that content node is re-instantiating the plugin.

It might be a good idea to never instantiate plugins when they're touched through the wrapper that we use to protect chrome from content.

Comment 8

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Would it be hard to modify nsHTMLPluginObjElementSH::GetPluginInstanceIfSafe to do that?

We probably should also add a check to nsHTMLPluginObjElementSH::GetPluginInstanceIfSafe to bail out if the document is not active.

Comment 9

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Attachment: don't try to instantiate plugin in an inactive document

Comment 10

[Robert O'Callahan (:roc) (email my personal email if necessary)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/22b3bd76eaa5

Comment 11

[(no longer active)]

https://hg.mozilla.org/mozilla-central/rev/22b3bd76eaa5