Specialize RenderedPosition for bidi adjustment in SelectionController

This patch removes the general member functions in RenderedPosition
that inspect properties of bidi runs, and replace them by functions
providing the exact information needed by SelectionController, which
is the only client of RenderedPosition.

Details:
- Introduced RenderedPosition::BidiBoundaryType, so that we can obtain
  whether an RP is at bidi boundary without re-comparing its offset.
- Replaced almost all existing member functions by
 - AtBidiBoundary(), replacing AtL/RBoundaryOfBidiRun()
 - BidiRunContains(other_rp), replacing
   |other_rp == this->L/RBoundaryOfBidiRun(other_rp.BidiLevelOnR/L())|
 - IsPossiblyOtherBoundaryOf(other_rp), replacing
   |this->AtL/RBoundaryOfBidiRun(other_rp.BidiLevelOnL/R())|


Bug: 811502
Change-Id: I0b59b76e8060a5953295e09c98fcf7127a636ecc
Reviewed-on: https://chromium-review.googlesource.com/1056220Reviewed-by: Yoshifumi Inoue <yosin@chromium.org>
Commit-Queue: Xiaocheng Hu <xiaochengh@chromium.org>
Cr-Commit-Position: refs/heads/master@{#558202}

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

@@ -65,123 +65,63 @@ RenderedPosition RenderedPosition::Create(
   // For example, abc FED |ghi should be changed into abc FED| ghi
   if (offset == box->CaretLeftmostOffset()) {
     const InlineBox* prev_box = box->PrevLeafChildIgnoringLineBreak();
-    if (prev_box && prev_box->BidiLevel() > box->BidiLevel())
-      return RenderedPosition(prev_box, prev_box->CaretRightmostOffset());
+    if (prev_box && prev_box->BidiLevel() > box->BidiLevel()) {
+      return RenderedPosition(prev_box, prev_box->CaretRightmostOffset(),
+                              BidiBoundaryType::kRightBoundary);
+    }
+    BidiBoundaryType type =
+        prev_box && prev_box->BidiLevel() == box->BidiLevel()
+            ? BidiBoundaryType::kNotBoundary
+            : BidiBoundaryType::kLeftBoundary;
+    return RenderedPosition(box, offset, type);
   }
 
   // For example, abc| FED ghi should be changed into abc |FED ghi
   if (offset == box->CaretRightmostOffset()) {
     const InlineBox* next_box = box->NextLeafChildIgnoringLineBreak();
-    if (next_box && next_box->BidiLevel() > box->BidiLevel())
-      return RenderedPosition(next_box, next_box->CaretLeftmostOffset());
+    if (next_box && next_box->BidiLevel() > box->BidiLevel()) {
+      return RenderedPosition(next_box, next_box->CaretLeftmostOffset(),
+                              BidiBoundaryType::kLeftBoundary);
+    }
+    BidiBoundaryType type =
+        next_box && next_box->BidiLevel() == box->BidiLevel()
+            ? BidiBoundaryType::kNotBoundary
+            : BidiBoundaryType::kRightBoundary;
+    return RenderedPosition(box, offset, type);
   }
 
-  return RenderedPosition(box, offset);
-}
-
-const InlineBox* RenderedPosition::PrevLeafChild() const {
-  if (!prev_leaf_child_.has_value())
-    prev_leaf_child_ = inline_box_->PrevLeafChildIgnoringLineBreak();
-  return prev_leaf_child_.value();
-}
-
-const InlineBox* RenderedPosition::NextLeafChild() const {
-  if (!next_leaf_child_.has_value())
-    next_leaf_child_ = inline_box_->NextLeafChildIgnoringLineBreak();
-  return next_leaf_child_.value();
-}
-
-unsigned char RenderedPosition::BidiLevelOnLeft() const {
-  const InlineBox* box =
-      AtLeftmostOffsetInBox() ? PrevLeafChild() : inline_box_;
-  return box ? box->BidiLevel() : 0;
-}
-
-unsigned char RenderedPosition::BidiLevelOnRight() const {
-  const InlineBox* box =
-      AtRightmostOffsetInBox() ? NextLeafChild() : inline_box_;
-  return box ? box->BidiLevel() : 0;
-}
-
-RenderedPosition RenderedPosition::LeftBoundaryOfBidiRun(
-    unsigned char bidi_level_of_run) const {
-  if (!inline_box_ || bidi_level_of_run > inline_box_->BidiLevel())
-    return RenderedPosition();
-
-  const InlineBox& box =
-      InlineBoxTraversal::FindLeftBoundaryOfEntireBidiRunIgnoringLineBreak(
-          *inline_box_, bidi_level_of_run);
-  return RenderedPosition(&box, box.CaretLeftmostOffset());
-}
-
-RenderedPosition RenderedPosition::RightBoundaryOfBidiRun(
-    unsigned char bidi_level_of_run) const {
-  if (!inline_box_ || bidi_level_of_run > inline_box_->BidiLevel())
-    return RenderedPosition();
-
-  const InlineBox& box =
-      InlineBoxTraversal::FindRightBoundaryOfEntireBidiRunIgnoringLineBreak(
-          *inline_box_, bidi_level_of_run);
-  return RenderedPosition(&box, box.CaretRightmostOffset());
+  return RenderedPosition(box, offset, BidiBoundaryType::kNotBoundary);
 }
 
-bool RenderedPosition::AtLeftBoundaryOfBidiRun(
-    ShouldMatchBidiLevel should_match_bidi_level,
-    unsigned char bidi_level_of_run) const {
-  if (!inline_box_)
+bool RenderedPosition::IsPossiblyOtherBoundaryOf(
+    const RenderedPosition& other) const {
+  DCHECK(other.AtBidiBoundary());
+  if (!AtBidiBoundary())
     return false;
-
-  if (AtLeftmostOffsetInBox()) {
-    if (should_match_bidi_level == kIgnoreBidiLevel)
-      return !PrevLeafChild() ||
-             PrevLeafChild()->BidiLevel() < inline_box_->BidiLevel();
-    return inline_box_->BidiLevel() >= bidi_level_of_run &&
-           (!PrevLeafChild() ||
-            PrevLeafChild()->BidiLevel() < bidi_level_of_run);
-  }
-
-  if (AtRightmostOffsetInBox()) {
-    if (should_match_bidi_level == kIgnoreBidiLevel)
-      return NextLeafChild() &&
-             inline_box_->BidiLevel() < NextLeafChild()->BidiLevel();
-    return NextLeafChild() && inline_box_->BidiLevel() < bidi_level_of_run &&
-           NextLeafChild()->BidiLevel() >= bidi_level_of_run;
-  }
-
+  if (bidi_boundary_type_ == other.bidi_boundary_type_)
     return false;
+  return inline_box_->BidiLevel() >= other.inline_box_->BidiLevel();
 }
 
-bool RenderedPosition::AtRightBoundaryOfBidiRun(
-    ShouldMatchBidiLevel should_match_bidi_level,
-    unsigned char bidi_level_of_run) const {
-  if (!inline_box_)
-    return false;
-
-  if (AtRightmostOffsetInBox()) {
-    if (should_match_bidi_level == kIgnoreBidiLevel)
-      return !NextLeafChild() ||
-             NextLeafChild()->BidiLevel() < inline_box_->BidiLevel();
-    return inline_box_->BidiLevel() >= bidi_level_of_run &&
-           (!NextLeafChild() ||
-            NextLeafChild()->BidiLevel() < bidi_level_of_run);
-  }
-
-  if (AtLeftmostOffsetInBox()) {
-    if (should_match_bidi_level == kIgnoreBidiLevel)
-      return PrevLeafChild() &&
-             inline_box_->BidiLevel() < PrevLeafChild()->BidiLevel();
-    return PrevLeafChild() && inline_box_->BidiLevel() < bidi_level_of_run &&
-           PrevLeafChild()->BidiLevel() >= bidi_level_of_run;
-  }
-
+bool RenderedPosition::BidiRunContains(const RenderedPosition& other) const {
+  DCHECK(AtBidiBoundary());
+  DCHECK(!other.IsNull());
+  UBiDiLevel level = inline_box_->BidiLevel();
+  if (level > other.inline_box_->BidiLevel())
     return false;
+  const InlineBox& boundary_of_other =
+      bidi_boundary_type_ == BidiBoundaryType::kLeftBoundary
+          ? InlineBoxTraversal::
+                FindLeftBoundaryOfEntireBidiRunIgnoringLineBreak(
+                    *other.inline_box_, level)
+          : InlineBoxTraversal::
+                FindRightBoundaryOfEntireBidiRunIgnoringLineBreak(
+                    *other.inline_box_, level);
+  return inline_box_ == &boundary_of_other;
 }
 
 PositionInFlatTree RenderedPosition::GetPosition() const {
-  DCHECK(AtLeftBoundaryOfBidiRun() || AtRightBoundaryOfBidiRun());
-  DCHECK_EQ(AtLeftmostOffsetInBox(), AtLeftBoundaryOfBidiRun());
-  DCHECK_EQ(AtRightmostOffsetInBox(), AtRightBoundaryOfBidiRun());
-
+  DCHECK(AtBidiBoundary());
   return PositionInFlatTree::EditingPositionOf(
       inline_box_->GetLineLayoutItem().GetNode(), offset_);
 }

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

@@ -44,42 +44,33 @@ struct CompositedSelection;
 
 // TODO(xiaochengh): RenderedPosition is deprecated. It's currently only used in
 // |SelectionController| for bidi adjustment. We should break this class and
-// move relevant code to |inline_box_traversal.cc|, and templatize it so that it
+// move relevant code to |inline_box_traversal.cc|, and generalize it so that it
 // can be reused in LayoutNG.
 class CORE_EXPORT RenderedPosition {
   STACK_ALLOCATED();
 
  public:
-  RenderedPosition();
+  RenderedPosition() = default;
   static RenderedPosition Create(const VisiblePositionInFlatTree&);
 
   bool IsNull() const { return !inline_box_; }
   bool operator==(const RenderedPosition& other) const {
-    return inline_box_ == other.inline_box_ && offset_ == other.offset_;
+    return inline_box_ == other.inline_box_ && offset_ == other.offset_ &&
+           bidi_boundary_type_ == other.bidi_boundary_type_;
   }
 
-  unsigned char BidiLevelOnLeft() const;
-  unsigned char BidiLevelOnRight() const;
-  RenderedPosition LeftBoundaryOfBidiRun(unsigned char bidi_level_of_run) const;
-  RenderedPosition RightBoundaryOfBidiRun(
-      unsigned char bidi_level_of_run) const;
-
-  enum ShouldMatchBidiLevel { kMatchBidiLevel, kIgnoreBidiLevel };
-  bool AtLeftBoundaryOfBidiRun() const {
-    return AtLeftBoundaryOfBidiRun(kIgnoreBidiLevel, 0);
-  }
-  bool AtRightBoundaryOfBidiRun() const {
-    return AtRightBoundaryOfBidiRun(kIgnoreBidiLevel, 0);
-  }
-  // The following two functions return true only if the current position is
-  // at the end of the bidi run of the specified bidi embedding level.
-  bool AtLeftBoundaryOfBidiRun(unsigned char bidi_level_of_run) const {
-    return AtLeftBoundaryOfBidiRun(kMatchBidiLevel, bidi_level_of_run);
-  }
-  bool AtRightBoundaryOfBidiRun(unsigned char bidi_level_of_run) const {
-    return AtRightBoundaryOfBidiRun(kMatchBidiLevel, bidi_level_of_run);
+  bool AtBidiBoundary() const {
+    return bidi_boundary_type_ != BidiBoundaryType::kNotBoundary;
   }
 
+  // Given |other|, which is a boundary of a bidi run, returns true if |this|
+  // can be the other boundary of that run by checking some conditions.
+  bool IsPossiblyOtherBoundaryOf(const RenderedPosition& other) const;
+
+  // Callable only when |this| is at boundary of a bidi run. Returns true if
+  // |other| is in that bidi run.
+  bool BidiRunContains(const RenderedPosition& other) const;
+
   PositionInFlatTree GetPosition() const;
 
   // TODO(editing-dev): This function doesn't use RenderedPosition
@@ -87,33 +78,18 @@ class CORE_EXPORT RenderedPosition {
   static CompositedSelection ComputeCompositedSelection(const FrameSelection&);
 
  private:
-  explicit RenderedPosition(const InlineBox*, int offset);
+  enum class BidiBoundaryType { kNotBoundary, kLeftBoundary, kRightBoundary };
+  explicit RenderedPosition(const InlineBox*, int offset, BidiBoundaryType);
 
-  const InlineBox* PrevLeafChild() const;
-  const InlineBox* NextLeafChild() const;
-  bool AtLeftmostOffsetInBox() const {
-    return inline_box_ && offset_ == inline_box_->CaretLeftmostOffset();
-  }
-  bool AtRightmostOffsetInBox() const {
-    return inline_box_ && offset_ == inline_box_->CaretRightmostOffset();
-  }
-  bool AtLeftBoundaryOfBidiRun(ShouldMatchBidiLevel,
-                               unsigned char bidi_level_of_run) const;
-  bool AtRightBoundaryOfBidiRun(ShouldMatchBidiLevel,
-                                unsigned char bidi_level_of_run) const;
-
-  const InlineBox* inline_box_;
-  int offset_;
-
-  mutable base::Optional<const InlineBox*> prev_leaf_child_;
-  mutable base::Optional<const InlineBox*> next_leaf_child_;
+  const InlineBox* inline_box_ = nullptr;
+  int offset_ = 0;
+  BidiBoundaryType bidi_boundary_type_ = BidiBoundaryType::kNotBoundary;
 };
 
-inline RenderedPosition::RenderedPosition()
-    : inline_box_(nullptr), offset_(0) {}
-
-inline RenderedPosition::RenderedPosition(const InlineBox* box, int offset)
-    : inline_box_(box), offset_(offset) {}
+inline RenderedPosition::RenderedPosition(const InlineBox* box,
+                                          int offset,
+                                          BidiBoundaryType type)
+    : inline_box_(box), offset_(offset), bidi_boundary_type_(type) {}
 
 }  // namespace blink
 

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

@@ -766,28 +766,17 @@ void SelectionController::SelectClosestWordOrLinkFromMouseEvent(
 
 static bool ShouldAdjustBaseAtBidiBoundary(const RenderedPosition& base,
                                            const RenderedPosition& extent) {
-  if (base.AtLeftBoundaryOfBidiRun()) {
-    return !extent.AtRightBoundaryOfBidiRun(base.BidiLevelOnRight()) &&
-           base == extent.LeftBoundaryOfBidiRun(base.BidiLevelOnRight());
-  }
-
-  if (base.AtRightBoundaryOfBidiRun()) {
-    return !extent.AtLeftBoundaryOfBidiRun(base.BidiLevelOnLeft()) &&
-           base == extent.RightBoundaryOfBidiRun(base.BidiLevelOnLeft());
-  }
-
+  DCHECK(base.AtBidiBoundary());
+  if (extent.IsPossiblyOtherBoundaryOf(base))
     return false;
+  return base.BidiRunContains(extent);
 }
 
 static bool ShouldAdjustExtentAtBidiBoundary(const RenderedPosition& base,
                                              const RenderedPosition& extent) {
-  if (extent.AtLeftBoundaryOfBidiRun())
-    return extent == base.LeftBoundaryOfBidiRun(extent.BidiLevelOnRight());
-
-  if (extent.AtRightBoundaryOfBidiRun())
-    return extent == base.RightBoundaryOfBidiRun(extent.BidiLevelOnLeft());
-
+  if (!extent.AtBidiBoundary())
     return false;
+  return extent.BidiRunContains(base);
 }
 
 static SelectionInFlatTree AdjustEndpointsAtBidiBoundary(
@@ -808,7 +797,7 @@ static SelectionInFlatTree AdjustEndpointsAtBidiBoundary(
   if (base.IsNull() || extent.IsNull() || base == extent)
     return unchanged_selection;
 
-  if (base.AtLeftBoundaryOfBidiRun() || base.AtRightBoundaryOfBidiRun()) {
+  if (base.AtBidiBoundary()) {
     if (ShouldAdjustBaseAtBidiBoundary(base, extent)) {
       const PositionInFlatTree adjusted_base =
           CreateVisiblePosition(base.GetPosition()).DeepEquivalent();