[Backport] Fix for security issue 1050090

Reland "Don't validate clients for display items copied from cached subsequence"

This is a reland of 0aec6da69cf422ed80e2b743314d1e6d1f3b2b98

This reland reverts the change about paint chunks, and modified the
test to test client of display item only.

Original change's description:
> Don't validate clients for display items copied from cached subsequence
>
> This avoids callbacks to clients that are already validated in the
> previous cycle, and checks or protects from under-invalidation of
> subsequences when any contained client has been deleted.
>
> Bug: 1050090
> Change-Id: I1ba11bd82d64b8e80c09fd6f0dee3ad6924eb161
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2033822
> Reviewed-by: Philip Rogers <pdr@chromium.org>
> Commit-Queue: Xianzhu Wang <wangxianzhu@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#739552}

TBR=pdr@chromium.org, wangxianzhu@chromium.org

(cherry picked from commit 2aaf264e6f60ebc2db2fa857968dffd7d73a9bf2)

Bug: 1050090
Change-Id: I65cfb07520e9fb2b336a3f214c559a609b7d3197
Reviewed-by: Xianzhu Wang <wangxianzhu@chromium.org>
Commit-Queue: Xianzhu Wang <wangxianzhu@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#739776}
Cr-Commit-Position: refs/branch-heads/4044@{#270}
Cr-Branched-From: a6d9daf149a473ceea37f629c41d4527bf2055bd-refs/heads/master@{#737173}
Reviewed-by: Jüri Valdmann <juri.valdmann@qt.io>
Reviewed-by: Allan Sandfeld Jensen <allan.jensen@qt.io>

4 files changed, 63 insertions, 7 deletions

diff --git a/chromium/third_party/blink/renderer/platform/graphics/paint/display_item.h b/chromium/third_party/blink/renderer/platform/graphics/paint/display_item.h
index be665246f7b..4dc58b2ad2d 100644
--- a/chromium/third_party/blink/renderer/platform/graphics/paint/display_item.h
+++ b/chromium/third_party/blink/renderer/platform/graphics/paint/display_item.h
@@ -154,10 +154,11 @@ class PLATFORM_EXPORT DisplayItem {
         fragment_(0),
         // TODO(pdr): Should this return true for IsScrollHitTestType too?
         is_cacheable_(client.IsCacheable() && IsDrawingType(type)),
-        is_tombstone_(false) {
+        is_tombstone_(false),
+        is_copied_from_cached_subsequence_(false) {
     // |derived_size| must fit in |derived_size_|.
     // If it doesn't, enlarge |derived_size_| and fix this assert.
-    SECURITY_DCHECK(derived_size < (1 << 8));
+    SECURITY_DCHECK(derived_size < (1 << 7));
     SECURITY_DCHECK(derived_size >= sizeof(*this));
   }
 
@@ -254,6 +255,13 @@ class PLATFORM_EXPORT DisplayItem {
 
   bool IsCacheable() const { return is_cacheable_; }
 
+  bool IsCopiedFromCachedSubsequence() const {
+    return is_copied_from_cached_subsequence_;
+  }
+  void SetCopiedFromCachedSubsequence(bool b) {
+    is_copied_from_cached_subsequence_ = b;
+  }
+
   virtual bool Equals(const DisplayItem& other) const {
     // Failure of this DCHECK would cause bad casts in subclasses.
     SECURITY_CHECK(!is_tombstone_);
@@ -293,10 +301,11 @@ class PLATFORM_EXPORT DisplayItem {
 
   static_assert(kTypeLast < (1 << 8), "DisplayItem::Type should fit in 8 bits");
   unsigned type_ : 8;
-  unsigned derived_size_ : 8;  // size of the actual derived class
+  unsigned derived_size_ : 7;  // size of the actual derived class
   unsigned fragment_ : 14;
   unsigned is_cacheable_ : 1;
   unsigned is_tombstone_ : 1;
+  unsigned is_copied_from_cached_subsequence_ : 1;
 };
 
 inline bool operator==(const DisplayItem::Id& a, const DisplayItem::Id& b) {
diff --git a/chromium/third_party/blink/renderer/platform/graphics/paint/display_item_list.h b/chromium/third_party/blink/renderer/platform/graphics/paint/display_item_list.h
index 5a6df085081..3389b428c6f 100644
--- a/chromium/third_party/blink/renderer/platform/graphics/paint/display_item_list.h
+++ b/chromium/third_party/blink/renderer/platform/graphics/paint/display_item_list.h
@@ -44,7 +44,8 @@ class PLATFORM_EXPORT DisplayItemList
         ContiguousContainer::AppendByMoving(item, item.DerivedSize());
     // ContiguousContainer::AppendByMoving() calls an in-place constructor
     // on item which replaces it with a tombstone/"dead display item" that
-    // can be safely destructed but should never be used except for debugging.
+    // can be safely destructed but should never be used except for debugging
+    // and raster invalidation (see below).
     DCHECK(item.IsTombstone());
     DCHECK(item.GetId() == result.GetId());
     // We need |visual_rect_| and |outset_for_raster_effects_| of the old
@@ -53,6 +54,7 @@ class PLATFORM_EXPORT DisplayItemList
     // original values back from |result|.
     item.visual_rect_ = result.visual_rect_;
     item.outset_for_raster_effects_ = result.outset_for_raster_effects_;
+    result.SetCopiedFromCachedSubsequence(false);
     return result;
   }
 
diff --git a/chromium/third_party/blink/renderer/platform/graphics/paint/paint_controller.cc b/chromium/third_party/blink/renderer/platform/graphics/paint/paint_controller.cc
index 745cfbf980c..a59a05cd8f2 100644
--- a/chromium/third_party/blink/renderer/platform/graphics/paint/paint_controller.cc
+++ b/chromium/third_party/blink/renderer/platform/graphics/paint/paint_controller.cc
@@ -458,7 +458,9 @@ void PaintController::CopyCachedSubsequence(size_t begin_index,
     }
 #endif
 
-    ProcessNewItem(MoveItemFromCurrentListToNewList(current_index));
+    auto& item = MoveItemFromCurrentListToNewList(current_index);
+    item.SetCopiedFromCachedSubsequence(true);
+    ProcessNewItem(item);
     DCHECK((!new_paint_chunks_.LastChunk().is_cacheable &&
             !cached_chunk->is_cacheable) ||
            new_paint_chunks_.LastChunk().Matches(*cached_chunk));
@@ -541,13 +543,25 @@ void PaintController::FinishCycle() {
     }
     for (const auto& item : current_paint_artifact_->GetDisplayItemList()) {
       const auto& client = item.Client();
+
+      if (item.IsCopiedFromCachedSubsequence()) {
+        // We don't need to validate the clients of a display item that is
+        // copied from a cached subsequence, because it should be already
+        // valid. See http://crbug.com/1050090 for more details.
+#if DCHECK_IS_ON()
+        DCHECK(client.IsAlive());
+        DCHECK(client.IsValid() || !client.IsCacheable());
+#endif
+        continue;
+      }
       client.ClearPartialInvalidationVisualRect();
       if (client.IsCacheable())
         client.Validate();
     }
     for (const auto& chunk : current_paint_artifact_->PaintChunks()) {
-      if (chunk.id.client.IsCacheable())
-        chunk.id.client.Validate();
+        const auto& client = chunk.id.client;
+        if (client.IsCacheable())
+          client.Validate();
     }
   }
 
diff --git a/chromium/third_party/blink/renderer/platform/graphics/paint/paint_controller_test.cc b/chromium/third_party/blink/renderer/platform/graphics/paint/paint_controller_test.cc
index be7dd5659c4..57b32a6bd48 100644
--- a/chromium/third_party/blink/renderer/platform/graphics/paint/paint_controller_test.cc
+++ b/chromium/third_party/blink/renderer/platform/graphics/paint/paint_controller_test.cc
@@ -1648,6 +1648,37 @@ TEST_P(PaintControllerTest, DuplicatedSubsequences) {
   CommitAndFinishCycle();
 }
 
+TEST_P(PaintControllerTest, DeletedClientInUnderInvaldiatedSubsequence) {
+  if (RuntimeEnabledFeatures::PaintUnderInvalidationCheckingEnabled())
+    return;
+
+  FakeDisplayItemClient container("container");
+  auto content = std::make_unique<FakeDisplayItemClient>("content");
+  GraphicsContext context(GetPaintController());
+
+  InitRootChunk();
+  {
+    SubsequenceRecorder r(context, container);
+    GetPaintController().ForceNewChunk(container, kBackgroundType);
+    DrawRect(context, *content, kBackgroundType, FloatRect(100, 100, 300, 300));
+  }
+  CommitAndFinishCycle();
+
+  content = nullptr;
+  InitRootChunk();
+  // Leave container not invalidated.
+#if DCHECK_IS_ON()
+  ASSERT_DEATH(
+      SubsequenceRecorder::UseCachedSubsequenceIfPossible(context, container),
+      "");
+#else
+  // This should not crash.
+  EXPECT_TRUE(
+      SubsequenceRecorder::UseCachedSubsequenceIfPossible(context, container));
+  CommitAndFinishCycle();
+#endif
+}
+
 class PaintControllerUnderInvalidationTest
     : public PaintControllerTestBase,
       private ScopedPaintUnderInvalidationCheckingForTest {