PositionIterator: Fix Increment/Decrement behavior for SELECT with two or more children

PositoinIterator returned a wrong result if INPUT, SELECT, or TEXTAREA
has two or more children.

Strategy::LastOffsetForEditing() doesn't take into account of
IsUserSelectContain(). It's inconsistent with
ShouldTraverseChildren<Strategy>. So, this CL introduces
an IsUserSelectContain()-aware version of LatOffsetForEditing().

Bug: 697283
Change-Id: Id04cf8ce5cbe3702a4c51a1b6057693c7c669cef
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2029536
Commit-Queue: Kent Tamura <tkent@chromium.org>
Reviewed-by: Yoshifumi Inoue <yosin@chromium.org>
Cr-Commit-Position: refs/heads/master@{#737208}

third_party/blink/renderer/core/editing/position_iterator.cc

@@ -39,6 +39,11 @@ bool ShouldTraverseChildren(const Node& node) {
   return Strategy::HasChildren(node) && !IsUserSelectContain(node);
 }
 
+template <typename Strategy>
+int LastOffsetForPositionIterator(const Node* node) {
+  return IsUserSelectContain(*node) ? 1 : Strategy::LastOffsetForEditing(node);
+}
+
 // TODO(editing-dev): We should replace usages of |parent()| in
 // |PositionIterator| to |selectableParentOf()|.
 template <typename Strategy>
@@ -187,7 +192,8 @@ void PositionIteratorAlgorithm<Strategy>::Increment() {
 
   if (anchor_node_->GetLayoutObject() &&
       !ShouldTraverseChildren<Strategy>(*anchor_node_) &&
-      offset_in_anchor_ < Strategy::LastOffsetForEditing(anchor_node_)) {
+      offset_in_anchor_ <
+          LastOffsetForPositionIterator<Strategy>(anchor_node_)) {
     // Case #2. This is the next of Case #1 or #2 itself.
     // Position is (|anchor|, |offset_in_anchor_|).
     // In this case |anchor| is a leaf(E,F,C,G or H) and
@@ -249,9 +255,10 @@ void PositionIteratorAlgorithm<Strategy>::Decrement() {
       // Let |anchor| is B and |child| is F,
       // next |anchor| is E and |child| is null.
       node_after_position_in_anchor_ = nullptr;
-      offset_in_anchor_ = ShouldTraverseChildren<Strategy>(*anchor_node_)
-                              ? 0
-                              : Strategy::LastOffsetForEditing(anchor_node_);
+      offset_in_anchor_ =
+          ShouldTraverseChildren<Strategy>(*anchor_node_)
+              ? 0
+              : LastOffsetForPositionIterator<Strategy>(anchor_node_);
       // Decrement offset of |child| or initialize if it have never been
       // used.
       if (offsets_in_anchor_node_[depth_to_anchor_node_] == kInvalidOffset)
@@ -293,9 +300,10 @@ void PositionIteratorAlgorithm<Strategy>::Decrement() {
     // Case #2. This is a reverse of increment()::Case3-b.
     // Let |anchor| is B, next |anchor| is F.
     anchor_node_ = Strategy::LastChild(*anchor_node_);
-    offset_in_anchor_ = ShouldTraverseChildren<Strategy>(*anchor_node_)
-                            ? 0
-                            : Strategy::LastOffsetForEditing(anchor_node_);
+    offset_in_anchor_ =
+        ShouldTraverseChildren<Strategy>(*anchor_node_)
+            ? 0
+            : LastOffsetForPositionIterator<Strategy>(anchor_node_);
     // Decrement depth initializing with -1 because
     // |node_after_position_in_anchor_| is null so still unneeded.
     if (depth_to_anchor_node_ >= offsets_in_anchor_node_.size())

third_party/blink/renderer/core/editing/position_iterator_test.cc

@@ -29,12 +29,6 @@ void PositionIteratorTest::TestDecrement(const char* markup) {
   dom_iterator.Decrement();
   EXPECT_EQ(Position::AfterNode(element), dom_iterator.ComputePosition());
   dom_iterator.Decrement();
-  if (element.HasTagName(html_names::kSelectTag)) {
-    EXPECT_EQ(Position::AfterNode(element), dom_iterator.ComputePosition())
-        << "This is redundant result, we should not have. see "
-           "http://crbug.com/697283";
-    dom_iterator.Decrement();
-  }
   EXPECT_EQ(Position::BeforeNode(element), dom_iterator.ComputePosition());
   dom_iterator.Decrement();
   EXPECT_EQ(Position(body, 1), dom_iterator.ComputePosition());
@@ -72,12 +66,6 @@ void PositionIteratorTest::TestIncrement(const char* markup) {
   dom_iterator.Increment();
   EXPECT_EQ(Position::AfterNode(element), dom_iterator.ComputePosition());
   dom_iterator.Increment();
-  if (element.HasTagName(html_names::kSelectTag)) {
-    EXPECT_EQ(Position::AfterNode(element), dom_iterator.ComputePosition())
-        << "This is redundant result, we should not have. see "
-           "http://crbug.com/697283";
-    dom_iterator.Increment();
-  }
   EXPECT_EQ(Position(body, 1), dom_iterator.ComputePosition());
   dom_iterator.Increment();
   EXPECT_EQ(Position(text, 0), dom_iterator.ComputePosition());