Prevent leak on completion of drop operation.

Prevent leak on completion of drop operation.

If the 'drop' event handler signals that the event has been handled and
no default handling is to be done, we clear the document and consider
the operation complete. Also clear out the drag caret selection at that
time, as it may hold onto an anchor node (and its document.)

R=dcheng
BUG=364417
TEST=fast/dom/shadow/drop-event-for-input-in-shadow.html

Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=184768

[sof, 2014-10-31 16:22:56]

sigbjornf@opera.com changed reviewers:
+ dcheng@chromium.org

[sof, 2014-10-31 16:22:56]

Please take a look.

I _believe_ it is acceptable to clear the caret controller at the completion of
this kind of drop.

[dcheng, 2014-10-31 17:53:22]

Do you mind explaining why this leaks without the change?

[sof, 2014-10-31 19:14:49]

On 2014/10/31 17:53:22, dcheng wrote:
> Do you mind explaining why this leaks without the change?

The test is over a shadow rooted element, but the leak reproduces without UA
shadow root elements, so let's explain why that version leaks instead --

The hit testing for the drag operation maps onto an <input> element, so the
caret position kept by the drag caret controller is set to a Position referring
to that <input> element. Should the 'drop' event handler disable further default
handling for the drop event, the code path will be the one changed here & we
will simply clear the document-under-mouse reference, but leave the caret
position set & still referring to the <input> element.

Hence it leaks wrt what the leak detector reports as it shuts down without
initiating other drag operations with other outcomes. The drag caret
controller's caret position will only be cleared out during the later page
destruction.

There are many DragController code paths that take care to clear out the drag
caret controller upon completion of the operation (whatever the outcome); this
looks like one that just doesn't.

[sof, 2014-10-31 21:02:41]

Checking with Chromium bots; reporting green.

[dcheng, 2014-10-31 22:26:54]

OK, makes sense.

Is there any way we can reduce the amount of manual calls to cancelDrag() that
are needed? For example, can we move the call up higher in the stack? The drag
and drop state machine is already really complex and hard to understand.

[sof, 2014-11-01 05:43:00]

On 2014/10/31 22:26:54, dcheng wrote:
> OK, makes sense.
> 
> Is there any way we can reduce the amount of manual calls to cancelDrag() that
> are needed? For example, can we move the call up higher in the stack? The drag
> and drop state machine is already really complex and hard to understand.

I don't see how that can be done here, we bottom out from an EventHandler caller
and there really isn't any other place for that cleanup.

DragController can certainly be worked on/with, but that would be an open-ended
effort worth putting time into. But not here, IMHO.

[dcheng, 2014-11-01 06:27:24]

lgtm

[sof, 2014-11-01 06:36:31]

The CQ bit was checked by sigbjornf@opera.com

[commit-bot: I haz the power, 2014-11-01 06:36:44]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/689103003/1

[commit-bot: I haz the power, 2014-11-01 07:10:42]

Committed patchset #1 (id:1) as 184768