[Editing] Refactor DeleteSelectionCommand to not use VisibleSelection

Currently DeleteSelectionCommand accept a VisibleSelection as the
parameter, which is not necessary in many places. This CL converts the
VisibleSelection |selection_to_delete_| to SelectionForUndoStep. So we
could avoid canonicalization.

This enables us to avoiding to use
VisibleSelection::CreateWithoutValidationDeprecated() later.

Bug: 1024738
Change-Id: I4b32507afca4e77338e9f6a83c01b3a30ea3b188
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2013905Reviewed-by: Xiaocheng Hu <xiaochengh@chromium.org>
Commit-Queue: Shimi Zhang <ctzsm@chromium.org>
Cr-Commit-Position: refs/heads/master@{#734513}

third_party/blink/renderer/core/editing/commands/delete_selection_command.cc

@@ -89,10 +89,10 @@ DeleteSelectionCommand::DeleteSelectionCommand(
       reference_move_position_(reference_move_position) {}
 
 DeleteSelectionCommand::DeleteSelectionCommand(
-    const VisibleSelection& selection,
+    const SelectionForUndoStep& selection,
     const DeleteSelectionOptions& options,
     InputEvent::InputType input_type)
-    : CompositeEditCommand(*selection.Start().GetDocument()),
+    : CompositeEditCommand(*selection.Base().GetDocument()),
       options_(options),
       has_selection_to_delete_(true),
       merge_blocks_after_delete_(options.IsMergeBlocksAfterDelete()),
@@ -137,9 +137,10 @@ void DeleteSelectionCommand::InitializeStartEnd(Position& start,
       break;
 
     if (CreateVisiblePosition(start).DeepEquivalent() !=
-            selection_to_delete_.VisibleStart().DeepEquivalent() ||
+            CreateVisiblePosition(selection_to_delete_.Start())
+                .DeepEquivalent() ||
         CreateVisiblePosition(end).DeepEquivalent() !=
-            selection_to_delete_.VisibleEnd().DeepEquivalent())
+            CreateVisiblePosition(selection_to_delete_.End()).DeepEquivalent())
       break;
 
     // If we're going to expand to include the startSpecialContainer, it must be
@@ -561,7 +562,8 @@ void DeleteSelectionCommand::DeleteTextFromNode(Text* node,
 void DeleteSelectionCommand::
     MakeStylingElementsDirectChildrenOfEditableRootToPreventStyleLoss(
         EditingState* editing_state) {
-  Range* range = CreateRange(selection_to_delete_.ToNormalizedEphemeralRange());
+  Range* range = CreateRange(CreateVisibleSelection(selection_to_delete_)
+                                 .ToNormalizedEphemeralRange());
   Node* node = range->FirstNode();
   while (node && node != range->PastLastNode()) {
     Node* next_node = NodeTraversal::Next(*node);
@@ -1020,7 +1022,7 @@ void DeleteSelectionCommand::CalculateTypingStyleAfterDelete() {
 }
 
 void DeleteSelectionCommand::ClearTransientState() {
-  selection_to_delete_ = VisibleSelection();
+  selection_to_delete_ = SelectionForUndoStep();
   upstream_start_ = Position();
   downstream_start_ = Position();
   upstream_end_ = Position();
@@ -1059,12 +1061,14 @@ void DeleteSelectionCommand::RemoveRedundantBlocks(
 void DeleteSelectionCommand::DoApply(EditingState* editing_state) {
   // If selection has not been set to a custom selection when the command was
   // created, use the current ending selection.
-  if (!has_selection_to_delete_)
-    selection_to_delete_ = EndingVisibleSelection();
+  if (!has_selection_to_delete_) {
+    selection_to_delete_ =
+        SelectionForUndoStep::From(EndingVisibleSelection().AsSelection());
+  }
 
   if (!selection_to_delete_.IsValidFor(GetDocument()) ||
       !selection_to_delete_.IsRange() ||
-      !selection_to_delete_.IsContentEditable()) {
+      !IsEditablePosition(selection_to_delete_.Base())) {
     // editing/execCommand/delete-non-editable-range-crash.html reaches here.
     return;
   }
@@ -1084,20 +1088,26 @@ void DeleteSelectionCommand::DoApply(EditingState* editing_state) {
       (downstream_end.ComputeContainerNode()->IsTextNode() &&
        downstream_end.ComputeContainerNode()->parentNode() ==
            RootEditableElement(*downstream_end.ComputeContainerNode()));
+  VisiblePosition visible_start = CreateVisiblePosition(
+      selection_to_delete_.Start(),
+      selection_to_delete_.IsRange() ? TextAffinity::kDownstream : affinity);
+  VisiblePosition visible_end = CreateVisiblePosition(
+      selection_to_delete_.End(),
+      selection_to_delete_.IsRange() ? TextAffinity::kUpstream : affinity);
+
   bool line_break_at_end_of_selection_to_delete =
-      LineBreakExistsAtVisiblePosition(selection_to_delete_.VisibleEnd());
-  need_placeholder_ = !root_will_stay_open_without_placeholder &&
-                      IsStartOfParagraph(selection_to_delete_.VisibleStart(),
-                                         kCanCrossEditingBoundary) &&
-                      IsEndOfParagraph(selection_to_delete_.VisibleEnd(),
-                                       kCanCrossEditingBoundary) &&
-                      !line_break_at_end_of_selection_to_delete;
+      LineBreakExistsAtVisiblePosition(visible_end);
+
+  need_placeholder_ =
+      !root_will_stay_open_without_placeholder &&
+      IsStartOfParagraph(visible_start, kCanCrossEditingBoundary) &&
+      IsEndOfParagraph(visible_end, kCanCrossEditingBoundary) &&
+      !line_break_at_end_of_selection_to_delete;
   if (need_placeholder_) {
     // Don't need a placeholder when deleting a selection that starts just
     // before a table and ends inside it (we do need placeholders to hold
     // open empty cells, but that's handled elsewhere).
-    if (Element* table =
-            TableElementJustAfter(selection_to_delete_.VisibleStart())) {
+    if (Element* table = TableElementJustAfter(visible_start)) {
       if (selection_to_delete_.End().AnchorNode()->IsDescendantOf(table))
         need_placeholder_ = false;
     }

third_party/blink/renderer/core/editing/commands/delete_selection_command.h

@@ -43,7 +43,7 @@ class CORE_EXPORT DeleteSelectionCommand final : public CompositeEditCommand {
       InputEvent::InputType input_type = InputEvent::InputType::kNone,
       const Position& reference_move_position = Position());
   DeleteSelectionCommand(
-      const VisibleSelection&,
+      const SelectionForUndoStep&,
       const DeleteSelectionOptions&,
       InputEvent::InputType input_type = InputEvent::InputType::kNone);
 
@@ -85,7 +85,7 @@ class CORE_EXPORT DeleteSelectionCommand final : public CompositeEditCommand {
 
   // This data is transient and should be cleared at the end of the doApply
   // function.
-  VisibleSelection selection_to_delete_;
+  SelectionForUndoStep selection_to_delete_;
   Position upstream_start_;
   Position downstream_start_;
   Position upstream_end_;

third_party/blink/renderer/core/editing/commands/typing_command.cc

@@ -205,8 +205,9 @@ void TypingCommand::DeleteSelection(Document& document, Options options) {
       ->Apply();
 }
 
-void TypingCommand::DeleteSelectionIfRange(const VisibleSelection& selection,
-                                           EditingState* editing_state) {
+void TypingCommand::DeleteSelectionIfRange(
+    const SelectionForUndoStep& selection,
+    EditingState* editing_state) {
   if (!selection.IsRange())
     return;
   // Although the 'selection' to delete is indeed a Range, it may have been
@@ -785,8 +786,8 @@ void TypingCommand::DeleteKeyPressed(TextGranularity granularity,
     return;
 
   if (EndingSelection().IsRange()) {
-    DeleteKeyPressedInternal(EndingVisibleSelection(), EndingSelection(),
-                             kill_ring, editing_state);
+    DeleteKeyPressedInternal(EndingSelection(), EndingSelection(), kill_ring,
+                             editing_state);
     return;
   }
 
@@ -894,7 +895,7 @@ void TypingCommand::DeleteKeyPressed(TextGranularity granularity,
   if (!StartingSelection().IsRange() ||
       selection_to_delete.Base() != StartingSelection().Start()) {
     DeleteKeyPressedInternal(
-        selection_to_delete,
+        SelectionForUndoStep::From(selection_to_delete.AsSelection()),
         SelectionForUndoStep::From(selection_to_delete.AsSelection()),
         kill_ring, editing_state);
     return;
@@ -908,12 +909,13 @@ void TypingCommand::DeleteKeyPressed(TextGranularity granularity,
               CreateVisiblePosition(selection_to_delete.Extent())
                   .DeepEquivalent())
           .Build();
-  DeleteKeyPressedInternal(selection_to_delete, selection_after_undo, kill_ring,
-                           editing_state);
+  DeleteKeyPressedInternal(
+      SelectionForUndoStep::From(selection_to_delete.AsSelection()),
+      selection_after_undo, kill_ring, editing_state);
 }
 
 void TypingCommand::DeleteKeyPressedInternal(
-    const VisibleSelection& selection_to_delete,
+    const SelectionForUndoStep& selection_to_delete,
     const SelectionForUndoStep& selection_after_undo,
     bool kill_ring,
     EditingState* editing_state) {
@@ -927,9 +929,10 @@ void TypingCommand::DeleteKeyPressedInternal(
   LocalFrame* frame = GetDocument().GetFrame();
   DCHECK(frame);
 
-  if (kill_ring)
-    frame->GetEditor().AddToKillRing(
-        selection_to_delete.ToNormalizedEphemeralRange());
+  if (kill_ring) {
+    frame->GetEditor().AddToKillRing(CreateVisibleSelection(selection_to_delete)
+                                         .ToNormalizedEphemeralRange());
+  }
   // On Mac, make undo select everything that has been deleted, unless an undo
   // will undo more than just this deletion.
   // FIXME: This behaves like TextEdit except for the case where you open with
@@ -969,7 +972,7 @@ void TypingCommand::ForwardDeleteKeyPressed(TextGranularity granularity,
     return;
 
   if (EndingSelection().IsRange()) {
-    ForwardDeleteKeyPressedInternal(EndingVisibleSelection(), EndingSelection(),
+    ForwardDeleteKeyPressedInternal(EndingSelection(), EndingSelection(),
                                     kill_ring, editing_state);
     return;
   }
@@ -1041,7 +1044,7 @@ void TypingCommand::ForwardDeleteKeyPressed(TextGranularity granularity,
       MostBackwardCaretPosition(selection_to_delete.Base()) !=
           StartingSelection().Start()) {
     ForwardDeleteKeyPressedInternal(
-        selection_to_delete,
+        SelectionForUndoStep::From(selection_to_delete.AsSelection()),
         SelectionForUndoStep::From(selection_to_delete.AsSelection()),
         kill_ring, editing_state);
     return;
@@ -1054,12 +1057,13 @@ void TypingCommand::ForwardDeleteKeyPressed(TextGranularity granularity,
               ComputeExtentForForwardDeleteUndo(selection_to_delete,
                                                 StartingSelection().End()))
           .Build();
-  ForwardDeleteKeyPressedInternal(selection_to_delete, selection_after_undo,
-                                  kill_ring, editing_state);
+  ForwardDeleteKeyPressedInternal(
+      SelectionForUndoStep::From(selection_to_delete.AsSelection()),
+      selection_after_undo, kill_ring, editing_state);
 }
 
 void TypingCommand::ForwardDeleteKeyPressedInternal(
-    const VisibleSelection& selection_to_delete,
+    const SelectionForUndoStep& selection_to_delete,
     const SelectionForUndoStep& selection_after_undo,
     bool kill_ring,
     EditingState* editing_state) {
@@ -1073,9 +1077,10 @@ void TypingCommand::ForwardDeleteKeyPressedInternal(
   LocalFrame* frame = GetDocument().GetFrame();
   DCHECK(frame);
 
-  if (kill_ring)
-    frame->GetEditor().AddToKillRing(
-        selection_to_delete.ToNormalizedEphemeralRange());
+  if (kill_ring) {
+    frame->GetEditor().AddToKillRing(CreateVisibleSelection(selection_to_delete)
+                                         .ToNormalizedEphemeralRange());
+  }
   // Make undo select what was deleted on Mac alone
   if (frame->GetEditor().Behavior().ShouldUndoOfDeleteSelectText())
     SetStartingSelection(selection_after_undo);

third_party/blink/renderer/core/editing/commands/typing_command.h

@@ -145,15 +145,15 @@ class CORE_EXPORT TypingCommand final : public CompositeEditCommand {
   bool IsIncrementalInsertion() const { return is_incremental_insertion_; }
 
   void DeleteKeyPressedInternal(
-      const VisibleSelection& selection_to_delete,
+      const SelectionForUndoStep& selection_to_delete,
       const SelectionForUndoStep& selection_after_undo,
       bool kill_ring,
       EditingState*);
 
-  void DeleteSelectionIfRange(const VisibleSelection&, EditingState*);
+  void DeleteSelectionIfRange(const SelectionForUndoStep&, EditingState*);
 
   void ForwardDeleteKeyPressedInternal(
-      const VisibleSelection& selection_to_delete,
+      const SelectionForUndoStep& selection_to_delete,
       const SelectionForUndoStep& selection_after_undo,
       bool kill_ring,
       EditingState*);