Invalidate layer when its composited scrolling status changed

When a composited scrolling layer's object is set 'pointer-events:none', the layer will no longer use composited scrolling (though it is still composited). This is controlled by ComputedStyle::VisibleToHitTesting(). When this happens, no other conditions trigger invalidation of the layer. Now invalidate a layer when its composited scrolling status changed. [email protected] (cherry picked from commit 9ba2227) Bug: 758474 Cq-Include-Trybots: master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2 Change-Id: I3c01a543a5424d8c746e901047abc606874e0290 Reviewed-on: https://chromium-review.googlesource.com/717028 Reviewed-by: Philip Rogers <[email protected]> Commit-Queue: Xianzhu Wang <[email protected]> Cr-Original-Commit-Position: refs/heads/master@{#509172} Reviewed-on: https://chromium-review.googlesource.com/726379 Reviewed-by: Xianzhu Wang <[email protected]> Cr-Commit-Position: refs/branch-heads/3239@{#53} Cr-Branched-From: adb61db-refs/heads/master@{#508578}
nwjs · Oct 18, 2017 · 2dcf849 · 2dcf849
1 parent f9a91c4
commit 2dcf849
Show file tree

Hide file tree

Showing 7 changed files with 106 additions and 5 deletions.
diff --git a/third_party/WebKit/LayoutTests/FlagExpectations/enable-slimming-paint-v2 b/third_party/WebKit/LayoutTests/FlagExpectations/enable-slimming-paint-v2
@@ -1717,6 +1717,8 @@ crbug.com/762098 compositing/overflow/non-composited-sticky-element-in-main-thre
 
 # Subsequence under-invalidation of scroller related to ScrollHitTest.
 crbug.com/738613 compositing/overflow/overflow-scroll-with-pointer-events-toggle.html [ Failure Crash ]
+# Null pointer in PaintArtifactCompositor::ScrollHitTestLayerForPendingLayer().
+crbug.com/738613 paint/invalidation/compositing/pointer-events-composited-scrolling.html [ Crash ]
 
 # See comment regarding this test in NeverFixTests. It also fails for other
 # reasons, in particular that the composited layerization algorithm provides

diff --git a/...outTests/paint/invalidation/compositing/pointer-events-composited-scrolling-expected.html b/...outTests/paint/invalidation/compositing/pointer-events-composited-scrolling-expected.html
@@ -0,0 +1,2 @@
+<!DOCTYPE html>
+<div id="content" style="margin-top: 38px; width: 50px; height: 50px; background: blue"></div>
diff --git a/...youtTests/paint/invalidation/compositing/pointer-events-composited-scrolling-expected.txt b/...youtTests/paint/invalidation/compositing/pointer-events-composited-scrolling-expected.txt
@@ -0,0 +1,67 @@
+{
+  "layers": [
+    {
+      "name": "LayoutView #document",
+      "bounds": [800, 600],
+      "contentsOpaque": true,
+      "backgroundColor": "#FFFFFF"
+    },
+    {
+      "name": "LayoutBlockFlow DIV id='target'",
+      "position": [8, 8],
+      "bounds": [100, 100],
+      "paintInvalidations": [
+        {
+          "object": "LayoutBlockFlow DIV id='target'",
+          "rect": [0, 0, 100, 100],
+          "reason": "full"
+        },
+        {
+          "object": "LayoutBlockFlow DIV id='target'",
+          "rect": [0, 0, 100, 100],
+          "reason": "compositing update"
+        },
+        {
+          "object": "LayoutBlockFlow DIV id='content'",
+          "rect": [0, 30, 50, 50],
+          "reason": "compositing update"
+        }
+      ]
+    },
+    {
+      "name": "Overflow Controls Host Layer",
+      "position": [8, 8],
+      "bounds": [100, 100],
+      "drawsContent": false
+    },
+    {
+      "name": "Horizontal Scrollbar Layer",
+      "position": [8, 108],
+      "bounds": [100, 0]
+    },
+    {
+      "name": "Vertical Scrollbar Layer",
+      "position": [108, 8],
+      "bounds": [0, 100]
+    }
+  ],
+  "objectPaintInvalidations": [
+    {
+      "object": "LayoutBlockFlow DIV id='target'",
+      "reason": "compositing update"
+    },
+    {
+      "object": "LayoutBlockFlow DIV id='target'",
+      "reason": "geometry"
+    },
+    {
+      "object": "LayoutBlockFlow DIV id='target'",
+      "reason": "geometry"
+    },
+    {
+      "object": "LayoutBlockFlow DIV id='content'",
+      "reason": "compositing update"
+    }
+  ]
+}
+
diff --git a/...ebKit/LayoutTests/paint/invalidation/compositing/pointer-events-composited-scrolling.html b/...ebKit/LayoutTests/paint/invalidation/compositing/pointer-events-composited-scrolling.html
@@ -0,0 +1,15 @@
+<!DOCTYPE html>
+<style>*::-webkit-scrollbar { display: none }</style>
+<div id="target" style="overflow: scroll; will-change: transform; width: 100px; height: 100px">
+  <div id="content" style="margin: 80px 0; width: 50px; height: 50px; background: blue"></div>
+</div>
+<script src="../resources/text-based-repaint.js"></script>
+<script>
+function repaintTest() {
+  target.style.pointerEvents = 'none';
+}
+onload = function() {
+  target.scrollTop = 50;
+  runRepaintAndPixelTest();
+}
+</script>
diff --git a/third_party/WebKit/LayoutTests/paint/invalidation/destroy-composited-scrollbar-expected.txt b/third_party/WebKit/LayoutTests/paint/invalidation/destroy-composited-scrollbar-expected.txt
@@ -25,16 +25,25 @@
         },
         {
           "object": "LayoutBlockFlow (positioned) DIV",
-          "rect": [185, 0, 15, 200],
-          "reason": "scroll control"
+          "rect": [0, 0, 200, 200],
+          "reason": "compositing update"
+        },
+        {
+          "object": "LayoutBlockFlow DIV id='content'",
+          "rect": [0, 0, 100, 100],
+          "reason": "compositing update"
         }
       ]
     }
   ],
   "objectPaintInvalidations": [
     {
       "object": "LayoutBlockFlow (positioned) DIV",
-      "reason": "geometry"
+      "reason": "compositing update"
+    },
+    {
+      "object": "LayoutBlockFlow DIV id='content'",
+      "reason": "compositing update"
     }
   ]
 }

diff --git a/third_party/WebKit/Source/core/paint/compositing/GraphicsLayerUpdater.cpp b/third_party/WebKit/Source/core/paint/compositing/GraphicsLayerUpdater.cpp
@@ -107,8 +107,14 @@ void GraphicsLayerUpdater::UpdateRecursive(
     CompositedLayerMapping* mapping = layer.GetCompositedLayerMapping();
 
     if (update_type == kForceUpdate || mapping->NeedsGraphicsLayerUpdate()) {
-      if (mapping->UpdateGraphicsLayerConfiguration())
+      bool had_scrolling_layer = mapping->ScrollingLayer();
+      if (mapping->UpdateGraphicsLayerConfiguration()) {
         needs_rebuild_tree_ = true;
+        // Change of existence of scrolling layer affects visual rect offsets of
+        // descendants via LayoutObject::ScrollAdjustmentForPaintInvalidation().
+        if (had_scrolling_layer != !!mapping->ScrollingLayer())
+          layers_needing_paint_invalidation.push_back(&layer);
+      }
       mapping->UpdateGraphicsLayerGeometry(context.CompositingContainer(layer),
                                            context.CompositingStackingContext(),
                                            layers_needing_paint_invalidation);

diff --git a/third_party/WebKit/Source/platform/graphics/paint/PaintController.cpp b/third_party/WebKit/Source/platform/graphics/paint/PaintController.cpp
@@ -1026,7 +1026,7 @@ void PaintController::ShowSequenceUnderInvalidationError(
   LOG(ERROR) << under_invalidation_message_prefix_ << " " << reason;
   LOG(ERROR) << "Subsequence client: " << client.DebugName();
 #ifndef NDEBUG
-//  showDebugData();
+  ShowDebugData();
 #else
   LOG(ERROR) << "Run debug build to get more details.";
 #endif
Original file line number	Diff line number	Diff line change
		@@ -0,0 +1,2 @@
		<!DOCTYPE html>
		<div id="content" style="margin-top: 38px; width: 50px; height: 50px; background: blue"></div>