Commit a5c22c52 authored by Vladimir Levin's avatar Vladimir Levin Committed by Chromium LUCI CQ

LayoutShiftTracker: Reset post-prepaint state even if we early out.

This patch ensures that we reset some state that we accumulate and
only reset if we actually report a layout shift.

The problem is that the accumulated values can later cause us _not_
to early out from ObjectShifted and report a layout shift where we
should not have reported one.

By resetting the state after every prepaint, we ensure that we reset
the accumulated state.

R=skobes@chromium.org

Change-Id: I2e764d3d847669cbdf06219da183cda02df57684
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2639862
Commit-Queue: vmpstr <vmpstr@chromium.org>
Reviewed-by: default avatarSteve Kobes <skobes@chromium.org>
Cr-Commit-Position: refs/heads/master@{#845904}
parent 12ca52fd
...@@ -423,7 +423,7 @@ double LayoutShiftTracker::SubframeWeightingFactor() const { ...@@ -423,7 +423,7 @@ double LayoutShiftTracker::SubframeWeightingFactor() const {
main_frame_size.Area(); main_frame_size.Area();
} }
void LayoutShiftTracker::NotifyPrePaintFinished() { void LayoutShiftTracker::NotifyPrePaintFinishedInternal() {
if (!is_active_) if (!is_active_)
return; return;
if (region_.IsEmpty()) if (region_.IsEmpty())
...@@ -465,8 +465,13 @@ void LayoutShiftTracker::NotifyPrePaintFinished() { ...@@ -465,8 +465,13 @@ void LayoutShiftTracker::NotifyPrePaintFinished() {
if (!region_.IsEmpty()) if (!region_.IsEmpty())
SetLayoutShiftRects(region_.GetRects()); SetLayoutShiftRects(region_.GetRects());
region_.Reset(); }
void LayoutShiftTracker::NotifyPrePaintFinished() {
NotifyPrePaintFinishedInternal();
// Reset accumulated state.
region_.Reset();
frame_max_distance_ = 0.0; frame_max_distance_ = 0.0;
frame_scroll_delta_ = ScrollOffset(); frame_scroll_delta_ = ScrollOffset();
attributions_.fill(Attribution()); attributions_.fill(Attribution());
......
...@@ -157,6 +157,7 @@ class CORE_EXPORT LayoutShiftTracker final ...@@ -157,6 +157,7 @@ class CORE_EXPORT LayoutShiftTracker final
void UpdateInputTimestamp(base::TimeTicks timestamp); void UpdateInputTimestamp(base::TimeTicks timestamp);
LayoutShift::AttributionList CreateAttributionList() const; LayoutShift::AttributionList CreateAttributionList() const;
void SubmitPerformanceEntry(double score_delta, bool input_detected) const; void SubmitPerformanceEntry(double score_delta, bool input_detected) const;
void NotifyPrePaintFinishedInternal();
Member<LocalFrameView> frame_view_; Member<LocalFrameView> frame_view_;
bool is_active_; bool is_active_;
......
...@@ -5,6 +5,7 @@ ...@@ -5,6 +5,7 @@
#include "third_party/blink/renderer/core/layout/layout_shift_tracker.h" #include "third_party/blink/renderer/core/layout/layout_shift_tracker.h"
#include "third_party/blink/public/common/input/web_mouse_event.h" #include "third_party/blink/public/common/input/web_mouse_event.h"
#include "third_party/blink/renderer/core/dom/dom_token_list.h"
#include "third_party/blink/renderer/core/frame/local_dom_window.h" #include "third_party/blink/renderer/core/frame/local_dom_window.h"
#include "third_party/blink/renderer/core/frame/visual_viewport.h" #include "third_party/blink/renderer/core/frame/visual_viewport.h"
#include "third_party/blink/renderer/core/frame/web_local_frame_impl.h" #include "third_party/blink/renderer/core/frame/web_local_frame_impl.h"
...@@ -623,4 +624,46 @@ TEST_F(LayoutShiftTrackerTest, ClipByVisualViewport) { ...@@ -623,4 +624,46 @@ TEST_F(LayoutShiftTrackerTest, ClipByVisualViewport) {
GetLayoutShiftTracker().Score()); GetLayoutShiftTracker().Score());
} }
TEST_F(LayoutShiftTrackerTest, ScrollThenCauseScrollAnchoring) {
SetBodyInnerHTML(R"HTML(
<style>
.big {
width: 100px;
height: 500px;
background: blue;
}
.small {
width: 100px;
height: 100px;
background: green;
}
</style>
<div class=big id=target></div>
<div class=big></div>
<div class=big></div>
<div class=big></div>
<div class=big></div>
<div class=big></div>
)HTML");
auto* target_element = GetDocument().getElementById("target");
// Scroll the window which accumulates a scroll in the layout shift tracker.
GetDocument().domWindow()->scrollBy(0, 1000);
UpdateAllLifecyclePhasesForTest();
EXPECT_FLOAT_EQ(0, GetLayoutShiftTracker().Score());
target_element->classList().Remove("big");
target_element->classList().Add("small");
UpdateAllLifecyclePhasesForTest();
EXPECT_FLOAT_EQ(0, GetLayoutShiftTracker().Score());
target_element->classList().Remove("small");
target_element->classList().Add("big");
UpdateAllLifecyclePhasesForTest();
EXPECT_FLOAT_EQ(0, GetLayoutShiftTracker().Score());
}
} // namespace blink } // namespace blink
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