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.

Change-Id: I4daf0d1f03507ca45f3e78bf2d3f93b4b6848efd
Bug: 971639
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1644939Reviewed-by: Xianzhu Wang <wangxianzhu@chromium.org>
Commit-Queue: Bryan McQuade <bmcquade@chromium.org>
Cr-Commit-Position: refs/heads/master@{#667309}

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 @@ static bool SmallerThanRegionGranularity(const FloatRect& rect,
          rect.Height() * granularity_scale < 0.5;
 }
 
-static const PropertyTreeState PropertyTreeStateFor(LayoutObject& object) {
+static const PropertyTreeState PropertyTreeStateFor(
+    const LayoutObject& object) {
   return object.FirstFragment().LocalBorderBoxProperties();
 }
 
@@ -119,7 +119,7 @@ JankTracker::JankTracker(LocalFrameView* frame_view)
       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())
@@ -149,21 +149,19 @@ void JankTracker::AccumulateJank(const LayoutObject& source,
   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);
@@ -209,24 +207,25 @@ void JankTracker::AccumulateJank(const LayoutObject& source,
   }
 }
 
-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;
 
@@ -236,7 +235,8 @@ void JankTracker::NotifyCompositedLayerMoved(const PaintLayer& paint_layer,
   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 {

third_party/blink/renderer/core/layout/jank_tracker.h

@@ -17,7 +17,7 @@ namespace blink {
 class IntRect;
 class LayoutObject;
 class LocalFrameView;
-class PaintLayer;
+class PropertyTreeState;
 class TracedValue;
 class WebInputEvent;
 
@@ -30,9 +30,10 @@ class CORE_EXPORT JankTracker {
   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 @@ class CORE_EXPORT JankTracker {
 
  private:
   void AccumulateJank(const LayoutObject&,
-                      const PaintLayer&,
+                      const PropertyTreeState&,
                       FloatRect old_rect,
                       FloatRect new_rect);
   void TimerFired(TimerBase*) {}

third_party/blink/renderer/core/layout/jank_tracker_test.cc

@@ -366,6 +366,32 @@ TEST_F(JankTrackerTest, ShiftInToViewport) {
   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) {

third_party/blink/renderer/core/paint/compositing/composited_layer_mapping.cc

@@ -1316,7 +1316,7 @@ void CompositedLayerMapping::UpdateMainGraphicsLayerGeometry(
 
     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(

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 @@ void PaintInvalidator::UpdateVisualRect(const LayoutObject& object,
          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(

third_party/blink/renderer/core/paint/pre_paint_tree_walk.cc

@@ -391,10 +391,6 @@ void PrePaintTreeWalk::WalkInternal(const LayoutObject& object,
     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) {