Handle removed display items in under-invalidation checking in cached subsequences

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

 // Copyright 2014 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "platform/graphics/paint/PaintController.h"
 
 #include "platform/TraceEvent.h"
 #include "platform/graphics/GraphicsLayer.h"
 #include "platform/graphics/paint/DrawingDisplayItem.h"
 #include "third_party/skia/include/core/SkPictureAnalyzer.h"
@@ skipping 121 matching lines @@
         return;
 
 #if DCHECK_IS_ON()
     // Also remove the index pointing to the removed display item.
     DisplayItemIndicesByClientMap::iterator it = m_newDisplayItemIndicesByClient.find(&m_newDisplayItemList.last().client());
     if (it != m_newDisplayItemIndicesByClient.end()) {
         Vector<size_t>& indices = it->value;
         if (!indices.isEmpty() && indices.last() == (m_newDisplayItemList.size() - 1))
             indices.removeLast();
     }
+
+    if (RuntimeEnabledFeatures::slimmingPaintUnderInvalidationCheckingEnabled() && isCheckingUnderInvalidation()) {
+        if (m_skippedUnderInvalidationCount) {
+            --m_skippedUnderInvalidationCount;
+        } else {
+            DCHECK(m_underInvalidationCheckingBegin);
+            --m_underInvalidationCheckingBegin;
+        }
+    }
 #endif
     m_newDisplayItemList.removeLast();
 
     if (RuntimeEnabledFeatures::slimmingPaintV2Enabled())
         m_newPaintChunks.decrementDisplayItemIndex();
 }
 
 void PaintController::processNewItem(DisplayItem& displayItem)
 {
     DCHECK(!m_constructionDisabled);
@@ skipping 242 matching lines @@
     return enclosingIntRect(visualRect);
 }
 
 void PaintController::resetCurrentListIndices()
 {
     m_nextItemToMatch = 0;
     m_nextItemToIndex = 0;
 #if DCHECK_IS_ON()
     m_underInvalidationCheckingBegin = 0;
     m_underInvalidationCheckingEnd = 0;
+    m_skippedUnderInvalidationCount = 0;
 #endif
 }
 
 void PaintController::commitNewDisplayItems(const LayoutSize& offsetFromLayoutObject)
 {
     TRACE_EVENT2("blink,benchmark", "PaintController::commitNewDisplayItems",
         "current_display_list_size", (int)m_currentPaintArtifact.getDisplayItemList().size(),
         "num_non_cached_new_items", (int)m_newDisplayItemList.size() - m_numCachedNewItems);
     m_numCachedNewItems = 0;
 
@@ skipping 83 matching lines @@
 void PaintController::appendDebugDrawingAfterCommit(const DisplayItemClient& displayItemClient, PassRefPtr<SkPicture> picture, const LayoutSize& offsetFromLayoutObject)
 {
     DCHECK(m_newDisplayItemList.isEmpty());
     DrawingDisplayItem& displayItem = m_currentPaintArtifact.getDisplayItemList().allocateAndConstruct<DrawingDisplayItem>(displayItemClient, DisplayItem::DebugDrawing, picture);
     displayItem.setSkippedCache();
     m_currentPaintArtifact.getDisplayItemList().appendVisualRect(visualRectForDisplayItem(displayItem, offsetFromLayoutObject));
 }
 
 #if DCHECK_IS_ON()
 
-void PaintController::showUnderInvalidationError(const char* reason, const DisplayItem& newItem, const DisplayItem& oldItem) const
+void PaintController::showUnderInvalidationError(const char* reason, const DisplayItem& newItem, const DisplayItem* oldItem) const
 {
     LOG(ERROR) << m_underInvalidationMessagePrefix << " " << reason;
 #ifndef NDEBUG
     LOG(ERROR) << "New display item: " << newItem.asDebugString();
-    LOG(ERROR) << "Old display item: " << oldItem.asDebugString();
+    LOG(ERROR) << "Old display item: " << (oldItem ? oldItem->asDebugString() : "None");
 #else
     LOG(ERROR) << "Run debug build to get more details.";
 #endif
     LOG(ERROR) << "See http://crbug.com/619103.";
 
 #ifndef NDEBUG
-    if (newItem.isDrawing()) {
-        RefPtr<const SkPicture> newPicture = static_cast<const DrawingDisplayItem&>(newItem).picture();
-        RefPtr<const SkPicture> oldPicture = static_cast<const DrawingDisplayItem&>(oldItem).picture();
-        LOG(INFO) << "new picture:\n" << (newPicture ? pictureAsDebugString(newPicture.get()) : "None");
-        LOG(INFO) << "old picture:\n" << (oldPicture ? pictureAsDebugString(oldPicture.get()) : "None");
-    }
+    const SkPicture* newPicture = newItem.isDrawing() ? static_cast<const DrawingDisplayItem&>(newItem).picture() : nullptr;
+    const SkPicture* oldPicture = oldItem && oldItem->isDrawing() ? static_cast<const DrawingDisplayItem*>(oldItem)->picture() : nullptr;
+    LOG(INFO) << "new picture:\n" << (newPicture ? pictureAsDebugString(newPicture) : "None");
+    LOG(INFO) << "old picture:\n" << (oldPicture ? pictureAsDebugString(oldPicture) : "None");
+
+    showDebugData();
 #endif // NDEBUG
 }
 
 void PaintController::checkUnderInvalidation()
 {
     DCHECK(RuntimeEnabledFeatures::slimmingPaintUnderInvalidationCheckingEnabled());
 
     if (!isCheckingUnderInvalidation())
         return;
 
     const DisplayItem& newItem = m_newDisplayItemList.last();
-    const DisplayItem& oldItem = m_currentPaintArtifact.getDisplayItemList()[m_underInvalidationCheckingBegin++];
+    size_t oldItemIndex = m_underInvalidationCheckingBegin + m_skippedUnderInvalidationCount;
+    const DisplayItem* oldItem = oldItemIndex < m_currentPaintArtifact.getDisplayItemList().size() ? &m_currentPaintArtifact.getDisplayItemList()[oldItemIndex] : nullptr;
 
     if (newItem.isCacheable() && !clientCacheIsValid(newItem.client())) {
         showUnderInvalidationError("under-invalidation of PaintLayer: invalidated in cached subsequence", newItem, oldItem);
         NOTREACHED();
     }
-    if (!newItem.equals(oldItem)) {
-        showUnderInvalidationError("under-invalidation: display item changed", newItem, oldItem);
+
+    bool oldAndNewEqual = oldItem && newItem.equals(*oldItem);
+    if (!oldAndNewEqual) {

[chrishtr, 2016/07/28 16:55:36]

Don't you need to reset m_skippedUnderInvalidationCount to zero as soon as the
pattern of begin/end or compositebegin/drawing/end does *not* happen?

[Xianzhu, 2016/07/28 17:20:48]

No. If the pattern does not happen, non-zero m_skippedUnderInvalidationCount
means real under-invalidation because the mismatching display items actually
exist. Line 568-274 below will report under-invalidation in the case.

[chrishtr, 2016/07/28 17:29:21]

Suppose the display list is this:

BeginCompositing - Drawing - Drawing - EndCompositing

After the first two are processed, won't m_skippedUnderInvalidationCount be 2?
What resets it to 0 when the second Drawing is encountered?

[Xianzhu, 2016/07/28 17:38:40]

It's not reset to 0. It is non-zero only if we have seen mismatching display
items in a subsequence that is supposed to use the cache. It's 2 only if the
BeginCompositing and the first Drawing didn't match the cached display items. At
the second Drawing, we know that the skipped probable under-invalidations are
real under-invalidations and will report the mismatching BeginCompositing as
under-invalidation.

If all the new display items are matching cached ones,
m_skippedUnderInvalidationCount will be always 0.

+        if (newItem.isBegin()) {
+            // Ignore mismatching begin display item which may be removed when we remove a no-op pair.
+            ++m_skippedUnderInvalidationCount;
+            return;
+        }
+        if (newItem.isDrawing() && m_skippedUnderInvalidationCount == 1 && m_newDisplayItemList[m_newDisplayItemList.size() - 2].getType() == DisplayItem::BeginCompositing) {

[chrishtr, 2016/07/28 16:55:36]

Check if the display list has length at least 2.

[Xianzhu, 2016/07/28 17:20:48]

Added a DCHECK. The condition should be always true because
m_newDisplayItemList.size() > m_skippedUnderInvalidationCount.

+            // This might be a drawing item between a pair of begin/end compositing display items that will be folded into a single drawing display item.
+            ++m_skippedUnderInvalidationCount;
+            return;
+        }
+    }
+
+    if (m_skippedUnderInvalidationCount || !oldAndNewEqual) {
+        // If we ever skipped reporting any under-invalidations, report the earliest one.
+        showUnderInvalidationError("under-invalidation: display item changed",
+            m_newDisplayItemList[m_newDisplayItemList.size() - m_skippedUnderInvalidationCount - 1],
+            &m_currentPaintArtifact.getDisplayItemList()[m_underInvalidationCheckingBegin]);
         NOTREACHED();
     }
+
+    ++m_underInvalidationCheckingBegin;
 }
 
 #endif // DCHECK_IS_ON()
 
 #ifndef NDEBUG
 
 String PaintController::displayItemListAsDebugString(const DisplayItemList& list) const
 {
     StringBuilder stringBuilder;
     size_t i = 0;
@@ skipping 20 matching lines @@
 
 void PaintController::showDebugData() const
 {
     WTFLogAlways("current display item list: [%s]\n", displayItemListAsDebugString(m_currentPaintArtifact.getDisplayItemList()).utf8().data());
     WTFLogAlways("new display item list: [%s]\n", displayItemListAsDebugString(m_newDisplayItemList).utf8().data());
 }
 
 #endif // ifndef NDEBUG
 
 } // namespace blink