Commit 94766d09 authored by Yoichi Osato's avatar Yoichi Osato Committed by Commit Bot

Revert "Let SelectionPaintRange iterator iterate on a flat tree."

This reverts commit deb6bb60.

Reason for revert: This causes much crash on clusterfuzz.
Original change's description:
> Let SelectionPaintRange iterator iterate on a flat tree.
> 
> SelectionPaintRange::Iterator iterated LayoutObjects using layout order
> but we marks SelectionStatus on flat tree order.
> This causes invalidation leak if they are not same order.
> Ruby element is a reported example for that.
> 
> This patch changes SelectionPaintRange::Iterator iterate on a flat tree
> considering first-letter.
> 
> Bug: 843144
> Cq-Include-Trybots: luci.chromium.try:linux_layout_tests_layout_ng
> Change-Id: I02cbad86e64d0a7781f8fb37e2d13c7aa00228fb
> Reviewed-on: https://chromium-review.googlesource.com/1063521
> Commit-Queue: Yoichi Osato <yoichio@chromium.org>
> Reviewed-by: Xiaocheng Hu <xiaochengh@chromium.org>
> Reviewed-by: Yoshifumi Inoue <yosin@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#569168}

TBR=yosin@chromium.org,yoichio@chromium.org,xiaochengh@chromium.org

# Not skipping CQ checks because original CL landed > 1 day ago.

Bug: 843144, 855026
Change-Id: I745ab57ed70bd10e59bac20cf4f6fd591e170abd
Cq-Include-Trybots: luci.chromium.try:linux_layout_tests_layout_ng
Reviewed-on: https://chromium-review.googlesource.com/1118098Reviewed-by: default avatarYoichi Osato <yoichio@chromium.org>
Reviewed-by: default avatarYoshifumi Inoue <yosin@chromium.org>
Commit-Queue: Yoichi Osato <yoichio@chromium.org>
Cr-Commit-Position: refs/heads/master@{#571076}
parent e2decead
...@@ -76,49 +76,13 @@ base::Optional<unsigned> SelectionPaintRange::EndOffset() const { ...@@ -76,49 +76,13 @@ base::Optional<unsigned> SelectionPaintRange::EndOffset() const {
return end_offset_; return end_offset_;
} }
static LayoutTextFragment* FirstLetterPartFor(
const LayoutObject* layout_object) {
if (!layout_object->IsText())
return nullptr;
if (!ToLayoutText(layout_object)->IsTextFragment())
return nullptr;
return ToLayoutTextFragment(const_cast<LayoutObject*>(
AssociatedLayoutObjectOf(*layout_object->GetNode(), 0)));
}
static LayoutObject* NextLayoutObjectOnFlatTree(
const LayoutObject& layout_object) {
// If |layout_object| is a first letter part, return remaining part.
if (layout_object.IsText() && ToLayoutText(layout_object).IsTextFragment() &&
!ToLayoutTextFragment(layout_object).IsRemainingTextLayoutObject()) {
const Node* associated_node =
ToLayoutTextFragment(layout_object).AssociatedTextNode();
if (!associated_node)
return nullptr;
return associated_node->GetLayoutObject();
}
// Otherwise, find the first layouted object of following nodes.
const Node* node = layout_object.GetNode();
DCHECK(node);
for (const Node* next = FlatTreeTraversal::Next(*node); next;
next = FlatTreeTraversal::Next(*next)) {
LayoutObject* layout_object = next->GetLayoutObject();
if (!layout_object)
continue;
if (LayoutObject* first_letter = FirstLetterPartFor(layout_object))
return first_letter;
return layout_object;
}
return nullptr;
}
SelectionPaintRange::Iterator::Iterator(const SelectionPaintRange* range) { SelectionPaintRange::Iterator::Iterator(const SelectionPaintRange* range) {
if (!range || range->IsNull()) { if (!range || range->IsNull()) {
current_ = nullptr; current_ = nullptr;
return; return;
} }
current_ = range->StartLayoutObject(); current_ = range->StartLayoutObject();
stop_ = NextLayoutObjectOnFlatTree(*range->EndLayoutObject()); stop_ = range->EndLayoutObject()->NextInPreOrder();
} }
LayoutObject* SelectionPaintRange::Iterator::operator*() const { LayoutObject* SelectionPaintRange::Iterator::operator*() const {
...@@ -128,7 +92,7 @@ LayoutObject* SelectionPaintRange::Iterator::operator*() const { ...@@ -128,7 +92,7 @@ LayoutObject* SelectionPaintRange::Iterator::operator*() const {
SelectionPaintRange::Iterator& SelectionPaintRange::Iterator::operator++() { SelectionPaintRange::Iterator& SelectionPaintRange::Iterator::operator++() {
DCHECK(current_); DCHECK(current_);
current_ = NextLayoutObjectOnFlatTree(*current_); current_ = current_->NextInPreOrder();
if (current_ && current_ != stop_) if (current_ && current_ != stop_)
return *this; return *this;
...@@ -461,6 +425,15 @@ static base::Optional<unsigned> ComputeEndOffset( ...@@ -461,6 +425,15 @@ static base::Optional<unsigned> ComputeEndOffset(
return ToText(layout_node)->length(); return ToText(layout_node)->length();
} }
static LayoutTextFragment* FirstLetterPartFor(LayoutObject* layout_object) {
if (!layout_object->IsText())
return nullptr;
if (!ToLayoutText(layout_object)->IsTextFragment())
return nullptr;
return ToLayoutTextFragment(const_cast<LayoutObject*>(
AssociatedLayoutObjectOf(*layout_object->GetNode(), 0)));
}
static void MarkSelected(SelectedLayoutObjects* selected_objects, static void MarkSelected(SelectedLayoutObjects* selected_objects,
LayoutObject* layout_object, LayoutObject* layout_object,
SelectionState state) { SelectionState state) {
......
...@@ -76,20 +76,18 @@ using IsTypeOfSimple = bool(const LayoutObject& layout_object); ...@@ -76,20 +76,18 @@ using IsTypeOfSimple = bool(const LayoutObject& layout_object);
return layout_object.member_func(); \ return layout_object.member_func(); \
} }
USING_LAYOUTOBJECT_FUNC(IsBR);
USING_LAYOUTOBJECT_FUNC(IsLayoutBlock); USING_LAYOUTOBJECT_FUNC(IsLayoutBlock);
USING_LAYOUTOBJECT_FUNC(IsLayoutBlockFlow); USING_LAYOUTOBJECT_FUNC(IsLayoutBlockFlow);
USING_LAYOUTOBJECT_FUNC(IsLayoutButton);
USING_LAYOUTOBJECT_FUNC(IsLayoutEmbeddedContent);
USING_LAYOUTOBJECT_FUNC(IsLayoutImage);
USING_LAYOUTOBJECT_FUNC(IsLayoutInline);
USING_LAYOUTOBJECT_FUNC(IsLayoutNGBlockFlow); USING_LAYOUTOBJECT_FUNC(IsLayoutNGBlockFlow);
USING_LAYOUTOBJECT_FUNC(IsLayoutInline);
USING_LAYOUTOBJECT_FUNC(IsBR);
USING_LAYOUTOBJECT_FUNC(IsListItem); USING_LAYOUTOBJECT_FUNC(IsListItem);
USING_LAYOUTOBJECT_FUNC(IsListMarker); USING_LAYOUTOBJECT_FUNC(IsListMarker);
USING_LAYOUTOBJECT_FUNC(IsRuby); USING_LAYOUTOBJECT_FUNC(IsLayoutImage);
USING_LAYOUTOBJECT_FUNC(IsRubyText); USING_LAYOUTOBJECT_FUNC(IsLayoutButton);
USING_LAYOUTOBJECT_FUNC(IsSVGRoot); USING_LAYOUTOBJECT_FUNC(IsSVGRoot);
USING_LAYOUTOBJECT_FUNC(IsSVGText); USING_LAYOUTOBJECT_FUNC(IsSVGText);
USING_LAYOUTOBJECT_FUNC(IsLayoutEmbeddedContent);
static IsTypeOf IsLayoutTextFragmentOf(const String& text) { static IsTypeOf IsLayoutTextFragmentOf(const String& text) {
return WTF::BindRepeating( return WTF::BindRepeating(
...@@ -688,41 +686,6 @@ TEST_F(LayoutSelectionTest, Embed) { ...@@ -688,41 +686,6 @@ TEST_F(LayoutSelectionTest, Embed) {
TEST_NO_NEXT_LAYOUT_OBJECT(); TEST_NO_NEXT_LAYOUT_OBJECT();
} }
// http:/crbug.com/843144
TEST_F(LayoutSelectionTest, Ruby) {
Selection().SetSelectionAndEndTyping(
SetSelectionTextToBody("^<ruby>foo<rt>bar</rt></ruby>|"));
Selection().CommitAppearanceIfNeeded();
TEST_NEXT(IsLayoutBlock, kContain, NotInvalidate);
TEST_NEXT(IsRuby, kNone, NotInvalidate);
TEST_NEXT(IsLayoutBlock, kContain, NotInvalidate);
TEST_NEXT(IsRubyText, kContain, NotInvalidate);
TEST_NEXT("bar", kEnd, ShouldInvalidate);
TEST_NEXT(IsLayoutBlock, kContain, NotInvalidate);
TEST_NEXT("foo", kStart, ShouldInvalidate);
TEST_NO_NEXT_LAYOUT_OBJECT();
UpdateAllLifecyclePhases();
TEST_NEXT(IsLayoutBlock, kContain, NotInvalidate);
TEST_NEXT(IsRuby, kNone, NotInvalidate);
TEST_NEXT(IsLayoutBlock, kContain, NotInvalidate);
TEST_NEXT(IsRubyText, kContain, NotInvalidate);
TEST_NEXT("bar", kEnd, NotInvalidate);
TEST_NEXT(IsLayoutBlock, kContain, NotInvalidate);
TEST_NEXT("foo", kStart, NotInvalidate);
TEST_NO_NEXT_LAYOUT_OBJECT();
Selection().ClearLayoutSelection();
TEST_NEXT(IsLayoutBlock, kNone, NotInvalidate);
TEST_NEXT(IsRuby, kNone, NotInvalidate);
TEST_NEXT(IsLayoutBlock, kNone, NotInvalidate);
TEST_NEXT(IsRubyText, kNone, NotInvalidate);
TEST_NEXT("bar", kNone, ShouldInvalidate);
TEST_NEXT(IsLayoutBlock, kNone, NotInvalidate);
TEST_NEXT("foo", kNone, ShouldInvalidate);
TEST_NO_NEXT_LAYOUT_OBJECT();
}
static const NGPaintFragment* FindNGPaintFragmentInternal( static const NGPaintFragment* FindNGPaintFragmentInternal(
const NGPaintFragment* paint, const NGPaintFragment* paint,
const LayoutObject* layout_object) { const LayoutObject* layout_object) {
......
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