396829 - Crash [@ SampleMouseAndKeyboard] when dragging text whose document goes away

Status: VERIFIED FIXED

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

Description

[Jesse Ruderman]

Attachment: testcase (see comment 0 for steps)

The testcase is a simple page with an iframe that "refreshes" every 2 seconds.

Steps to reproduce:
1. Load the testcase.
2. Select some text in the iframe.
3. Drag the text to outside the iframe, keeping the mouse button held down.
4. Wait for the iframe to refresh.
5. Move the cursor again (?).

Result: crash [@ SampleMouseAndKeyboard] [@ objc_msgSend] dereferencing a random address.

This only crashes after the bug 395397 landing, because before that landing, timers did not fire during drags.  I don't know whether it was possible to make this happen in other ways before the bug 395397 landing (e.g. using a timer in another window?).  I don't know whether this is Mac-specific.

Comment 1

[Josh Aas]

Attachment: fix v1.0

Comment 2

[Josh Aas]

Attachment: fix v1.1

autoreleasing is probably better, it seems weird for an object to call the final release on itself in its own method though apparently it wasn't a problem in this case.

Comment 3

[Josh Aas]

Attachment: fix v2.0

This also fixes the bug and adds an additional measure of safety and efficiency. We retain/release much less often during a drag and there is no need to mess with autorelease.

Comment 4

[Steven Michaud [:smichaud] (Retired)]

Comment on attachment 281624 [details] [diff] [review]
fix v2.0

Your patch looks fine to me, Josh.

But there are a number of places (in nsDragService.mm) where
gLastDragView and gLastDragEvent are used without null checking.
Because of the way Objective-C works, these could never cause crashes.
But you'd (presumably) still get strange results if they were ever
null.

Comment 5

[Josh Aas]

Comment on attachment 281624 [details] [diff] [review]
fix v2.0

We do depend on the mouseDragged being on the stack before every place we use the globals. That is always true and like you said if it wasn't then we wouldn't crash.

Comment 6

[Josh Aas]

landed on trunk

Comment 7

[Carsten Book [:Tomcat]]

verified fixed using Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9b3pre) Gecko/2007123104 Minefield/3.0b3pre and the testcase from this bug - no crash on testcase

-> Verified fixed

Comment 8

[Jesse Ruderman]

Following the steps in comment 0 does not crash on branch.