Commit b6aea068 authored by Yoshifumi Inoue's avatar Yoshifumi Inoue Committed by Commit Bot

Change NGInlineNodeEditor::SetTextWithOffset() to use character diff instead of offsets

This patch changes |NGInlineNodeEditor| to reuse |ShapeResult| before
and after changed region by equality of substring instead of calculating
it from offsets to handle generated line break markers, e.g. leading
spaces in pre-wrap, and increase chance to reuse, e.g. replacing
"abc" with "abX", before this patch we reuse nothing but after this
patch we reuse "ab".

This patch also gets rid of |NGInlineNodeData::can_use_flag_editing_|
because we don't use it.

Bug: 1131315
Change-Id: Ic4471340245522ced6b2a4d3d8bf43b23a2ff077
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2437831
Auto-Submit: Yoshifumi Inoue <yosin@chromium.org>
Commit-Queue: Koji Ishii <kojii@chromium.org>
Reviewed-by: default avatarKoji Ishii <kojii@chromium.org>
Cr-Commit-Position: refs/heads/master@{#812062}
parent 376b2999
...@@ -249,8 +249,11 @@ TEST_F(LayoutNGTextTest, SetTextWithOffsetDeleteWithGeneratedBreakOpportunity) { ...@@ -249,8 +249,11 @@ TEST_F(LayoutNGTextTest, SetTextWithOffsetDeleteWithGeneratedBreakOpportunity) {
Text& text = To<Text>(*GetElementById("target")->firstChild()); Text& text = To<Text>(*GetElementById("target")->firstChild());
text.deleteData(2, 1, ASSERT_NO_EXCEPTION); // remove "\n" text.deleteData(2, 1, ASSERT_NO_EXCEPTION); // remove "\n"
EXPECT_EQ("LayoutText has NeedsCollectInlines", EXPECT_EQ(
GetItemsAsString(*text.GetLayoutObject())); "*{'ab', ShapeResult=0+2}\n"
"{''}\n"
"{''}\n",
GetItemsAsString(*text.GetLayoutObject()));
} }
// http://crbug.com/1123251 // http://crbug.com/1123251
......
...@@ -563,7 +563,6 @@ void NGInlineItemsBuilderTemplate< ...@@ -563,7 +563,6 @@ void NGInlineItemsBuilderTemplate<
// after a forced break. // after a forced break.
if (item->Type() != NGInlineItem::kControl || if (item->Type() != NGInlineItem::kControl ||
text_[item->StartOffset()] != kNewlineCharacter) { text_[item->StartOffset()] != kNewlineCharacter) {
can_use_fast_editing_ = false;
AppendGeneratedBreakOpportunity(layout_object); AppendGeneratedBreakOpportunity(layout_object);
} }
} }
...@@ -1236,7 +1235,6 @@ void NGInlineItemsBuilderTemplate< ...@@ -1236,7 +1235,6 @@ void NGInlineItemsBuilderTemplate<
// |SegmentText()| will analyze the text and reset |is_bidi_enabled_| if it // |SegmentText()| will analyze the text and reset |is_bidi_enabled_| if it
// doesn't contain any RTL characters. // doesn't contain any RTL characters.
data->is_bidi_enabled_ = MayBeBidiEnabled(); data->is_bidi_enabled_ = MayBeBidiEnabled();
data->can_use_fast_editing_ = CanUseFastEditing();
// Note: Even if |IsEmptyInline()| is true, |text_| isn't empty, e.g. it // Note: Even if |IsEmptyInline()| is true, |text_| isn't empty, e.g. it
// holds U+FFFC(ORC) for float or abspos. // holds U+FFFC(ORC) for float or abspos.
data->has_line_even_if_empty_ = data->has_line_even_if_empty_ =
......
...@@ -56,9 +56,6 @@ class NGInlineItemsBuilderTemplate { ...@@ -56,9 +56,6 @@ class NGInlineItemsBuilderTemplate {
// Returns whether the items contain any Bidi controls. // Returns whether the items contain any Bidi controls.
bool HasBidiControls() const { return has_bidi_controls_; } bool HasBidiControls() const { return has_bidi_controls_; }
// Returns whether the items contain any generated break opportunity.
bool CanUseFastEditing() const { return can_use_fast_editing_; }
// Returns if the inline node has no content. For example: // Returns if the inline node has no content. For example:
// <span></span> or <span><float></float></span>. // <span></span> or <span><float></float></span>.
bool IsEmptyInline() const { return is_empty_inline_; } bool IsEmptyInline() const { return is_empty_inline_; }
...@@ -185,7 +182,6 @@ class NGInlineItemsBuilderTemplate { ...@@ -185,7 +182,6 @@ class NGInlineItemsBuilderTemplate {
bool has_bidi_controls_ = false; bool has_bidi_controls_ = false;
bool has_ruby_ = false; bool has_ruby_ = false;
bool can_use_fast_editing_ = true;
bool is_empty_inline_ = true; bool is_empty_inline_ = true;
bool is_block_level_ = true; bool is_block_level_ = true;
bool changes_may_affect_earlier_lines_ = false; bool changes_may_affect_earlier_lines_ = false;
......
...@@ -525,7 +525,7 @@ class NGInlineNodeDataEditor final { ...@@ -525,7 +525,7 @@ class NGInlineNodeDataEditor final {
block_flow_->GetDocument().NeedsLayoutTreeUpdate() || block_flow_->GetDocument().NeedsLayoutTreeUpdate() ||
!block_flow_->GetNGInlineNodeData() || !block_flow_->GetNGInlineNodeData() ||
block_flow_->GetNGInlineNodeData()->text_content.IsNull() || block_flow_->GetNGInlineNodeData()->text_content.IsNull() ||
!block_flow_->GetNGInlineNodeData()->CanUseFastEditing()) block_flow_->GetNGInlineNodeData()->items.IsEmpty())
return nullptr; return nullptr;
// Because of current text content has secured text, e.g. whole text is // Because of current text content has secured text, e.g. whole text is
...@@ -544,109 +544,97 @@ class NGInlineNodeDataEditor final { ...@@ -544,109 +544,97 @@ class NGInlineNodeDataEditor final {
const NGOffsetMapping* const offset_mapping = const NGOffsetMapping* const offset_mapping =
NGInlineNode::GetOffsetMapping(block_flow_); NGInlineNode::GetOffsetMapping(block_flow_);
DCHECK(offset_mapping); DCHECK(offset_mapping);
const auto units =
offset_mapping->GetMappingUnitsForLayoutObject(layout_text_);
start_offset_ = ConvertDOMOffsetToTextContent(units, offset);
end_offset_ = ConvertDOMOffsetToTextContent(units, offset + length);
DCHECK_LE(start_offset_, end_offset_);
data_.reset(block_flow_->TakeNGInlineNodeData()); data_.reset(block_flow_->TakeNGInlineNodeData());
return data_.get(); return data_.get();
} }
void Run() { void Run() {
const NGInlineNodeData& new_data = *block_flow_->GetNGInlineNodeData(); const NGInlineNodeData& new_data = *block_flow_->GetNGInlineNodeData();
const int diff = const unsigned old_length = data_->text_content.length();
new_data.text_content.length() - data_->text_content.length(); const unsigned new_length = new_data.text_content.length();
// |inserted_text_length| can be negative when white space is collapsed const unsigned start_offset = Mismatch(*data_, new_data);
// after text change.
// * "ab cd ef" => delete "cd" => "ab ef" // * "ab cd ef" => delete "cd" => "ab ef"
// We should not reuse " " before "ef" // We should not reuse " " before "ef"
// * "a bc" => delete "bc" => "a" // * "a bc" => delete "bc" => "a"
// There are no spaces after "a". // There are no spaces after "a".
const int inserted_text_length = end_offset_ - start_offset_ + diff; const unsigned matched_length = MismatchFromEnd(
DCHECK_GE(inserted_text_length, -1); *data_, new_data,
const unsigned start_offset = std::min(old_length - start_offset, new_length - start_offset));
inserted_text_length < 0 && end_offset_ == data_->text_content.length() DCHECK_LE(start_offset, old_length - matched_length);
? start_offset_ - 1 DCHECK_LE(start_offset, new_length - matched_length);
: start_offset_; const unsigned end_offset = old_length - matched_length;
const unsigned end_offset =
inserted_text_length < 0 && start_offset_ == start_offset
? end_offset_ + 1
: end_offset_;
DCHECK_LE(end_offset, data_->text_content.length());
DCHECK_LE(start_offset, end_offset); DCHECK_LE(start_offset, end_offset);
#if DCHECK_IS_ON()
if (start_offset_ != start_offset) {
DCHECK_EQ(data_->text_content[start_offset], ' ');
DCHECK_EQ(end_offset, end_offset_);
}
if (end_offset_ != end_offset) {
DCHECK_EQ(data_->text_content[end_offset_], ' ');
DCHECK_EQ(start_offset, start_offset_);
}
#endif
Vector<NGInlineItem> items; Vector<NGInlineItem> items;
// +3 for before and after replaced text. // +3 for before and after replaced text.
items.ReserveInitialCapacity(data_->items.size() + 3); items.ReserveInitialCapacity(data_->items.size() + 3);
// Copy items before replaced range // Copy items before replaced range
auto const* end = data_->items.end();
auto* it = data_->items.begin(); auto* it = data_->items.begin();
while (it->end_offset_ < start_offset || while (it != end && it->end_offset_ < start_offset) {
it->layout_object_ != layout_text_) {
DCHECK(it != data_->items.end()); DCHECK(it != data_->items.end());
items.push_back(*it); items.push_back(*it);
++it; ++it;
} }
DCHECK_EQ(it->layout_object_, layout_text_); for (;;) {
if (it == end)
break;
// Copy part of item before replaced range. // Copy part of item before replaced range.
if (it->start_offset_ < start_offset) { if (it->start_offset_ < start_offset) {
const NGInlineItem& new_item = CopyItemBefore(*it, start_offset); const NGInlineItem& new_item = CopyItemBefore(*it, start_offset);
items.push_back(new_item); items.push_back(new_item);
if (new_item.EndOffset() < start_offset) { if (new_item.EndOffset() < start_offset) {
items.push_back( items.push_back(
NGInlineItem(*it, new_item.EndOffset(), start_offset, nullptr)); NGInlineItem(*it, new_item.EndOffset(), start_offset, nullptr));
}
} }
}
// Skip items in replaced range. // Skip items in replaced range.
while (it->end_offset_ < end_offset) while (it != end && it->end_offset_ < end_offset)
++it; ++it;
if (it == end)
break;
// Inserted text // Inserted text
if (it->layout_object_ == layout_text_) { const int diff = new_length - old_length;
if (inserted_text_length > 0) { const unsigned inserted_end = AdjustOffset(end_offset, diff);
const unsigned inserted_start_offset = if (start_offset < inserted_end)
items.IsEmpty() ? 0 : items.back().end_offset_; items.push_back(NGInlineItem(*it, start_offset, inserted_end, nullptr));
const unsigned inserted_end_offset =
inserted_start_offset + inserted_text_length; // Copy part of item after replaced range.
items.push_back(NGInlineItem(*it, inserted_start_offset, if (end_offset < it->end_offset_) {
inserted_end_offset, nullptr)); const NGInlineItem& new_item = CopyItemAfter(*it, end_offset);
if (end_offset < new_item.StartOffset()) {
items.push_back(
NGInlineItem(*it, end_offset, new_item.StartOffset(), nullptr));
ShiftItem(&items.back(), diff);
}
items.push_back(new_item);
ShiftItem(&items.back(), diff);
} }
} else {
DCHECK_LE(inserted_text_length, 0);
}
// Copy part of item after replaced range. // Copy items after replaced range
if (end_offset < it->end_offset_) { ++it;
const NGInlineItem& new_item = CopyItemAfter(*it, end_offset); while (it != end) {
if (end_offset < new_item.StartOffset()) { DCHECK_LE(end_offset, it->start_offset_);
items.push_back( items.push_back(*it);
NGInlineItem(*it, end_offset, new_item.StartOffset(), nullptr));
ShiftItem(&items.back(), diff); ShiftItem(&items.back(), diff);
++it;
} }
items.push_back(new_item); break;
ShiftItem(&items.back(), diff);
} }
// Copy items after replaced range if (items.IsEmpty()) {
++it; items.push_back(NGInlineItem(data_->items.front(), 0,
while (it != data_->items.end()) { new_data.text_content.length(), nullptr));
DCHECK_LE(end_offset, it->start_offset_); } else if (items.back().end_offset_ < new_data.text_content.length()) {
items.push_back(*it); items.push_back(NGInlineItem(data_->items.back(),
ShiftItem(&items.back(), diff); items.back().end_offset_,
++it; new_data.text_content.length(), nullptr));
} }
VerifyItems(items); VerifyItems(items);
...@@ -700,7 +688,6 @@ class NGInlineNodeDataEditor final { ...@@ -700,7 +688,6 @@ class NGInlineNodeDataEditor final {
unsigned end_offset) const { unsigned end_offset) const {
DCHECK_LT(item.start_offset_, end_offset); DCHECK_LT(item.start_offset_, end_offset);
DCHECK_LE(end_offset, item.end_offset_); DCHECK_LE(end_offset, item.end_offset_);
DCHECK_EQ(item.layout_object_, layout_text_);
const unsigned safe_end_offset = GetLastSafeToReuse(item, end_offset); const unsigned safe_end_offset = GetLastSafeToReuse(item, end_offset);
const unsigned start_offset = item.start_offset_; const unsigned start_offset = item.start_offset_;
if (start_offset == safe_end_offset) if (start_offset == safe_end_offset)
...@@ -717,7 +704,6 @@ class NGInlineNodeDataEditor final { ...@@ -717,7 +704,6 @@ class NGInlineNodeDataEditor final {
unsigned end_offset) const { unsigned end_offset) const {
DCHECK_LT(item.start_offset_, end_offset); DCHECK_LT(item.start_offset_, end_offset);
DCHECK_LE(end_offset, item.end_offset_); DCHECK_LE(end_offset, item.end_offset_);
DCHECK_EQ(item.layout_object_, layout_text_);
const unsigned start_offset = item.start_offset_; const unsigned start_offset = item.start_offset_;
if (!item.shape_result_ || item.shape_result_->IsAppliedSpacing() || if (!item.shape_result_ || item.shape_result_->IsAppliedSpacing() ||
end_offset - start_offset <= 1) end_offset - start_offset <= 1)
...@@ -728,6 +714,65 @@ class NGInlineNodeDataEditor final { ...@@ -728,6 +714,65 @@ class NGInlineNodeDataEditor final {
return item.shape_result_->CachedPreviousSafeToBreakOffset(end_offset - 1); return item.shape_result_->CachedPreviousSafeToBreakOffset(end_offset - 1);
} }
template <typename Span1, typename Span2>
static unsigned MismatchInternal(const Span1& span1, const Span2& span2) {
const auto old_new =
std::mismatch(span1.begin(), span1.end(), span2.begin(), span2.end());
return static_cast<unsigned>(old_new.first - span1.begin());
}
static unsigned Mismatch(const NGInlineItemsData& old_data,
const NGInlineItemsData& new_data) {
const StringImpl& old_text = *old_data.text_content.Impl();
const StringImpl& new_text = *new_data.text_content.Impl();
if (old_text.Is8Bit()) {
const auto old_span8 = old_text.Span8();
if (new_text.Is8Bit())
return MismatchInternal(old_span8, new_text.Span8());
return MismatchInternal(old_span8, new_text.Span16());
}
const auto old_span16 = old_text.Span16();
if (new_text.Is8Bit())
return MismatchInternal(old_span16, new_text.Span8());
return MismatchInternal(old_span16, new_text.Span16());
}
template <typename Span1, typename Span2>
static unsigned MismatchFromEnd(const Span1& span1, const Span2& span2) {
const auto old_new = std::mismatch(span1.rbegin(), span1.rend(),
span2.rbegin(), span2.rend());
return static_cast<unsigned>(old_new.first - span1.rbegin());
}
static unsigned MismatchFromEnd(const NGInlineItemsData& old_data,
const NGInlineItemsData& new_data,
unsigned max_length) {
const StringImpl& old_text = *old_data.text_content.Impl();
const StringImpl& new_text = *new_data.text_content.Impl();
const unsigned old_length = old_text.length();
const unsigned new_length = new_text.length();
DCHECK_LE(max_length, old_length);
DCHECK_LE(max_length, new_length);
const unsigned old_start = old_length - max_length;
const unsigned new_start = new_length - max_length;
if (old_text.Is8Bit()) {
const auto old_span8 = old_text.Span8().subspan(old_start, max_length);
if (new_text.Is8Bit()) {
return MismatchFromEnd(old_span8,
new_text.Span8().subspan(new_start, max_length));
}
return MismatchFromEnd(old_span8,
new_text.Span16().subspan(new_start, max_length));
}
const auto old_span16 = old_text.Span16().subspan(old_start, max_length);
if (new_text.Is8Bit()) {
return MismatchFromEnd(old_span16,
new_text.Span8().subspan(new_start, max_length));
}
return MismatchFromEnd(old_span16,
new_text.Span16().subspan(new_start, max_length));
}
static void ShiftItem(NGInlineItem* item, int delta) { static void ShiftItem(NGInlineItem* item, int delta) {
if (delta == 0) if (delta == 0)
return; return;
...@@ -748,7 +793,7 @@ class NGInlineNodeDataEditor final { ...@@ -748,7 +793,7 @@ class NGInlineNodeDataEditor final {
DCHECK_LE(item.start_offset_, item.end_offset_); DCHECK_LE(item.start_offset_, item.end_offset_);
DCHECK_EQ(last_offset, item.start_offset_); DCHECK_EQ(last_offset, item.start_offset_);
last_offset = item.end_offset_; last_offset = item.end_offset_;
if (!item.shape_result_ || item.layout_object_ != layout_text_) if (!item.shape_result_)
continue; continue;
DCHECK_LT(item.start_offset_, item.end_offset_); DCHECK_LT(item.start_offset_, item.end_offset_);
DCHECK_EQ(item.shape_result_->StartIndex(), item.start_offset_); DCHECK_EQ(item.shape_result_->StartIndex(), item.start_offset_);
...@@ -762,8 +807,6 @@ class NGInlineNodeDataEditor final { ...@@ -762,8 +807,6 @@ class NGInlineNodeDataEditor final {
std::unique_ptr<NGInlineNodeData> data_; std::unique_ptr<NGInlineNodeData> data_;
LayoutBlockFlow* const block_flow_; LayoutBlockFlow* const block_flow_;
const LayoutText& layout_text_; const LayoutText& layout_text_;
unsigned start_offset_ = 0;
unsigned end_offset_ = 0;
}; };
// static // static
......
...@@ -22,7 +22,6 @@ struct CORE_EXPORT NGInlineNodeData : NGInlineItemsData { ...@@ -22,7 +22,6 @@ struct CORE_EXPORT NGInlineNodeData : NGInlineItemsData {
return static_cast<TextDirection>(base_direction_); return static_cast<TextDirection>(base_direction_);
} }
bool CanUseFastEditing() const { return can_use_fast_editing_; }
bool HasLineEvenIfEmpty() const { return has_line_even_if_empty_; } bool HasLineEvenIfEmpty() const { return has_line_even_if_empty_; }
bool HasRuby() const { return has_ruby_; } bool HasRuby() const { return has_ruby_; }
bool IsEmptyInline() const { return is_empty_inline_; } bool IsEmptyInline() const { return is_empty_inline_; }
...@@ -59,9 +58,6 @@ struct CORE_EXPORT NGInlineNodeData : NGInlineItemsData { ...@@ -59,9 +58,6 @@ struct CORE_EXPORT NGInlineNodeData : NGInlineItemsData {
unsigned is_bidi_enabled_ : 1; unsigned is_bidi_enabled_ : 1;
unsigned base_direction_ : 1; // TextDirection unsigned base_direction_ : 1; // TextDirection
// True if we can use fast editing path.
unsigned can_use_fast_editing_ : 1;
// True if there are no inline item items and the associated block is root // True if there are no inline item items and the associated block is root
// editable element or having "-internal-empty-line-height:fabricated", // editable element or having "-internal-empty-line-height:fabricated",
// e.g. <div contenteditable></div>, <input type=button value=""> // e.g. <div contenteditable></div>, <input type=button value="">
......
<!doctype html>
<script src="../../resources/ahem.js"></script>
<style>
.sample {
border: solid 1px; green;
font: 20px monospace;
padding: 5px;
white-space: pre-wrap;
}
</style>
<div class="sample" id="target">XYZ bc</div>
<br>
<div class="sample">XYZ bc</div>
<!doctype html>
<script src="../../resources/ahem.js"></script>
<style>
.sample {
border: solid 1px; green;
font: 20px monospace;
padding: 5px;
white-space: pre-wrap;
}
</style>
<div class="sample" id="target"> bc</div>
<br>
<div class="sample">XYZ bc</div>
<script>
const target = document.getElementById('target');
const text = target.firstChild;
document.body.offsetHeight;
text.insertData(0, 'XYZ');
</script>
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment