[PE] Cached subsequence should retain previous paint result

If a subsequence previously partially painted, when we use the cached
without repainting, we should still treat the layer as partially
painted, otherwise we'll fail to repaint on paint dirty rect change.

Bug: 898435
Change-Id: I27ec2689377aaf408a819f2849675640c439fcd9
Reviewed-on: https://chromium-review.googlesource.com/c/1329701Reviewed-by: Chris Harrelson <chrishtr@chromium.org>
Commit-Queue: Xianzhu Wang <wangxianzhu@chromium.org>
Cr-Commit-Position: refs/heads/master@{#607041}

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

@@ -381,8 +381,9 @@ PaintResult PaintLayerPainter::PaintLayerContents(
     if (!ShouldRepaintSubsequence(paint_layer_, painting_info,
                                   respect_overflow_clip) &&
         SubsequenceRecorder::UseCachedSubsequenceIfPossible(context,
-                                                            paint_layer_))
-      return result;
+                                                            paint_layer_)) {
+      return paint_layer_.PreviousPaintResult();
+    }
     DCHECK(paint_layer_.SupportsSubsequenceCaching());
     subsequence_recorder.emplace(context, paint_layer_);
   }

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

@@ -323,6 +323,87 @@ TEST_P(PaintLayerPainterTest,
                           IsSameId(&content2, kBackgroundType)));
 }
 
+TEST_P(PaintLayerPainterTest, CachedSubsequenceRetainsPreviousPaintResult) {
+  // TODO(wangxianzhu): SPv2 deals with interest rect differently, so disable
+  // this test for SPv2 temporarily.
+  if (RuntimeEnabledFeatures::SlimmingPaintV2Enabled())
+    return;
+
+  SetBodyInnerHTML(R"HTML(
+    <style>
+      html, body { height: 100%; margin: 0 }
+      ::-webkit-scrollbar { display:none }
+    </style>
+    <div id="target" style="height: 8000px; contain: paint">
+      <div id="content1" style="height: 100px; background: blue"></div>
+      <div style="height: 6000px"></div>
+      <div id="content2" style="height: 100px; background: blue"></div>
+    </div>
+    <div id="change" style="display: none"></div>
+  )HTML");
+
+  const auto* target_layer =
+      ToLayoutBoxModelObject(GetLayoutObjectByElementId("target"))->Layer();
+  const auto* content1 = GetLayoutObjectByElementId("content1");
+  const auto* content2 = GetLayoutObjectByElementId("content2");
+  const auto& view_client = ViewScrollingBackgroundClient();
+  // |content2| is out of the interest rect.
+  EXPECT_THAT(RootPaintController().GetDisplayItemList(),
+              ElementsAre(IsSameId(&view_client, kDocumentBackgroundType),
+                          IsSameId(content1, kBackgroundType)));
+  // |target| is partially painted.
+  EXPECT_EQ(kMayBeClippedByPaintDirtyRect, target_layer->PreviousPaintResult());
+  EXPECT_EQ(LayoutRect(0, 0, 800, 4600),
+            target_layer->PreviousPaintDirtyRect());
+  // |target| created subsequence.
+  EXPECT_SUBSEQUENCE(*target_layer, 1u, 2u);
+
+  // Change something that triggers a repaint but |target| should use cached
+  // subsequence.
+  GetDocument().getElementById("change")->setAttribute(html_names::kStyleAttr,
+                                                       "display: block");
+  GetDocument().View()->UpdateAllLifecyclePhasesExceptPaint();
+  EXPECT_FALSE(target_layer->NeedsRepaint());
+  EXPECT_TRUE(PaintWithoutCommit());
+  EXPECT_EQ(2, NumCachedNewItems());
+  CommitAndFinishCycle();
+
+  // Painted result is the same.
+  EXPECT_THAT(RootPaintController().GetDisplayItemList(),
+              ElementsAre(IsSameId(&view_client, kDocumentBackgroundType),
+                          IsSameId(content1, kBackgroundType)));
+  // |target| is still partially painted.
+  EXPECT_EQ(kMayBeClippedByPaintDirtyRect, target_layer->PreviousPaintResult());
+  EXPECT_EQ(LayoutRect(0, 0, 800, 4600),
+            target_layer->PreviousPaintDirtyRect());
+  // |target| still created subsequence (cached).
+  EXPECT_SUBSEQUENCE(*target_layer, 1u, 2u);
+
+  // Scroll the view so that both |content1| and |content2| are in the interest
+  // rect.
+  GetLayoutView().GetScrollableArea()->SetScrollOffset(ScrollOffset(0, 3000),
+                                                       kProgrammaticScroll);
+  GetDocument().View()->UpdateAllLifecyclePhasesExceptPaint();
+  // Scrolling doesn't set NeedsRepaint flag. Change of paint dirty rect of
+  // a partially painted layer will trigger repaint.
+  EXPECT_FALSE(target_layer->NeedsRepaint());
+  EXPECT_TRUE(PaintWithoutCommit());
+  EXPECT_EQ(2, NumCachedNewItems());
+  CommitAndFinishCycle();
+
+  // Painted result should include both |content1| and |content2|.
+  EXPECT_THAT(RootPaintController().GetDisplayItemList(),
+              ElementsAre(IsSameId(&view_client, kDocumentBackgroundType),
+                          IsSameId(content1, kBackgroundType),
+                          IsSameId(content2, kBackgroundType)));
+  // |target| is still partially painted.
+  EXPECT_EQ(kMayBeClippedByPaintDirtyRect, target_layer->PreviousPaintResult());
+  EXPECT_EQ(LayoutRect(0, 0, 800, 7600),
+            target_layer->PreviousPaintDirtyRect());
+  // |target| still created subsequence (repainted).
+  EXPECT_SUBSEQUENCE(*target_layer, 1u, 3u);
+}
+
 TEST_P(PaintLayerPainterTest, PaintPhaseOutline) {
   AtomicString style_without_outline =
       "width: 50px; height: 50px; background-color: green";