Skip to content

Commit

Permalink
Change how iteration range is passed to TextIteratorTextNodeHandler
Browse files Browse the repository at this point in the history
TextIteratorTextNodeHandler used to store the full iteration range, and
have comparisons between |text_node_| and |start/end_container_| scattered
in the class to decide the offset range on |text_node_| from which text
should be emitted.

This patch changes it that:
- TextIterator, when calling HandleTextNode(), compares the current text
  node with the start and end containers to decide the offset range on it,
  and pass the offset range to TextNodeHandler
- TextNodeHandler no longer stores the full iteration, but only the offset
  range passed from TextIterator.
- TextNodeHandler no longer decides the offset range by itself, but uses
  the passed-in offset range directly.

BUG=721957
TEST=n/a; no behavioral change

Review-Url: https://codereview.chromium.org/2913673004
Cr-Commit-Position: refs/heads/master@{#476336}
  • Loading branch information
xiaochengh authored and Commit Bot committed Jun 1, 2017
1 parent 3c5fcee commit 3c26208
Show file tree
Hide file tree
Showing 3 changed files with 62 additions and 50 deletions.
22 changes: 17 additions & 5 deletions third_party/WebKit/Source/core/editing/iterators/TextIterator.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -188,9 +188,6 @@ TextIteratorAlgorithm<Strategy>::TextIteratorAlgorithm(
Node* const end_container = end.ComputeContainerNode();
const int end_offset = end.ComputeOffsetInContainerNode();

text_node_handler_.Initialize(start_container, start_offset, end_container,
end_offset);

// Remember the range - this does not change.
start_container_ = start_container;
start_offset_ = start_offset;
Expand Down Expand Up @@ -484,8 +481,23 @@ void TextIteratorAlgorithm<Strategy>::HandleTextNode() {

DCHECK_NE(last_text_node_, node_)
<< "We should never call HandleTextNode on the same node twice";
last_text_node_ = ToText(node_);
text_node_handler_.HandleTextNode(ToText(node_));
Text* text = ToText(node_);
last_text_node_ = text;

// TODO(editing-dev): Introduce a |DOMOffsetRange| class so that we can pass
// an offset range with unbounded endpoint(s) in an easy but still clear way.
if (node_ != start_container_) {
if (node_ != end_container_)
text_node_handler_.HandleTextNodeWhole(text);
else
text_node_handler_.HandleTextNodeEndAt(text, end_offset_);
return;
}
if (node_ != end_container_) {
text_node_handler_.HandleTextNodeStartFrom(text, start_offset_);
return;
}
text_node_handler_.HandleTextNodeInRange(text, start_offset_, end_offset_);
}

template <typename Strategy>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,22 +18,6 @@ TextIteratorTextNodeHandler::TextIteratorTextNodeHandler(
TextIteratorTextState* text_state)
: behavior_(behavior), text_state_(*text_state) {}

void TextIteratorTextNodeHandler::Initialize(Node* start_container,
int start_offset,
Node* end_container,
int end_offset) {
// This function should be called only once.
DCHECK(!start_container_);
DCHECK_EQ(start_offset_, 0);
DCHECK(!end_container_);
DCHECK_EQ(end_offset_, 0);

start_container_ = start_container;
start_offset_ = start_offset;
end_container_ = end_container;
end_offset_ = end_offset;
}

bool TextIteratorTextNodeHandler::HandleRemainingTextRuns() {
if (ShouldProceedToRemainingText())
ProceedToRemainingText();
Expand Down Expand Up @@ -100,7 +84,6 @@ void TextIteratorTextNodeHandler::HandlePreFormattedTextNode() {
const String first_letter = first_letter_text_->GetText();
const unsigned run_start = offset_;
const bool stops_in_first_letter =
text_node_ == end_container_ &&
end_offset_ <= static_cast<int>(first_letter.length());
const unsigned run_end =
stops_in_first_letter ? end_offset_ : first_letter.length();
Expand All @@ -123,9 +106,7 @@ void TextIteratorTextNodeHandler::HandlePreFormattedTextNode() {
DCHECK_GE(static_cast<unsigned>(offset_), layout_object->TextStartOffset());
const unsigned run_start = offset_ - layout_object->TextStartOffset();
const unsigned str_length = str.length();
const unsigned end = (text_node_ == end_container_)
? end_offset_ - layout_object->TextStartOffset()
: str_length;
const unsigned end = end_offset_ - layout_object->TextStartOffset();
const unsigned run_end = std::min(str_length, end);

if (run_start >= run_end)
Expand All @@ -134,9 +115,25 @@ void TextIteratorTextNodeHandler::HandlePreFormattedTextNode() {
EmitText(text_node_, text_node_->GetLayoutObject(), run_start, run_end);
}

void TextIteratorTextNodeHandler::HandleTextNode(Text* node) {
void TextIteratorTextNodeHandler::HandleTextNodeInRange(Text* node,
int start_offset,
int end_offset) {
DCHECK(node);
DCHECK_GE(start_offset, 0);

// TODO(editing-dev): Add the following DCHECK once we stop assuming equal
// number of code units in DOM string and LayoutText::GetText(). Currently
// violated by
// - external/wpt/innerText/getter.html
// - fast/css/case-transform.html
// DCHECK_LE(end_offset, static_cast<int>(node->data().length()));

// TODO(editing-dev): Stop passing in |start_offset == end_offset|.
DCHECK_LE(start_offset, end_offset);

text_node_ = node;
offset_ = text_node_ == start_container_ ? start_offset_ : 0;
offset_ = start_offset;
end_offset_ = end_offset;
handled_first_letter_ = false;
first_letter_text_ = nullptr;

Expand Down Expand Up @@ -186,6 +183,22 @@ void TextIteratorTextNodeHandler::HandleTextNode(Text* node) {
HandleTextBox();
}

void TextIteratorTextNodeHandler::HandleTextNodeStartFrom(Text* node,
int start_offset) {
HandleTextNodeInRange(node, start_offset,
node->GetLayoutObject()->TextStartOffset() +
node->GetLayoutObject()->GetText().length());
}

void TextIteratorTextNodeHandler::HandleTextNodeEndAt(Text* node,
int end_offset) {
HandleTextNodeInRange(node, 0, end_offset);
}

void TextIteratorTextNodeHandler::HandleTextNodeWhole(Text* node) {
HandleTextNodeStartFrom(node, 0);
}

// Restore the collapsed space for copy & paste. See http://crbug.com/318925
size_t TextIteratorTextNodeHandler::RestoreCollapsedTrailingSpace(
InlineTextBox* next_text_box,
Expand Down Expand Up @@ -228,10 +241,7 @@ void TextIteratorTextNodeHandler::HandleTextBox() {
// Start and end offsets in |str|, i.e., str[start..end - 1] should be
// emitted (after handling whitespace collapsing).
const unsigned start = offset_ - layout_object->TextStartOffset();
const unsigned end =
(text_node_ == end_container_)
? static_cast<unsigned>(end_offset_) - text_start_offset
: INT_MAX;
const unsigned end = static_cast<unsigned>(end_offset_) - text_start_offset;
while (text_box_) {
const unsigned text_box_start = text_box_->Start();
const unsigned run_start = std::max(text_box_start, start);
Expand Down Expand Up @@ -357,8 +367,6 @@ void TextIteratorTextNodeHandler::HandleTextBox() {
bool TextIteratorTextNodeHandler::ShouldProceedToRemainingText() const {
if (text_box_ || !remaining_text_box_)
return false;
if (text_node_ != end_container_)
return true;
return offset_ < end_offset_;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,15 +26,6 @@ class TextIteratorTextNodeHandler final {
TextIteratorTextNodeHandler(const TextIteratorBehavior&,
TextIteratorTextState*);

// Initializes the full iteration range of the TextIterator. This function
// should be called only once from TextIterator::Initialize.
// TODO(xiaochengh): TextNodeHandler doesn't need to know the full iteration
// range; The offset range in the current node suffices. Remove this function.
void Initialize(Node* start_container,
int start_offset,
Node* end_container,
int end_offset);

Text* GetNode() const { return text_node_; }

// Returns true if more text is emitted without traversing to the next node.
Expand All @@ -45,7 +36,13 @@ class TextIteratorTextNodeHandler final {

void ResetCollapsedWhiteSpaceFixup();

void HandleTextNode(Text*);
// Emit plain text from the given text node.
void HandleTextNodeWhole(Text*);

// Variants that emit plain text within the given DOM offset range.
void HandleTextNodeStartFrom(Text*, int start_offset);
void HandleTextNodeEndAt(Text*, int end_offset);
void HandleTextNodeInRange(Text*, int start_offset, int end_offset);

private:
void HandlePreFormattedTextNode();
Expand All @@ -72,15 +69,10 @@ class TextIteratorTextNodeHandler final {
int text_start_offset,
int text_end_offset);

// The range.
Member<Node> start_container_;
int start_offset_ = 0;
Member<Node> end_container_;
int end_offset_ = 0;

// The current text node and offset, from which text is being emitted.
// The current text node and offset range, from which text should be emitted.
Member<Text> text_node_;
int offset_ = 0;
int end_offset_ = 0;

InlineTextBox* text_box_ = nullptr;

Expand Down

0 comments on commit 3c26208

Please sign in to comment.