Skip to content

Commit

Permalink
Merge "DeferredShaping: Fix stale references to LayoutObjects in NGPh…
Browse files Browse the repository at this point in the history
…ysicalFragments" to M103 branch

crrev.com/986592 added a ClearLayoutResults() call. However it didn't
clear LayoutObject fields of NGPhysicalFragments in the LayoutResult,
and they could be stale if the LayoutObject is destroyed.

This CL clears the LayoutObject fields before ClearLayoutResults().

(cherry picked from commit 9e8b4ac)

Bug: 1327891
Change-Id: I4f780910c98462cb87a3e44dcef88f08338e61a0
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3662744
Reviewed-by: Koji Ishii <kojii@chromium.org>
Auto-Submit: Kent Tamura <tkent@chromium.org>
Commit-Queue: Koji Ishii <kojii@chromium.org>
Cr-Original-Commit-Position: refs/heads/main@{#1006775}
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3668408
Bot-Commit: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
Commit-Queue: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
Cr-Commit-Position: refs/branch-heads/5060@{#253}
Cr-Branched-From: b83393d-refs/heads/main@{#1002911}
  • Loading branch information
tkent-google authored and Chromium LUCI CQ committed May 26, 2022
1 parent c354d88 commit e874701
Show file tree
Hide file tree
Showing 4 changed files with 34 additions and 9 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -660,6 +660,7 @@ bool DisplayLockContext::MarkForLayoutIfNeeded() {
layout_object->SetIntrinsicLogicalWidthsDirty();
layout_object->SetChildNeedsLayout();
// Make sure we don't use cached NGFragmentItem objects.
To<LayoutBox>(layout_object)->DisassociatePhysicalFragments();
To<LayoutBox>(layout_object)->ClearLayoutResults();
}
}
Expand Down
18 changes: 18 additions & 0 deletions third_party/blink/renderer/core/layout/deferred_shaping_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -244,6 +244,24 @@ MMM MMMMM MMMMM MMM MMMMM MMMM MMM MMMM MMM.
.width);
}

// crbug.com/1327891
TEST_F(DeferredShapingTest, FragmentAssociationAfterUnlock) {
SetBodyInnerHTML(R"HTML(
<div style="height:1800px"></div>
<div id="target">IFC</div>)HTML");
UpdateAllLifecyclePhasesForTest();
EXPECT_TRUE(IsDefer("target"));
EXPECT_TRUE(IsLocked("target"));
auto* box = GetLayoutBoxByElementId("target");
auto* fragment = box->GetPhysicalFragment(0);
EXPECT_EQ(box, fragment->GetLayoutObject());

ScrollAndWaitForIntersectionCheck(1800);
EXPECT_FALSE(IsDefer("target"));
EXPECT_FALSE(IsLocked("target"));
EXPECT_EQ(nullptr, fragment->GetLayoutObject());
}

TEST_F(DeferredShapingTest, UpdateTextInDeferred) {
SetBodyInnerHTML(R"HTML(
<div style="height:1800px"></div>
Expand Down
22 changes: 13 additions & 9 deletions third_party/blink/renderer/core/layout/layout_box.cc
Original file line number Diff line number Diff line change
Expand Up @@ -496,14 +496,7 @@ void LayoutBox::WillBeDestroyed() {
ShapeOutsideInfo::RemoveInfo(*this);

if (!DocumentBeingDestroyed()) {
if (FirstInlineFragmentItemIndex()) {
NGFragmentItems::LayoutObjectWillBeDestroyed(*this);
ClearFirstInlineFragmentItemIndex();
}
if (measure_result_)
measure_result_->PhysicalFragment().LayoutObjectWillBeDestroyed();
for (auto result : layout_results_)
result->PhysicalFragment().LayoutObjectWillBeDestroyed();
DisassociatePhysicalFragments();
GetDocument()
.GetFrame()
->GetInputMethodController()
Expand All @@ -514,6 +507,17 @@ void LayoutBox::WillBeDestroyed() {
LayoutBoxModelObject::WillBeDestroyed();
}

void LayoutBox::DisassociatePhysicalFragments() {
if (FirstInlineFragmentItemIndex()) {
NGFragmentItems::LayoutObjectWillBeDestroyed(*this);
ClearFirstInlineFragmentItemIndex();
}
if (measure_result_)
measure_result_->PhysicalFragment().LayoutObjectWillBeDestroyed();
for (auto result : layout_results_)
result->PhysicalFragment().LayoutObjectWillBeDestroyed();
}

void LayoutBox::InsertedIntoTree() {
NOT_DESTROYED();
LayoutBoxModelObject::InsertedIntoTree();
Expand Down Expand Up @@ -3407,7 +3411,7 @@ void LayoutBox::InvalidateItems(const NGLayoutResult& result) {
// Column fragments are not really associated with a layout object.
if (IsLayoutFlowThread())
DCHECK(box_fragment.IsColumnBox());
else
else if (!IsShapingDeferred())
DCHECK_EQ(this, box_fragment.GetLayoutObject());
#endif
ObjectPaintInvalidator(*this).SlowSetPaintingLayerNeedsRepaint();
Expand Down
2 changes: 2 additions & 0 deletions third_party/blink/renderer/core/layout/layout_box.h
Original file line number Diff line number Diff line change
Expand Up @@ -1192,6 +1192,8 @@ class CORE_EXPORT LayoutBox : public LayoutBoxModelObject {
void RestoreLegacyLayoutResults(const NGLayoutResult* measure_result,
const NGLayoutResult* layout_result);
void ClearLayoutResults();
// Clear LayoutObject fields of physical fragments.
void DisassociatePhysicalFragments();

// Call when NG fragment count or size changed. Only call if the fragment
// count is or was larger than 1.
Expand Down

0 comments on commit e874701

Please sign in to comment.