ALL-IN-ONE: Refactor ComputeInlineBoxPosition()

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

 /*
  * Copyright (C) 2004, 2005, 2006, 2007, 2008, 2009 Apple Inc. All rights
  * reserved.
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions
  * are met:
  * 1. Redistributions of source code must retain the above copyright
  *    notice, this list of conditions and the following disclaimer.
  * 2. Redistributions in binary form must reproduce the above copyright
@@ skipping 864 matching lines @@
 static bool IsStartOfDifferentDirection(const InlineBox* inline_box) {
   InlineBox* prev_box = inline_box->PrevLeafChild();
   if (!prev_box)
     return true;
   if (prev_box->Direction() == inline_box->Direction())
     return true;
   DCHECK_NE(prev_box->BidiLevel(), inline_box->BidiLevel());
   return prev_box->BidiLevel() > inline_box->BidiLevel();
 }
 
+// TODO(yosin): We have a forward declaration here to reduce patch size. We'll
+// replica this with an implementation once review finished.
+static InlineBoxPosition ComputeInlineBoxPositionInternal(LayoutObject*,
+                                                          InlineBox*,
+                                                          int,
+                                                          TextDirection);
+
+// TODO(yosin): We have a forward declaration here to reduce patch size. We'll
+// replica this with an implementation once review finished.
+static InlineBoxPosition ComputeInlineBoxPositionForTextNode(LayoutText*,
+                                                             int,
+                                                             TextAffinity,
+                                                             TextDirection);
+
 template <typename Strategy>
 static InlineBoxPosition ComputeInlineBoxPositionTemplate(
     const PositionTemplate<Strategy>& position,
     TextAffinity affinity,
     TextDirection primary_direction) {
-  InlineBox* inline_box = nullptr;
-  int caret_offset = position.ComputeEditingOffset();
+  const int caret_offset = position.ComputeEditingOffset();
   Node* const anchor_node = position.AnchorNode();
-  LayoutObject* layout_object =
+  LayoutObject* const layout_object =
       anchor_node->IsShadowRoot()
           ? ToShadowRoot(anchor_node)->host().GetLayoutObject()
           : anchor_node->GetLayoutObject();
 
   DCHECK(layout_object) << position;
 
-  if (!layout_object->IsText()) {
-    inline_box = 0;
-    if (CanHaveChildrenForEditing(anchor_node) &&
-        layout_object->IsLayoutBlockFlow() &&
-        HasRenderedNonAnonymousDescendantsWithHeight(layout_object)) {
-      // Try a visually equivalent position with possibly opposite
-      // editability. This helps in case |this| is in an editable block
-      // but surrounded by non-editable positions. It acts to negate the
-      // logic at the beginning of
-      // |LayoutObject::createPositionWithAffinity()|.
-      PositionTemplate<Strategy> equivalent =
-          DownstreamIgnoringEditingBoundaries(position);
-      if (equivalent == position) {
-        equivalent = UpstreamIgnoringEditingBoundaries(position);
-        if (equivalent == position ||
-            DownstreamIgnoringEditingBoundaries(equivalent) == position)
-          return InlineBoxPosition(inline_box, caret_offset);
-      }
+  if (layout_object->IsText()) {
+    return ComputeInlineBoxPositionForTextNode(
+        ToLayoutText(layout_object), caret_offset, affinity, primary_direction);
+  }
+  if (CanHaveChildrenForEditing(anchor_node) &&
+      layout_object->IsLayoutBlockFlow() &&
+      HasRenderedNonAnonymousDescendantsWithHeight(layout_object)) {
+    // Try a visually equivalent position with possibly opposite
+    // editability. This helps in case |this| is in an editable block
+    // but surrounded by non-editable positions. It acts to negate the
+    // logic at the beginning of
+    // |LayoutObject::createPositionWithAffinity()|.
+    const PositionTemplate<Strategy>& downstream_equivalent =
+        DownstreamIgnoringEditingBoundaries(position);
+    if (downstream_equivalent != position) {
+      return ComputeInlineBoxPosition(
+          downstream_equivalent, TextAffinity::kUpstream, primary_direction);
+    }
+    const PositionTemplate<Strategy>& upstream_equivalent =
+        UpstreamIgnoringEditingBoundaries(position);
+    if (upstream_equivalent == position ||
+        DownstreamIgnoringEditingBoundaries(upstream_equivalent) == position)
+      return InlineBoxPosition();
+    return ComputeInlineBoxPosition(upstream_equivalent,
+                                    TextAffinity::kUpstream, primary_direction);
+  }
+  if (!layout_object->IsBox())
+    return InlineBoxPosition();
+  InlineBox* const box = ToLayoutBox(layout_object)->InlineBoxWrapper();
+  if (!box)
+    return InlineBoxPosition();
+  if (caret_offset > box->CaretMinOffset() &&
+      caret_offset < box->CaretMaxOffset()) {
+    return InlineBoxPosition(box, caret_offset);
+  }
+  return ComputeInlineBoxPositionInternal(layout_object, box, caret_offset,
+                                          primary_direction);
+}
 
-      return ComputeInlineBoxPosition(equivalent, TextAffinity::kUpstream,
-                                      primary_direction);
+static InlineBoxPosition ComputeInlineBoxPositionForTextNode(
+    LayoutText* text_layout_object,
+    int caret_offset,
+    TextAffinity affinity,
+    TextDirection primary_direction) {
+  InlineTextBox* inline_box = nullptr;
+  InlineTextBox* candidate = nullptr;
+  for (InlineTextBox* box : InlineTextBoxesOf(*text_layout_object)) {
+    const int caret_min_offset = box->CaretMinOffset();
+    const int caret_max_offset = box->CaretMaxOffset();
+
+    if (caret_offset < caret_min_offset || caret_offset > caret_max_offset ||
+        (caret_offset == caret_max_offset && box->IsLineBreak()))
+      continue;
+
+    if (caret_offset > caret_min_offset && caret_offset < caret_max_offset)
+      return InlineBoxPosition(box, caret_offset);
+
+    if (((caret_offset == caret_max_offset) ^
+         (affinity == TextAffinity::kDownstream)) ||
+        ((caret_offset == caret_min_offset) ^
+         (affinity == TextAffinity::kUpstream)) ||
+        (caret_offset == caret_max_offset && box->NextLeafChild() &&
+         box->NextLeafChild()->IsLineBreak())) {
+      inline_box = box;
+      break;
     }
-    if (layout_object->IsBox()) {
-      inline_box = ToLayoutBox(layout_object)->InlineBoxWrapper();
-      if (!inline_box || (caret_offset > inline_box->CaretMinOffset() &&
-                          caret_offset < inline_box->CaretMaxOffset()))
-        return InlineBoxPosition(inline_box, caret_offset);
+
+    candidate = box;
+  }
+  if (candidate && candidate == text_layout_object->LastTextBox() &&
+      affinity == TextAffinity::kDownstream) {
+    if (InlineBox* found = SearchAheadForBetterMatch(text_layout_object)) {
+      return ComputeInlineBoxPositionInternal(text_layout_object, found,
+                                              found->CaretMinOffset(),
+                                              primary_direction);
     }
-  } else {
-    LayoutText* text_layout_object = ToLayoutText(layout_object);
+  }
+  if (inline_box) {
+    return ComputeInlineBoxPositionInternal(text_layout_object, inline_box,
+                                            caret_offset, primary_direction);
+  }
+  if (candidate) {
+    return ComputeInlineBoxPositionInternal(text_layout_object, candidate,
+                                            caret_offset, primary_direction);
+  }
+  return InlineBoxPosition();
+}
 
-    InlineTextBox* candidate = nullptr;
-
-    for (InlineTextBox* box : InlineTextBoxesOf(*text_layout_object)) {
-      int caret_min_offset = box->CaretMinOffset();
-      int caret_max_offset = box->CaretMaxOffset();
-
-      if (caret_offset < caret_min_offset || caret_offset > caret_max_offset ||
-          (caret_offset == caret_max_offset && box->IsLineBreak()))
-        continue;
-
-      if (caret_offset > caret_min_offset && caret_offset < caret_max_offset)
-        return InlineBoxPosition(box, caret_offset);
-
-      if (((caret_offset == caret_max_offset) ^
-           (affinity == TextAffinity::kDownstream)) ||
-          ((caret_offset == caret_min_offset) ^
-           (affinity == TextAffinity::kUpstream)) ||
-          (caret_offset == caret_max_offset && box->NextLeafChild() &&
-           box->NextLeafChild()->IsLineBreak())) {
-        inline_box = box;
-        break;
-      }
-
-      candidate = box;
-    }
-    if (candidate && candidate == text_layout_object->LastTextBox() &&
-        affinity == TextAffinity::kDownstream) {
-      inline_box = SearchAheadForBetterMatch(text_layout_object);

[Xiaocheng, 2017/06/08 21:34:12]

I don't understand this part.

It seems to be setting the value of |inline_box| repeatedly in the loop without
returning. Why it can be changed to an early return?

[yosin_UTC9, 2017/06/09 01:55:55]

Setting |inline_box| is one in the loop at L976. After setting |iline_box|, we
break loop.

This loop is can be represents as:

(is_early_return, inline_box, candidate) = this_loop(...);
if (is_eraly_return)
  return InlineBoxPosition(inline_box, caret_offset); // as L968
...

-      if (inline_box)
-        caret_offset = inline_box->CaretMinOffset();
-    }
-    if (!inline_box)
-      inline_box = candidate;
-  }
-
-  if (!inline_box)
-    return InlineBoxPosition(inline_box, caret_offset);
-
+static InlineBoxPosition ComputeInlineBoxPositionInternal(
+    LayoutObject* layout_object,
+    InlineBox* inline_box,
+    int caret_offset,
+    TextDirection primary_direction) {
   unsigned char level = inline_box->BidiLevel();
 
   if (inline_box->Direction() == primary_direction) {
     if (caret_offset == inline_box->CaretRightmostOffset()) {
       InlineBox* next_box = inline_box->NextLeafChild();
       if (!next_box || next_box->BidiLevel() >= level)
         return InlineBoxPosition(inline_box, caret_offset);
 
       level = next_box->BidiLevel();
       InlineBox* prev_box = inline_box;
@@ skipping 54 matching lines @@
                  inline_box->PrevLeafChildIgnoringLineBreak()) {
         if (tertiary_box->BidiLevel() <= level)
           break;
         inline_box = tertiary_box;
       }
       return InlineBoxPosition(inline_box, inline_box->CaretLeftmostOffset());
     }
     return InlineBoxPosition(inline_box, caret_offset);
   }
 
-  if (layout_object &&
-      layout_object->Style()->GetUnicodeBidi() == UnicodeBidi::kPlaintext) {
+  if (layout_object->Style()->GetUnicodeBidi() == UnicodeBidi::kPlaintext) {
     if (inline_box->BidiLevel() < level)
       return InlineBoxPosition(inline_box, inline_box->CaretLeftmostOffset());
     return InlineBoxPosition(inline_box, inline_box->CaretRightmostOffset());
   }
 
   InlineBox* next_box = inline_box->NextLeafChildIgnoringLineBreak();
   if (!next_box || next_box->BidiLevel() < level) {
     // Right edge of a secondary run. Set to the left edge of the entire
     // run.
     while (InlineBox* prev_box = inline_box->PrevLeafChildIgnoringLineBreak()) {
@@ skipping 66 matching lines @@
   }
   Node* node = position.AnchorNode();
 
   layout_object = node->GetLayoutObject();
   if (!layout_object)
     return LayoutRect();
 
   InlineBoxPosition box_position =
       ComputeInlineBoxPosition(position.GetPosition(), position.Affinity());
 
-  if (box_position.inline_box)
-    layout_object = LineLayoutAPIShim::LayoutObjectFrom(
-        box_position.inline_box->GetLineLayoutItem());
+  if (box_position.inline_box) {
+    return LineLayoutAPIShim::LayoutObjectFrom(
+               box_position.inline_box->GetLineLayoutItem())
+        ->LocalCaretRect(box_position.inline_box, box_position.offset_in_box);
+  }
 
-  return layout_object->LocalCaretRect(box_position.inline_box,
-                                       box_position.offset_in_box);
+  return layout_object->LocalCaretRect(

[yosin_UTC9, 2017/06/08 10:19:24]

Since ComputeInlineBoxPosition() returns offset zero when there are no inline
box for
position, we should compute offset from position.

+      nullptr, position.GetPosition().ComputeEditingOffset());
 }
 
 // This function was added because the caret rect that is calculated by
 // using the line top value instead of the selection top.
 template <typename Strategy>
 LayoutRect LocalSelectionRectOfPositionTemplate(
     const PositionWithAffinityTemplate<Strategy>& position,
     LayoutObject*& layout_object) {
   if (position.IsNull()) {
     layout_object = nullptr;
@@ skipping 1470 matching lines @@
 
 IntRect ComputeTextRect(const EphemeralRangeInFlatTree& range) {
   return EnclosingIntRect(ComputeTextRectTemplate(range));
 }
 
 FloatRect ComputeTextFloatRect(const EphemeralRange& range) {
   return ComputeTextRectTemplate(range);
 }
 
 }  // namespace blink