Skip to content

Commit

Permalink
Remove offset_ from RangeSelectionAdjuster::RenderedPosition
Browse files Browse the repository at this point in the history
|RangeSelectionAdjuster::RenderedPosition::offset_| is a redundant
member of the class. It is used at only two places and can be replaced:

1. In GetPosition(), where the RenderedPosition must be at bidi boundary,
   in which case |offset_| is implied by |bidi_boundary_type_|.


2. In operator==(). However,
  - If |inline_box_| or |bidi_boundary_type_| of the two operands
    differ, we don't need to compare |offset_|
  - If |bidi_boundary_type_| is not |kNotBoundary|, |offset_| is implied
    and doesn't need to be compared
  - If |bidi_boundary_type_| is |kNotBoundary| (for both operands), we
    can early-stop the bidi adjustment, and still don't need the exact
    value of |offset_|

Hence, this patch removes the member, and modifies the two use sites
accordingly.

This patch also helps generalizing |RangeSelectionAdjuster| for
LayoutNG, where we don't necessarily have an offset on
|NGPaintFragment|.

Bug: 811502
Change-Id: I14bd6236073dcf262c34f98209d57871665f463c
Reviewed-on: https://chromium-review.googlesource.com/1070471
Commit-Queue: Xiaocheng Hu <xiaochengh@chromium.org>
Reviewed-by: Emil A Eklund <eae@chromium.org>
Cr-Commit-Position: refs/heads/master@{#561247}
  • Loading branch information
xiaochengh authored and Commit Bot committed May 23, 2018
1 parent 08877de commit 8ca9db2
Showing 1 changed file with 14 additions and 13 deletions.
27 changes: 14 additions & 13 deletions third_party/blink/renderer/core/editing/inline_box_traversal.cc
Original file line number Diff line number Diff line change
Expand Up @@ -606,7 +606,8 @@ class RangeSelectionAdjuster {
visible_extent.DeepEquivalent())
.Build();

if (base.IsNull() || extent.IsNull() || base == extent)
if (base.IsNull() || extent.IsNull() || base == extent ||
(!base.AtBidiBoundary() && !extent.AtBidiBoundary()))
return unchanged_selection;

if (base.AtBidiBoundary()) {
Expand Down Expand Up @@ -641,7 +642,7 @@ class RangeSelectionAdjuster {

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_ &&
bidi_boundary_type_ == other.bidi_boundary_type_;
}

Expand Down Expand Up @@ -681,17 +682,19 @@ class RangeSelectionAdjuster {

PositionInFlatTree GetPosition() const {
DCHECK(AtBidiBoundary());
const int offset = bidi_boundary_type_ == BidiBoundaryType::kLeftBoundary
? inline_box_->CaretLeftmostOffset()
: inline_box_->CaretRightmostOffset();
return PositionInFlatTree::EditingPositionOf(
inline_box_->GetLineLayoutItem().GetNode(), offset_);
inline_box_->GetLineLayoutItem().GetNode(), offset);
}

private:
enum class BidiBoundaryType { kNotBoundary, kLeftBoundary, kRightBoundary };
RenderedPosition(const InlineBox* box, int offset, BidiBoundaryType type)
: inline_box_(box), offset_(offset), bidi_boundary_type_(type) {}
RenderedPosition(const InlineBox* box, BidiBoundaryType type)
: inline_box_(box), bidi_boundary_type_(type) {}

const InlineBox* inline_box_ = nullptr;
int offset_ = 0;
BidiBoundaryType bidi_boundary_type_ = BidiBoundaryType::kNotBoundary;
};

Expand Down Expand Up @@ -731,31 +734,29 @@ RangeSelectionAdjuster::RenderedPosition::Create(
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(),
BidiBoundaryType::kRightBoundary);
return RenderedPosition(prev_box, BidiBoundaryType::kRightBoundary);
}
BidiBoundaryType type =
prev_box && prev_box->BidiLevel() == box->BidiLevel()
? BidiBoundaryType::kNotBoundary
: BidiBoundaryType::kLeftBoundary;
return RenderedPosition(box, offset, type);
return RenderedPosition(box, 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(),
BidiBoundaryType::kLeftBoundary);
return RenderedPosition(next_box, BidiBoundaryType::kLeftBoundary);
}
BidiBoundaryType type =
next_box && next_box->BidiLevel() == box->BidiLevel()
? BidiBoundaryType::kNotBoundary
: BidiBoundaryType::kRightBoundary;
return RenderedPosition(box, offset, type);
return RenderedPosition(box, type);
}

return RenderedPosition(box, offset, BidiBoundaryType::kNotBoundary);
return RenderedPosition(box, BidiBoundaryType::kNotBoundary);
}
} // namespace

Expand Down

0 comments on commit 8ca9db2

Please sign in to comment.