[InputEvent] Move 'beforeinput' logic into |CompositeEditCommand::willApplyEditing()| (3/3)

third_party/WebKit/Source/core/page/DragController.cpp

Index: third_party/WebKit/Source/core/page/DragController.cpp
diff --git a/third_party/WebKit/Source/core/page/DragController.cpp b/third_party/WebKit/Source/core/page/DragController.cpp
index 0be7eaeadf9e19192115fdf043a20fa3d9f9fab6..6e7ca85ea9924b8581c1eb853c2b2cba950c976f 100644
--- a/third_party/WebKit/Source/core/page/DragController.cpp
+++ b/third_party/WebKit/Source/core/page/DragController.cpp
@@ -601,9 +601,13 @@ bool DragController::concludeEditDrag(DragData* dragData) {
               ? InsertMode::Smart
               : InsertMode::Simple;
 
-      if (!innerFrame->editor().deleteSelectionAfterDraggingWithEvents(

[chongz, 2016/12/20 23:27:53]

Removed, drag deletion are covered by |DeleteSelectionCommand =>
willApplyEditing()|.

-              innerFrame->editor().findEventTargetFromSelection(), deleteMode,
-              dragCaret.base()))
+      innerFrame->editor().deleteSelectionWithSmartDelete(
+          EditCommandSource::kMenuOrKeyBinding, deleteMode,
+          InputEvent::InputType::DeleteByDrag, dragCaret.base());
+
+      // 'beforeinput' event handler may destroy drop frame.

[Xiaocheng, 2016/12/21 02:56:39]

Is it just 'beforeinput'?

In fact is it a bug that the current drag&drop doesn't abort even if other
events fired by DeleteSelectionCommand destroys the frame?

[chongz, 2016/12/21 23:59:24]

I've tested 'input' and it won't crash, so I think it's only 'beforeinput'.

Other events are dispatched through |dispatchScopedEvent()| and my debug log
told me |ScopedEventQueue::instance()->shouldQueueEvents()| was true (however
I'm not sure where the scope starts). 

We have to use |dispatchEvent()| for 'beforeinput' as JS might prevent default.

+      if (innerFrame->document()->frame() != innerFrame ||
+          !element->isConnected())
         return false;
 
       innerFrame->selection().setSelection(
@@ -612,19 +616,16 @@ bool DragController::concludeEditDrag(DragData* dragData) {
               .build());
       if (innerFrame->selection().isAvailable()) {
         DCHECK(m_documentUnderMouse);
-        if (!innerFrame->editor().replaceSelectionAfterDraggingWithEvents(

[chongz, 2016/12/20 23:27:53]

Removed, drop replacing are covered by |ReplaceSelectionCommand =>
willApplyEditing()|.

-                element, dragData, fragment, range, insertMode, dragSourceType))
-          return false;
+        innerFrame->editor().replaceSelectionAfterDragging(fragment, insertMode,
+                                                           dragSourceType);
       }
     } else {
       if (setSelectionToDragCaret(innerFrame, dragCaret, range, point)) {
         DCHECK(m_documentUnderMouse);
-        if (!innerFrame->editor().replaceSelectionAfterDraggingWithEvents(

[chongz, 2016/12/20 23:27:53]

Removed, drop replacing are covered by |ReplaceSelectionCommand =>
willApplyEditing()|.

-                element, dragData, fragment, range,
-                dragData->canSmartReplace() ? InsertMode::Smart
-                                            : InsertMode::Simple,
-                dragSourceType))
-          return false;
+        innerFrame->editor().replaceSelectionAfterDragging(
+            fragment, dragData->canSmartReplace() ? InsertMode::Smart
+                                                  : InsertMode::Simple,
+            dragSourceType);
       }
     }
   } else {
@@ -634,11 +635,9 @@ bool DragController::concludeEditDrag(DragData* dragData) {
 
     if (setSelectionToDragCaret(innerFrame, dragCaret, range, point)) {
       DCHECK(m_documentUnderMouse);
-      if (!innerFrame->editor().replaceSelectionAfterDraggingWithEvents(

[chongz, 2016/12/20 23:27:53]

Removed, drop replacing are covered by |ReplaceSelectionCommand =>
willApplyEditing()|.

-              element, dragData,
-              createFragmentFromText(EphemeralRange(range), text), range,
-              InsertMode::Simple, DragSourceType::PlainTextSource))
-        return false;
+      innerFrame->editor().replaceSelectionAfterDragging(
+          createFragmentFromText(EphemeralRange(range), text),
+          InsertMode::Simple, DragSourceType::PlainTextSource);
     }
   }
 

[Xiaocheng, 2016/12/21 02:56:39]

We should check whether the frame is destroyed here, and abort if so.

[chongz, 2016/12/21 23:59:24]

Done.