Fix layout shift tracking with 2d transforms

This fixes the following issues:

1. crbug.com/1109053 we reported transform changes as layout shifts.
  According to https://github.com/WICG/layout-instability#transform-changes
  we should ignore transform changes.
  This is fixed by just accumulating offsets of PaintOffsetTranslations
  for the additional offset for layout shift
  (previous offset_to_2d_translation_root,
   now additional_offset_to_layout_shift_root)
 Test: external/wpt/layout-instability/transform-change.html

2. Movement of transformed element itself was ignored. This is fixed by
  resetting the additional offset for layout shift after updating
  paint properties for child instead of during updating the properties
  because the offset applies to the transformed element itself.
 Test: external/wpt/layout-instability/move-transformed.html

3. Stale old additional offset for layout shift (previously baked in
  FragmentData::VisualRectTo2DTranslationRoot). We don't always update
  the offset when it changes because it's neither a paint property nor
  a paint invalidation data, thus the old data may be stale. To avoid
  that, now save VisualRectForLayoutShiftTracking() which is in the
  local transform space and is always updated, and recover the old
  additional offset from the old paint properties.

Bug: 1109053,1104794
Change-Id: I36a3dec0695f9674251e7f5ce85ae2411b135eda
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2316783Reviewed-by: Chris Harrelson <chrishtr@chromium.org>
Reviewed-by: Steve Kobes <skobes@chromium.org>
Commit-Queue: Xianzhu Wang <wangxianzhu@chromium.org>
Cr-Commit-Position: refs/heads/master@{#792312}

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

@@ -177,7 +177,7 @@ struct SameSizeAsLayoutObject : ImageResourceObserver, DisplayItemClient {
   Member<void*> members[1];
   // The following fields are in FragmentData.
   PhysicalOffset paint_offset_;
-  PhysicalRect visual_rect_in_2d_translation_root_;
+  PhysicalRect visual_rect_for_layout_shift_tracking;
   std::unique_ptr<int> rare_data_;
 };
 

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

@@ -593,7 +593,7 @@ void ReattachHook::NotifyDetach(const Node& node) {
   auto& fragment = layout_object->GetMutableForPainting().FirstFragment();
 
   // Save the visual rect for restoration on future reattachment.
-  PhysicalRect visual_rect = fragment.VisualRectIn2DTranslationRoot();
+  PhysicalRect visual_rect = fragment.VisualRectForLayoutShiftTracking();
   if (visual_rect.IsEmpty())
     return;
   map.Set(&node, visual_rect);
@@ -613,7 +613,7 @@ void ReattachHook::NotifyAttach(const Node& node) {
   auto iter = map.find(&node);
   if (iter == map.end())
     return;
-  fragment.SetVisualRectIn2DTranslationRoot(iter->value);
+  fragment.SetVisualRectForLayoutShiftTracking(iter->value);
 }
 
 void ReattachHook::Trace(Visitor* visitor) const {

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

@@ -34,18 +34,15 @@ class CORE_EXPORT LayoutShiftTracker final
   ~LayoutShiftTracker() = default;
   // |old_visual_rect| and |new_visual_rect| are in the local transform space:
   // |property_tree_state.Transform()|. As we don't save the old property tree
-  // state, the caller should adjust |old_rect| as if the difference between the
-  // old and the new local and ancestor transforms [1] caused the difference
-  // between the locations of |old_visual_rect| and |new_visual_rect|, so that
-  // we can calculate the shift caused by the changed transforms, in addition to
-  // the shift in the local transform space, by comparing locations of
-  // |old_visual_rect| and |new_visual_rect|.
-  //
-  // [1] We may stop at a certain ancestor transform and ignore changes of all
-  // higher transforms. This is how we ignore scrolls in layout shift tracking.
-  // We also can't accumulate offsets across non-2d-translation transforms.
-  // See PaintPropertyTreeBuilderFragmentContext::
-  //    ContainingBlockContext::offset_to_2d_translation_root.
+  // state, the caller should adjust |old_visual_rect| as if the difference
+  // between the old and new additional offsets to the layout shift root[1]
+  // caused the difference between the locations of |old_visual_rect| and
+  // |new_visual_rect|, in addition to that caused by the difference between
+  // the old and new paint offsets in the local transform space, so that we can
+  // calculate the total shift from the layout shift root by comparing locations
+  // of |old_visual_rect| and |new_visual_rect|.
+  // [1] See PaintPropertyTreeBuilderFragmentContext::ContainingBlockContext
+  // ::additional_offset_to_layout_shift_root_delta.
   void NotifyObjectPrePaint(const LayoutObject& object,
                             const PropertyTreeStateOrAlias& property_tree_state,
                             const PhysicalRect& old_visual_rect,

third_party/blink/renderer/core/paint/fragment_data.h

@@ -35,15 +35,12 @@ class CORE_EXPORT FragmentData {
     paint_offset_ = paint_offset;
   }
 
-  // This is for LayoutShiftTracker.
-  // See PaintPropertyTreeBuilderFragmentContext::
-  //    ContainingBlockContext::offset_to_2d_translation_root for definition
-  // of 2d translation root.
-  const PhysicalRect& VisualRectIn2DTranslationRoot() const {
-    return visual_rect_in_2d_translation_root_;
+  // Visual rect in the space of the the local transform space.
+  const PhysicalRect& VisualRectForLayoutShiftTracking() const {
+    return visual_rect_for_layout_shift_tracking_;
   }
-  void SetVisualRectIn2DTranslationRoot(const PhysicalRect& rect) {
-    visual_rect_in_2d_translation_root_ = rect;
+  void SetVisualRectForLayoutShiftTracking(const PhysicalRect& rect) {
+    visual_rect_for_layout_shift_tracking_ = rect;
   }
 
   // An id for this object that is unique for the lifetime of the WebView.
@@ -264,7 +261,7 @@ class CORE_EXPORT FragmentData {
   RareData& EnsureRareData();
 
   PhysicalOffset paint_offset_;
-  PhysicalRect visual_rect_in_2d_translation_root_;
+  PhysicalRect visual_rect_for_layout_shift_tracking_;
 
   std::unique_ptr<RareData> rare_data_;
 };

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

@@ -145,38 +145,38 @@ void PaintInvalidator::UpdateForPaintOffsetChange(
     const LayoutObject& object,
     FragmentData& fragment_data,
     PaintInvalidatorContext& context) {
-  DCHECK(context.tree_builder_context_);
-  DCHECK_EQ(context.tree_builder_context_->current.paint_offset,
-            fragment_data.PaintOffset());
+  const auto* tree_context = context.tree_builder_context_;
+  DCHECK(tree_context);
+  DCHECK_EQ(tree_context->current.paint_offset, fragment_data.PaintOffset());
 
   // LayoutShiftTracker doesn't track SVG children. Also the visual rect
   // calculation below works for non-SVG-child objects only.
   if (!object.IsSVGChild()) {
     PhysicalRect new_visual_rect = object.LocalVisualRect();
     new_visual_rect.Move(fragment_data.PaintOffset());
-    // Adjust old_visual_rect so that LayoutShiftTracker can see the change of
-    // offset caused by change of transforms below the 2d translation root.
-    PhysicalRect old_visual_rect =
-        fragment_data.VisualRectIn2DTranslationRoot();
-    old_visual_rect.Move(
-        -context.tree_builder_context_->current.offset_to_2d_translation_root);
-    object.GetFrameView()->GetLayoutShiftTracker().NotifyObjectPrePaint(
-        object,
-        PropertyTreeStateOrAlias(
-            *context.tree_builder_context_->current.transform,
-            *context.tree_builder_context_->current.clip,
-            *context.tree_builder_context_->current_effect),
-        old_visual_rect, PhysicalRect(new_visual_rect));
-
-    new_visual_rect.Move(
-        context.tree_builder_context_->current.offset_to_2d_translation_root);
-    fragment_data.SetVisualRectIn2DTranslationRoot(new_visual_rect);
+    // If the layout shift root has changed, LayoutShiftTracker can't use the
+    // current paint property tree to map the old visual rect.
+    if (!tree_context->current.layout_shift_root_changed) {
+      // Adjust old_visual_rect so that LayoutShiftTracker can see the change of
+      // offset caused by change of transforms below the 2d translation root.
+      PhysicalRect old_visual_rect =
+          fragment_data.VisualRectForLayoutShiftTracking();
+      old_visual_rect.Move(
+          -tree_context->current.additional_offset_to_layout_shift_root_delta);
+      object.GetFrameView()->GetLayoutShiftTracker().NotifyObjectPrePaint(
+          object,
+          PropertyTreeStateOrAlias(*tree_context->current.transform,
+                                   *tree_context->current.clip,
+                                   *tree_context->current_effect),
+          old_visual_rect, new_visual_rect);
+    }
+    fragment_data.SetVisualRectForLayoutShiftTracking(new_visual_rect);
   }
 
   // For performance, we ignore subpixel movement of composited layers for paint
   // invalidation. This will result in imperfect pixel-snapped painting.
   // See crbug.com/833083 for details.
-  if (context.tree_builder_context_->current
+  if (tree_context->current
           .directly_composited_container_paint_offset_subpixel_delta ==
       fragment_data.PaintOffset() - context.old_paint_offset)
     context.old_paint_offset = fragment_data.PaintOffset();

third_party/blink/renderer/core/paint/paint_property_tree_builder.h

@@ -46,15 +46,23 @@ struct PaintPropertyTreeBuilderFragmentContext {
     // fragmentation offsets. See FragmentContext for the fragmented version.
     PhysicalOffset paint_offset;
 
-    // The 2D translation root is the nearest transform node, inclusive of
-    // |transform|, that is one of:
+    // "Additional offset to layout shift root" is the accumulation of paint
+    // offsets encoded in PaintOffsetTranslations between the local transform
+    // space and the layout shift root. The layout shift root is the nearest
+    // transform node (not including the transform nodes for the current object)
+    // that is one of:
     //   * The transform property tree state of the containing LayoutView
-    //   * Not an identity or 2D translation
+    //   * A transform that is not identity or 2d translation
     //   * A scroll translation
-    // |offset_to_2d_translation_root| is the offset representation of the
-    // product of the the transforms starting at |transform| but not inclusive
-    // of the 2D translation root.
-    PhysicalOffset offset_to_2d_translation_root;
+    //   * A replaced contents transform
+    //   * A transform isolation node
+    // The offset plus paint_offset is the offset for layout shift tracking.
+    // It doesn't include transforms because we need to ignore transform changes
+    // for layout shift tracking, see
+    //    https://github.com/WICG/layout-instability#transform-changes
+    // This field is the diff between the new and the old additional offsets to
+    // layout shift root.
+    PhysicalOffset additional_offset_to_layout_shift_root_delta;
 
     // For paint invalidation optimization for subpixel movement under
     // composited layer. It's reset to zero if subpixel can't be propagated
@@ -74,6 +82,11 @@ struct PaintPropertyTreeBuilderFragmentContext {
     // root of the FrameView (and hence above its scroll).
     bool fixed_position_children_fixed_to_root = false;
 
+    // True if the layout shift root (see
+    // additional_offset_to_layout_shift_root_delta for the definition) of this
+    // object has changed.
+    bool layout_shift_root_changed = false;
+
     // Rendering context for 3D sorting. See
     // TransformPaintPropertyNode::renderingContextId.
     unsigned rendering_context_id = 0;
@@ -253,13 +266,15 @@ class PaintPropertyTreeBuilder {
   PaintPropertyChangeType UpdateForChildren();
 
  private:
-  ALWAYS_INLINE void InitFragmentPaintProperties(FragmentData&,
-                                                 bool needs_paint_properties);
+  ALWAYS_INLINE void InitFragmentPaintProperties(
+      FragmentData&,
+      bool needs_paint_properties,
+      PaintPropertyTreeBuilderFragmentContext&);
   ALWAYS_INLINE void InitFragmentPaintPropertiesForLegacy(
       FragmentData&,
       bool needs_paint_properties,
-      const PhysicalOffset& pagination_offset = PhysicalOffset(),
-      LayoutUnit logical_top_in_flow_thread = LayoutUnit());
+      const PhysicalOffset& pagination_offset,
+      PaintPropertyTreeBuilderFragmentContext&);
   ALWAYS_INLINE void InitFragmentPaintPropertiesForNG(
       bool needs_paint_properties);
   ALWAYS_INLINE void InitSingleFragmentFromParent(bool needs_paint_properties);

third_party/blink/web_tests/external/wpt/layout-instability/move-transformed.html

+<!DOCTYPE html>
+<title>Layout Instability: shift of a transformed container</title>
+<link rel="help" href="https://wicg.github.io/layout-instability/" />
+<style>
+body { margin: 0; }
+#transformed { position: relative; transform: translateX(20px); width: 100px; height: 100px; }
+</style>
+<div id="transformed"></div>
+<script src="/resources/testharness.js"></script>
+<script src="/resources/testharnessreport.js"></script>
+<script src="resources/util.js"></script>
+<script>
+
+promise_test(async () => {
+  const watcher = new ScoreWatcher;
+
+  // Wait for the initial render to complete.
+  await waitForAnimationFrames(2);
+
+  document.querySelector("#transformed").style = "top: 50px";
+
+  const expectedScore = computeExpectedScore(100 * (100 + 50), 50);
+
+  await watcher.promise;
+  assert_equals(watcher.score, expectedScore);
+}, 'Move transformed container');
+
+</script>

third_party/blink/web_tests/external/wpt/layout-instability/transform-change.html

+<!DOCTYPE html>
+<title>Layout Instability: transform change</title>
+<link rel="help" href="https://wicg.github.io/layout-instability/" />
+<style>
+body { margin: 0; }
+#transformed { position: relative; transform: translateX(20px); width: 100px; height: 100px; }
+#child { width: 400px; height: 400px; }
+</style>
+<div id="transformed">
+  <div id="child"></div>
+</div>
+<script src="/resources/testharness.js"></script>
+<script src="/resources/testharnessreport.js"></script>
+<script src="resources/util.js"></script>
+<script>
+
+promise_test(async () => {
+  const watcher = new ScoreWatcher;
+
+  // Wait for the initial render to complete.
+  await waitForAnimationFrames(2);
+
+  // Modify the transform, for which no shift should be reported.
+  document.querySelector("#transformed").style = "transform: translateY(100px)";
+  // Change size of child, for which no shift should be reported, either.
+  document.querySelector("#child").style = "width: 300px";
+
+  await waitForAnimationFrames(2);
+  // No shift should be reported.
+  assert_equals(watcher.score, 0);
+}, 'transform change');
+
+</script>