Merge LocalCaretRect's line end hack into ComputeInlineBoxPosition()

As a half-baked utility function, ComputeInlineBoxPosition() used to
return null for positions after linebreak. Some callers have ad hoc
hacks to fix up such null returns.

r547639 added such a fixup in LocalCaretRectOfPosition().

This patch merges the fixup into ComputeInlineBoxPosition(), since
LocalCaretRectOfPosition() is supposed to be a wrapper of CIBP().

For changes in SelectionModifierCharacter: the class has code handling
positions after linebreaks, but misses the case of '\n' in
'white-space: pre' style, and handles the case by other fixup path.
With the merge, we no longer hit the fixup path, and have to fix the
linebreak handling in it.

Bug: 812535
Change-Id: I9308dcd2bd911b63b0d2268a2db7a0cb94e63d4c
Reviewed-on: https://chromium-review.googlesource.com/1014606
Commit-Queue: Xiaocheng Hu <xiaochengh@chromium.org>
Reviewed-by: Yoichi Osato <yoichio@chromium.org>
Reviewed-by: Yoshifumi Inoue <yosin@chromium.org>
Cr-Commit-Position: refs/heads/master@{#551408}

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

@@ -204,7 +204,7 @@ InlineBoxPosition AdjustInlineBoxPositionForPrimaryDirection(
 }
 
 // TODO(xiaochengh): Unify left and right edge handling with template.
-InlineBoxPosition AdjustInlineBoxPositionForTextDirectionInternal(
+InlineBoxPosition AdjustInlineBoxPositionForTextDirection(
     InlineBox* inline_box,
     int caret_offset,
     UnicodeBidi unicode_bidi) {
@@ -394,9 +394,59 @@ PositionWithAffinityTemplate<Strategy> ComputeInlineAdjustedPositionAlgorithm(
   return AdjustBlockFlowPositionToInline(position.GetPosition());
 }
 
+// Returns true if |layout_object| and |offset| points after line end.
+template <typename Strategy>
+bool NeedsLineEndAdjustment(
+    const PositionWithAffinityTemplate<Strategy>& adjusted) {
+  const PositionTemplate<Strategy>& position = adjusted.GetPosition();
+  const LayoutObject& layout_object = *position.AnchorNode()->GetLayoutObject();
+  if (!layout_object.IsText())
+    return false;
+  const LayoutText& layout_text = ToLayoutText(layout_object);
+  if (layout_text.IsBR())
+    return position.IsAfterAnchor();
+  // For normal text nodes.
+  if (!layout_text.Style()->PreserveNewline())
+    return false;
+  if (!layout_text.TextLength() ||
+      layout_text.CharacterAt(layout_text.TextLength() - 1) != '\n')
+    return false;
+  if (position.IsAfterAnchor())
+    return true;
+  return position.IsOffsetInAnchor() &&
+         position.OffsetInContainerNode() ==
+             static_cast<int>(layout_text.TextLength());
+}
+
+// Returns the first InlineBoxPosition at next line of last InlineBoxPosition
+// in |layout_object| if it exists to avoid making InlineBoxPosition at end of
+// line.
+template <typename Strategy>
+InlineBoxPosition NextLinePositionOf(
+    const PositionWithAffinityTemplate<Strategy>& adjusted) {
+  const PositionTemplate<Strategy>& position = adjusted.GetPosition();
+  const LayoutText& layout_text =
+      ToLayoutTextOrDie(*position.AnchorNode()->GetLayoutObject());
+  InlineTextBox* const last = layout_text.LastTextBox();
+  if (!last)
+    return InlineBoxPosition();
+  const RootInlineBox& root = last->Root();
+  const RootInlineBox* const next_root = root.NextRootBox();
+  if (!next_root)
+    return InlineBoxPosition();
+  InlineBox* const inline_box = next_root->FirstLeafChild();
+  auto ans = AdjustInlineBoxPositionForTextDirection(
+      inline_box, inline_box->CaretMinOffset(),
+      layout_text.Style()->GetUnicodeBidi());
+  return ans;
+}
+
 template <typename Strategy>
 InlineBoxPosition ComputeInlineBoxPositionForInlineAdjustedPositionAlgorithm(
     const PositionWithAffinityTemplate<Strategy>& adjusted) {
+  if (NeedsLineEndAdjustment(adjusted))
+    return NextLinePositionOf(adjusted);
+
   const PositionTemplate<Strategy>& position = adjusted.GetPosition();
   DCHECK(!position.AnchorNode()->IsShadowRoot()) << adjusted;
   DCHECK(position.AnchorNode()->GetLayoutObject()) << adjusted;
@@ -470,12 +520,4 @@ InlineBoxPosition ComputeInlineBoxPositionForInlineAdjustedPosition(
   return ComputeInlineBoxPositionForInlineAdjustedPositionAlgorithm(position);
 }
 
-InlineBoxPosition AdjustInlineBoxPositionForTextDirection(
-    InlineBox* inline_box,
-    int caret_offset,
-    UnicodeBidi unicode_bidi) {
-  return AdjustInlineBoxPositionForTextDirectionInternal(
-      inline_box, caret_offset, unicode_bidi);
-}
-
 }  // namespace blink

third_party/blink/renderer/core/editing/inline_box_position.h

@@ -64,13 +64,6 @@ struct InlineBoxPosition {
   }
 };
 
-// TODO(yoichio): ComputeInlineBoxPosition returns null if position is at the
-// end of line and We fixed LocalCaretRectOfPosition for such position with
-// NeedsLineEndAdjustment and NextLinePositionOf.
-// We should include the fix into ComputeInlineBoxPosition however
-// SelectionModifierCharacter and SelectionModifierWord
-// depend on the null-line-end behavior of CIBP.
-// Move the fix into the CIBP while fixing the modifier functions.
 CORE_EXPORT InlineBoxPosition
 ComputeInlineBoxPosition(const PositionWithAffinity&);
 CORE_EXPORT InlineBoxPosition
@@ -87,10 +80,6 @@ InlineBoxPosition ComputeInlineBoxPositionForInlineAdjustedPosition(
 InlineBoxPosition ComputeInlineBoxPositionForInlineAdjustedPosition(
     const PositionInFlatTreeWithAffinity&);
 
-InlineBoxPosition AdjustInlineBoxPositionForTextDirection(InlineBox*,
-                                                          int,
-                                                          UnicodeBidi);
-
 // The print for |InlineBoxPosition| is available only for testing
 // in "webkit_unit_tests", and implemented in
 // "core/editing/InlineBoxPositionTest.cpp".

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

@@ -45,52 +45,6 @@ namespace blink {
 
 namespace {
 
-// Returns true if |layout_object| and |offset| points after line end.
-template <typename Strategy>
-bool NeedsLineEndAdjustment(
-    const PositionWithAffinityTemplate<Strategy>& adjusted) {
-  const PositionTemplate<Strategy>& position = adjusted.GetPosition();
-  const LayoutObject& layout_object = *position.AnchorNode()->GetLayoutObject();
-  if (!layout_object.IsText())
-    return false;
-  const LayoutText& layout_text = ToLayoutText(layout_object);
-  if (layout_text.IsBR())
-    return position.IsAfterAnchor();
-  // For normal text nodes.
-  if (!layout_text.Style()->PreserveNewline())
-    return false;
-  if (!layout_text.TextLength() ||
-      layout_text.CharacterAt(layout_text.TextLength() - 1) != '\n')
-    return false;
-  if (position.IsAfterAnchor())
-    return true;
-  return position.IsOffsetInAnchor() &&
-         position.OffsetInContainerNode() ==
-             static_cast<int>(layout_text.TextLength());
-}
-
-// Returns the first InlineBoxPosition at next line of last InlineBoxPosition
-// in |layout_object| if it exists to avoid making InlineBoxPosition at end of
-// line.
-template <typename Strategy>
-InlineBoxPosition NextLinePositionOf(
-    const PositionWithAffinityTemplate<Strategy>& adjusted) {
-  const PositionTemplate<Strategy>& position = adjusted.GetPosition();
-  const LayoutText& layout_text =
-      ToLayoutTextOrDie(*position.AnchorNode()->GetLayoutObject());
-  InlineTextBox* const last = layout_text.LastTextBox();
-  if (!last)
-    return InlineBoxPosition();
-  const RootInlineBox& root = last->Root();
-  const RootInlineBox* const next_root = root.NextRootBox();
-  if (!next_root)
-    return InlineBoxPosition();
-  InlineBox* const inline_box = next_root->FirstLeafChild();
-  return AdjustInlineBoxPositionForTextDirection(
-      inline_box, inline_box->CaretMinOffset(),
-      layout_text.Style()->GetUnicodeBidi());
-}
-
 template <typename Strategy>
 LocalCaretRect LocalCaretRectOfPositionTemplate(
     const PositionWithAffinityTemplate<Strategy>& position,
@@ -118,9 +72,7 @@ LocalCaretRect LocalCaretRectOfPositionTemplate(
     DCHECK_EQ(PrimaryDirectionOf(*position.AnchorNode()),
               PrimaryDirectionOf(*adjusted.AnchorNode()));
     const InlineBoxPosition& box_position =
-        NeedsLineEndAdjustment(adjusted)
-            ? NextLinePositionOf(adjusted)
-            : ComputeInlineBoxPositionForInlineAdjustedPosition(adjusted);
+        ComputeInlineBoxPositionForInlineAdjustedPosition(adjusted);
 
     if (box_position.inline_box) {
       const LayoutObject* box_layout_object =

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

@@ -234,6 +234,15 @@ struct TraversalRight {
   }
 };
 
+template <typename Traversal>
+bool IsAfterAtomicInlineOrLineBreak(const InlineBox& box, int offset) {
+  if (offset != Traversal::CaretEndOffsetOf(box))
+    return false;
+  if (box.IsText() && ToInlineTextBox(box).IsLineBreak())
+    return true;
+  return box.GetLineLayoutItem().IsAtomicInlineLevel();
+}
+
 // TODO(yosin): We should rename local variables and comments in
 // |TraverseInternalAlgorithm()| to generic name based on |Traversal| instead of
 // assuming right-to-left traversal.
@@ -265,8 +274,7 @@ static PositionTemplate<Strategy> TraverseInternalAlgorithm(
     LineLayoutItem line_layout_item = box->GetLineLayoutItem();
 
     while (true) {
-      if ((line_layout_item.IsAtomicInlineLevel() || line_layout_item.IsBR()) &&
-          offset == Traversal::CaretEndOffsetOf(*box)) {
+      if (IsAfterAtomicInlineOrLineBreak<Traversal>(*box, offset)) {
         return Traversal::ForwardVisuallyDistinctCandidateOf(box->Direction(),
                                                              deep_position);
       }