Introduce Find{Left,Right}BoundaryOfBidiRun() and variations

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 16 matching lines @@
 #include "core/editing/VisibleUnits.h"
 
 #include "core/HTMLNames.h"
 #include "core/dom/Document.h"
 #include "core/dom/Element.h"
 #include "core/dom/FirstLetterPseudoElement.h"
 #include "core/dom/NodeTraversal.h"
 #include "core/dom/Text.h"
 #include "core/editing/EditingUtilities.h"
 #include "core/editing/FrameSelection.h"
+#include "core/editing/InlineBoxTraversal.h"
 #include "core/editing/Position.h"
 #include "core/editing/PositionIterator.h"
 #include "core/editing/RenderedPosition.h"
 #include "core/editing/TextAffinity.h"
 #include "core/editing/VisiblePosition.h"
 #include "core/editing/iterators/BackwardsCharacterIterator.h"
 #include "core/editing/iterators/BackwardsTextBuffer.h"
 #include "core/editing/iterators/CharacterIterator.h"
 #include "core/editing/iterators/ForwardsTextBuffer.h"
 #include "core/editing/iterators/SimplifiedBackwardsTextIterator.h"
@@ skipping 827 matching lines @@
       InlineBox* prev_box = inline_box;
       do {
         prev_box = prev_box->PrevLeafChild();
       } while (prev_box && prev_box->BidiLevel() > level);
 
       // For example, abc FED 123 ^ CBA
       if (prev_box && prev_box->BidiLevel() == level)
         return InlineBoxPosition(inline_box, caret_offset);
 
       // For example, abc 123 ^ CBA
-      while (InlineBox* next_box = inline_box->NextLeafChild()) {

[Xiaocheng, 2017/06/15 01:11:48]

The existing code is so bad that a loop variable and a local variable have the
same name |next_box|...

It's good that we caught that.

[yosin_UTC9, 2017/06/15 03:42:14]

FYI: MSVC has a compile option to detect multiple occurrences of variable names
in function.
C# doesn't allow name confliction in same scope in the spec.

-        if (next_box->BidiLevel() < level)
-          break;
-        inline_box = next_box;
-      }
+      inline_box =
+          InlineBoxTraversal::FindRightBoundaryOfEntireBidiRun(*next_box);

[yosin_UTC9, 2017/06/14 09:41:34]

Because |level = next_box->BidiLevel()| at L874, we use |next_box| to
avoid pass both |InlineBox*| and BiDi level to
|indRightBoundaryOfEntireBidiRun|.

[Xiaocheng, 2017/06/15 01:11:48]

We should pass in |inline_box|, otherwise |next_box| is skipped in
FindBoudnaryOfEntireBidiRun().

However, do we still pass in the correct Bidi level if |inline_box| is passed
in?

[yosin_UTC9, 2017/06/15 03:42:14]

Add |level| parameter to |FindXXX()|. Once code cleaner, we'll remove |level|
parameter.

       return InlineBoxPosition(inline_box, inline_box->CaretRightmostOffset());
     }
 
     if (IsStartOfDifferentDirection(inline_box))
       return InlineBoxPosition(inline_box, caret_offset);
 
     level = inline_box->PrevLeafChild()->BidiLevel();
     InlineBox* next_box = inline_box;
     do {
       next_box = next_box->NextLeafChild();
     } while (next_box && next_box->BidiLevel() > level);
 
     if (next_box && next_box->BidiLevel() == level)
       return InlineBoxPosition(inline_box, caret_offset);
 
-    while (InlineBox* prev_box = inline_box->PrevLeafChild()) {
-      if (prev_box->BidiLevel() < level)
-        break;
-      inline_box = prev_box;
-    }
+    inline_box = InlineBoxTraversal::FindLeftBoundaryOfEntireBidiRun(
+        *inline_box->PrevLeafChild());

[yosin_UTC9, 2017/06/14 09:41:34]

To avoid pass both |InlineBox*| and BiDi level to
|FindLeftBoundaryOfEntireBidiRun()|,
we use |inline_box->PrevLeafChild()| since |level =
inline_box->PrevLeafChild()->BidiLevel()| at L893.

[Xiaocheng, 2017/06/15 01:11:48]

Ditto.

[yosin_UTC9, 2017/06/15 03:42:14]

Add |level| parameter to |FindXXX()|. Once code cleaner, we'll remove |level|
parameter.

     return InlineBoxPosition(inline_box, inline_box->CaretLeftmostOffset());
   }
 
   if (caret_offset == inline_box->CaretLeftmostOffset()) {
     InlineBox* prev_box = inline_box->PrevLeafChildIgnoringLineBreak();
     if (!prev_box || prev_box->BidiLevel() < level) {
       // Left edge of a secondary run. Set to the right edge of the entire
       // run.
-      while (InlineBox* next_box =
-                 inline_box->NextLeafChildIgnoringLineBreak()) {
-        if (next_box->BidiLevel() < level)
-          break;
-        inline_box = next_box;
-      }
+      inline_box =
+          InlineBoxTraversal::FindRightBoundaryOfEntireBidiRunIgnoringLineBreak(
+              *inline_box);

[Xiaocheng, 2017/06/15 01:11:48]

Do we pass in the correct bidi level here?

[yosin_UTC9, 2017/06/15 03:42:14]

Done.

       return InlineBoxPosition(inline_box, inline_box->CaretRightmostOffset());
     }
 
     if (prev_box->BidiLevel() > level) {
       // Right edge of a "tertiary" run. Set to the left edge of that run.
-      while (InlineBox* tertiary_box =
-                 inline_box->PrevLeafChildIgnoringLineBreak()) {
-        if (tertiary_box->BidiLevel() <= level)
-          break;
-        inline_box = tertiary_box;
-      }
+      inline_box =
+          InlineBoxTraversal::FindLeftBoundaryOfBidiRunIgnoringLineBreak(

[Xiaocheng, 2017/06/15 01:11:48]

Do we pass in the correct bidi level here?

[yosin_UTC9, 2017/06/15 03:42:14]

Done.

+              *inline_box);
       return InlineBoxPosition(inline_box, inline_box->CaretLeftmostOffset());
     }
     return InlineBoxPosition(inline_box, caret_offset);
   }
 
   if (unicode_bidi == 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()) {
-      if (prev_box->BidiLevel() < level)
-        break;
-      inline_box = prev_box;
-    }
+    inline_box =
+        InlineBoxTraversal::FindLeftBoundaryOfEntireBidiRunIgnoringLineBreak(

[Xiaocheng, 2017/06/15 01:11:48]

Do we pass in the correct bidi level here?

[yosin_UTC9, 2017/06/15 03:42:14]

Done.

+            *inline_box);
     return InlineBoxPosition(inline_box, inline_box->CaretLeftmostOffset());
   }
 
   if (next_box->BidiLevel() <= level)
     return InlineBoxPosition(inline_box, caret_offset);
 
   // Left edge of a "tertiary" run. Set to the right edge of that run.
-  while (InlineBox* tertiary_box =
-             inline_box->NextLeafChildIgnoringLineBreak()) {
-    if (tertiary_box->BidiLevel() <= level)
-      break;
-    inline_box = tertiary_box;
-  }
+  inline_box = InlineBoxTraversal::FindRightBoundaryOfBidiRunIgnoringLineBreak(

[Xiaocheng, 2017/06/15 01:11:48]

Do we pass in the correct bidi level here?

[yosin_UTC9, 2017/06/15 03:42:14]

Done.

+      *inline_box);
   return InlineBoxPosition(inline_box, inline_box->CaretRightmostOffset());
 }
 
 template <typename Strategy>
 static InlineBoxPosition ComputeInlineBoxPositionTemplate(
     const PositionTemplate<Strategy>& position,
     TextAffinity affinity) {
   return ComputeInlineBoxPositionTemplate<Strategy>(
       position, affinity, PrimaryDirectionOf(*position.AnchorNode()));
 }
@@ skipping 1100 matching lines @@
 
 IntRect ComputeTextRect(const EphemeralRangeInFlatTree& range) {
   return EnclosingIntRect(ComputeTextRectTemplate(range));
 }
 
 FloatRect ComputeTextFloatRect(const EphemeralRange& range) {
   return ComputeTextRectTemplate(range);
 }
 
 }  // namespace blink