Find cached display items directly during painting

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 13 matching lines @@
     return PaintChunker::DefaultBehavior;
 }
 
 const PaintArtifact& PaintController::paintArtifact() const
 {
     DCHECK(m_newDisplayItemList.isEmpty());
     DCHECK(m_newPaintChunks.isInInitialState());
     return m_currentPaintArtifact;
 }
 
+bool PaintController::useCachedDrawingIfPossible(const DisplayItemClient& client, DisplayItem::Type type)
+{
+    DCHECK(DisplayItem::isDrawingType(type));
+
+    if (displayItemConstructionIsDisabled())
+        return false;
+
+    if (!clientCacheIsValid(client))
+        return false;
+
+#if ENABLE(ASSERT)
+    // When under-invalidation checking is enabled, we output CachedDrawing display item
+    // followed by the display item containing forced painting.
+    if (RuntimeEnabledFeatures::slimmingPaintUnderInvalidationCheckingEnabled())
+        return false;
+#endif
+
+    DisplayItemList::iterator it = findCachedItem(DisplayItem::Id(client, type));
+    if (it == m_currentPaintArtifact.getDisplayItemList().end()) {
+        NOTREACHED();
+        return false;
+    }
+
+    ++m_numCachedNewItems;
+    ensureNewDisplayItemListInitialCapacity();
+    processNewItem(m_newDisplayItemList.appendByMoving(*it));
+
+    m_nextItemToMatch = it + 1;
+    // Items before m_nextItemToMatch have been copied so we don't need to index them.
+    if (m_nextItemToMatch - m_nextItemToIndex > 0)
+        m_nextItemToIndex = m_nextItemToMatch;
+
+    return true;
+}
+
+bool PaintController::useCachedSubsequenceIfPossible(const DisplayItemClient& client)
+{
+    // TODO(crbug.com/596983): Implement subsequence caching for spv2.
+    if (RuntimeEnabledFeatures::slimmingPaintV2Enabled())
+        return false;
+
+    if (displayItemConstructionIsDisabled() || subsequenceCachingIsDisabled())
+        return false;
+
+    if (!clientCacheIsValid(client))
+        return false;
+
+#if ENABLE(ASSERT)
+    // When under-invalidation checking is enabled, we output CachedDrawing display item
+    // followed by the display item containing forced painting.
+    if (RuntimeEnabledFeatures::slimmingPaintUnderInvalidationCheckingEnabled())
+        return false;
+#endif
+
+    DisplayItemList::iterator it = findCachedItem(DisplayItem::Id(client, DisplayItem::Subsequence));
+    if (it == m_currentPaintArtifact.getDisplayItemList().end()) {
+        NOTREACHED();
+        return false;
+    }
+
+    // |it| will point to the first item after the subsequence or end of the current list.
+    ensureNewDisplayItemListInitialCapacity();
+    copyCachedSubsequence(it);
+
+    m_nextItemToMatch = it;
+    // Items before |it| have been copied so we don't need to index them.
+    if (it - m_nextItemToIndex > 0)
+        m_nextItemToIndex = it;
+
+    return true;
+}
+
 bool PaintController::lastDisplayItemIsNoopBegin() const
 {
     if (m_newDisplayItemList.isEmpty())
         return false;
 
     const auto& lastDisplayItem = m_newDisplayItemList.last();
     return lastDisplayItem.isBegin() && !lastDisplayItem.drawsContent();
 }
 
 void PaintController::removeLastDisplayItem()
@@ skipping 12 matching lines @@
 #endif
     m_newDisplayItemList.removeLast();
 
     if (RuntimeEnabledFeatures::slimmingPaintV2Enabled())
         m_newPaintChunks.decrementDisplayItemIndex();
 }
 
 void PaintController::processNewItem(DisplayItem& displayItem)
 {
     DCHECK(!m_constructionDisabled);
-    DCHECK(!skippingCache() || !displayItem.isCached());
 
 #if CHECK_DISPLAY_ITEM_CLIENT_ALIVENESS
     if (!skippingCache()) {
-        if (displayItem.isCacheable() || displayItem.isCached()) {
+        if (displayItem.isCacheable()) {
             // 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())
-        ++m_numCachedNewItems;
-
 #if DCHECK_IS_ON()
     // Verify noop begin/end pairs have been removed.
     if (m_newDisplayItemList.size() >= 2 && displayItem.isEnd()) {
         const auto& beginDisplayItem = m_newDisplayItemList[m_newDisplayItemList.size() - 2];
         if (beginDisplayItem.isBegin() && beginDisplayItem.getType() != DisplayItem::Subsequence && !beginDisplayItem.drawsContent())
             DCHECK(!displayItem.isEndAndPairedWith(beginDisplayItem.getType()));
     }
 #endif
 
     if (skippingCache())
         displayItem.setSkippedCache();
 
 #if DCHECK_IS_ON()
-    size_t index = findMatchingItemFromIndex(displayItem.nonCachedId(), m_newDisplayItemIndicesByClient, m_newDisplayItemList);
+    size_t index = findMatchingItemFromIndex(displayItem.getId(), m_newDisplayItemIndicesByClient, m_newDisplayItemList);
     if (index != kNotFound) {
 #ifndef NDEBUG
         showDebugData();
         WTFLogAlways("DisplayItem %s has duplicated id with previous %s (index=%d)\n",
             displayItem.asDebugString().utf8().data(), m_newDisplayItemList[index].asDebugString().utf8().data(), static_cast<int>(index));
 #endif
         NOTREACHED();
     }
     addItemToIndexIfNeeded(displayItem, m_newDisplayItemList.size() - 1, m_newDisplayItemIndicesByClient);
 #endif // DCHECK_IS_ON()
@@ skipping 33 matching lines @@
 size_t PaintController::findMatchingItemFromIndex(const DisplayItem::Id& id, const DisplayItemIndicesByClientMap& displayItemIndicesByClient, const DisplayItemList& list)
 {
     DisplayItemIndicesByClientMap::const_iterator it = displayItemIndicesByClient.find(&id.client);
     if (it == displayItemIndicesByClient.end())
         return kNotFound;
 
     const Vector<size_t>& indices = it->value;
     for (size_t index : indices) {
         const DisplayItem& existingItem = list[index];
         DCHECK(!existingItem.hasValidClient() || existingItem.client() == id.client);
-        if (id.matches(existingItem))
+        if (id == existingItem.getId())
             return index;
     }
 
     return kNotFound;
 }
 
 void PaintController::addItemToIndexIfNeeded(const DisplayItem& displayItem, size_t index, DisplayItemIndicesByClientMap& displayItemIndicesByClient)
 {
     if (!displayItem.isCacheable())
         return;
 
     DisplayItemIndicesByClientMap::iterator it = displayItemIndicesByClient.find(&displayItem.client());
     Vector<size_t>& indices = it == displayItemIndicesByClient.end() ?
         displayItemIndicesByClient.add(&displayItem.client(), Vector<size_t>()).storedValue->value : it->value;
     indices.append(index);
 }
 
-struct PaintController::OutOfOrderIndexContext {
-    STACK_ALLOCATED();
-    OutOfOrderIndexContext(DisplayItemList::iterator begin) : nextItemToIndex(begin) { }
-
-    DisplayItemList::iterator nextItemToIndex;
-    DisplayItemIndicesByClientMap displayItemIndicesByClient;
-};
-
-DisplayItemList::iterator PaintController::findOutOfOrderCachedItem(const DisplayItem::Id& id, OutOfOrderIndexContext& context)
+DisplayItemList::iterator PaintController::findCachedItem(const DisplayItem::Id& id)
 {
     DCHECK(clientCacheIsValid(id.client));
 
-    size_t foundIndex = findMatchingItemFromIndex(id, context.displayItemIndicesByClient, m_currentPaintArtifact.getDisplayItemList());
-    if (foundIndex != kNotFound)
+    // Try to find the item sequentially first. This is fast if the current list and the new list are in
+    // the same order around the new item. If found, we don't need to update and lookup the index.
+    DisplayItemList::iterator end = m_currentPaintArtifact.getDisplayItemList().end();
+    DisplayItemList::iterator it = m_nextItemToMatch;
+    for (; it != end; ++it) {
+        // We encounter an item that has already been copied which indicates we can't do sequential matching.
+        if (!it->hasValidClient())
+            break;
+        if (id == it->getId()) {
+#if DCHECK_IS_ON()
+            ++m_numSequentialMatches;
+#endif
+            return it;
+        }
+        // We encounter a different cacheable item which also indicates we can't do sequential matching.
+        if (it->isCacheable())
+            break;
+    }
+
+    size_t foundIndex = findMatchingItemFromIndex(id, m_outOfOrderItemIndices, m_currentPaintArtifact.getDisplayItemList());
+    if (foundIndex != kNotFound) {
+#if DCHECK_IS_ON()
+        ++m_numOutOfOrderMatches;
+#endif
         return m_currentPaintArtifact.getDisplayItemList().begin() + foundIndex;
+    }
 
-    return findOutOfOrderCachedItemForward(id, context);
+    return findOutOfOrderCachedItemForward(id);
 }
 
 // Find forward for the item and index all skipped indexable items.
-DisplayItemList::iterator PaintController::findOutOfOrderCachedItemForward(const DisplayItem::Id& id, OutOfOrderIndexContext& context)
+DisplayItemList::iterator PaintController::findOutOfOrderCachedItemForward(const DisplayItem::Id& id)
 {
-    DisplayItemList::iterator currentEnd = m_currentPaintArtifact.getDisplayItemList().end();
-    for (; context.nextItemToIndex != currentEnd; ++context.nextItemToIndex) {
-        const DisplayItem& item = *context.nextItemToIndex;
+    DisplayItemList::iterator end = m_currentPaintArtifact.getDisplayItemList().end();
+    for (DisplayItemList::iterator it = m_nextItemToIndex; it != end; ++it) {
+        const DisplayItem& item = *it;
         DCHECK(item.hasValidClient());
-        if (id.matches(item))
-            return context.nextItemToIndex++;
-        if (item.isCacheable())
-            addItemToIndexIfNeeded(item, context.nextItemToIndex - m_currentPaintArtifact.getDisplayItemList().begin(), context.displayItemIndicesByClient);
+        if (id == item.getId()) {
+#if DCHECK_IS_ON()
+            ++m_numSequentialMatches;
+#endif
+            return it;
+        }
+        if (item.isCacheable()) {
+#if DCHECK_IS_ON()
+            ++m_numIndexedItems;
+#endif
+            addItemToIndexIfNeeded(item, it - m_currentPaintArtifact.getDisplayItemList().begin(), m_outOfOrderItemIndices);
+        }
     }
-    return currentEnd;
+
+#ifndef NDEBUG
+    showDebugData();
+    LOG(ERROR) << id.client.debugName() << ":" << DisplayItem::typeAsDebugString(id.type) << " not found in current display item list";
+#endif
+    NOTREACHED();
+    // We did not find the cached display item. This should be impossible, but may occur if there is a bug
+    // in the system, such as under-invalidation, incorrect cache checking or duplicate display ids.
+    // In this case, the caller should fall back to repaint the display item.
+    return end;
 }
 
-void PaintController::copyCachedSubsequence(const DisplayItemList& currentList, DisplayItemList::iterator& currentIt, DisplayItemList& updatedList, SkPictureGpuAnalyzer& gpuAnalyzer)
+// On return, |it| points to the item after the EndSubsequence item of the subsequence.
+void PaintController::copyCachedSubsequence(DisplayItemList::iterator& it)
 {
-    DCHECK(currentIt->getType() == DisplayItem::Subsequence);
-    DisplayItem::Id endSubsequenceId(currentIt->client(), DisplayItem::EndSubsequence);
+    DCHECK(it->getType() == DisplayItem::Subsequence);
+    DisplayItem::Id endSubsequenceId(it->client(), DisplayItem::EndSubsequence);
     do {
         // We should always find the EndSubsequence display item.
-        DCHECK(currentIt != m_currentPaintArtifact.getDisplayItemList().end());
-        DCHECK(currentIt->hasValidClient());
+        DCHECK(it != m_currentPaintArtifact.getDisplayItemList().end());
+        DCHECK(it->hasValidClient());
 #if CHECK_DISPLAY_ITEM_CLIENT_ALIVENESS
-        CHECK(currentIt->client().isAlive());
+        CHECK(it->client().isAlive());
 #endif
-        updatedList.appendByMoving(*currentIt, currentList.visualRect(currentIt - m_currentPaintArtifact.getDisplayItemList().begin()), gpuAnalyzer);
-        ++currentIt;
-    } while (!endSubsequenceId.matches(updatedList.last()));
+        ++m_numCachedNewItems;
+        processNewItem(m_newDisplayItemList.appendByMoving(*it));
+        ++it;
+    } while (endSubsequenceId != m_newDisplayItemList.last().getId());
 }
 
 static IntRect visualRectForDisplayItem(const DisplayItem& displayItem, const LayoutSize& offsetFromLayoutObject)
 {
     LayoutRect visualRect = displayItem.client().visualRect();
     visualRect.move(-offsetFromLayoutObject);
     return enclosingIntRect(visualRect);
 }
 
-// Update the existing display items by removing invalidated entries, updating
-// repainted ones, and appending new items.
-// - For cached drawing display item, copy the corresponding cached DrawingDisplayItem;
-// - For cached subsequence display item, copy the cached display items between the
-//   corresponding SubsequenceDisplayItem and EndSubsequenceDisplayItem (incl.);
-// - Otherwise, copy the new display item.
-//
-// The algorithm is O(|m_currentDisplayItemList| + |m_newDisplayItemList|).
-// Coefficients are related to the ratio of out-of-order CachedDisplayItems
-// and the average number of (Drawing|Subsequence)DisplayItems per client.
-//
+void PaintController::resetCurrentListIterators()
+{
+    m_nextItemToMatch = m_currentPaintArtifact.getDisplayItemList().begin();
+    m_nextItemToIndex = m_nextItemToMatch;
+}
+
 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;
 
     // These data structures are used during painting only.
     DCHECK(!skippingCache());
 #if DCHECK_IS_ON()
     m_newDisplayItemIndicesByClient.clear();
 #endif
 
     SkPictureGpuAnalyzer gpuAnalyzer;
 
-    if (m_currentPaintArtifact.isEmpty()) {
-#if DCHECK_IS_ON()
-        for (const auto& item : m_newDisplayItemList)
-            DCHECK(!item.isCached());
+    m_currentCacheGeneration = DisplayItemClient::CacheGenerationOrInvalidationReason::next();
+    for (const auto& item : m_newDisplayItemList) {
+        // No reason to continue the analysis once we have a veto.
+        if (gpuAnalyzer.suitableForGpuRasterization())
+            item.analyzeForGpuRasterization(gpuAnalyzer);
+
+        m_newDisplayItemList.appendVisualRect(visualRectForDisplayItem(item, offsetFromLayoutObject));
+
+        if (item.isCacheable())
+            item.client().setDisplayItemsCached(m_currentCacheGeneration);
+    }
+
+    m_newDisplayItemList.removeEmptyBuffers();

[pdr., 2016/07/12 02:19:51]

Can you add a comment here, like:
// The new list will not be appended to again so we can release unused memory.

[Xianzhu, 2016/07/12 17:56:47]

Done.

+    m_currentPaintArtifact = PaintArtifact(std::move(m_newDisplayItemList), m_newPaintChunks.releasePaintChunks(), gpuAnalyzer.suitableForGpuRasterization());
+    resetCurrentListIterators();
+    m_outOfOrderItemIndices.clear();
+
+    // We'll allocate the initial buffer when we start the next paint.
+    m_newDisplayItemList = DisplayItemList(0);

[pdr., 2016/07/12 02:19:52]

Is the "m_newDisplayItemList.removeEmptyBuffers();" call above needed if we just
re-create the list here?

Do you know if the approach here is needed for performance reasons? I'm unsure
if it is worth it to lazily allocate the initial buffer... it seems more
expensive to have an empty check on every append call than it would to just
pre-allocate kInitialDisplayItemListCapacityBytes immediately. By keeping around
the same initial buffer, we can also avoid allocations.

[Xianzhu, 2016/07/12 17:56:47]

The removeEmptyBuffers() is for the m_currentPaintArtifact which holds the
m_newDisplayItemList before line 372.

Actually the above statement is not necessary but may be clearer about the
intent.
This is a balance between memory and cpu performance. If we allocate the initial
buffer now, we'll keep the buffer during the time between paint phases which may
have much longer duration than the paint phases, e.g. when the page is idle.

However, this will be not a problem when we make PaintController live in a paint
phase only
(https://codereview.chromium.org/2107103002/diff/300001/third_party/WebKit/Sou...).

+
+#if CHECK_DISPLAY_ITEM_CLIENT_ALIVENESS
+    CHECK(m_currentSubsequenceClients.isEmpty());
+    DisplayItemClient::endShouldKeepAliveAllClients(this);
 #endif
 
-        for (const auto& item : m_newDisplayItemList) {
-            m_newDisplayItemList.appendVisualRect(visualRectForDisplayItem(item, offsetFromLayoutObject));
-            // No reason to continue the analysis once we have a veto.
-            if (gpuAnalyzer.suitableForGpuRasterization())
-                item.analyzeForGpuRasterization(gpuAnalyzer);
-        }
-        m_currentPaintArtifact = PaintArtifact(std::move(m_newDisplayItemList), m_newPaintChunks.releasePaintChunks(), gpuAnalyzer.suitableForGpuRasterization());
-        m_newDisplayItemList = DisplayItemList(kInitialDisplayItemListCapacityBytes);
-        updateCacheGeneration();
-        return;
-    }
-
-    // Stores indices to valid DrawingDisplayItems in m_currentDisplayItems that have not been matched
-    // by CachedDisplayItems during synchronized matching. The indexed items will be matched
-    // by later out-of-order CachedDisplayItems in m_newDisplayItemList. This ensures that when
-    // out-of-order CachedDisplayItems occur, we only traverse at most once over m_currentDisplayItems
-    // looking for potential matches. Thus we can ensure that the algorithm runs in linear time.
-    OutOfOrderIndexContext outOfOrderIndexContext(m_currentPaintArtifact.getDisplayItemList().begin());
-
-    // TODO(jbroman): Consider revisiting this heuristic.
-    DisplayItemList updatedList(std::max(m_currentPaintArtifact.getDisplayItemList().usedCapacityInBytes(), m_newDisplayItemList.usedCapacityInBytes()));
-    Vector<PaintChunk> updatedPaintChunks;
-    DisplayItemList::iterator currentIt = m_currentPaintArtifact.getDisplayItemList().begin();
-    DisplayItemList::iterator currentEnd = m_currentPaintArtifact.getDisplayItemList().end();
-    for (DisplayItemList::iterator newIt = m_newDisplayItemList.begin(); newIt != m_newDisplayItemList.end(); ++newIt) {
-        const DisplayItem& newDisplayItem = *newIt;
-        const DisplayItem::Id newDisplayItemId = newDisplayItem.nonCachedId();
-        bool newDisplayItemHasCachedType = newDisplayItem.getType() != newDisplayItemId.type;
-
-        bool isSynchronized = currentIt != currentEnd && newDisplayItemId.matches(*currentIt);
-
-        if (newDisplayItemHasCachedType) {
-            DCHECK(newDisplayItem.isCached());
-#if CHECK_DISPLAY_ITEM_CLIENT_ALIVENESS
-            CHECK(clientCacheIsValid(newDisplayItem.client()));
+#if DCHECK_IS_ON()
+    m_numSequentialMatches = 0;
+    m_numOutOfOrderMatches = 0;
+    m_numIndexedItems = 0;
 #endif
-            if (!isSynchronized) {
-                currentIt = findOutOfOrderCachedItem(newDisplayItemId, outOfOrderIndexContext);
-
-                if (currentIt == currentEnd) {
-#ifndef NDEBUG
-                    showDebugData();
-                    WTFLogAlways("%s not found in m_currentDisplayItemList\n", newDisplayItem.asDebugString().utf8().data());
-#endif
-                    NOTREACHED();
-                    // We did not find the cached display item. This should be impossible, but may occur if there is a bug
-                    // in the system, such as under-invalidation, incorrect cache checking or duplicate display ids.
-                    // In this case, attempt to recover rather than crashing or bailing on display of the rest of the display list.
-                    continue;
-                }
-            }
-#if DCHECK_IS_ON()
-            if (RuntimeEnabledFeatures::slimmingPaintUnderInvalidationCheckingEnabled()) {
-                DisplayItemList::iterator temp = currentIt;
-                checkUnderInvalidation(newIt, temp);
-            }
-#endif
-            if (newDisplayItem.isCachedDrawing()) {
-                updatedList.appendByMoving(*currentIt, m_currentPaintArtifact.getDisplayItemList().visualRect(currentIt - m_currentPaintArtifact.getDisplayItemList().begin()),
-                    gpuAnalyzer);
-                ++currentIt;
-            } else {
-                DCHECK(newDisplayItem.getType() == DisplayItem::CachedSubsequence);
-                copyCachedSubsequence(m_currentPaintArtifact.getDisplayItemList(), currentIt, updatedList, gpuAnalyzer);
-                DCHECK(updatedList.last().getType() == DisplayItem::EndSubsequence);
-            }
-        } else {
-            DCHECK(!newDisplayItem.isDrawing()
-                || newDisplayItem.skippedCache()
-                || !clientCacheIsValid(newDisplayItem.client()));
-
-            updatedList.appendByMoving(*newIt, visualRectForDisplayItem(*newIt, offsetFromLayoutObject), gpuAnalyzer);
-
-            if (isSynchronized)
-                ++currentIt;
-        }
-        // Items before currentIt should have been copied so we don't need to index them.
-        if (currentIt - outOfOrderIndexContext.nextItemToIndex > 0)
-            outOfOrderIndexContext.nextItemToIndex = currentIt;
-    }
-
-    // TODO(jbroman): When subsequence caching applies to SPv2, we'll need to
-    // merge the paint chunks as well.
-    m_currentPaintArtifact = PaintArtifact(std::move(updatedList), m_newPaintChunks.releasePaintChunks(), gpuAnalyzer.suitableForGpuRasterization());
-
-    m_newDisplayItemList = DisplayItemList(kInitialDisplayItemListCapacityBytes);
-    updateCacheGeneration();
 }
 
 size_t PaintController::approximateUnsharedMemoryUsage() const
 {
     size_t memoryUsage = sizeof(*this);
 
     // Memory outside this class due to m_currentPaintArtifact.
     memoryUsage += m_currentPaintArtifact.approximateUnsharedMemoryUsage() - sizeof(m_currentPaintArtifact);
 
     // TODO(jbroman): If display items begin to have significant external memory
     // usage that's not shared with the embedder, we should account for it here.
     //
     // External objects, shared with the embedder, such as SkPicture, should be
     // excluded to avoid double counting. It is the embedder's responsibility to
     // count such objects.
     //
     // At time of writing, the only known case of unshared external memory was
     // the rounded clips vector in ClipDisplayItem, which is not expected to
     // contribute significantly to memory usage.
 
     // Memory outside this class due to m_newDisplayItemList.
     DCHECK(m_newDisplayItemList.isEmpty());
     memoryUsage += m_newDisplayItemList.memoryUsageInBytes();
 
     return memoryUsage;
 }
 
-void PaintController::updateCacheGeneration()
-{
-    m_currentCacheGeneration = DisplayItemClient::CacheGenerationOrInvalidationReason::next();
-    for (const DisplayItem& displayItem : m_currentPaintArtifact.getDisplayItemList()) {
-        if (!displayItem.isCacheable())
-            continue;
-        displayItem.client().setDisplayItemsCached(m_currentCacheGeneration);
-    }
-#if CHECK_DISPLAY_ITEM_CLIENT_ALIVENESS
-    CHECK(m_currentSubsequenceClients.isEmpty());
-    DisplayItemClient::endShouldKeepAliveAllClients(this);
-#endif
-}
-
 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));
+
+    resetCurrentListIterators();
 }
 
-#if DCHECK_IS_ON()
+#if 0 // DCHECK_IS_ON()

[pdr., 2016/07/12 02:19:51]

Did you plan on fixing this in this patch?

[Xianzhu, 2016/07/12 17:56:47]

https://codereview.chromium.org/2114653003/ based on this patch will fix this.

We can either review and land them one by one, review them one by one but land
them together, or review and land them as one. Which do you prefer? 

[pdr., 2016/07/12 20:09:35]

Separate patches sounds good to me.

+// TODO(wangxianzhu): Fix under-invalidation checking for the new caching method.
 
 void PaintController::checkUnderInvalidation(DisplayItemList::iterator& newIt, DisplayItemList::iterator& currentIt)
 {
     DCHECK(RuntimeEnabledFeatures::slimmingPaintUnderInvalidationCheckingEnabled());
-    DCHECK(newIt->isCached());
 
     // When under-invalidation-checking is enabled, the forced painting is following the cached display item.
     DisplayItem::Type nextItemType = DisplayItem::nonCachedType(newIt->getType());
     ++newIt;
     DCHECK(newIt->getType() == nextItemType);
 
     if (newIt->isDrawing()) {
         checkCachedDisplayItemIsUnchanged("", *newIt, *currentIt);
         return;
     }
@@ skipping 97 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