Commit 2477aab0 authored by Chris Harrelson's avatar Chris Harrelson Committed by Chromium LUCI CQ

[CLS] Fix bug when recording paint offset translation deltas

Previously, a negative delta was added to
additional_offset_to_layout_shift_root_delta to account for a
paint offset translation transform on an object from the prior frame,
which is meant to be added to the new paint offset translation and
result in a delta. However, this negative delta was added to the
containing block context, which is incorrect in cases when the
current object is not a containing block for some descendants, such
as nested fixed-position elements (there are other cases as well).

Instead, store off the negative delta and apply it to the fragment
context once it has been updated for the current object.

Bug: 1141739

Change-Id: Ic667aff457be0a192de7f92a424996bcded47b10
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2586969Reviewed-by: default avatarXianzhu Wang <wangxianzhu@chromium.org>
Commit-Queue: Chris Harrelson <chrishtr@chromium.org>
Cr-Commit-Position: refs/heads/master@{#836416}
parent 6ef3ba86
...@@ -556,4 +556,24 @@ TEST_F(LayoutShiftTrackerTest, ...@@ -556,4 +556,24 @@ TEST_F(LayoutShiftTrackerTest,
EXPECT_EQ(LayoutSize(100, 100), onscreen->Size()); EXPECT_EQ(LayoutSize(100, 100), onscreen->Size());
} }
TEST_F(LayoutShiftTrackerTest, NestedFixedPos) {
SetBodyInnerHTML(R"HTML(
<div id=parent style="position: fixed; top: 0; left: -100%; width: 100%">
<div id=target style="position: fixed; top: 0; width: 100%; height: 100%;
left: 0"></div>
</div>
<div style="height: 5000px"></div>
</div>
)HTML");
auto* target = To<LayoutBox>(GetLayoutObjectByElementId("target"));
EXPECT_FLOAT_EQ(0, GetLayoutShiftTracker().Score());
// Test that repaint of #target does not record a layout shift.
target->SetNeedsPaintPropertyUpdate();
target->SetSubtreeShouldDoFullPaintInvalidation();
UpdateAllLifecyclePhasesForTest();
EXPECT_FLOAT_EQ(0, GetLayoutShiftTracker().Score());
}
} // namespace blink } // namespace blink
...@@ -2597,6 +2597,11 @@ void FragmentPaintPropertyTreeBuilder::UpdatePaintOffset() { ...@@ -2597,6 +2597,11 @@ void FragmentPaintPropertyTreeBuilder::UpdatePaintOffset() {
} }
context_.current.paint_offset += context_.repeating_paint_offset_adjustment; context_.current.paint_offset += context_.repeating_paint_offset_adjustment;
context_.current.additional_offset_to_layout_shift_root_delta +=
context_.pending_additional_offset_to_layout_shift_root_delta;
context_.pending_additional_offset_to_layout_shift_root_delta =
PhysicalOffset();
} }
void FragmentPaintPropertyTreeBuilder::SetNeedsPaintPropertyUpdateIfNeeded() { void FragmentPaintPropertyTreeBuilder::SetNeedsPaintPropertyUpdateIfNeeded() {
...@@ -2829,13 +2834,15 @@ void PaintPropertyTreeBuilder::InitFragmentPaintProperties( ...@@ -2829,13 +2834,15 @@ void PaintPropertyTreeBuilder::InitFragmentPaintProperties(
PaintPropertyTreeBuilderFragmentContext& context) { PaintPropertyTreeBuilderFragmentContext& context) {
if (const auto* properties = fragment.PaintProperties()) { if (const auto* properties = fragment.PaintProperties()) {
if (const auto* translation = properties->PaintOffsetTranslation()) { if (const auto* translation = properties->PaintOffsetTranslation()) {
auto* containing_block_context = &context.current; // If there is a paint offset translation, it only causes a net change
if (object_.StyleRef().GetPosition() == EPosition::kAbsolute) // in additional_offset_to_layout_shift_root_delta by the amount the
containing_block_context = &context.absolute_position; // paint offset translation changed from the prior frame. To implement
else if (object_.StyleRef().GetPosition() == EPosition::kFixed) // this, we record a negative offset here, and then re-add it in
containing_block_context = &context.fixed_position; // UpdatePaintOffsetTranslation. The net effect is that the value
containing_block_context->additional_offset_to_layout_shift_root_delta -= // of additional_offset_to_layout_shift_root_delta is the difference
PhysicalOffset::FromFloatSizeRound(translation->Translation2D()); // between the old and new paint offset translation.
context.pending_additional_offset_to_layout_shift_root_delta =
-PhysicalOffset::FromFloatSizeRound(translation->Translation2D());
} }
} }
......
...@@ -142,6 +142,12 @@ struct PaintPropertyTreeBuilderFragmentContext { ...@@ -142,6 +142,12 @@ struct PaintPropertyTreeBuilderFragmentContext {
PhysicalOffset repeating_paint_offset_adjustment; PhysicalOffset repeating_paint_offset_adjustment;
PhysicalOffset old_paint_offset; PhysicalOffset old_paint_offset;
// An additional offset that applies to the current fragment, but is detected
// *before* the ContainingBlockContext is updated for it. Once the
// ContainingBlockContext is set, this value should be added to
// ContainingBlockContext::additional_offset_to_layout_shift_root_delta.
PhysicalOffset pending_additional_offset_to_layout_shift_root_delta;
}; };
struct PaintPropertyTreeBuilderContext { struct PaintPropertyTreeBuilderContext {
......
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