19941 - can't drag url from outside app into content area, then crash

Status: VERIFIED FIXED

(Core :: XUL, defect, P3)

Description

[Mike Pinkerton (not reading bugmail)]

Drag some text from WordPad and hover over the content area on win32. The calls
into the dragSession are throwing in C++, but we can't catch them so the go up to
OLE and are thrown away. it looks like our internal data members are bad, but
only when the app is in the bg. This all works fine when the drag comes from us.

This will then crash at some later time GC'ing in JS, probably because we never
finish executing the JS routine in the drag handler.

Comment 1

[Mike Pinkerton (not reading bugmail)]

First response from jband:

Oh man, you are playing with OLE here!

I tracked it into your call to dragSession.isDataFlavorSupported.
At about line 227 in nsDragService.cpp you have
  nsDataObjCollection * dataObjCol = (nsDataObjCollection
*)mDataObject;
  PRUint32 i;
  PRUint32 cnt = dataObjCol->GetNumDataObjects();
But dataObjCol does not look valid. It has some bogus data. When
it tries to look at one of that object's memebers it throws bad
exceptions in Win32. I see in the debugger:
First-chance exception in mozilla.exe (XPCOM.DLL): 0xC0000005:
Access Violation.
First-chance exception in mozilla.exe (RPCRT4.DLL): 0x80010105:
(no name).

These blow right past *our* code but since you are running this
all inside an OLE scope, when the stack unwinds up to there OLE
catches it and eats the expection. This completely screws us but
OLE continues on.

So, I don't know why your drag service has a bogus mDataObject.
Normally this would just crash imediately. But our friend OLE is
just trying to help.

----------------------

2nd message from jband:

Yeah, even in nsDragService::SetIDataObject (where I assume this
value is first set) doing a cast of (nsDataObjCollection
*)mDataObject yields an object that has some funny looking
members. Are you getting handed something different from what you
expect?

--------------------------------

3rd message from jband:

In msdev (once the target app is running) in the debug menu there
is an 'Exceptions' item that brings up a dialog in which you can
set options for handling particular kinds of exceptions. The
default for most types is "stop if not handled". When working
with OLE you might want to set the access violation to "stop
always" so that you can see the exception in the debugger before
OLE eats it.

Comment 2

[Mike Pinkerton (not reading bugmail)]

m13

Comment 3

[Mike Pinkerton (not reading bugmail)]

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

Comment 4

[Mike Pinkerton (not reading bugmail)]

marking help wanted. this is some bizarre js threading problem that i need help
from brendan/jband on to really fix. probably means camping out in my cube for a
while ;)

Comment 5

[Brendan Eich [:brendan]]

I don't see what js and threads have to do with this; sounds like you need
expert OLE help (who doesn't?).  Jband?

/be

Comment 6

[Peter Trudelle]

drag&drop is once again post-beta according to marketing. moving all d&d bugs to
M15.

Comment 7

[John Bandhauer]

So, is this disabled in some way? or does it just crash if the user drags
something onto the app?

I'm no OLE guru, but I have some Win32 experience and can try to help with this
problem.

Comment 8

[Claudius Gayle]

has this been disabled? I looked with the 2000011908 builds on NT
and when try to drag some selection form wordpad to Seamonkey
all I get is the international symbol for 'no'. Thereafter no ill affects.

Comment 9

[Mike Pinkerton (not reading bugmail)]

all d&d was hidden behind a pref yesterday, that is off by default.

Comment 10

[Mike Pinkerton (not reading bugmail)]

I want this fixed for m14. 

Comment 11

[Peter Trudelle]

Putting on beta1 radar.

Comment 12

[leger]

Adding "crash" keyword to all known open crasher bugs.

Comment 13

[leger]

Putting on PDT+ radar for beta1.  Stop the crash as opposed to implementing the 
feature.

Comment 14

[leger]

ccing jevering per his request.

Comment 15

[Mike Pinkerton (not reading bugmail)]

...though blizzard is making good progress on linux d&d....

anyway, I don't think this needs to be pdt+, as the offending code never gets 
called for normal users (read: it won't crash). Only people who have the d&d pref 
turned on (eg, developers working on implementing d&d) will see this crash. I 
want seriously to fix this for M14 (it gets in the way for d&d developers) but we 
should _not_ hold beta for this bug.

removing the pdt+ for reconsideration.

Comment 16

[Peter Trudelle]

removing beta1 nomination.

Comment 17

[Mike Pinkerton (not reading bugmail)]

Sigh, five seconds of thought and consideration on this one and i know what the 
problem is. looking back, i have no idea where i got the multi-thread idea into 
my head. jband was right on the money, though i stumbled onto this one 
independantly.

i should have this fixed in a day or so. then perhaps we can get d&d turned on 
everywhere. removing "help wanted" from keywords since i can fix this one.

Comment 18

[Mike Pinkerton (not reading bugmail)]

fixed and dragging to the content area has been re-enabled on all platforms.

Comment 19

[Eli Goldberg]

Verified fixed using 2.7.00 AM builds on Win32 & Mac OS.