202843 - Trunk crash [@ nsEventStateManager::GenerateDragGesture]

Status: VERIFIED FIXED

(Core :: DOM: Copy & Paste and Drag & Drop, defect)

Description

[Jay Patel [:jay]]

Not sure if I chose the right component, but there have been quite a few crashes
in the past few days with this stack signature.  Crash is a regression that
started with 4/19 builds.  Here is the latest from Talkback:

Rank    StackSignature    Count  

18   nsEventStateManager::GenerateDragGesture   11 

 
 	Source File :
c:/builds/seamonkey/mozilla/content/events/src/nsEventStateManager.cpp line : 1422
 
====================================================================================================
     Count   Offset    Real Signature
[ 5   nsEventStateManager::GenerateDragGesture 06252130 -
nsEventStateManager::GenerateDragGesture ]
[ 2   nsEventStateManager::GenerateDragGesture ba9e11e0 -
nsEventStateManager::GenerateDragGesture ]
[ 1   nsEventStateManager::GenerateDragGesture c3595777 -
nsEventStateManager::GenerateDragGesture ]
[ 1   nsEventStateManager::GenerateDragGesture b4bde213 -
nsEventStateManager::GenerateDragGesture ]
[ 1   nsEventStateManager::GenerateDragGesture 23644598 -
nsEventStateManager::GenerateDragGesture ]
[ 1   nsEventStateManager::GenerateDragGesture 2292a04a -
nsEventStateManager::GenerateDragGesture ]
 
     Crash date range: 2003-04-20 to 2003-04-20
     Min/Max Seconds since last crash: 46 - 71849
     Min/Max Runtime: 46 - 71849
 
     Count   Platform List 
     8   Windows NT 5.1 build 2600
     1   Windows NT 5.0 build 2195
     1   Windows 98 4.10 build 67766446
     1   Windows 95 4.0 build 67306684
 
     Count   Build Id List 
     5   2003042008
     4   2003041908
     2   2003042004
 
     No of Unique Users        11
 
 Stack trace(Frame) 

	 nsEventStateManager::GenerateDragGesture
[c:/builds/seamonkey/mozilla/content/events/src/nsEventStateManager.cpp  line 1422] 
	 nsEventStateManager::PreHandleEvent
[c:/builds/seamonkey/mozilla/content/events/src/nsEventStateManager.cpp  line 394] 
	 PresShell::HandleEventInternal
[c:/builds/seamonkey/mozilla/layout/html/base/src/nsPresShell.cpp  line 6324] 
	 PresShell::HandleEvent
[c:/builds/seamonkey/mozilla/layout/html/base/src/nsPresShell.cpp  line 6273] 
	 nsViewManager::HandleEvent
[c:/builds/seamonkey/mozilla/view/src/nsViewManager.cpp  line 2299] 
	 nsView::HandleEvent	[c:/builds/seamonkey/mozilla/view/src/nsView.cpp  line 309] 
	 nsViewManager::DispatchEvent
[c:/builds/seamonkey/mozilla/view/src/nsViewManager.cpp  line 2035] 
	 HandleEvent	[c:/builds/seamonkey/mozilla/view/src/nsView.cpp  line 83] 
	 nsWindow::DispatchEvent
[c:/builds/seamonkey/mozilla/widget/src/windows/nsWindow.cpp  line 1057] 
	 nsWindow::DispatchWindowEvent
[c:/builds/seamonkey/mozilla/widget/src/windows/nsWindow.cpp  line 1074] 
	 nsWindow::DispatchMouseEvent
[c:/builds/seamonkey/mozilla/widget/src/windows/nsWindow.cpp  line 5177] 
	 ChildWindow::DispatchMouseEvent
[c:/builds/seamonkey/mozilla/widget/src/windows/nsWindow.cpp  line 5432] 
	 nsWindow::ProcessMessage
[c:/builds/seamonkey/mozilla/widget/src/windows/nsWindow.cpp  line 3956] 
	 nsWindow::WindowProc
[c:/builds/seamonkey/mozilla/widget/src/windows/nsWindow.cpp  line 1337] 
	 USER32.dll + 0x27ad7 (0x77d67ad7)  
	 USER32.dll + 0x2ccd4 (0x77d6ccd4)  
	 USER32.dll + 0x4455 (0x77d44455)  
	 USER32.dll + 0x95d5 (0x77d495d5)  
	 nsAppShellService::Run
[c:/builds/seamonkey/mozilla/xpfe/appshell/src/nsAppShellService.cpp  line 479] 
	 main1	[c:/builds/seamonkey/mozilla/xpfe/bootstrap/nsAppRunner.cpp  line 1287] 
	 main	[c:/builds/seamonkey/mozilla/xpfe/bootstrap/nsAppRunner.cpp  line 1653] 
	 WinMain	[c:/builds/seamonkey/mozilla/xpfe/bootstrap/nsAppRunner.cpp  line 1675] 
	 WinMainCRTStartup()  
	 kernel32.dll + 0x214c7 (0x77e814c7)   
 
     (19327085)	URL: http://www.originaltrilogy.com/petition.cfm
     (19327085)	Comments: Scrolling down in listbox to selected province/state
or country.

Looking at CVS Blame, it looks like the following checkin might have introduced
this crash:

1.433 <[e-mail]> 18 Apr 2003 17:46
Fix bug 200745 -- we were using the wrong event coords to start a drag. Fix the
ESM to use the coords of the mouse down, not the mouse move, so that dragging
small objects is easier. r=jkeiser, sr=bryner.

Adding crash, topcrash keywords and making this zt4newcrash so we can get it
looked at right away.

Comment 1

[Simon Fraser [no longer active]]

I can't reproduce the crash on Mac, but am adding a null-check on
mGestureDownFrame, which is where is seems to be crashing.

I do note that the state dropdown on http://www.originaltrilogy.com/sign.cfm
doesn't scroll if you try and click-drag the scrollbar thumb. This seems to be
broken with or without my changes.

Comment 2

[Simon Fraser [no longer active]]

Attachment: Null check mGestureDownFrame in nsEventStateManager::GenerateDragGesture

Comment 3

[John Keiser (jkeiser)]

I can't reproduce it either, but I can see how this could happen ... I believe
the proper fix for this is in ClearFrameRefs().  What's happening here is (I
believe), the gesture-down frame is being destroyed between the mousedown and
the drag, which clears the frame but does not set mIsTrackingDragGesture =
PR_FALSE.  What needs to happen is to call StopTrackingDragGesture() in
ClearFrameRefs() if the gesture down frame is destroyed, instead of just
clearing the frame.

Comment 4

[Simon Fraser [no longer active]]

Testcase from bug 200745.

Comment 5

[John Keiser (jkeiser)]

Attachment: Patch (StopTrackingDragGesture in ClearFrameRefs)

This patch should nip the problem more at its root.  I have verified that this
does in fact fix the crash.

Comment 6

[John Keiser (jkeiser)]

Taking for tracking.

Comment 7

[Simon Fraser [no longer active]]

Should we still test mGestureDownFrame for null before the line
      mGestureDownFrame->GetWindow(aPresContext, getter_AddRefs(targetWidget));
?

Comment 8

[John Keiser (jkeiser)]

I don't believe so, no ... the frame and the boolean should go hand in hand
(actually the boolean could be removed and replaced with a null check against
the frame).

Comment 9

[Simon Fraser [no longer active]]

Comment on attachment 121553 [details] [diff] [review]
Patch (StopTrackingDragGesture in ClearFrameRefs)

Try bryner for r=. We need to get this in.

Comment 10

[Asa Dotzler [:asa]]

Comment on attachment 121553 [details] [diff] [review]
Patch (StopTrackingDragGesture in ClearFrameRefs)

a=asa (on behalf of drivers) for checkin to 1.4b

Comment 11

[chris hofmann]

is this ready for checkin?

Comment 12

[John Keiser (jkeiser)]

yeah, it's been waiting for my tree to compile so I can retest, which finally
just happened.

Comment 13

[John Keiser (jkeiser)]

Fix checked in.

Comment 14

[Matthias Versen [:Matti]]

*** Bug 204487 has been marked as a duplicate of this bug. ***

Comment 15

[Jay Patel [:jay]]

No crashes since 5/2 checkin according to Talkback.  Verified.