296464 - [FIX] crash when setting textbox value from ondragdrop handler

Status: VERIFIED FIXED

(Core :: Layout, defect)

Description

[Brian Ryner (not reading)]

If you attempt to set the value of a XUL textbox from an ondragdrop handler, you
crash.  I think this bug has been around for some time but I couldn't find it on
file.  Here's a stack trace from my trunk build on Windows.  It appears that the
frame (mLastCaretFrame) may be pointing to garbage.

00000000()
nsCaret::GetViewForRendering(nsIFrame * 0x0353d0ac, nsICaret::EViewCoordinates
eRenderingViewCoordinates, nsPoint & {...}, nsRect & {...}, nsIView * *
0x0012d204, nsIView * * 0x00000000) line 750
nsCaret::GetCaretRectAndInvert() line 922
nsCaret::DrawCaret() line 906
nsCaret::EraseCaret(nsCaret * const 0x0357f8c0) line 401
nsTextEditorDragListener::DragDrop(nsTextEditorDragListener * const 0x03541770,
nsIDOMEvent * 0x0357f788) line 624
DispatchToInterface(nsIDOMEvent * 0x0357f788, nsIDOMEventListener * 0x03541770,
unsigned int (nsIDOMEvent *)* 0x020c3230 `vcall'(nsIDOMEvent *), const nsID &
{...}, int * 0x0012d2e0) line 136 + 11 bytes
nsEventListenerManager::HandleEvent(nsEventListenerManager * const 0x0353faf8,
nsPresContext * 0x03515170, nsEvent * 0x0012de4c, nsIDOMEvent * * 0x0012da44,
nsIDOMEventTarget * 0x03586668, unsigned int 2, nsEventStatus * 0x0012dc94) line
1662 + 35 bytes
nsGenericElement::HandleDOMEvent(nsPresContext * 0x03515170, nsEvent *
0x0012de4c, nsIDOMEvent * * 0x0012da44, unsigned int 2, nsEventStatus *
0x0012dc94) line 2128
nsHTMLInputElement::HandleDOMEvent(nsPresContext * 0x03515170, nsEvent *
0x0012de4c, nsIDOMEvent * * 0x0012da44, unsigned int 2, nsEventStatus *
0x0012dc94) line 1380 + 31 bytes
nsGenericElement::HandleDOMEvent(nsPresContext * 0x03515170, nsEvent *
0x0012de4c, nsIDOMEvent * * 0x0012da44, unsigned int 7, nsEventStatus *
0x0012dc94) line 2157 + 57 bytes
PresShell::HandleEventInternal(nsEvent * 0x0012de4c, nsIView * 0x03541578,
unsigned int 1, nsEventStatus * 0x0012dc94) line 6324 + 58 bytes
PresShell::HandleEvent(PresShell * const 0x03517e0c, nsIView * 0x03541578,
nsGUIEvent * 0x0012de4c, nsEventStatus * 0x0012dc94, int 0, int & 1) line 6167 +
25 bytes
nsViewManager::HandleEvent(nsView * 0x03516628, nsGUIEvent * 0x0012de4c, int 0)
line 2502
nsViewManager::DispatchEvent(nsViewManager * const 0x03516570, nsGUIEvent *
0x0012de4c, nsEventStatus * 0x0012ddb4) line 2224 + 20 bytes
HandleEvent(nsGUIEvent * 0x0012de4c) line 174
nsWindow::DispatchEvent(nsWindow * const 0x0351671c, nsGUIEvent * 0x0012de4c,
nsEventStatus & nsEventStatus_eIgnore) line 1180 + 10 bytes
nsNativeDragTarget::DispatchDragDropEvent(unsigned int 1403, _POINTL {...}) line 211
nsNativeDragTarget::ProcessDrag(IDataObject * 0x0356e850, unsigned int 1403,
unsigned long 0, _POINTL {...}, unsigned long * 0x0012e09c) line 237
nsNativeDragTarget::Drop(nsNativeDragTarget * const 0x03516840, IDataObject *
0x0356e850, unsigned long 0, _POINTL {...}, unsigned long * 0x0012e09c) line 360
OLE32! 775f8e86()
OLE32! 775f90c8()
OLE32! 775cfc98()
OLE32! 775cfb20()
nsDragService::StartInvokingDragSession(nsDragService * const 0x01c665d8,
IDataObject * 0x0356e850, unsigned int 3) line 188 + 25 bytes
nsDragService::InvokeDragSession(nsDragService * const 0x01c665d8, nsIDOMNode *
0x03080200, nsISupportsArray * 0x0356e7d8, nsIScriptableRegion * 0x00000000,
unsigned int 3) line 149 + 25 bytes
nsPlaintextEditor::DoDrag(nsPlaintextEditor * const 0x03098540, nsIDOMEvent *
0x0356e350) line 408 + 47 bytes
nsTextEditorDragListener::DragGesture(nsTextEditorDragListener * const
0x03091288, nsIDOMEvent * 0x0356e350) line 508 + 25 bytes
DispatchToInterface(nsIDOMEvent * 0x0356e350, nsIDOMEventListener * 0x03091288,
unsigned int (nsIDOMEvent *)* 0x020c3240 `vcall'(nsIDOMEvent *), const nsID &
{...}, int * 0x0012e238) line 136 + 11 bytes
nsEventListenerManager::HandleEvent(nsEventListenerManager * const 0x01b2ce80,
nsPresContext * 0x02b6f2d8, nsEvent * 0x0012ea5c, nsIDOMEvent * * 0x0012e9fc,
nsIDOMEventTarget * 0x0356e6d0, unsigned int 2, nsEventStatus * 0x0012eaac) line
1662 + 35 bytes
nsGenericElement::HandleDOMEvent(nsPresContext * 0x02b6f2d8, nsEvent *
0x0012ea5c, nsIDOMEvent * * 0x0012e9fc, unsigned int 2, nsEventStatus *
0x0012eaac) line 2128
nsHTMLInputElement::HandleDOMEvent(nsPresContext * 0x02b6f2d8, nsEvent *
0x0012ea5c, nsIDOMEvent * * 0x0012e9fc, unsigned int 2, nsEventStatus *
0x0012eaac) line 1380 + 31 bytes
nsGenericElement::HandleDOMEvent(nsPresContext * 0x02b6f2d8, nsEvent *
0x0012ea5c, nsIDOMEvent * * 0x0012e9fc, unsigned int 2, nsEventStatus *
0x0012eaac) line 2157 + 57 bytes
nsGenericDOMDataNode::HandleDOMEvent(nsPresContext * 0x02b6f2d8, nsEvent *
0x0012ea5c, nsIDOMEvent * * 0x0012e9fc, unsigned int 7, nsEventStatus *
0x0012eaac) line 794 + 36 bytes
nsEventStateManager::GenerateDragGesture(nsPresContext * 0x02b6f2d8,
nsMouseEvent * 0x0012f1ac) line 1533
nsEventStateManager::PreHandleEvent(nsEventStateManager * const 0x02b6f600,
nsPresContext * 0x02b6f2d8, nsEvent * 0x0012f1ac, nsIFrame * 0x0308e254,
nsEventStatus * 0x0012ef68, nsIView * 0x03091090) line 477
PresShell::HandleEventInternal(nsEvent * 0x0012f1ac, nsIView * 0x03091090,
unsigned int 1, nsEventStatus * 0x0012ef68) line 6315 + 61 bytes
PresShell::HandleEvent(PresShell * const 0x02b1ff2c, nsIView * 0x03091090,
nsGUIEvent * 0x0012f1ac, nsEventStatus * 0x0012ef68, int 1, int & 0) line 6167 +
25 bytes
nsViewManager::HandleEvent(nsView * 0x03091090, nsGUIEvent * 0x0012f1ac, int 1)
line 2502
nsViewManager::DispatchEvent(nsViewManager * const 0x02b1f688, nsGUIEvent *
0x0012f1ac, nsEventStatus * 0x0012f088) line 2224 + 20 bytes
HandleEvent(nsGUIEvent * 0x0012f1ac) line 174
nsWindow::DispatchEvent(nsWindow * const 0x02b1f834, nsGUIEvent * 0x0012f1ac,
nsEventStatus & nsEventStatus_eIgnore) line 1180 + 10 bytes
nsWindow::DispatchWindowEvent(nsGUIEvent * 0x0012f1ac) line 1201
nsWindow::DispatchMouseEvent(unsigned int 300, unsigned int 1, nsPoint *
0x00000000) line 5917 + 21 bytes
ChildWindow::DispatchMouseEvent(unsigned int 300, unsigned int 1, nsPoint *
0x00000000) line 6172
nsWindow::ProcessMessage(unsigned int 512, unsigned int 1, long 3080497, long *
0x0012f6b0) line 4546 + 28 bytes
nsWindow::WindowProc(HWND__ * 0x002506ce, unsigned int 512, unsigned int 1, long
3080497) line 1472 + 27 bytes
USER32! 77d48734()
USER32! 77d48816()
USER32! 77d489cd()
USER32! 77d48a10()
nsAppShell::Run(nsAppShell * const 0x00ffb698) line 135
nsAppStartup::Run(nsAppStartup * const 0x01003b58) line 145
XRE_main(int 1, char * * 0x003e8418, const nsXREAppData * 0x0041f01c kAppData)
line 2059 + 35 bytes
main(int 1, char * * 0x003e8418) line 61 + 18 bytes
mainCRTStartup() line 338 + 17 bytes
KERNEL32! 7c816d4f()

Comment 1

[Brian Ryner (not reading)]

Attachment: testcase

Comment 2

[:Gavin Sharp [email: [e-mail]]]

Some workarounds in the tree (to be removed when this is fixed, I guess):
http://bonsai.mozilla.org/cvsblame.cgi?file=mozilla/browser/base/content/search.xml&rev=1.23#338
http://bonsai.mozilla.org/cvsblame.cgi?file=mozilla/browser/base/content/browser.js&rev=1.425#2161

Comment 3

[Boris Zbarsky [:bzbarsky]]

Yeah, mLastCaretFrame seems to be garbage indeed... Not sure how that can happen
given that we do flag it as NS_FRAME_EXTERNAL_REFERENCE and null it out in
ClearFrameRefs.

Not only that, but it's really garbage, not 0xdddddddd....

Comment 4

[Mats Palmgren (inactive)]

Attachment: Stack

The problem is that there are two nsCaret's involved here and the PresShell
only notifies the one it owns.
http://bonsai.mozilla.org/cvsblame.cgi?file=mozilla/layout/base/nsPresShell.cpp&rev=3.848&root=/cvsroot&mark=3875-3877#3870


The other one is created here:
http://bonsai.mozilla.org/cvsblame.cgi?file=mozilla/editor/libeditor/text/nsEditorEventListeners.cpp&rev=1.234&root=/cvsroot&mark=522#514


and it is not notified of the frame the is destroyed.

I have two alternative fixes for this, the first adds a couple of methods to
nsIPresShell to add/remove "external" carets so that they get notified.
The other, which I prefer, removes the need to notify nsCaret in the first
place by not keeping frame pointers around...

Comment 5

[Mats Palmgren (inactive)]

Attachment: Patch rev. 1

The return value from DrawAtPositionWithHint() should probably be changed to
a 'nsresult' instead, but I kept as is for now to limit the regression risc,
let me know if you want me to change it.

This also fixes bug 300977. It has been tested on GTK2/GTK1/Windows XP.

Comment 6

[Blake Kaplan (:mrbkap) (inactive)]

Comment on attachment 191771 [details] [diff] [review]
Patch rev. 1

I like this patch a lot. Great work! r=me

Comment 7

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

Comment on attachment 191771 [details] [diff] [review]
Patch rev. 1

+, mLastContent(nsnull)

Not needed, nsCOMPtrs always initialize to null

Replacing durable frame references with content references is always good :-)

Comment 8

[Mats Palmgren (inactive)]

Checked in to trunk at 2005-08-10 20:44 PDT

-> FIXED

Comment 9

[Stephen Donner [:stephend] Not actively reading bugmail]

Verified FIXED using https://bugzilla.mozilla.org/attachment.cgi?id=185224 as my
testcase with build 2005-08-16-12 on Windows XP SeaMonkey trunk.

Comment 10

[Uri Bernstein]

This patch caused bug 307533 (verified by backing it out).