Ensure JankTracker uses object's PropertyTreeState
Previously, we used the PaintLayer's LayoutObject.
Xianzhu pointed out that there can be cases where a LayoutObject does
not have its own PaintLayer, and that we should instead use the
PropertyTreeState directly associated with the currently painting
object.
(cherry picked from commit 1d53d79569b70f1cf90729814b78ce251d0b58fa)
Change-Id: I4daf0d1f03507ca45f3e78bf2d3f93b4b6848efd
Bug: 971639
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1644939
Reviewed-by: Xianzhu Wang <wangxianzhu@chromium.org>
Commit-Queue: Bryan McQuade <bmcquade@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#667309}
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1660699
Reviewed-by: Bryan McQuade <bmcquade@chromium.org>
Cr-Commit-Position: refs/branch-heads/3809@{#318}
Cr-Branched-From: d82dec1a818f378c464ba307ddd9c92133eac355-refs/heads/master@{#665002}
diff --git a/third_party/blink/renderer/core/layout/jank_tracker.cc b/third_party/blink/renderer/core/layout/jank_tracker.cc
index 8829462..5a91de35 100644
--- a/third_party/blink/renderer/core/layout/jank_tracker.cc
+++ b/third_party/blink/renderer/core/layout/jank_tracker.cc
@@ -15,7 +15,6 @@
#include "third_party/blink/renderer/core/layout/layout_view.h"
#include "third_party/blink/renderer/core/page/chrome_client.h"
#include "third_party/blink/renderer/core/page/page.h"
-#include "third_party/blink/renderer/core/paint/paint_layer.h"
#include "third_party/blink/renderer/core/timing/dom_window_performance.h"
#include "third_party/blink/renderer/core/timing/performance_entry.h"
#include "third_party/blink/renderer/core/timing/window_performance.h"
@@ -71,7 +70,8 @@
rect.Height() * granularity_scale < 0.5;
}
-static const PropertyTreeState PropertyTreeStateFor(LayoutObject& object) {
+static const PropertyTreeState PropertyTreeStateFor(
+ const LayoutObject& object) {
return object.FirstFragment().LocalBorderBoxProperties();
}
@@ -118,7 +118,7 @@
observed_input_or_scroll_(false) {}
void JankTracker::AccumulateJank(const LayoutObject& source,
- const PaintLayer& painting_layer,
+ const PropertyTreeState& property_tree_state,
FloatRect old_rect,
FloatRect new_rect) {
if (old_rect.IsEmpty() || new_rect.IsEmpty())
@@ -148,21 +148,19 @@
if (source.IsSVG())
return;
- const auto local_state =
- PropertyTreeStateFor(painting_layer.GetLayoutObject());
const auto root_state = PropertyTreeStateFor(*source.View());
FloatClipRect clip_rect =
- GeometryMapper::LocalToAncestorClipRect(local_state, root_state);
+ GeometryMapper::LocalToAncestorClipRect(property_tree_state, root_state);
// If the clip region is empty, then the resulting layout shift isn't visible
// in the viewport so ignore it.
if (!clip_rect.IsInfinite() && clip_rect.Rect().IsEmpty())
return;
- GeometryMapper::SourceToDestinationRect(local_state.Transform(),
+ GeometryMapper::SourceToDestinationRect(property_tree_state.Transform(),
root_state.Transform(), old_rect);
- GeometryMapper::SourceToDestinationRect(local_state.Transform(),
+ GeometryMapper::SourceToDestinationRect(property_tree_state.Transform(),
root_state.Transform(), new_rect);
FloatRect clipped_old_rect(old_rect), clipped_new_rect(new_rect);
@@ -208,24 +206,25 @@
}
}
-void JankTracker::NotifyObjectPrePaint(const LayoutObject& object,
- const IntRect& old_visual_rect,
- const PaintLayer& painting_layer) {
+void JankTracker::NotifyObjectPrePaint(
+ const LayoutObject& object,
+ const PropertyTreeState& property_tree_state,
+ const IntRect& old_visual_rect,
+ const IntRect& new_visual_rect) {
if (!IsActive())
return;
- AccumulateJank(object, painting_layer, FloatRect(old_visual_rect),
- FloatRect(object.FirstFragment().VisualRect()));
+ AccumulateJank(object, property_tree_state, FloatRect(old_visual_rect),
+ FloatRect(new_visual_rect));
}
-void JankTracker::NotifyCompositedLayerMoved(const PaintLayer& paint_layer,
+void JankTracker::NotifyCompositedLayerMoved(const LayoutObject& layout_object,
FloatRect old_layer_rect,
FloatRect new_layer_rect) {
if (!IsActive())
return;
// Make sure we can access a transform node.
- LayoutObject& layout_object = paint_layer.GetLayoutObject();
if (!layout_object.FirstFragment().HasLocalBorderBoxProperties())
return;
@@ -235,7 +234,8 @@
old_layer_rect.MoveBy(transform_parent_offset);
new_layer_rect.MoveBy(transform_parent_offset);
- AccumulateJank(layout_object, paint_layer, old_layer_rect, new_layer_rect);
+ AccumulateJank(layout_object, PropertyTreeStateFor(layout_object),
+ old_layer_rect, new_layer_rect);
}
double JankTracker::SubframeWeightingFactor() const {
diff --git a/third_party/blink/renderer/core/layout/jank_tracker.h b/third_party/blink/renderer/core/layout/jank_tracker.h
index e38fcc7..c44a7217 100644
--- a/third_party/blink/renderer/core/layout/jank_tracker.h
+++ b/third_party/blink/renderer/core/layout/jank_tracker.h
@@ -17,7 +17,7 @@
class IntRect;
class LayoutObject;
class LocalFrameView;
-class PaintLayer;
+class PropertyTreeState;
class TracedValue;
class WebInputEvent;
@@ -30,9 +30,10 @@
JankTracker(LocalFrameView*);
~JankTracker() {}
void NotifyObjectPrePaint(const LayoutObject& object,
+ const PropertyTreeState& property_tree_state,
const IntRect& old_visual_rect,
- const PaintLayer& painting_layer);
- void NotifyCompositedLayerMoved(const PaintLayer&,
+ const IntRect& new_visual_rect);
+ void NotifyCompositedLayerMoved(const LayoutObject& object,
FloatRect old_layer_rect,
FloatRect new_layer_rect);
void NotifyPrePaintFinished();
@@ -48,7 +49,7 @@
private:
void AccumulateJank(const LayoutObject&,
- const PaintLayer&,
+ const PropertyTreeState&,
FloatRect old_rect,
FloatRect new_rect);
void TimerFired(TimerBase*) {}
diff --git a/third_party/blink/renderer/core/layout/jank_tracker_test.cc b/third_party/blink/renderer/core/layout/jank_tracker_test.cc
index 78959ec..b31c65c 100644
--- a/third_party/blink/renderer/core/layout/jank_tracker_test.cc
+++ b/third_party/blink/renderer/core/layout/jank_tracker_test.cc
@@ -366,6 +366,32 @@
EXPECT_FLOAT_EQ(0.25, GetJankTracker().Score());
}
+TEST_F(JankTrackerTest, ClipWithoutPaintLayer) {
+ SetBodyInnerHTML(R"HTML(
+ <style>
+ #scroller { overflow: scroll; width: 200px; height: 500px; }
+ #space { height: 1000px; margin-bottom: -500px; }
+ #j { width: 150px; height: 150px; background: yellow; }
+ </style>
+ <div id='scroller'>
+ <div id='space'></div>
+ <div id='j'></div>
+ </div>
+ )HTML");
+
+ // Increase j's top margin by 100px. Since j is clipped by the scroller, this
+ // should not generate jank. However, due to the issue in crbug/971639, this
+ // case was erroneously reported as janking, before that bug was fixed. This
+ // test ensures we do not regress this behavior.
+ GetDocument().getElementById("j")->setAttribute(
+ html_names::kStyleAttr, AtomicString("margin-top: 100px"));
+
+ UpdateAllLifecyclePhases();
+ // Make sure no jank score is reported, since the element that moved is fully
+ // clipped by the scroller.
+ EXPECT_FLOAT_EQ(0.0, GetJankTracker().Score());
+}
+
class JankTrackerSimTest : public SimTest {};
TEST_F(JankTrackerSimTest, SubframeWeighting) {
diff --git a/third_party/blink/renderer/core/paint/compositing/composited_layer_mapping.cc b/third_party/blink/renderer/core/paint/compositing/composited_layer_mapping.cc
index 989d333..12e381a 100644
--- a/third_party/blink/renderer/core/paint/compositing/composited_layer_mapping.cc
+++ b/third_party/blink/renderer/core/paint/compositing/composited_layer_mapping.cc
@@ -1316,7 +1316,7 @@
LocalFrameView* frame_view = layout_object.View()->GetFrameView();
frame_view->GetJankTracker().NotifyCompositedLayerMoved(
- OwningLayer(), FloatRect(old_position, FloatSize(old_size)),
+ layout_object, FloatRect(old_position, FloatSize(old_size)),
FloatRect(new_position, FloatSize(new_size)));
}
graphics_layer_->SetOffsetFromLayoutObject(
diff --git a/third_party/blink/renderer/core/paint/paint_invalidator.cc b/third_party/blink/renderer/core/paint/paint_invalidator.cc
index e0b2fab..00a11989 100644
--- a/third_party/blink/renderer/core/paint/paint_invalidator.cc
+++ b/third_party/blink/renderer/core/paint/paint_invalidator.cc
@@ -9,6 +9,7 @@
#include "third_party/blink/renderer/core/frame/local_frame.h"
#include "third_party/blink/renderer/core/frame/local_frame_view.h"
#include "third_party/blink/renderer/core/frame/settings.h"
+#include "third_party/blink/renderer/core/layout/jank_tracker.h"
#include "third_party/blink/renderer/core/layout/layout_block_flow.h"
#include "third_party/blink/renderer/core/layout/layout_table.h"
#include "third_party/blink/renderer/core/layout/layout_table_section.h"
@@ -270,6 +271,13 @@
fragment_data.PaintOffset());
fragment_data.SetVisualRect(ComputeVisualRect(object, context));
+
+ object.GetFrameView()->GetJankTracker().NotifyObjectPrePaint(
+ object,
+ PropertyTreeState(*context.tree_builder_context_->current.transform,
+ *context.tree_builder_context_->current.clip,
+ *context.tree_builder_context_->current_effect),
+ context.old_visual_rect, fragment_data.VisualRect());
}
void PaintInvalidator::UpdateEmptyVisualRectFlag(
diff --git a/third_party/blink/renderer/core/paint/pre_paint_tree_walk.cc b/third_party/blink/renderer/core/paint/pre_paint_tree_walk.cc
index ad7db2f..0f5ee2a 100644
--- a/third_party/blink/renderer/core/paint/pre_paint_tree_walk.cc
+++ b/third_party/blink/renderer/core/paint/pre_paint_tree_walk.cc
@@ -391,10 +391,6 @@
ToLayoutBoxModelObject(object).Layer()->SetNeedsRepaint();
CompositingLayerPropertyUpdater::Update(object);
-
- object.GetFrameView()->GetJankTracker().NotifyObjectPrePaint(
- object, paint_invalidator_context.old_visual_rect,
- *paint_invalidator_context.painting_layer);
}
void PrePaintTreeWalk::Walk(const LayoutObject& object) {