Reland of Check DisplayItemCient aliveness in cached subsequences (patchset #1...

Reland of Check DisplayItemCient aliveness in cached subsequences (patchset #1 id:1 of https://codereview.chromium.org/2078393003/ )

Reason for revert:
It was not acceptable to revert the OwnPtr removal
patch just for this revert.

Relanding this to land OwnPtr commit again.

Original issue's description:
> Revert of Check DisplayItemCient aliveness in cached subsequences (patchset #6 id:100001 of https://codereview.chromium.org/2066603005/ )
>
> Reason for revert:
> Based on r400605 that is to be reverted.
>
> Original issue's description:
> > Check DisplayItemCient aliveness in cached subsequences
> >
> > BUG=609218
> >
> > Committed: https://crrev.com/127e5fadca5f431a5de1bef939f7f43aec1d136f
> > Cr-Commit-Position: refs/heads/master@{#400607}
>
> TBR=chrishtr@chromium.org,wangxianzhu@chromium.org
> # Not skipping CQ checks because original CL landed more than 1 days ago.
> BUG=609218
>
> Committed: https://chromium.googlesource.com/chromium/src/+/6ece4aaeb5e2c53eca2617d42de64ff917cee884

TBR=chrishtr@chromium.org,wangxianzhu@chromium.org,vasilii@chromium.org
# Skipping CQ checks because original CL landed less than 1 days ago.
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG=609218

Review-Url: https://codereview.chromium.org/2082553002
Cr-Commit-Position: refs/heads/master@{#400645}

third_party/WebKit/Source/core/layout/LayoutView.cpp

@@ -961,6 +961,12 @@ void LayoutView::setIsInWindow(bool isInWindow)
 {
     if (m_compositor)
         m_compositor->setIsInWindow(isInWindow);
+#if CHECK_DISPLAY_ITEM_CLIENT_ALIVENESS
+    // We don't invalidate layers during document detach(), so must clear the should-keep-alive
+    // DisplayItemClients which may be deleted before the layers being subsequence owners.
+    if (!isInWindow && layer())
+        layer()->endShouldKeepAliveAllClientsRecursive();
+#endif
 }
 
 IntervalArena* LayoutView::intervalArena()

third_party/WebKit/Source/core/paint/PaintLayer.cpp

@@ -2888,8 +2888,10 @@ void PaintLayer::markCompositingContainerChainForNeedsRepaint()
                 break;
             container = owner->enclosingLayer();
         }
+
         if (container->m_needsRepaint)
             break;
+
         container->setNeedsRepaintInternal();
         layer = container;
     }
@@ -2909,6 +2911,15 @@ PaintTiming* PaintLayer::paintTiming()
     return nullptr;
 }
 
+void PaintLayer::endShouldKeepAliveAllClientsRecursive()
+{
+    for (PaintLayer* child = firstChild(); child; child = child->nextSibling())
+        child->endShouldKeepAliveAllClientsRecursive();
+#if CHECK_DISPLAY_ITEM_CLIENT_ALIVENESS
+    DisplayItemClient::endShouldKeepAliveAllClients(this);
+#endif
+}
+
 DisableCompositingQueryAsserts::DisableCompositingQueryAsserts()
     : m_disabler(gCompositingQueryMode, CompositingQueriesAreAllowed) { }
 

third_party/WebKit/Source/core/paint/PaintLayer.h

@@ -705,6 +705,10 @@ public:
     bool update3DTransformedDescendantStatus();
     bool has3DTransformedDescendant() const { DCHECK(!m_3DTransformedDescendantStatusDirty); return m_has3DTransformedDescendant; }
 
+#if CHECK_DISPLAY_ITEM_CLIENT_ALIVENESS
+    void endShouldKeepAliveAllClientsRecursive();
+#endif
+
 private:
     // Bounding box in the coordinates of this layer.
     LayoutRect logicalBoundingBox() const;

third_party/WebKit/Source/platform/graphics/paint/DisplayItemClient.cpp

@@ -39,6 +39,8 @@ DisplayItemClient::~DisplayItemClient()
         }
     }
     liveDisplayItemClients->remove(this);
+    // In case this object is a subsequence owner.
+    endShouldKeepAliveAllClients(this);
 }
 
 bool DisplayItemClient::isAlive() const
@@ -46,20 +48,20 @@ bool DisplayItemClient::isAlive() const
     return liveDisplayItemClients && liveDisplayItemClients->contains(this);
 }
 
-void DisplayItemClient::beginShouldKeepAlive(const void* paintController) const
+void DisplayItemClient::beginShouldKeepAlive(const void* owner) const
 {
     CHECK(isAlive());
     if (!displayItemClientsShouldKeepAlive)
         displayItemClientsShouldKeepAlive = new HashMap<const void*, HashMap<const DisplayItemClient*, String>>();
-    auto addResult = displayItemClientsShouldKeepAlive->add(paintController, HashMap<const DisplayItemClient*, String>()).storedValue->value.add(this, "");
+    auto addResult = displayItemClientsShouldKeepAlive->add(owner, HashMap<const DisplayItemClient*, String>()).storedValue->value.add(this, "");
     if (addResult.isNewEntry)
         addResult.storedValue->value = debugName();
 }
 
-void DisplayItemClient::endShouldKeepAliveAllClients(const void* paintController)
+void DisplayItemClient::endShouldKeepAliveAllClients(const void* owner)
 {
     if (displayItemClientsShouldKeepAlive)
-        displayItemClientsShouldKeepAlive->remove(paintController);
+        displayItemClientsShouldKeepAlive->remove(owner);
 }
 
 void DisplayItemClient::endShouldKeepAliveAllClients()

third_party/WebKit/Source/platform/graphics/paint/DisplayItemClient.h

@@ -64,14 +64,19 @@ public:
     // Tests if this DisplayItemClient object has been created and has not been deleted yet.
     bool isAlive() const;
     // Called when any DisplayItem of this DisplayItemClient is added into PaintController
-    // using PaintController::createAndAppend().
-    void beginShouldKeepAlive(const void* paintController) const;
+    // using PaintController::createAndAppend() or into a cached subsequence.
+    void beginShouldKeepAlive(const void* owner) const;
     // Clears all should-keep-alive DisplayItemClients of a PaintController. Called after
-    // PaintController commits new display items.
-    static void endShouldKeepAliveAllClients(const void* paintController);
+    // PaintController commits new display items or the subsequence owner is invalidated.
+    static void endShouldKeepAliveAllClients(const void* owner);
     static void endShouldKeepAliveAllClients();
+
+    // Called to clear should-keep-alive of DisplayItemClients in a subsequence if this
+    // object is a subsequence.
+#define ON_DISPLAY_ITEM_CLIENT_INVALIDATION() endShouldKeepAliveAllClients(this)
 #else
     virtual ~DisplayItemClient() { }
+#define ON_DISPLAY_ITEM_CLIENT_INVALIDATION()
 #endif
 
     virtual String debugName() const = 0;
@@ -88,7 +93,11 @@ public:
 #define DISPLAY_ITEM_CACHE_STATUS_IMPLEMENTATION \
     bool displayItemsAreCached(DisplayItemCacheGeneration cacheGeneration) const final { return m_cacheGeneration.matches(cacheGeneration); } \
     void setDisplayItemsCached(DisplayItemCacheGeneration cacheGeneration) const final { m_cacheGeneration = cacheGeneration; } \
-    void setDisplayItemsUncached() const final { m_cacheGeneration.invalidate(); } \
+    void setDisplayItemsUncached() const final \
+    { \
+        m_cacheGeneration.invalidate(); \
+        ON_DISPLAY_ITEM_CLIENT_INVALIDATION(); \
+    } \
     mutable DisplayItemCacheGeneration m_cacheGeneration;
 
 #define DISPLAY_ITEM_CACHE_STATUS_UNCACHEABLE_IMPLEMENTATION \

third_party/WebKit/Source/platform/graphics/paint/PaintController.cpp

@@ -66,8 +66,26 @@ void PaintController::processNewItem(DisplayItem& displayItem)
     DCHECK(!skippingCache() || !displayItem.isCached());
 
 #if CHECK_DISPLAY_ITEM_CLIENT_ALIVENESS
-    if (!skippingCache() && (displayItem.isCacheable() || displayItem.isCached()))
-        displayItem.client().beginShouldKeepAlive(this);
+    if (!skippingCache()) {
+        if (displayItem.isCacheable() || displayItem.isCached()) {
+            // Mark the client shouldKeepAlive under this PaintController.
+            // The status will end after the new display items are committed.
+            displayItem.client().beginShouldKeepAlive(this);
+
+            if (!m_currentSubsequenceClients.isEmpty()) {
+                // Mark the client shouldKeepAlive under the current subsequence.
+                // The status will end when the subsequence owner is invalidated or deleted.
+                displayItem.client().beginShouldKeepAlive(m_currentSubsequenceClients.last());
+            }
+        }
+
+        if (displayItem.getType() == DisplayItem::Subsequence) {
+            m_currentSubsequenceClients.append(&displayItem.client());
+        } else if (displayItem.getType() == DisplayItem::EndSubsequence) {
+            CHECK(m_currentSubsequenceClients.last() == &displayItem.client());
+            m_currentSubsequenceClients.removeLast();
+        }
+    }
 #endif
 
     if (displayItem.isCached())
@@ -370,6 +388,7 @@ void PaintController::updateCacheGeneration()
         displayItem.client().setDisplayItemsCached(m_currentCacheGeneration);
     }
 #if CHECK_DISPLAY_ITEM_CLIENT_ALIVENESS
+    CHECK(m_currentSubsequenceClients.isEmpty());
     DisplayItemClient::endShouldKeepAliveAllClients(this);
 #endif
 }

third_party/WebKit/Source/platform/graphics/paint/PaintController.h

@@ -216,6 +216,11 @@ private:
 #endif
 
     DisplayItemCacheGeneration m_currentCacheGeneration;
+
+#if CHECK_DISPLAY_ITEM_CLIENT_ALIVENESS
+    // A stack recording subsequence clients that are currently painting.
+    Vector<const DisplayItemClient*> m_currentSubsequenceClients;
+#endif
 };
 
 } // namespace blink

third_party/WebKit/Source/platform/graphics/paint/PaintControllerTest.cpp

@@ -502,6 +502,10 @@ TEST_F(PaintControllerTest, CachedSubsequenceSwapOrder)
         TestDisplayItem(content1, foregroundDrawingType),
         TestDisplayItem(container1, foregroundDrawingType),
         TestDisplayItem(container1, DisplayItem::EndSubsequence));
+
+#if CHECK_DISPLAY_ITEM_CLIENT_ALIVENESS
+    DisplayItemClient::endShouldKeepAliveAllClients();
+#endif
 }
 
 TEST_F(PaintControllerTest, OutOfOrderNoCrash)
@@ -620,6 +624,10 @@ TEST_F(PaintControllerTest, CachedNestedSubsequenceUpdate)
         TestDisplayItem(content1, DisplayItem::EndSubsequence),
         TestDisplayItem(container1, foregroundDrawingType),
         TestDisplayItem(container1, DisplayItem::EndSubsequence));
+
+#if CHECK_DISPLAY_ITEM_CLIENT_ALIVENESS
+    DisplayItemClient::endShouldKeepAliveAllClients();
+#endif
 }
 
 TEST_F(PaintControllerTest, SkipCache)
@@ -893,6 +901,10 @@ TEST_F(PaintControllerTest, IsNotSuitableForGpuRasterizationSinglePictureManyPat
     EXPECT_TRUE(SubsequenceRecorder::useCachedSubsequenceIfPossible(context, container));
     getPaintController().commitNewDisplayItems(LayoutSize());
     EXPECT_FALSE(getPaintController().paintArtifact().isSuitableForGpuRasterization());
+
+#if CHECK_DISPLAY_ITEM_CLIENT_ALIVENESS
+    DisplayItemClient::endShouldKeepAliveAllClients();
+#endif
 }
 
 // Temporarily disabled (pref regressions due to GPU veto stickiness: http://crbug.com/603969).