Ensure update of scrollbar and caret visual rects on ancestor clip ch…

…ange When an ancestor clip changes, we set kSubtreeVisualRectUpdate to update descendant visual rects which may be affected by the clip change. Previously we early returned in PaintInvalidator::InvalidatePaint() for descendants that had only kSubtreeVisualRectUpdate flag set. This caused other visual rects (of e.g. scrollbars, carets) that are updated during LayoutObject::InvalidatePaint() not updated. Now move the early return from PaintInvalidator::InvalidatePaint() into ObjectPaintInvalidator::ComputePaintInvalidationReason() to ensure the visual rects are updated. [email protected] (cherry picked from commit 6dfcf90) Bug: 727155 Cq-Include-Trybots: master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2 Change-Id: I28f92281363c693b16121f48d21ab4f808990f14 Reviewed-on: https://chromium-review.googlesource.com/719716 Commit-Queue: Xianzhu Wang <[email protected]> Reviewed-by: Chris Harrelson <[email protected]> Cr-Original-Commit-Position: refs/heads/master@{#509648} Reviewed-on: https://chromium-review.googlesource.com/728300 Reviewed-by: Xianzhu Wang <[email protected]> Cr-Commit-Position: refs/branch-heads/3239@{#66} Cr-Branched-From: adb61db-refs/heads/master@{#508578}
nwjs · Oct 19, 2017 · 470d6c4 · 470d6c4
1 parent 73d1627
commit 470d6c4
Show file tree

Hide file tree

Showing 11 changed files with 220 additions and 8 deletions.
diff --git a/third_party/WebKit/LayoutTests/FlagExpectations/enable-slimming-paint-v2 b/third_party/WebKit/LayoutTests/FlagExpectations/enable-slimming-paint-v2
@@ -903,6 +903,7 @@ Bug(none) paint/invalidation/overflow-changed-on-child-of-composited-layer.html
 Bug(none) paint/invalidation/overflow-hidden-yet-scrolled-with-custom-scrollbar.html [ Failure ]
 Bug(none) paint/invalidation/overflow-hidden-yet-scrolled.html [ Failure ]
 Bug(none) paint/invalidation/paint-invalidation-with-reparent-across-frame-boundaries.html [ Failure ]
+Bug(none) paint/invalidation/scrollbar-ancestor-clip-change.html [ Failure ]
 Bug(none) paint/invalidation/svg/resize-svg-invalidate-children-2.html [ Failure ]
 Bug(none) paint/invalidation/svg/resize-svg-invalidate-children.html [ Failure ]
 

diff --git a/...cific/enable-slimming-paint-v2/paint/invalidation/caret-ancestor-clip-change-expected.txt b/...cific/enable-slimming-paint-v2/paint/invalidation/caret-ancestor-clip-change-expected.txt
@@ -0,0 +1,33 @@
+{
+  "layers": [
+    {
+      "name": "LayoutView #document",
+      "bounds": [800, 600],
+      "contentsOpaque": true,
+      "backgroundColor": "#FFFFFF",
+      "paintInvalidations": [
+        {
+          "object": "LayoutBlockFlow DIV",
+          "rect": [8, 8, 95, 112],
+          "reason": "paint property change"
+        },
+        {
+          "object": "LayoutTextControl INPUT id='target'",
+          "rect": [8, 8, 95, 112],
+          "reason": "paint property change"
+        },
+        {
+          "object": "LayoutBlockFlow DIV",
+          "rect": [8, 8, 95, 50],
+          "reason": "paint property change"
+        },
+        {
+          "object": "LayoutTextControl INPUT id='target'",
+          "rect": [8, 8, 95, 50],
+          "reason": "paint property change"
+        }
+      ]
+    }
+  ]
+}
+
diff --git a/third_party/WebKit/LayoutTests/paint/invalidation/caret-ancestor-clip-change-expected.html b/third_party/WebKit/LayoutTests/paint/invalidation/caret-ancestor-clip-change-expected.html
@@ -0,0 +1,7 @@
+<!DOCTYPE html>
+<script>
+onload = function() { target.focus(); }
+</script>
+<div style="width: 200px; height: 300px; overflow: hidden">
+  <input id="target" style="font-size: 100px; width: 100px; margin-left: -10px; margin-top: -10px">
+</div>
diff --git a/third_party/WebKit/LayoutTests/paint/invalidation/caret-ancestor-clip-change-expected.txt b/third_party/WebKit/LayoutTests/paint/invalidation/caret-ancestor-clip-change-expected.txt
@@ -0,0 +1,46 @@
+{
+  "layers": [
+    {
+      "name": "LayoutView #document",
+      "bounds": [800, 600],
+      "contentsOpaque": true,
+      "backgroundColor": "#FFFFFF",
+      "paintInvalidations": [
+        {
+          "object": "LayoutBlockFlow DIV",
+          "rect": [8, 8, 200, 300],
+          "reason": "geometry"
+        },
+        {
+          "object": "LayoutBlockFlow DIV",
+          "rect": [8, 8, 200, 300],
+          "reason": "geometry"
+        },
+        {
+          "object": "LayoutBlockFlow DIV id='clip'",
+          "rect": [8, 58, 200, 250],
+          "reason": "incremental"
+        }
+      ]
+    }
+  ],
+  "objectPaintInvalidations": [
+    {
+      "object": "LayoutBlockFlow DIV id='clip'",
+      "reason": "incremental"
+    },
+    {
+      "object": "LayoutBlockFlow DIV",
+      "reason": "geometry"
+    },
+    {
+      "object": "LayoutBlockFlow DIV",
+      "reason": "geometry"
+    },
+    {
+      "object": "RootInlineBox",
+      "reason": "geometry"
+    }
+  ]
+}
+
diff --git a/third_party/WebKit/LayoutTests/paint/invalidation/caret-ancestor-clip-change.html b/third_party/WebKit/LayoutTests/paint/invalidation/caret-ancestor-clip-change.html
@@ -0,0 +1,21 @@
+<!DOCTYPE html>
+<script src="resources/text-based-repaint.js"></script>
+<script>
+function repaintTest() {
+  clip.style.height = '300px';
+}
+onload = function() {
+  target.focus();
+  runRepaintAndPixelTest();
+}
+</script>
+<div id="clip" style="width: 200px; height: 50px; overflow: hidden">
+  <!-- These two divs isolate layout of |target| from |clip|, so that we won't
+       layout |target| on |clip|'s height change. -->
+  <div style="width: 300px; height: 300px; overflow: hidden">
+    <div style="width: 300px; height: 300px; overflow: hidden">
+      <!-- Use negative margins to avoid scroll on focus which would cause extra invalidations. -->
+      <input id="target" style="font-size: 100px; width: 100px; margin-left: -10px; margin-top: -10px">
+    </div>
+  </div>
+</div>
diff --git a/..._party/WebKit/LayoutTests/paint/invalidation/scrollbar-ancestor-clip-change-expected.html b/..._party/WebKit/LayoutTests/paint/invalidation/scrollbar-ancestor-clip-change-expected.html
@@ -0,0 +1,4 @@
+<!DOCTYPE html>
+<div style="width: 100px; height: 200px; overflow: scroll">
+  <div style="width: 400px; height: 400px"></div>
+</div>
diff --git a/...d_party/WebKit/LayoutTests/paint/invalidation/scrollbar-ancestor-clip-change-expected.txt b/...d_party/WebKit/LayoutTests/paint/invalidation/scrollbar-ancestor-clip-change-expected.txt
@@ -0,0 +1,78 @@
+{
+  "layers": [
+    {
+      "name": "LayoutView #document",
+      "bounds": [800, 600],
+      "contentsOpaque": true,
+      "backgroundColor": "#FFFFFF",
+      "paintInvalidations": [
+        {
+          "object": "LayoutBlockFlow DIV",
+          "rect": [8, 8, 100, 300],
+          "reason": "geometry"
+        },
+        {
+          "object": "LayoutBlockFlow DIV",
+          "rect": [8, 8, 100, 300],
+          "reason": "geometry"
+        },
+        {
+          "object": "LayoutBlockFlow DIV id='clip'",
+          "rect": [8, 108, 100, 200],
+          "reason": "incremental"
+        },
+        {
+          "object": "LayoutBlockFlow DIV id='target'",
+          "rect": [8, 193, 85, 15],
+          "reason": "scroll control"
+        },
+        {
+          "object": "LayoutBlockFlow DIV id='target'",
+          "rect": [93, 8, 15, 185],
+          "reason": "scroll control"
+        },
+        {
+          "object": "LayoutBlockFlow DIV id='target'",
+          "rect": [93, 8, 15, 100],
+          "reason": "scroll control"
+        },
+        {
+          "object": "LayoutBlockFlow DIV id='target'",
+          "rect": [93, 193, 15, 15],
+          "reason": "scroll control"
+        }
+      ]
+    }
+  ],
+  "objectPaintInvalidations": [
+    {
+      "object": "LayoutBlockFlow DIV id='clip'",
+      "reason": "incremental"
+    },
+    {
+      "object": "LayoutBlockFlow DIV",
+      "reason": "geometry"
+    },
+    {
+      "object": "LayoutBlockFlow DIV",
+      "reason": "geometry"
+    },
+    {
+      "object": "LayoutBlockFlow DIV id='target'",
+      "reason": "geometry"
+    },
+    {
+      "object": "HorizontalScrollbar",
+      "reason": "scroll control"
+    },
+    {
+      "object": "LayoutBlockFlow DIV id='target'",
+      "reason": "geometry"
+    },
+    {
+      "object": "VerticalScrollbar",
+      "reason": "scroll control"
+    }
+  ]
+}
+
diff --git a/third_party/WebKit/LayoutTests/paint/invalidation/scrollbar-ancestor-clip-change.html b/third_party/WebKit/LayoutTests/paint/invalidation/scrollbar-ancestor-clip-change.html
@@ -0,0 +1,19 @@
+<!DOCTYPE html>
+<script src="resources/text-based-repaint.js"></script>
+<script>
+function repaintTest() {
+  clip.style.height = '300px';
+}
+onload = runRepaintAndPixelTest;
+</script>
+<div id="clip" style="width: 100px; height: 100px; overflow: hidden">
+  <!-- These two divs isolate layout of |target| from |clip|, so that we won't
+       layout |target| on |clip|'s height change. -->
+  <div style="width: 300px; height: 300px; overflow: hidden">
+    <div style="width: 300px; height: 300px; overflow: hidden">
+      <div id="target" style="width: 100px; height: 200px; overflow: scroll">
+        <div style="width: 400px; height: 400px"></div>
+      </div>
+    </div>
+  </div>
+</div>
diff --git a/third_party/WebKit/Source/core/paint/BoxPaintInvalidatorTest.cpp b/third_party/WebKit/Source/core/paint/BoxPaintInvalidatorTest.cpp
@@ -189,6 +189,7 @@ TEST_P(BoxPaintInvalidatorTest, ComputePaintInvalidationReasonBasic) {
   target.setAttribute(HTMLNames::styleAttr, "background: blue");
   GetDocument().View()->UpdateAllLifecyclePhases();
 
+  box.SetMayNeedPaintInvalidation();
   LayoutRect visual_rect = box.VisualRect();
   EXPECT_EQ(LayoutRect(0, 0, 50, 100), visual_rect);
 

diff --git a/third_party/WebKit/Source/core/paint/ObjectPaintInvalidator.cpp b/third_party/WebKit/Source/core/paint/ObjectPaintInvalidator.cpp
@@ -439,6 +439,16 @@ ObjectPaintInvalidatorWithContext::ComputePaintInvalidationReason() {
     background_obscuration_changed = true;
   }
 
+  if (!object_.ShouldCheckForPaintInvalidation() &&
+      (!context_.subtree_flags ||
+       context_.subtree_flags ==
+           PaintInvalidatorContext::kSubtreeVisualRectUpdate)) {
+    // No paint invalidation flag, or just kSubtreeVisualRectUpdate (which has
+    // been handled in PaintInvalidator). No paint invalidation is needed.
+    DCHECK(!background_obscuration_changed);
+    return PaintInvalidationReason::kNone;
+  }
+
   if (context_.subtree_flags &
       PaintInvalidatorContext::kSubtreeFullInvalidation)
     return PaintInvalidationReason::kSubtree;

diff --git a/third_party/WebKit/Source/core/paint/PaintInvalidator.cpp b/third_party/WebKit/Source/core/paint/PaintInvalidator.cpp
@@ -504,14 +504,6 @@ void PaintInvalidator::InvalidatePaint(
   UpdateEmptyVisualRectFlag(object, context);
   UpdateVisualRectIfNeeded(object, tree_builder_context, context);
 
-  if (!object.ShouldCheckForPaintInvalidation() &&
-      !(context.subtree_flags &
-        ~PaintInvalidatorContext::kSubtreeVisualRectUpdate)) {
-    // We are done updating anything needed. No other paint invalidation work to
-    // do for this object.
-    return;
-  }
-
   PaintInvalidationReason reason = object.InvalidatePaint(context);
   switch (reason) {
     case PaintInvalidationReason::kDelayedFull: