Remove extra calls to SetNeedsGraphicsLayerUpdate

In non-BGPT mode, we only need to do a kGraphicsLayerUpdateLocal update,
and in BGPT mode, we now call SetNeedsCommit() directly from
PropertyTreeManager::EnsureCompositorTransformNode, so no graphics layer
update is needed at all.

Bug: 895109
Change-Id: I970b29bc05a43720cfb9747bfcfa50e42dc8639e
Reviewed-on: https://chromium-review.googlesource.com/c/1403639Reviewed-by: Chris Harrelson <chrishtr@chromium.org>
Reviewed-by: Philip Rogers <pdr@chromium.org>
Commit-Queue: Mason Freed <masonfreed@chromium.org>
Cr-Commit-Position: refs/heads/master@{#622979}

cc/layers/layer.h

@@ -765,6 +765,13 @@ class CC_EXPORT Layer : public base::RefCounted<Layer> {
 
   std::string ToString() const;
 
+  // Called when a property has been modified in a way that the layer knows
+  // immediately that a commit is required.  This implies SetNeedsPushProperties
+  // to push that property.
+  // This is public, so that it can be called directly when needed, for example
+  // in PropertyTreeManager when handling scroll offsets.
+  void SetNeedsCommit();
+
  protected:
   friend class LayerImpl;
   friend class TreeSynchronizer;
@@ -773,11 +780,8 @@ class CC_EXPORT Layer : public base::RefCounted<Layer> {
   virtual ~Layer();
 
   // These SetNeeds functions are in order of severity of update:
-  //
-  // Called when a property has been modified in a way that the layer knows
-  // immediately that a commit is required.  This implies SetNeedsPushProperties
-  // to push that property.
-  void SetNeedsCommit();
+
+  // See SetNeedsCommit() above - it belongs here in the order of severity.
 
   // Called when there's been a change in layer structure.  Implies
   // SetNeedsCommit and property tree rebuld, but not SetNeedsPushProperties

third_party/blink/renderer/core/page/scrolling/scrolling_coordinator_test.cc

@@ -199,6 +199,11 @@ TEST_P(ScrollingCoordinatorTest, fastScrollingByDefault) {
 TEST_P(ScrollingCoordinatorTest, fastFractionalScrollingDiv) {
   ScopedFractionalScrollOffsetsForTest fractional_scroll_offsets(true);
 
+  // TODO(920417): Re-enable this test when main thread scrolling supports
+  // fractional scroll offsets.
+  if (RuntimeEnabledFeatures::BlinkGenPropertyTreesEnabled())
+    return;
+
   RegisterMockedHttpURLLoad("fractional-scroll-div.html");
   NavigateTo(base_url_ + "fractional-scroll-div.html");
   ForceFullCompositingUpdate();

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

@@ -2162,8 +2162,15 @@ void PaintLayerScrollableArea::UpdateCompositingLayersAfterScroll() {
         scrolling_coordinator->UpdateCompositedScrollOffset(this);
 
     if (!handled_scroll) {
-      Layer()->GetCompositedLayerMapping()->SetNeedsGraphicsLayerUpdate(
-          kGraphicsLayerUpdateSubtree);
+      if (!RuntimeEnabledFeatures::BlinkGenPropertyTreesEnabled()) {
+        // In non-BGPT mode, we need to do a full sub-tree update here, because
+        // we need to update the position property to compute
+        // offset_to_transform_parent. For more context, see the comment from
+        // chrishtr@ here:
+        // https://chromium-review.googlesource.com/c/chromium/src/+/1403639/6/third_party/blink/renderer/core/paint/paint_layer_scrollable_area.cc
+        Layer()->GetCompositedLayerMapping()->SetNeedsGraphicsLayerUpdate(
+            kGraphicsLayerUpdateSubtree);
+      }
       compositor->SetNeedsCompositingUpdate(
           kCompositingUpdateAfterGeometryChange);
     }

third_party/blink/renderer/platform/graphics/compositing/paint_artifact_compositor.cc

@@ -885,8 +885,10 @@ void PaintArtifactCompositor::Update(
     // Calling |PropertyTreeStateChanged| for every pending layer is
     // O(|property nodes|^2) and could be optimized by caching the lookup of
     // nodes known to be changed/unchanged.
-    if (PropertyTreeStateChanged(property_state))
+    if (PropertyTreeStateChanged(property_state)) {
       layer->SetSubtreePropertyChanged();
+      root_layer_->SetNeedsCommit();
+    }
   }
   property_tree_manager.Finalize();
   content_layer_clients_.swap(new_content_layer_clients);

third_party/blink/web_tests/fast/events/touch/compositor-touch-hit-rects-scroll-expected.txt

@@ -14,7 +14,7 @@ overflowwithhandler: layer(0,10 273x52) has hit test rect (0,0 273x52)
 overflowwithborder: layer(13,365 290x70) has hit test rect (0,0 290x70)
 overflowwithborder: layer(0,0 263x124) has hit test rect (4,4 255x116)
 
-withTransform: layer(15,435 271x12) has hit test rect (0,0 271x12)
+withTransform: layer(15,455 271x12) has hit test rect (0,0 271x12)
 withTransform: layer(0,0 282x77) has hit test rect (0,13 273x14)
 
 

third_party/blink/web_tests/paint/invalidation/compositing/scrolling-neg-z-index-descendants-expected.txt

@@ -24,7 +24,7 @@
     },
     {
       "name": "LayoutBlockFlow (relative positioned) DIV id='neg-z'",
-      "position": [9, -81],
+      "position": [9, 19],
       "bounds": [100, 410],
       "backfaceVisibility": "hidden"
     },

third_party/blink/web_tests/virtual/stable/paint/invalidation/compositing/scrolling-neg-z-index-descendants-expected.txt

+{
+  "layers": [
+    {
+      "name": "LayoutView #document",
+      "bounds": [800, 600],
+      "drawsContent": false,
+      "backgroundColor": "#FFFFFF"
+    },
+    {
+      "name": "Scrolling Layer",
+      "bounds": [800, 600],
+      "drawsContent": false
+    },
+    {
+      "name": "Scrolling Contents Layer",
+      "bounds": [800, 600],
+      "contentsOpaque": true,
+      "drawsContent": false,
+      "backgroundColor": "#FFFFFF"
+    },
+    {
+      "name": "LayoutBlockFlow HTML",
+      "bounds": [800, 318]
+    },
+    {
+      "name": "LayoutBlockFlow (relative positioned) DIV id='neg-z'",
+      "position": [9, -81],
+      "bounds": [100, 410]
+    },
+    {
+      "name": "LayoutBlockFlow HTML (foreground) Layer",
+      "bounds": [800, 318]
+    },
+    {
+      "name": "LayoutBlockFlow (relative positioned) DIV id='container'",
+      "position": [8, 8],
+      "bounds": [102, 302],
+      "backfaceVisibility": "hidden"
+    },
+    {
+      "name": "Scrolling Layer",
+      "position": [9, 9],
+      "bounds": [100, 300],
+      "drawsContent": false
+    },
+    {
+      "name": "Scrolling Contents Layer",
+      "position": [9, 9],
+      "bounds": [100, 430],
+      "drawsContent": false,
+      "transform": 1
+    },
+    {
+      "name": "Overflow Controls Host Layer",
+      "position": [8, 8],
+      "bounds": [102, 302],
+      "drawsContent": false,
+      "backfaceVisibility": "hidden"
+    },
+    {
+      "name": "Horizontal Scrollbar Layer",
+      "position": [9, 309],
+      "bounds": [100, 0]
+    },
+    {
+      "name": "Vertical Scrollbar Layer",
+      "position": [109, 9],
+      "bounds": [0, 300]
+    }
+  ],
+  "transforms": [
+    {
+      "id": 1,
+      "transform": [
+        [1, 0, 0, 0],
+        [0, 1, 0, 0],
+        [0, 0, 1, 0],
+        [0, -100, 0, 1]
+      ],
+      "flattenInheritedTransform": false
+    }
+  ]
+}
+