Closed Bug 491848 Opened 12 years ago Closed 12 years ago

Crash [@ nsDisplayListBuilder::EnterPresShell] when keeping mouse pressed down in position: fixed flash embed, while reloading

Categories

(Core :: Layout, defect)

x86
Windows XP
defect
Not set
critical

Tracking

()

RESOLVED FIXED

People

(Reporter: martijn.martijn, Assigned: tnikkel)

References

Details

(4 keywords)

Crash Data

Attachments

(3 files, 2 obsolete files)

Attached file testcase
See testcase, which crashes current trunk build while holding the mouse button down on the flash embed (the green bordered box).

This regressed between 2009-04-05 and 2009-04-06:
http://hg.mozilla.org/mozilla-central/pushloghtml?startdate=2009-04-05+04%3A00%3A00&enddate=2009-04-06+06%3A00%3A00
I'm guessing a regression from bug 485275.

The iframe content consists of this:
<embed type="application/x-shockwave-flash" style="position: fixed;  outline: 10px solid green;">

http://crash-stats.mozilla.com/report/index/b06b134c-b9ad-473e-912b-870fe2090507?p=1
0  	xul.dll  	nsDisplayListBuilder::EnterPresShell  	 layout/base/nsDisplayList.cpp:181
1 	xul.dll 	xul.dll@0x3b2b73 	
2 	xul.dll 	nsIFrame::BuildDisplayListForChild 	layout/generic/nsFrame.cpp:1618
3 	xul.dll 	DisplayLine 	layout/generic/nsBlockFrame.cpp:6055
4 	xul.dll 	nsBlockFrame::BuildDisplayList 	layout/generic/nsBlockFrame.cpp:6134
5 	xul.dll 	nsIFrame::BuildDisplayListForChild 	layout/generic/nsFrame.cpp:1576
6 	xul.dll 	DisplayLine 	layout/generic/nsBlockFrame.cpp:6055
7 	xul.dll 	nsBlockFrame::BuildDisplayList 	layout/generic/nsBlockFrame.cpp:6134
8 	xul.dll 	nsIFrame::BuildDisplayListForChild 	layout/generic/nsFrame.cpp:1618
9 	xul.dll 	CanvasFrame::BuildDisplayList 	layout/generic/nsHTMLFrame.cpp:502
10 	xul.dll 	nsIFrame::BuildDisplayListForChild 	layout/generic/nsFrame.cpp:1576
11 	xul.dll 	nsGfxScrollFrameInner::BuildDisplayList 	layout/generic/nsGfxScrollFrame.cpp:1349
12 	xul.dll 	nsHTMLScrollFrame::BuildDisplayList 	layout/generic/nsGfxScrollFrame.h:260
13 	xul.dll 	nsIFrame::BuildDisplayListForChild 	layout/generic/nsFrame.cpp:1576
14 	xul.dll 	ViewportFrame::BuildDisplayList 	layout/generic/nsViewportFrame.cpp:109
15 	xul.dll 	nsIFrame::BuildDisplayListForStackingContext 	layout/generic/nsFrame.cpp:1286
16 	xul.dll 	nsLayoutUtils::PaintFrame 	layout/base/nsLayoutUtils.cpp:1051
17 	xul.dll 	PresShell::Paint 	layout/base/nsPresShell.cpp:5573
18 	xul.dll 	nsViewManager::RenderViews 	view/src/nsViewManager.cpp:609
19 	xul.dll 	nsViewManager::Refresh 	view/src/nsViewManager.cpp:511
20 	xul.dll 	nsViewManager::DispatchEvent 	view/src/nsViewManager.cpp:1105
21 	xul.dll 	HandleEvent 	view/src/nsView.cpp:167
22 	xul.dll 	nsWindow::DispatchEvent 	widget/src/windows/nsWindow.cpp:947
23 	xul.dll 	nsWindow::DispatchWindowEvent 	widget/src/windows/nsWindow.cpp:972
24 	xul.dll 	nsWindow::OnPaint 	widget/src/windows/nsWindow.cpp:6127
25 	xul.dll 	nsWindow::ProcessMessage 	widget/src/windows/nsWindow.cpp:4279
26 	xul.dll 	nsWindow::WindowProc 	widget/src/windows/nsWindow.cpp:1163
27 	user32.dll 	user32.dll@0x8733 	
28 	user32.dll 	user32.dll@0x8815 	
29 	user32.dll 	user32.dll@0x18e9f 	
30 	user32.dll 	user32.dll@0x18eeb 	
31 	ntdll.dll 	ntdll.dll@0xe472 	
32 	xul.dll 	nsWindow::DispatchStarvedPaints 	widget/src/windows/nsWindow.cpp:4029
33 	user32.dll 	user32.dll@0x1a4e7 	
34 	user32.dll 	user32.dll@0x1b108 	
35 	xul.dll 	nsWindow::DispatchPendingEvents 	widget/src/windows/nsWindow.cpp:4060
36 	xul.dll 	nsReadFromRawBuffer 	xpcom/io/nsPipe3.cpp:1174
37 	xul.dll 	xul.dll@0x3ebed6 	
38 	xul.dll 	nsWindow::WindowProc 	widget/src/windows/nsWindow.cpp:1163
39 	xul.dll 	xul.dll@0x311c61 	
40 		@0x0
Flags: blocking1.9.2?
At first glance, if the part of bug 485275 that landed in that window had any effect in this stack trace then I would expect there to be two (or more) nsViewManager::RenderViews frames together in the stack trace.
Doesn't happen on Linux, so that makes it very hard for me to debug. Is it Windows only?

I can put a null check on the caret in nsDisplayListBuilder::EnterPresShell. This is probably a good idea regardless of this bug as I don't think there are any guarantees on mCaret being non-null (every access to mCaret in nsPresShell.cpp does a null check for example). But that doesn't answer why we are only seeing this now.
It's enough just to have a plugin in an iframe that has position: fixed, click it to give it focus, and then click reload, and that will cause it to crash. Reloading just the iframe doesn't do it, neither does back/forward on the iframe or the root document.

If anybody that does Windows builds wants to comment out lines 599 & 600 of nsViewManager.cpp ("if (displayRootVM && displayRootVM != this)" and "return displayRootVM->RenderViews(aView, aRC, aRegion);") and try the testcase that would tell us if bug 485275 is causing the problem.
Attached patch patch (obsolete) — Splinter Review
I finally got a Windows VM working to build and debug mozilla.

I get a different stack trace when using a debug build, but I think it's the same underlying problem. The stack is below, but here is my analysis. What is happening is the pres shell of the iframe is in the middle of being destroyed, as part of this the plugin is hidden with a call to nsWindow::Show, this goes into kernel code, and eventually a mouse move event  is sent to mozilla code (not sure exactly why, maybe Windows sends a mouse event to whatever window is now under the mouse after a hide), and this tries to build a display list to get the frame for the coordinates of the mouse event, which will fail when it recurses into the pres shell for the iframe.

The landing for bug 485275 was the trigger, but not directly. Because the crash in the debug build happens in event processing, and the crash in an opt build happens during painting, I think it just upset the balance and caused this race condition to fail. Bug 336582 is a similar bug involving focus and a pres shell for a subdocument being destroyed.

To fix this I think we should check in nsDisplayListBuilder::EnterPresShell if the pres shell is destroying, and return a boolean indicating to the caller if it can enter the pres shell.

And here is the stack trace.

 	ntdll.dll!7c90120e() 	
 	[Frames below may be incorrect and/or missing, no symbols loaded for ntdll.dll]	
>	xpcom_core.dll!Break(const char * aMsg=0x00000000)  Line 489	C++
 	xpcom_core.dll!NS_DebugBreak_P(unsigned int aSeverity=, const char * aStr=, const char * aExpr=, const char * aFile=, int aLine=)  Line 356	C++
 	gklayout.dll!nsFrameManager::GetPrimaryFrameFor(nsIContent * aContent=0x033aafb8, int aIndexHint=-1)  Line 335 + 0x16 bytes	C++
 	gklayout.dll!PresShell::GetPrimaryFrameFor(nsIContent * aContent=0x033aafb8)  Line 5092	C++
 	gklayout.dll!nsCSSRendering::FindRootFrame(nsIFrame * aForFrame=0x05056454)  Line 943 + 0x11 bytes	C++
 	gklayout.dll!nsCSSRendering::FindRootFrameBackground(nsIFrame * aForFrame=0x05056454)  Line 985 + 0x9 bytes	C++
 	gklayout.dll!PresShell::UpdateCanvasBackground()  Line 5602	C++
 	gklayout.dll!nsDisplayListBuilder::EnterPresShell(nsIFrame * aReferenceFrame=0x04f365a0, const nsRect & aDirtyRect={...})  Line 176	C++
 	gklayout.dll!nsSubDocumentFrame::BuildDisplayList(nsDisplayListBuilder * aBuilder=0x03876420, const nsRect & aDirtyRect={...}, const nsDisplayListSet & aLists={...})  Line 367	C++
 	gklayout.dll!nsIFrame::BuildDisplayListForChild(nsDisplayListBuilder * aBuilder=0x00000000, nsIFrame * aChild=0x04d30010, const nsRect & aDirtyRect={...}, const nsDisplayListSet & aLists={...}, unsigned int aFlags=4)  Line 1618 + 0xf bytes	C++
 	gklayout.dll!DisplayLine(nsDisplayListBuilder * aBuilder=0x0012e338, const nsRect & aLineArea={...}, const nsRect & aDirtyRect={...}, nsLineList_iterator & aLine={...}, int aDepth=0)  Line 6127 + 0x22 bytes	C++
 	gklayout.dll!nsBlockFrame::BuildDisplayList(nsDisplayListBuilder * aBuilder=0x0012e338, const nsRect & aDirtyRect={...}, const nsDisplayListSet & aLists={...})  Line 6206 + 0x1a bytes	C++
 	gklayout.dll!nsIFrame::BuildDisplayListForChild(nsDisplayListBuilder * aBuilder=0x00000000, nsIFrame * aChild=0x04d2fdb4, const nsRect & aDirtyRect={...}, const nsDisplayListSet & aLists={...}, unsigned int aFlags=0)  Line 1576 + 0xe bytes	C++
 	gklayout.dll!DisplayLine(nsDisplayListBuilder * aBuilder=0x0012e338, const nsRect & aLineArea={...}, const nsRect & aDirtyRect={...}, nsLineList_iterator & aLine={...}, int aDepth=0)  Line 6127 + 0x22 bytes	C++
 	gklayout.dll!nsBlockFrame::BuildDisplayList(nsDisplayListBuilder * aBuilder=0x0012e338, const nsRect & aDirtyRect={...}, const nsDisplayListSet & aLists={...})  Line 6206 + 0x1a bytes	C++
 	gklayout.dll!nsIFrame::BuildDisplayListForChild(nsDisplayListBuilder * aBuilder=0x00000000, nsIFrame * aChild=0x04d2fb8c, const nsRect & aDirtyRect={...}, const nsDisplayListSet & aLists={...}, unsigned int aFlags=1)  Line 1618 + 0xf bytes	C++
 	gklayout.dll!CanvasFrame::BuildDisplayList(nsDisplayListBuilder * aBuilder=0x0012e338, const nsRect & aDirtyRect={...}, const nsDisplayListSet & aLists={...})  Line 503	C++
 	gklayout.dll!nsIFrame::BuildDisplayListForChild(nsDisplayListBuilder * aBuilder=0x00000000, nsIFrame * aChild=0x050b3d04, const nsRect & aDirtyRect={...}, const nsDisplayListSet & aLists={...}, unsigned int aFlags=0)  Line 1576 + 0xe bytes	C++
 	gklayout.dll!nsGfxScrollFrameInner::BuildDisplayList(nsDisplayListBuilder * aBuilder=0x0012e338, const nsRect & aDirtyRect={...}, const nsDisplayListSet & aLists={...})  Line 1350	C++
 	gklayout.dll!nsHTMLScrollFrame::BuildDisplayList(nsDisplayListBuilder * aBuilder=0x0012e338, const nsRect & aDirtyRect={...}, const nsDisplayListSet & aLists={...})  Line 261	C++
 	gklayout.dll!nsIFrame::BuildDisplayListForChild(nsDisplayListBuilder * aBuilder=0x00000000, nsIFrame * aChild=0x050b3e84, const nsRect & aDirtyRect={...}, const nsDisplayListSet & aLists={...}, unsigned int aFlags=0)  Line 1576 + 0xe bytes	C++
 	gklayout.dll!ViewportFrame::BuildDisplayList(nsDisplayListBuilder * aBuilder=0x0012e338, const nsRect & aDirtyRect={...}, const nsDisplayListSet & aLists={...})  Line 109 + 0x13 bytes	C++
 	gklayout.dll!nsIFrame::BuildDisplayListForStackingContext(nsDisplayListBuilder * aBuilder=0x0012e338, const nsRect & aDirtyRect={...}, nsDisplayList * aList=0x0012e194)  Line 1287	C++
 	gklayout.dll!nsLayoutUtils::GetFrameForPoint(nsIFrame * aFrame=0x050b3c20, nsPoint aPt={...}, int aShouldIgnoreSuppression=0, int aIgnoreRootScrollFrame=0)  Line 925 + 0x16 bytes	C++
 	gklayout.dll!PresShell::HandleEvent(nsIView * aView=0x039d7210, nsGUIEvent * aEvent=0x0012e738, nsEventStatus * aEventStatus=0x0012e5f4)  Line 5929 + 0x1a bytes	C++
 	gklayout.dll!nsViewManager::HandleEvent(nsView * aView=0x04d2cdd0, nsPoint aPoint={...}, nsGUIEvent * aEvent=0x0012e738, int aCaptured=0)  Line 1349	C++
 	gklayout.dll!nsViewManager::DispatchEvent(nsGUIEvent * aEvent=0x039d7210, nsEventStatus * aStatus=0x0012e688)  Line 1325 + 0x1a bytes	C++
 	gklayout.dll!HandleEvent(nsGUIEvent * aEvent=0x0012e738)  Line 170	C++
 	gkwidget.dll!nsWindow::DispatchEvent(nsGUIEvent * event=0x0012e738, nsEventStatus & aStatus=nsEventStatus_eIgnore)  Line 967 + 0x3 bytes	C++
 	gkwidget.dll!nsWindow::DispatchWindowEvent(nsGUIEvent * event=0x00000000)  Line 988	C++
 	gkwidget.dll!nsWindow::DispatchMouseEvent(unsigned int aEventType=322, unsigned int wParam=0, long lParam=5374110, int aIsContextMenuKey=0, short aButton=100)  Line 6569 + 0xb bytes	C++
 	gkwidget.dll!ChildWindow::DispatchMouseEvent(unsigned int aEventType=322, unsigned int wParam=0, long lParam=5374110, int aIsContextMenuKey=0, short aButton=0)  Line 6716 + 0x14 bytes	C++
 	gkwidget.dll!nsWindow::DispatchMouseEvent(unsigned int aEventType=300, unsigned int wParam=0, long lParam=5374110, int aIsContextMenuKey=0, short aButton=100)  Line 6562	C++
 	gkwidget.dll!ChildWindow::DispatchMouseEvent(unsigned int aEventType=300, unsigned int wParam=0, long lParam=5374110, int aIsContextMenuKey=0, short aButton=0)  Line 6716 + 0x14 bytes	C++
 	gkwidget.dll!nsWindow::ProcessMessage(unsigned int msg=512, unsigned int & wParam=0, long & lParam=5374110, long * aRetValue=0x0012ead0)  Line 4442	C++
 	gkwidget.dll!nsWindow::WindowProc(HWND__ * hWnd=0x000202d4, unsigned int msg=0, unsigned int wParam=0, long lParam=5374110)  Line 1183 + 0x14 bytes	C++
 	user32.dll!7e418734() 	
 	user32.dll!7e418816() 	
 	user32.dll!7e4189cd() 	
 	user32.dll!7e41929b() 	
 	user32.dll!7e419402() 	
 	user32.dll!7e418a10() 	
 	gkwidget.dll!nsAppShell::ProcessNextNativeEvent(int mayWait=0)  Line 171	C++
 	gkwidget.dll!nsBaseAppShell::DoProcessNextNativeEvent(int mayWait=0)  Line 152	C++
 	gkwidget.dll!nsBaseAppShell::OnProcessNextEvent(nsIThreadInternal * thr=0x00a7f040, int mayWait=0, unsigned int recursionDepth=1)  Line 288	C++
 	xpcom_core.dll!nsThread::ProcessNextEvent(int mayWait=0, int * result=0x0012eccc)  Line 501	C++
 	xpcom_core.dll!NS_ProcessPendingEvents_P(nsIThread * thread=0x00000001, unsigned int timeout=100)  Line 183 + 0x16 bytes	C++
 	gkwidget.dll!nsWindow::DispatchPendingEvents()  Line 4068	C++
 	gkwidget.dll!nsWindow::ProcessMessageForPlugin(const tagMSG & aMsg={...}, long * aResult=0x00000000, int & aCallDefWndProc=1)  Line 5435	C++
 	gkwidget.dll!nsWindow::ProcessMessage(unsigned int msg=641, unsigned int & wParam=1, long & lParam=-1073741809, long * aRetValue=0x0012eee8)  Line 4107 + 0x18 bytes	C++
 	gkwidget.dll!nsWindow::WindowProc(HWND__ * hWnd=0x000302d6, unsigned int msg=0, unsigned int wParam=1, long lParam=-1073741809)  Line 1183 + 0x14 bytes	C++
 	user32.dll!7e418734() 	
 	user32.dll!7e418816() 	
 	gkwidget.dll!DetectWindowMove(int code=0, unsigned int wParam=22142644, long lParam=197334)  Line 163 + 0x5 bytes	C++
 	user32.dll!7e428ea0() 	
 	user32.dll!7e428eec() 	
 	ntdll.dll!7c90e473() 	
 	user32.dll!7e4194be() 	
 	user32.dll!7e42c174() 	
 	user32.dll!7e4292e3() 	
 	imm32.dll!76392b12() 	
 	user32.dll!7e46b90e() 	
 	user32.dll!7e46bdf2() 	
 	xpcom_core.dll!nsObserverService::Release()  Line 76 + 0x41 bytes	C++
 	gkplugin.dll!nsCOMPtr<nsIObserverService>::~nsCOMPtr<nsIObserverService>()  Line 510 + 0x6 bytes	C++
 	gkplugin.dll!NS_NotifyPluginCall(unsigned int startTime=)  Line 192 + 0x19 bytes	C++
 	nspr4.dll!_MD_CURRENT_THREAD()  Line 310	C
 	nspr4.dll!PR_GetThreadPrivate(unsigned int index=1)  Line 234	C
 	xpcom_core.dll!NS_LogAddRef_P(void * aPtr=0x002d5137, unsigned long aRefcnt=1, const char * aClazz=0x00bc8dc4, unsigned int classSize=8)  Line 915 + 0x12 bytes	C++
 	nspr4.dll!PR_GetThreadPrivate(unsigned int index=1241704)  Line 234	C
 	gkwidget.dll!nsBaseAppShell::Release()  Line 50 + 0x43 bytes	C++
 	xpcom_core.dll!nsAutoLockBase::~nsAutoLockBase()  Line 333 + 0xf bytes	C++
 	xpcom_core.dll!nsComponentManagerImpl::GetService(const nsID & aClass={...}, const nsID & aIID={...}, void * * result=0x00000001)  Line 1835 + 0x25 bytes	C++
 	nspr4.dll!_MD_CURRENT_THREAD()  Line 310	C
 	user32.dll!7e42b401() 	
 	gkwidget.dll!DetectWindowMove(int code=2118326563, unsigned int wParam=0, long lParam=0)  Line 163	C++
 	gkwidget.dll!DetectWindowMove(int code=1241872, unsigned int wParam=1969043519, long lParam=55775)  Line 163 + 0x5 bytes	C++
 	MSCTFIME.IME!755c44f5() 	
 	user32.dll!7e46c8cf() 	
 	user32.dll!7e428eec() 	
 	ntdll.dll!7c90e473() 	
 	user32.dll!7e42af62() 	
 	gkwidget.dll!nsWindow::Show(int bState=0)  Line 1708	C++
 	gklayout.dll!nsPluginInstanceOwner::PrepareToStop(int aDelayedStop=1)  Line 4288	C++
 	gklayout.dll!nsObjectFrame::StopPluginInternal(int aDelayedStop=1)  Line 2058	C++
 	gklayout.dll!nsObjectFrame::Destroy()  Line 617	C++
 	gklayout.dll!nsFrameList::DestroyFrames()  Line 67 + 0x8 bytes	C++
 	gklayout.dll!nsAbsoluteContainingBlock::DestroyFrames(nsIFrame * aDelegatingFrame=0x04f365a0)  Line 342	C++
 	gklayout.dll!ViewportFrame::Destroy()  Line 68	C++
 	gklayout.dll!nsFrameManager::Destroy()  Line 291	C++
 	gklayout.dll!PresShell::Destroy()  Line 1880	C++
 	gklayout.dll!DocumentViewerImpl::DestroyPresShell()  Line 4255	C++
 	gklayout.dll!DocumentViewerImpl::Destroy()  Line 1533	C++
 	docshell.dll!nsDocShell::Destroy()  Line 4254	C++
 	gklayout.dll!nsFrameLoader::Finalize()  Line 293	C++
 	gklayout.dll!nsDocument::MaybeInitializeFinalizeFrameLoaders()  Line 5167 + 0x17 bytes	C++
 	gklayout.dll!nsRunnableMethod<nsDocument,void>::Run()  Line 265	C++
 	gklayout.dll!nsContentUtils::AddScriptRunner(nsIRunnable * aRunnable=0x03aea9e8)  Line 4373	C++
 	gklayout.dll!nsDocument::FinalizeFrameLoader(nsFrameLoader * aLoader=0x03a66830)  Line 5122 + 0x6 bytes	C++
 	gklayout.dll!nsFrameLoader::Destroy()  Line 763 + 0x1e bytes	C++
 	gklayout.dll!nsGenericHTMLFrameElement::DestroyContent()  Line 2941	C++
 	gklayout.dll!nsGenericElement::DestroyContent()  Line 3518 + 0x12 bytes	C++
 	gklayout.dll!nsGenericElement::DestroyContent()  Line 3518 + 0x12 bytes	C++
 	gklayout.dll!nsDocument::Destroy()  Line 6886 + 0x12 bytes	C++
 	gklayout.dll!DocumentViewerImpl::Destroy()  Line 1498	C++
 	gklayout.dll!DocumentViewerImpl::Show()  Line 1839	C++
 	gklayout.dll!nsPresContext::EnsureVisible(int aUnsuppressFocus=0)  Line 1577	C++
 	gklayout.dll!PresShell::UnsuppressAndInvalidate()  Line 4507 + 0xb bytes	C++
 	gklayout.dll!PresShell::UnsuppressPainting()  Line 4555 + 0x7 bytes	C++
 	gklayout.dll!DocumentViewerImpl::LoadComplete(unsigned int aStatus=0)  Line 1037	C++
 	docshell.dll!nsDocShell::EndPageLoad(nsIWebProgress * aProgress=0x03641464, nsIChannel * aChannel=0x04d148e8, unsigned int aStatus=0)  Line 5717	C++
 	docshell.dll!nsDocShell::OnStateChange(nsIWebProgress * aProgress=0x03641464, nsIRequest * aRequest=0x04d148e8, unsigned int aStateFlags=80824552, unsigned int aStatus=0)  Line 5588	C++
 	docshell.dll!nsDocLoader::FireOnStateChange(nsIWebProgress * aProgress=0x03641464, nsIRequest * aRequest=0x04d148e8, int aStateFlags=131088, unsigned int aStatus=0)  Line 1259 + 0x1a bytes	C++
 	docshell.dll!nsDocLoader::doStopDocumentLoad(nsIRequest * request=0x04d148e8, unsigned int aStatus=0)  Line 891	C++
 	docshell.dll!nsDocLoader::DocLoaderIsEmpty(int aFlushLayout=31883016)  Line 787	C++
 	docshell.dll!nsDocLoader::ChildDoneWithOnload(nsIDocumentLoader * aChild=0x03b798b8)  Line 205 + 0x9 bytes	C++
 	docshell.dll!nsDocLoader::DocLoaderIsEmpty(int aFlushLayout=56890448)  Line 791	C++
 	docshell.dll!nsDocLoader::OnStopRequest(nsIRequest * aRequest=0x04fbcbf0, nsISupports * aCtxt=0x00000000, unsigned int aStatus=0)  Line 684	C++
 	necko.dll!nsLoadGroup::RemoveRequest(nsIRequest * request=0x04fbcbf0, nsISupports * ctxt=0x00000000, unsigned int aStatus=0)  Line 683	C++
 	gklayout.dll!nsDocument::DoUnblockOnload()  Line 7028	C++
 	gklayout.dll!nsDocument::UnblockOnload(int aFireSync=1)  Line 6974	C++
 	gklayout.dll!nsDocument::DispatchContentLoadedEvents()  Line 3918	C++
 	gklayout.dll!nsRunnableMethod<nsDocument,void>::Run()  Line 265	C++
 	xpcom_core.dll!nsThread::ProcessNextEvent(int mayWait=1, int * result=0x0012fc54)  Line 511	C++
 	xpcom_core.dll!NS_ProcessNextEvent_P(nsIThread * thread=0x00000001, int mayWait=1)  Line 230 + 0xd bytes	C++
 	gkwidget.dll!nsBaseAppShell::Run()  Line 170 + 0x9 bytes	C++
 	tkitcmps.dll!nsAppStartup::Run()  Line 194	C++
 	xul.dll!XRE_main(int argc=1, char * * argv=0x00a7af98, const nsXREAppData * aAppData=0x00a79b20)  Line 3361	C++
 	firefox.exe!NS_internal_main(int argc=1, char * * argv=0x00a7af98)  Line 157	C++
 	firefox.exe!wmain(int argc=10989464, unsigned short * * argv=0x00a794c0)  Line 112	C++
 	firefox.exe!__tmainCRTStartup()  Line 583 + 0x19 bytes	C
 	firefox.exe!wmainCRTStartup()  Line 403	C
 	kernel32.dll!7c817077()
Assignee: nobody → tnikkel
Attachment #376673 - Flags: review?(roc)
Comment on attachment 376673 [details] [diff] [review]
patch

+
+    rv =
+      aFrame->BuildDisplayListForStackingContext(&builder, dirtyRect, &list);

put that all on one line

+  if (builder.EnterPresShell(aRootFrame, rect)) {
 
   nsresult rv =

Lose the blank line

Otherwise great!
I've kinda lost track of which of your bugs should land on 1.9.1 ... can you make sure that anything that needs to land on 1.9.1 is nominated for blocking-1.9.1? Thanks!!!

Also, if you keep this up we should get you commit access soonish. That's a bit easier if you make a contribution to another module with a different reviewer.
Keywords: checkin-needed
Whiteboard: [needs landing]
Flags: blocking1.9.1?
Attached patch patch v2 (obsolete) — Splinter Review
Attachment #376673 - Attachment is obsolete: true
Attachment #376883 - Flags: review+
(In reply to comment #6)
> I've kinda lost track of which of your bugs should land on 1.9.1 ... can you
> make sure that anything that needs to land on 1.9.1 is nominated for
> blocking-1.9.1? Thanks!!!

Done.

> Also, if you keep this up we should get you commit access soonish. That's a bit
> easier if you make a contribution to another module with a different reviewer.

I will keep my eyes open for something to work on outside of layout. Thanks!
I was unable to get an automated test that actually crashed. This is what I got  in case anyone comes along later and wants to build on it. After the patch for this bug gets landed you will need to make nsDisplayListBuilder::EnterPresShell always return true so that the crash can actually be triggered.
Flags: in-testsuite?
I just realized that this patch isn't enough :-(. Basically, handling the mouse event during frame destruction can almost certainly be turned into a crash if the event handler reaches back into the document whose frames are being destroyed. But I'm not sure how to avoid hiding or moving the plugin widget during frame destruction here.

I guess scripts are blocked due to frame destruction so we don't have to worry about arbitrary badness. Still, CSS :hover rules could restyle things in arbitrary ways, say by forcing a frame reconstruct of the parent document ... bad news.

I wonder if on Windows we could detect synchronous mouse events using InSendMessage and redispatch them as asynchronous events instead?
Are we missing a script blocker somewhere? Because currently
PresShell::HandleEvent is very strict and doesn't allow any event handling
if there are script blockers.
At the start of PresShell::HandleEvent we check if the presshell is destroying (along with a script check and a few more), but then later in the function (nsPresShell.cpp:5952) the event is retargeted at another shell without checking if that new shell is being destroyed. I was investigating adding a check for that there before, but I let it go for some reason. So we should probably mirror the checks at the start of PresShell::HandleEvent when retargeting to a different shell at least.
I think there should be a scriptblocker somewhere in the stack when destroying a 
presshell. Apparently there isn't.
Good, OK, so we're just missing a script blocker. There should always be one during frame destruction, so I guess we need one somewhere in here:
     gklayout.dll!ViewportFrame::Destroy()  Line 68    C++
     gklayout.dll!nsFrameManager::Destroy()  Line 291    C++
     gklayout.dll!PresShell::Destroy()  Line 1880    C++
     gklayout.dll!DocumentViewerImpl::DestroyPresShell()  Line 4255    C++
     gklayout.dll!DocumentViewerImpl::Destroy()  Line 1533    C++
     docshell.dll!nsDocShell::Destroy()  Line 4254    C++
     gklayout.dll!nsFrameLoader::Finalize()  Line 293    C++

I'd suggest PresShell::Destroy or possibly DocumentViewerImpl::DestroyPresShell.

If we have that script blocker we shouldn't need any additional protection.
Maybe we should add the script blocker to DocumentViewerImpl::DestroyPresShell and have an assertion in PresShell::Destroy that scripts have been blocked by the caller.
(In reply to comment #14)
> Good, OK, so we're just missing a script blocker. There should always be one
> during frame destruction,

Can we also add an assertion to nsFrame::Destroy that we're in a script blocker?
Looks like we will still need the IsDestroying check in EnterPresShell. After adding the script blockers I now get a crash during a paint when entering the subpresshell (the View Manager has a script blocker already in place). This looks like Martijn's original stack trace. So I will create a patch doing both the destroying check and adding the script blockers.

Here is the stack.

 	ntdll.dll!7c90120e() 	
 	[Frames below may be incorrect and/or missing, no symbols loaded for ntdll.dll]	
>	xpcom_core.dll!Break(const char * aMsg=0x00000000)  Line 489	C++
 	xpcom_core.dll!NS_DebugBreak_P(unsigned int aSeverity=, const char * aStr=, const char * aExpr=, const char * aFile=, int aLine=)  Line 356	C++
 	gklayout.dll!nsFrameManager::GetPrimaryFrameFor(nsIContent * aContent=0x050a2a28, int aIndexHint=-1)  Line 335 + 0x16 bytes	C++
 	gklayout.dll!PresShell::GetPrimaryFrameFor(nsIContent * aContent=0x050a2a28)  Line 5095	C++
 	gklayout.dll!nsCSSRendering::FindRootFrame(nsIFrame * aForFrame=0x054bf8dc)  Line 943 + 0x11 bytes	C++
 	gklayout.dll!nsCSSRendering::FindRootFrameBackground(nsIFrame * aForFrame=0x054bf8dc)  Line 985 + 0x9 bytes	C++
 	gklayout.dll!PresShell::UpdateCanvasBackground()  Line 5605	C++
 	gklayout.dll!nsDisplayListBuilder::EnterPresShell(nsIFrame * aReferenceFrame=0x05b1bdc0, const nsRect & aDirtyRect={...})  Line 176	C++
 	gklayout.dll!nsSubDocumentFrame::BuildDisplayList(nsDisplayListBuilder * aBuilder=0x0567f4a8, const nsRect & aDirtyRect={...}, const nsDisplayListSet & aLists={...})  Line 370	C++
 	gklayout.dll!nsIFrame::BuildDisplayListForChild(nsDisplayListBuilder * aBuilder=0x00000000, nsIFrame * aChild=0x054b1338, const nsRect & aDirtyRect={...}, const nsDisplayListSet & aLists={...}, unsigned int aFlags=4)  Line 1621 + 0xf bytes	C++
 	gklayout.dll!DisplayLine(nsDisplayListBuilder * aBuilder=0x0012e304, const nsRect & aLineArea={...}, const nsRect & aDirtyRect={...}, nsLineList_iterator & aLine={...}, int aDepth=0)  Line 6127 + 0x22 bytes	C++
 	gklayout.dll!nsBlockFrame::BuildDisplayList(nsDisplayListBuilder * aBuilder=0x0012e304, const nsRect & aDirtyRect={...}, const nsDisplayListSet & aLists={...})  Line 6206 + 0x1a bytes	C++
 	gklayout.dll!nsIFrame::BuildDisplayListForChild(nsDisplayListBuilder * aBuilder=0x00000000, nsIFrame * aChild=0x054b10dc, const nsRect & aDirtyRect={...}, const nsDisplayListSet & aLists={...}, unsigned int aFlags=0)  Line 1579 + 0xe bytes	C++
 	gklayout.dll!DisplayLine(nsDisplayListBuilder * aBuilder=0x0012e304, const nsRect & aLineArea={...}, const nsRect & aDirtyRect={...}, nsLineList_iterator & aLine={...}, int aDepth=0)  Line 6127 + 0x22 bytes	C++
 	gklayout.dll!nsBlockFrame::BuildDisplayList(nsDisplayListBuilder * aBuilder=0x0012e304, const nsRect & aDirtyRect={...}, const nsDisplayListSet & aLists={...})  Line 6206 + 0x1a bytes	C++
 	gklayout.dll!nsIFrame::BuildDisplayListForChild(nsDisplayListBuilder * aBuilder=0x00000000, nsIFrame * aChild=0x054b0eb4, const nsRect & aDirtyRect={...}, const nsDisplayListSet & aLists={...}, unsigned int aFlags=1)  Line 1621 + 0xf bytes	C++
 	gklayout.dll!CanvasFrame::BuildDisplayList(nsDisplayListBuilder * aBuilder=0x0012e304, const nsRect & aDirtyRect={...}, const nsDisplayListSet & aLists={...})  Line 503	C++
 	gklayout.dll!nsIFrame::BuildDisplayListForChild(nsDisplayListBuilder * aBuilder=0x00000000, nsIFrame * aChild=0x054909ac, const nsRect & aDirtyRect={...}, const nsDisplayListSet & aLists={...}, unsigned int aFlags=0)  Line 1579 + 0xe bytes	C++
 	gklayout.dll!nsGfxScrollFrameInner::BuildDisplayList(nsDisplayListBuilder * aBuilder=0x0012e304, const nsRect & aDirtyRect={...}, const nsDisplayListSet & aLists={...})  Line 1350	C++
 	gklayout.dll!nsHTMLScrollFrame::BuildDisplayList(nsDisplayListBuilder * aBuilder=0x0012e304, const nsRect & aDirtyRect={...}, const nsDisplayListSet & aLists={...})  Line 261	C++
 	gklayout.dll!nsIFrame::BuildDisplayListForChild(nsDisplayListBuilder * aBuilder=0x00000000, nsIFrame * aChild=0x05490b2c, const nsRect & aDirtyRect={...}, const nsDisplayListSet & aLists={...}, unsigned int aFlags=0)  Line 1579 + 0xe bytes	C++
 	gklayout.dll!ViewportFrame::BuildDisplayList(nsDisplayListBuilder * aBuilder=0x0012e304, const nsRect & aDirtyRect={...}, const nsDisplayListSet & aLists={...})  Line 109 + 0x13 bytes	C++
 	gklayout.dll!nsIFrame::BuildDisplayListForStackingContext(nsDisplayListBuilder * aBuilder=0x0012e304, const nsRect & aDirtyRect={...}, nsDisplayList * aList=0x0012e2fc)  Line 1290	C++
 	gklayout.dll!nsLayoutUtils::PaintFrame(nsIRenderingContext * aRenderingContext=0x03b21138, nsIFrame * aFrame=0x054908c8, const nsRegion & aDirtyRegion={...}, unsigned int aBackground=4294967295)  Line 1051 + 0x13 bytes	C++
 	gklayout.dll!PresShell::Paint(nsIView * aView=0x037bb428, nsIRenderingContext * aRenderingContext=0x03b21138, const nsRegion & aDirtyRegion={...})  Line 5641 + 0xf bytes	C++
 	gklayout.dll!nsViewManager::RenderViews(nsView * aView=0x03acb508, nsIRenderingContext & aRC={...}, const nsRegion & aRegion={...})  Line 610	C++
 	gklayout.dll!nsViewManager::Refresh(nsView * aView=0x0542c208, nsIRenderingContext * aContext=0x0000003c, nsIRegion * aRegion=0x03b21138, unsigned int aUpdateFlags=1)  Line 513	C++
 	gklayout.dll!nsViewManager::DispatchEvent(nsGUIEvent * aEvent=0x03b16ed8, nsEventStatus * aStatus=0x0012e718)  Line 1107	C++
 	gklayout.dll!HandleEvent(nsGUIEvent * aEvent=0x0012e820)  Line 170	C++
 	gkwidget.dll!nsWindow::DispatchEvent(nsGUIEvent * event=0x0012e820, nsEventStatus & aStatus=nsEventStatus_eIgnore)  Line 967 + 0x3 bytes	C++
 	gkwidget.dll!nsWindow::DispatchWindowEvent(nsGUIEvent * event=0x0012e820, nsEventStatus & aStatus=nsEventStatus_eIgnore)  Line 993	C++
 	gkwidget.dll!nsWindow::OnPaint(HDC__ * aDC=0x00000000)  Line 6151 + 0x18 bytes	C++
 	gkwidget.dll!nsWindow::ProcessMessage(unsigned int msg=15, unsigned int & wParam=0, long & lParam=0, long * aRetValue=0x0012eb38)  Line 4299 + 0x9 bytes	C++
 	gkwidget.dll!nsWindow::WindowProc(HWND__ * hWnd=0x000203ee, unsigned int msg=0, unsigned int wParam=0, long lParam=0)  Line 1183 + 0x14 bytes	C++
 	user32.dll!7e418734() 	
 	user32.dll!7e418816() 	
 	gkwidget.dll!DetectWindowMove(int code=0, unsigned int wParam=22142644, long lParam=132078)  Line 163 + 0x5 bytes	C++
 	user32.dll!7e428ea0() 	
 	user32.dll!7e428eec() 	
 	ntdll.dll!7c90e473() 	
 	user32.dll!7e42aef1() 	
 	user32.dll!7e42aedc() 	
 	gkwidget.dll!nsWindow::DispatchStarvedPaints(HWND__ * aWnd=0x00020326, long aMsg=0)  Line 4045 + 0x7 bytes	C++
 	user32.dll!7e42a4e8() 	
 	user32.dll!7e42b109() 	
 	gkwidget.dll!nsWindow::DispatchPendingEvents()  Line 4080 + 0xe bytes	C++
 	gkwidget.dll!nsWindow::ProcessMessageForPlugin(const tagMSG & aMsg={...}, long * aResult=0x00000000, int & aCallDefWndProc=1)  Line 5435	C++
 	gkwidget.dll!nsWindow::ProcessMessage(unsigned int msg=641, unsigned int & wParam=1, long & lParam=-1073741809, long * aRetValue=0x0012eee8)  Line 4107 + 0x18 bytes	C++
 	gkwidget.dll!nsWindow::WindowProc(HWND__ * hWnd=0x000203f0, unsigned int msg=0, unsigned int wParam=1, long lParam=-1073741809)  Line 1183 + 0x14 bytes	C++
 	user32.dll!7e418734() 	
 	user32.dll!7e418816() 	
 	gkwidget.dll!DetectWindowMove(int code=0, unsigned int wParam=22142644, long lParam=132080)  Line 163 + 0x5 bytes	C++
 	user32.dll!7e428ea0() 	
 	user32.dll!7e428eec() 	
 	ntdll.dll!7c90e473() 	
 	user32.dll!7e4194be() 	
 	user32.dll!7e42c174() 	
 	user32.dll!7e4292e3() 	
 	imm32.dll!76392b12() 	
 	user32.dll!7e46b90e() 	
 	user32.dll!7e46bdf2() 	
 	xpcom_core.dll!nsObserverService::Release()  Line 76 + 0x41 bytes	C++
 	gkplugin.dll!nsCOMPtr<nsIObserverService>::~nsCOMPtr<nsIObserverService>()  Line 510 + 0x6 bytes	C++
 	gkplugin.dll!NS_NotifyPluginCall(unsigned int startTime=)  Line 192 + 0x19 bytes	C++
 	nspr4.dll!_MD_CURRENT_THREAD()  Line 310	C
 	nspr4.dll!PR_GetThreadPrivate(unsigned int index=1)  Line 234	C
 	xpcom_core.dll!NS_LogAddRef_P(void * aPtr=0x002d5137, unsigned long aRefcnt=1, const char * aClazz=0x00b8e5f4, unsigned int classSize=8)  Line 915 + 0x12 bytes	C++
 	nspr4.dll!PR_GetThreadPrivate(unsigned int index=1241704)  Line 234	C
 	gkwidget.dll!nsBaseAppShell::Release()  Line 50 + 0x43 bytes	C++
 	xpcom_core.dll!nsAutoLockBase::~nsAutoLockBase()  Line 333 + 0xf bytes	C++
 	xpcom_core.dll!nsComponentManagerImpl::GetService(const nsID & aClass={...}, const nsID & aIID={...}, void * * result=0x00000001)  Line 1835 + 0x25 bytes	C++
 	nspr4.dll!_MD_CURRENT_THREAD()  Line 310	C
 	user32.dll!7e42b401() 	
 	gkwidget.dll!DetectWindowMove(int code=2118326563, unsigned int wParam=0, long lParam=0)  Line 163	C++
 	gkwidget.dll!DetectWindowMove(int code=1241872, unsigned int wParam=1969043519, long lParam=21590)  Line 163 + 0x5 bytes	C++
 	MSCTFIME.IME!755c44f5() 	
 	user32.dll!7e46c8cf() 	
 	user32.dll!7e428eec() 	
 	ntdll.dll!7c90e473() 	
 	user32.dll!7e42af62() 	
 	gkwidget.dll!nsWindow::Show(int bState=0)  Line 1708	C++
 	gklayout.dll!nsPluginInstanceOwner::PrepareToStop(int aDelayedStop=1)  Line 4288	C++
 	gklayout.dll!nsObjectFrame::StopPluginInternal(int aDelayedStop=1)  Line 2058	C++
 	gklayout.dll!nsObjectFrame::Destroy()  Line 617	C++
 	gklayout.dll!nsFrameList::DestroyFrames()  Line 67 + 0x8 bytes	C++
 	gklayout.dll!nsAbsoluteContainingBlock::DestroyFrames(nsIFrame * aDelegatingFrame=0x05b1bdc0)  Line 342	C++
 	gklayout.dll!ViewportFrame::Destroy()  Line 68	C++
 	gklayout.dll!nsFrameManager::Destroy()  Line 291	C++
 	gklayout.dll!PresShell::Destroy()  Line 1883	C++
 	gklayout.dll!DocumentViewerImpl::DestroyPresShell()  Line 4256	C++
 	gklayout.dll!DocumentViewerImpl::Destroy()  Line 1533	C++
 	docshell.dll!nsDocShell::Destroy()  Line 4254	C++
 	gklayout.dll!nsFrameLoader::Finalize()  Line 293	C++
 	gklayout.dll!nsDocument::MaybeInitializeFinalizeFrameLoaders()  Line 5167 + 0x17 bytes	C++
 	gklayout.dll!nsRunnableMethod<nsDocument,void>::Run()  Line 265	C++
 	gklayout.dll!nsContentUtils::AddScriptRunner(nsIRunnable * aRunnable=0x05073878)  Line 4373	C++
 	gklayout.dll!nsDocument::FinalizeFrameLoader(nsFrameLoader * aLoader=0x054184d8)  Line 5122 + 0x6 bytes	C++
 	gklayout.dll!nsFrameLoader::Destroy()  Line 763 + 0x1e bytes	C++
 	gklayout.dll!nsGenericHTMLFrameElement::DestroyContent()  Line 2941	C++
 	gklayout.dll!nsGenericElement::DestroyContent()  Line 3518 + 0x12 bytes	C++
 	gklayout.dll!nsGenericElement::DestroyContent()  Line 3518 + 0x12 bytes	C++
 	gklayout.dll!nsDocument::Destroy()  Line 6886 + 0x12 bytes	C++
 	gklayout.dll!DocumentViewerImpl::Destroy()  Line 1498	C++
 	gklayout.dll!DocumentViewerImpl::Show()  Line 1839	C++
 	gklayout.dll!nsPresContext::EnsureVisible(int aUnsuppressFocus=0)  Line 1577	C++
 	gklayout.dll!PresShell::UnsuppressAndInvalidate()  Line 4510 + 0xb bytes	C++
 	gklayout.dll!PresShell::UnsuppressPainting()  Line 4558 + 0x7 bytes	C++
 	gklayout.dll!DocumentViewerImpl::LoadComplete(unsigned int aStatus=0)  Line 1037	C++
 	docshell.dll!nsDocShell::EndPageLoad(nsIWebProgress * aProgress=0x0356b52c, nsIChannel * aChannel=0x03996ed8, unsigned int aStatus=0)  Line 5717	C++
 	docshell.dll!nsDocShell::OnStateChange(nsIWebProgress * aProgress=0x0356b52c, nsIRequest * aRequest=0x03996ed8, unsigned int aStateFlags=60387032, unsigned int aStatus=0)  Line 5588	C++
 	docshell.dll!nsDocLoader::FireOnStateChange(nsIWebProgress * aProgress=0x0356b52c, nsIRequest * aRequest=0x03996ed8, int aStateFlags=131088, unsigned int aStatus=0)  Line 1259 + 0x1a bytes	C++
 	docshell.dll!nsDocLoader::doStopDocumentLoad(nsIRequest * request=0x03996ed8, unsigned int aStatus=0)  Line 891	C++
 	docshell.dll!nsDocLoader::DocLoaderIsEmpty(int aFlushLayout=31709712)  Line 787	C++
 	docshell.dll!nsDocLoader::ChildDoneWithOnload(nsIDocumentLoader * aChild=0x03655888)  Line 205 + 0x9 bytes	C++
 	docshell.dll!nsDocLoader::DocLoaderIsEmpty(int aFlushLayout=56014104)  Line 791	C++
 	docshell.dll!nsDocLoader::OnStopRequest(nsIRequest * aRequest=0x055bbd58, nsISupports * aCtxt=0x00000000, unsigned int aStatus=0)  Line 684	C++
 	necko.dll!nsLoadGroup::RemoveRequest(nsIRequest * request=0x055bbd58, nsISupports * ctxt=0x00000000, unsigned int aStatus=0)  Line 683	C++
 	gklayout.dll!nsDocument::DoUnblockOnload()  Line 7028	C++
 	gklayout.dll!nsDocument::UnblockOnload(int aFireSync=1)  Line 6974	C++
 	gklayout.dll!nsDocument::DispatchContentLoadedEvents()  Line 3918	C++
 	gklayout.dll!nsRunnableMethod<nsDocument,void>::Run()  Line 265	C++
 	xpcom_core.dll!nsThread::ProcessNextEvent(int mayWait=1, int * result=0x0012fc54)  Line 511	C++
 	xpcom_core.dll!NS_ProcessNextEvent_P(nsIThread * thread=0x00000001, int mayWait=1)  Line 230 + 0xd bytes	C++
 	gkwidget.dll!nsBaseAppShell::Run()  Line 170 + 0x9 bytes	C++
 	tkitcmps.dll!nsAppStartup::Run()  Line 194	C++
 	xul.dll!XRE_main(int argc=1, char * * argv=0x00a79d68, const nsXREAppData * aAppData=0x00a78aa0)  Line 3361	C++
 	firefox.exe!NS_internal_main(int argc=1, char * * argv=0x00a79d68)  Line 157	C++
 	firefox.exe!wmain(int argc=10984808, unsigned short * * argv=0x00a786b0)  Line 112	C++
 	firefox.exe!__tmainCRTStartup()  Line 583 + 0x19 bytes	C
 	firefox.exe!wmainCRTStartup()  Line 403	C
 	kernel32.dll!7c817077()
We should really avoid painting anything, or doing much of anything, while we're in frame destruction.

The basic problem is that Windows sends us synchronous events while we're in frame destruction. So far we've observed mouse-move being dispatched, but that (and other input messages) fizzles out in PresShell::HandleEvent with the script-blocker in place, OK. In the above stack we have a WM_IME_SETCONTEXT message being dispatched to the plugin (according to http://wiki.winehq.org/List_Of_Windows_Messages). That would be harmless except that ProcessMessageForPlugin decides to also synchronously paint a bunch of other windows at the same time via DispatchPendingEvents. We really should avoid doing that; ignoring the paints, in PresShell::Paint say, would be bad because we'll leave window contents as unpainted garbage.

The simplest thing we could do is just not call DispatchPendingEvents() when the plugin message was WM_IME_SETCONTEXT. If we observe other events being sent synchronously to the plugin we can do the same thing. It would actually be useful to fire up Spy++ --- or just some printfs in nsWindow::ProcessMessage --- to get a complete list of the Windows messages sent synchronously while we hide the plugin widget.
I used Winspector (Spy++ doesn't seem to come with the express edition of vc++) to observe the following messages being sent immediately after I clicked reload (after focusing the plugin). All messages have a matching return immediately after, except for three: two return where indicated (WM_NCDESTROY doesn't return at all).

WM_PAINT
WM_NCPAINT
WM_ERASEBKGND
WM_PAINT return
WM_IME_SETCONTEXT
WM_SETFOCUS
WM_SHOWWINDOW
WM_WINDOWPOSCHANGING
WM_WINDOWPOSCHANGED
WM_KILLFOCUS
WM_IME_SETCONTEXT
WM_CANCELMODE
WM_ENABLE
WM_WINDOWPOSCHANGING
WM_CHILDACTIVATE
WM_WINDOWPOSCHANGED
WM_MOVE
WM_WINDOWPOSCHANGED return
WM_SETICON
WM_SETICON
WM_DESTROY
WM_NCDESTROY
no return for WM_NCDESTROY

Of these only WM_IME_SETCONTEXT will cause nsWindow::ProcessMessageForPlugin to call DispatchPendingEvents(), the rest will return false and nsWindow::ProcessMessage will handle them. Making nsWindow::ProcessMessageForPlugin not call DispatchPendingEvents when the message is WM_IME_SETCONTEXT fixes the crash.

I get some assertations with reloading youtube videos, but I think (hope) they are unrelated. Will try a clean build later and hopefully post a patch.
Attached patch patch v3Splinter Review
The assertions were unrelated.

The checking of message type and determining when to dispatch synchronously seems kind of fragile to me. Is it not possible to remove the focus from the plugin at the very beginning of frame destruction (it gets removed in the middle of frame destruction anyway)?
Attachment #376883 - Attachment is obsolete: true
Attachment #377551 - Flags: review?(roc)
The problem is that removing focus itself dispatches synchronous messags and "the beginning of frame destruction" might not be safe either.

This should get easier to handle when my patches land in 1.9.2 that move plugins to be children of the toplevel window and also create a per-presshell list of windowed plugins.
Comment on attachment 377551 [details] [diff] [review]
patch v3

Thanks!!!

Would be nice to have an automated testcase here, but we won't block on that.
Attachment #377551 - Flags: review?(roc) → review+
http://hg.mozilla.org/mozilla-central/rev/691422bc0232
Status: NEW → RESOLVED
Closed: 12 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
Whiteboard: [needs landing] → [needs 191 approval]
Comment on attachment 377551 [details] [diff] [review]
patch v3

Closes a pretty nasty hole with crashy implications of undefined scope. Better fix it on 1.9.1. the patch is pretty safe.
Flags: blocking1.9.2?
Flags: blocking1.9.1?
Flags: blocking1.9.1+
Whiteboard: [needs 191 approval] → [needs 191 landing]
Attachment #377551 - Flags: approval1.9.1? → approval1.9.1+
Comment on attachment 377551 [details] [diff] [review]
patch v3

(blockers don't need approvals)
Depends on: 495728
The assert added in this bug is firing.  See bug 495728.
Depends on: 493842
Crash Signature: [@ nsDisplayListBuilder::EnterPresShell]
You need to log in before you can comment on or make changes to this bug.