682041 - When in a select menu, if the scroll bar is held down and you right click outside of the menu, Firefox crashes.

Status: VERIFIED FIXED

(Core :: Layout: Form Controls, defect)

Description

[schism0624]

User Agent: Mozilla/5.0 (Windows NT 5.1; rv:6.0) Gecko/20100101 Firefox/6.0
Build ID: 20110811165603

Steps to reproduce:

I was scrolling through a select drop-down list and accidentally right clicked just to the right of the drop-down menu.

This is one example of what I did to reproduce:
Enter http://www.serebii.net/pokedex-bw/ in the url bar and proceed to the website. There are a number of drop-downs in the middle of the page. Choose any, I chose the one that says "Kanto: 001-151". Click it to bring down the drop-down. Press your left mouse button on the scroll bar and begin to scroll down just a little ways. Now move mouse off to the right of the select in the open area, but keep the mouse clicked. Now right-click in that open area. At this point my Firefox crashed after about a second of hanging.


Actual results:

Firefox crashed and brought up the dialog to send an error report, I did so. I tested in both Windows and Linux, on multiple websites, and had a few friends try to confirm. It happened on all attempts.


Expected results:

It shouldn't crash.

Comment 1

[Thomas Ahlblom]

Please post one or more relevant Report ID:s from about:crashes into this bug so we know which crash reports this is about!

Are you able to reproduce the crash if you start Firefox in Safe Mode?
https://support.mozilla.com/en-US/kb/Safe+Mode

Comment 2

[:aceman]

Can confirm on FF6, Win XP, 32bit.
Crashlog:
https://crash-stats.mozilla.com/report/index/a069a60f-062f-4749-9911-98a022110826

Comment 3

[:aceman]

crash-stats.mozilla.com linked this crash log to bug 641893. Does it mean it is the same stack trace? Are these bugs the same?

Comment 4

[:aceman]

Can not see this on FF7, Linux.

Comment 5

[Alice0775 White]

Confirmed on
http://hg.mozilla.org/mozilla-central/rev/e87454393401
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:9.0a1) Gecko/20110826 Firefox/9.0a1 ID:20110826030805
bp-0514d24b-a38e-47c9-98ca-a06da2110826


Regression winodw(cached m-c hourly)
Works:
http://hg.mozilla.org/mozilla-central/rev/bc7c390b2b43
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:2.2a1pre) Gecko/20110324 Firefox/4.2a1pre ID:20110324130448
Crashes:
http://hg.mozilla.org/mozilla-central/rev/0798b52bb40d
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:2.2a1pre) Gecko/20110324 Firefox/4.2a1pre ID:20110324144234
Pushlog:
http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=bc7c390b2b43&tochange=0798b52bb40d
Suspected Bug:
165279f6a853	Ehsan Akhgari — Bug 610391 - Create the widget for drop-down comboboxes lazily, and tear it down when the drop-down is closed; r=roc

Comment 6

[Marcia Knous [:marcia]]

Bug 641893 was logged a while back, but it is likely the same crash based on the comments - many of them mention right clicking. Last week there were over 1800 crashes in this signature. Adding crash signature and reproducible keyword.

Comment 7

[(no longer active)]

This is the stack for the crash here:

 	xul.dll!nsWindow::OnDestroy()  Line 7347	C++
 	xul.dll!nsWindow::ProcessMessage(unsigned int msg=0x00000002, unsigned int & wParam=0x00000000, long & lParam=0x00000000, long * aRetValue=0x0044cbc4)  Line 4866	C++
 	xul.dll!nsWindow::WindowProcInternal(HWND__ * hWnd=0x000604e6, unsigned int msg=0x00000002, unsigned int wParam=0x00000000, long lParam=0x00000000)  Line 4436 + 0x20 bytes	C++
 	xul.dll!CallWindowProcCrashProtected(long (HWND__ *, unsigned int, unsigned int, long)* wndProc=0x68db3d60, HWND__ * hWnd=0x000604e6, unsigned int msg=0x00000002, unsigned int wParam=0x00000000, long lParam=0x00000000)  Line 65 + 0x13 bytes	C++
 	xul.dll!nsWindow::WindowProc(HWND__ * hWnd=0x000604e6, unsigned int msg=0x00000002, unsigned int wParam=0x00000000, long lParam=0x00000000)  Line 4378 + 0x1a bytes	C++
 	user32.dll!_InternalCallWinProc@20()  + 0x23 bytes	
 	user32.dll!_UserCallWinProcCheckWow@32()  + 0xb7 bytes	
 	user32.dll!_DispatchClientMessage@24()  + 0x51 bytes	
 	user32.dll!___fnDWORD@4()  + 0x2b bytes	
 	ntdll.dll!_KiUserCallbackDispatcher@12()  + 0x2e bytes	
 	user32.dll!_NtUserDestroyWindow@4()  + 0x15 bytes	
 	xul.dll!nsWindow::Destroy()  Line 701 + 0x10 bytes	C++
 	xul.dll!nsView::DestroyWidget()  Line 302	C++
 	xul.dll!nsIView::DestroyWidget()  Line 708	C++
 	xul.dll!nsComboboxControlFrame::ShowList(int aShowList=0x00000000)  Line 432	C++
 	xul.dll!nsComboboxControlFrame::RollupFromList()  Line 1304 + 0xd bytes	C++
 	xul.dll!nsListControlFrame::ComboboxFinish(int aIndex=0x00000000)  Line 1558	C++
 	xul.dll!nsListControlFrame::AboutToRollup()  Line 1712	C++
 	xul.dll!nsComboboxControlFrame::Rollup(unsigned int aCount=0xffffffff, nsIContent * * aLastRolledUp=0x00000000)  Line 1291	C++
 	xul.dll!nsWindow::DealWithPopups(HWND__ * inWnd=0x000604e6, unsigned int inMsg=0x00000204, unsigned int inWParam=0x00000003, long inLParam=0x003e007a, long * outResult=0x0044cf38)  Line 8756	C++
>	xul.dll!nsWindow::WindowProcInternal(HWND__ * hWnd=0x000604e6, unsigned int msg=0x00000204, unsigned int wParam=0x00000003, long lParam=0x003e007a)  Line 4417 + 0x19 bytes	C++
 	xul.dll!CallWindowProcCrashProtected(long (HWND__ *, unsigned int, unsigned int, long)* wndProc=0x68db3d60, HWND__ * hWnd=0x000604e6, unsigned int msg=0x00000204, unsigned int wParam=0x00000003, long lParam=0x003e007a)  Line 65 + 0x13 bytes	C++
 	xul.dll!nsWindow::WindowProc(HWND__ * hWnd=0x000604e6, unsigned int msg=0x00000204, unsigned int wParam=0x00000003, long lParam=0x003e007a)  Line 4378 + 0x1a bytes	C++
 	user32.dll!_InternalCallWinProc@20()  + 0x23 bytes	
 	user32.dll!_UserCallWinProcCheckWow@32()  + 0xb7 bytes	
 	user32.dll!_DispatchMessageWorker@8()  + 0xed bytes	
 	user32.dll!_DispatchMessageW@4()  + 0xf bytes	
 	xul.dll!nsAppShell::ProcessNextNativeEvent(int mayWait=0x00000001)  Line 347	C++
 	xul.dll!nsBaseAppShell::DoProcessNextNativeEvent(int mayWait=0x00000001)  Line 171 + 0x11 bytes	C++
 	xul.dll!nsBaseAppShell::OnProcessNextEvent(nsIThreadInternal * thr=0x00a263f0, int mayWait=0x00000001, unsigned int recursionDepth=0x00000000)  Line 324 + 0xf bytes	C++
 	xul.dll!nsThread::ProcessNextEvent(int mayWait=0x00000001, int * result=0x0044d1d4)  Line 598	C++
 	xul.dll!NS_ProcessNextEvent_P(nsIThread * thread=0x00a263f0, int mayWait=0x00000001)  Line 245 + 0x16 bytes	C++
 	xul.dll!mozilla::ipc::MessagePump::Run(base::MessagePump::Delegate * aDelegate=0x00a216b0)  Line 134 + 0xe bytes	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 191	C++
 	xul.dll!nsAppShell::Run()  Line 261 + 0x9 bytes	C++
 	xul.dll!nsAppStartup::Run()  Line 224 + 0x1c bytes	C++
 	xul.dll!XRE_main(int argc=0x00000001, char * * argv=0x00a17658, const nsXREAppData * aAppData=0x00a14770)  Line 3557 + 0x25 bytes	C++
 	firefox.exe!do_main(const char * exePath=0x0044fb44, int argc=0x00000001, char * * argv=0x00a17658)  Line 198 + 0x12 bytes	C++
 	firefox.exe!NS_internal_main(int argc=0x00000001, char * * argv=0x00a17658)  Line 281 + 0x14 bytes	C++
 	firefox.exe!wmain(int argc=0x00000001, wchar_t * * argv=0x00a11b90)  Line 107 + 0xd bytes	C++
 	firefox.exe!__tmainCRTStartup()  Line 583 + 0x19 bytes	C
 	firefox.exe!wmainCRTStartup()  Line 403	C
 	kernel32.dll!@BaseThreadInitThunk@12()  + 0x12 bytes	
 	ntdll.dll!___RtlUserThreadStart@8()  + 0x27 bytes	
 	ntdll.dll!__RtlUserThreadStart@8()  + 0x1b bytes	

What is happening is that during the processing of the WM_RBUTTONDOWN message which is dispatched to the select drop-down window, we end up destroying the widget and deleting the nsWindow object, and then when we get back to the windowproc function, we crash because someWindow is the deleted window object.

Comment 8

[(no longer active)]

Attachment: Patch (v1)

Comment 9

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

How do we know that the view is still alive when the runnable runs?

Comment 10

[(no longer active)]

(In reply to Robert O'Callahan (:roc) (Mozilla Corporation) from comment #9)
> How do we know that the view is still alive when the runnable runs?

Is there anything that might destroy it?  I thought that view destruction is explicit.  But if not, that's going to make this bug tricky, since as far as I can tell, views are not refcounted.  Is there any way to ensure their liveliness?

Comment 11

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

Something could destroy the DOM element which would destroy the view.

You could fire a runnable holding a strong reference to the select element. Then when the runnable runs, it can check to see if the element has a widget that should be destroyed, and if so, destroy it.

Comment 12

[Mozilla RelEng Bot]

Try run for ac29738b793e is complete.
Detailed breakdown of the results available here:
    http://tbpl.allizom.org/?tree=Try&usebuildbot=1&rev=ac29738b793e
Results (out of 123 total builds):
    exception: 14
    success: 38
    warnings: 35
    failure: 36
Builds available at http://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/[e-mail]-ac29738b793e

Comment 13

[(no longer active)]

Attachment: Patch (v2)

Comment 14

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

Comment on attachment 559280 [details] [diff] [review]
Patch (v2)

Review of attachment 559280 [details] [diff] [review]:
-----------------------------------------------------------------

You need to check whether the widget should actually be destroyed, somewhow. Otherwise, what if someone dismisses the popup and instantly opens it again, couldn't we destroy the widget while the popup is open?

Comment 15

[(no longer active)]

Attachment: Patch (v3)

Great catch!

Comment 16

[(no longer active)]

http://hg.mozilla.org/integration/mozilla-inbound/rev/fe6136c24fa4

Comment 17

[(no longer active)]

http://hg.mozilla.org/mozilla-central/rev/fe6136c24fa4
http://hg.mozilla.org/mozilla-central/rev/01dcbca466f3

Comment 18

[:aceman]

It does not crash for me on Nightly 10, verifying fix.