Unify base/extent and start/end in VisbileSelectionTemplate

This patch unifies base/extent and start/end in |VisbileSelectionTemplate| to
simplify |VisbileSelectionTemplate| for improving code health.

Before this patch, base/extent were used for holding start position of
granularity expansion and non-visibly shrunk positions, e.g.
start/end hold |Most{Back,For}wardCaretPostion()|.

After this patch base/extent and start/end are identical with considering
direction, in forward selection base == start and extent == end, in backward
selection base == end and extent == start.

For changes in "TypeCommand.cpp" and "SelectionController.cpp" are canceling
|Most{Back,For}wardCaretPostion()| of start/end for checking equality, for
example: "|<b>foo</>" (most backward caret position) vs. "<b>|foo</b>" (most
forward position). Note: |VisiblePosition| is tend to be most backward caret
position.

Following patch will get rid of |start_| and |end_| from
|VisbileSelectionTemplate|.

Note:
Changes for "TypeCommand.cpp" are for "undo-delete-boundary.html":
Changes for "SelectionController.cpp" are for "selection-bidi.html":

Bug: 230267
Change-Id: I255cbeb331a1da36ae6a569ce7bfd9c59b018eab
Reviewed-on: https://chromium-review.googlesource.com/569507Reviewed-by: Yoichi Osato <yoichio@chromium.org>
Commit-Queue: Yoshifumi Inoue <yosin@chromium.org>
Cr-Commit-Position: refs/heads/master@{#486716}

third_party/WebKit/Source/core/editing/SelectionController.cpp

@@ -765,8 +765,10 @@ void SelectionController::SetNonDirectionalSelectionIfNeeded(
     builder.SetBaseAndExtent(adjusted_selection.Base(),
                              adjusted_selection.Extent());
   } else if (original_base.IsNotNull()) {
-    if (Selection().ComputeVisibleSelectionInFlatTree().Base() ==
-        new_selection.Base()) {
+    if (CreateVisiblePosition(
+            Selection().ComputeVisibleSelectionInFlatTree().Base())
+            .DeepEquivalent() ==
+        CreateVisiblePosition(new_selection.Base()).DeepEquivalent()) {
       builder.SetBaseAndExtent(original_base.DeepEquivalent(),
                                new_selection.Extent());
     }

third_party/WebKit/Source/core/editing/SelectionControllerTest.cpp

@@ -58,7 +58,6 @@ TEST_F(SelectionControllerTest, setNonDirectionalSelectionIfNeeded) {
 
   Node* top = GetDocument().getElementById("top")->firstChild();
   Node* bottom = shadow_root->getElementById("bottom")->firstChild();
-  Node* host = GetDocument().getElementById("host");
 
   // top to bottom
   SetNonDirectionalSelectionIfNeeded(SelectionInFlatTree::Builder()
@@ -66,8 +65,10 @@ TEST_F(SelectionControllerTest, setNonDirectionalSelectionIfNeeded) {
                                          .Extend(PositionInFlatTree(bottom, 3))
                                          .Build(),
                                      TextGranularity::kCharacter);
-  EXPECT_EQ(Position(top, 1), VisibleSelectionInDOMTree().Base());
-  EXPECT_EQ(Position::BeforeNode(*host), VisibleSelectionInDOMTree().Extent());
+  EXPECT_EQ(VisibleSelectionInDOMTree().Start(),
+            VisibleSelectionInDOMTree().Base());
+  EXPECT_EQ(VisibleSelectionInDOMTree().End(),
+            VisibleSelectionInDOMTree().Extent());
   EXPECT_EQ(Position(top, 1), VisibleSelectionInDOMTree().Start());
   EXPECT_EQ(Position(top, 3), VisibleSelectionInDOMTree().End());
 
@@ -86,8 +87,9 @@ TEST_F(SelectionControllerTest, setNonDirectionalSelectionIfNeeded) {
           .Extend(PositionInFlatTree(top, 1))
           .Build(),
       TextGranularity::kCharacter);
-  EXPECT_EQ(Position(bottom, 3), VisibleSelectionInDOMTree().Base());
-  EXPECT_EQ(Position::BeforeNode(*bottom->parentNode()),
+  EXPECT_EQ(VisibleSelectionInDOMTree().End(),
+            VisibleSelectionInDOMTree().Base());
+  EXPECT_EQ(VisibleSelectionInDOMTree().Start(),
             VisibleSelectionInDOMTree().Extent());
   EXPECT_EQ(Position(bottom, 0), VisibleSelectionInDOMTree().Start());
   EXPECT_EQ(Position(bottom, 3), VisibleSelectionInDOMTree().End());

third_party/WebKit/Source/core/editing/VisibleSelection.cpp

@@ -487,20 +487,21 @@ void VisibleSelectionTemplate<Strategy>::Validate(
   AdjustSelectionToAvoidCrossingEditingBoundaries();
   UpdateSelectionType();
 
-  if (GetSelectionType() != kRangeSelection)
-    return;
-
-  // "Constrain" the selection to be the smallest equivalent range of
-  // nodes. This is a somewhat arbitrary choice, but experience shows that
-  // it is useful to make to make the selection "canonical" (if only for
-  // purposes of comparing selections). This is an ideal point of the code
-  // to do this operation, since all selection changes that result in a
-  // RANGE come through here before anyone uses it.
-  // TODO(yosin) Canonicalizing is good, but haven't we already done it
-  // (when we set these two positions to |VisiblePosition|
-  // |deepEquivalent()|s above)?
-  start_ = MostForwardCaretPosition(start_);
-  end_ = MostBackwardCaretPosition(end_);
+  if (GetSelectionType() == kRangeSelection) {
+    // "Constrain" the selection to be the smallest equivalent range of
+    // nodes. This is a somewhat arbitrary choice, but experience shows that
+    // it is useful to make to make the selection "canonical" (if only for
+    // purposes of comparing selections). This is an ideal point of the code
+    // to do this operation, since all selection changes that result in a
+    // RANGE come through here before anyone uses it.
+    // TODO(yosin) Canonicalizing is good, but haven't we already done it
+    // (when we set these two positions to |VisiblePosition|
+    // |DeepEquivalent()|s above)?
+    start_ = MostForwardCaretPosition(start_);
+    end_ = MostBackwardCaretPosition(end_);
+  }
+  base_ = base_is_first_ ? start_ : end_;
+  extent_ = base_is_first_ ? end_ : start_;
 }
 
 template <typename Strategy>

third_party/WebKit/Source/core/editing/VisibleSelectionTest.cpp

@@ -124,13 +124,13 @@ TEST_F(VisibleSelectionTest, expandUsingGranularity) {
   selection_in_flat_tree =
       ExpandUsingGranularity(selection_in_flat_tree, TextGranularity::kWord);
 
-  EXPECT_EQ(Position(one, 1), selection.Base());
-  EXPECT_EQ(Position(one, 1), selection.Extent());
+  EXPECT_EQ(selection.Start(), selection.Base());
+  EXPECT_EQ(selection.End(), selection.Extent());
   EXPECT_EQ(Position(one, 0), selection.Start());
   EXPECT_EQ(Position(two, 2), selection.End());
 
-  EXPECT_EQ(PositionInFlatTree(one, 1), selection_in_flat_tree.Base());
-  EXPECT_EQ(PositionInFlatTree(one, 1), selection_in_flat_tree.Extent());
+  EXPECT_EQ(selection_in_flat_tree.Start(), selection_in_flat_tree.Base());
+  EXPECT_EQ(selection_in_flat_tree.End(), selection_in_flat_tree.Extent());
   EXPECT_EQ(PositionInFlatTree(one, 0), selection_in_flat_tree.Start());
   EXPECT_EQ(PositionInFlatTree(five, 5), selection_in_flat_tree.End());
 
@@ -145,13 +145,13 @@ TEST_F(VisibleSelectionTest, expandUsingGranularity) {
   selection_in_flat_tree =
       ExpandUsingGranularity(selection_in_flat_tree, TextGranularity::kWord);
 
-  EXPECT_EQ(Position(two, 1), selection.Base());
-  EXPECT_EQ(Position(two, 1), selection.Extent());
+  EXPECT_EQ(selection.Start(), selection.Base());
+  EXPECT_EQ(selection.End(), selection.Extent());
   EXPECT_EQ(Position(one, 0), selection.Start());
   EXPECT_EQ(Position(two, 2), selection.End());
 
-  EXPECT_EQ(PositionInFlatTree(two, 1), selection_in_flat_tree.Base());
-  EXPECT_EQ(PositionInFlatTree(two, 1), selection_in_flat_tree.Extent());
+  EXPECT_EQ(selection_in_flat_tree.Start(), selection_in_flat_tree.Base());
+  EXPECT_EQ(selection_in_flat_tree.End(), selection_in_flat_tree.Extent());
   EXPECT_EQ(PositionInFlatTree(three, 0), selection_in_flat_tree.Start());
   EXPECT_EQ(PositionInFlatTree(four, 4), selection_in_flat_tree.End());
 
@@ -166,13 +166,13 @@ TEST_F(VisibleSelectionTest, expandUsingGranularity) {
   selection_in_flat_tree =
       ExpandUsingGranularity(selection_in_flat_tree, TextGranularity::kWord);
 
-  EXPECT_EQ(Position(three, 1), selection.Base());
-  EXPECT_EQ(Position(three, 1), selection.Extent());
+  EXPECT_EQ(selection.Start(), selection.Base());
+  EXPECT_EQ(selection.End(), selection.Extent());
   EXPECT_EQ(Position(three, 0), selection.Start());
   EXPECT_EQ(Position(four, 4), selection.End());
 
-  EXPECT_EQ(PositionInFlatTree(three, 1), selection_in_flat_tree.Base());
-  EXPECT_EQ(PositionInFlatTree(three, 1), selection_in_flat_tree.Extent());
+  EXPECT_EQ(selection_in_flat_tree.Start(), selection_in_flat_tree.Base());
+  EXPECT_EQ(selection_in_flat_tree.End(), selection_in_flat_tree.Extent());
   EXPECT_EQ(PositionInFlatTree(three, 0), selection_in_flat_tree.Start());
   EXPECT_EQ(PositionInFlatTree(four, 4), selection_in_flat_tree.End());
 
@@ -187,13 +187,13 @@ TEST_F(VisibleSelectionTest, expandUsingGranularity) {
   selection_in_flat_tree =
       ExpandUsingGranularity(selection_in_flat_tree, TextGranularity::kWord);
 
-  EXPECT_EQ(Position(four, 1), selection.Base());
-  EXPECT_EQ(Position(four, 1), selection.Extent());
+  EXPECT_EQ(selection.Start(), selection.Base());
+  EXPECT_EQ(selection.End(), selection.Extent());
   EXPECT_EQ(Position(three, 0), selection.Start());
   EXPECT_EQ(Position(four, 4), selection.End());
 
-  EXPECT_EQ(PositionInFlatTree(four, 1), selection_in_flat_tree.Base());
-  EXPECT_EQ(PositionInFlatTree(four, 1), selection_in_flat_tree.Extent());
+  EXPECT_EQ(selection_in_flat_tree.Start(), selection_in_flat_tree.Base());
+  EXPECT_EQ(selection_in_flat_tree.End(), selection_in_flat_tree.Extent());
   EXPECT_EQ(PositionInFlatTree(three, 0), selection_in_flat_tree.Start());
   EXPECT_EQ(PositionInFlatTree(four, 4), selection_in_flat_tree.End());
 
@@ -208,13 +208,13 @@ TEST_F(VisibleSelectionTest, expandUsingGranularity) {
   selection_in_flat_tree =
       ExpandUsingGranularity(selection_in_flat_tree, TextGranularity::kWord);
 
-  EXPECT_EQ(Position(five, 1), selection.Base());
-  EXPECT_EQ(Position(five, 1), selection.Extent());
+  EXPECT_EQ(selection.Start(), selection.Base());
+  EXPECT_EQ(selection.End(), selection.Extent());
   EXPECT_EQ(Position(five, 0), selection.Start());
   EXPECT_EQ(Position(five, 5), selection.End());
 
-  EXPECT_EQ(PositionInFlatTree(five, 1), selection_in_flat_tree.Base());
-  EXPECT_EQ(PositionInFlatTree(five, 1), selection_in_flat_tree.Extent());
+  EXPECT_EQ(selection_in_flat_tree.Start(), selection_in_flat_tree.Base());
+  EXPECT_EQ(selection_in_flat_tree.End(), selection_in_flat_tree.Extent());
   EXPECT_EQ(PositionInFlatTree(one, 0), selection_in_flat_tree.Start());
   EXPECT_EQ(PositionInFlatTree(five, 5), selection_in_flat_tree.End());
 }

third_party/WebKit/Source/core/editing/commands/TypingCommand.cpp

@@ -833,7 +833,8 @@ void TypingCommand::DeleteKeyPressed(TextGranularity granularity,
   // current state of the document and we'll get the wrong result.
   const VisibleSelection& selection_after_undo =
       VisibleSelection::CreateWithoutValidationDeprecated(
-          StartingSelection().End(), selection_to_delete.Extent(),
+          StartingSelection().End(),
+          CreateVisiblePosition(selection_to_delete.Extent()).DeepEquivalent(),
           selection_to_delete.Affinity());
   DeleteKeyPressedInternal(selection_to_delete, selection_after_undo, kill_ring,
                            editing_state);
@@ -961,7 +962,8 @@ void TypingCommand::ForwardDeleteKeyPressed(TextGranularity granularity,
       const VisibleSelection& selection_to_delete =
           selection_modifier.Selection();
       if (!StartingSelection().IsRange() ||
-          selection_to_delete.Base() != StartingSelection().Start()) {
+          MostBackwardCaretPosition(selection_to_delete.Base()) !=
+              StartingSelection().Start()) {
         ForwardDeleteKeyPressedInternal(
             selection_to_delete, selection_to_delete, kill_ring, editing_state);
         return;