Fix assertion failure during out-of-order matching

The assertion failure occurs in the following sequence:
- Out-of-order matching to n-th item;
- Synchronized matching to (n+m)-th item;
- Out-of-order matching to (n-k)-th item;
- Out-of-order matching to (n+m+l)-th.

The problem is that we only update nextItemToIndex during out-of-order
matching, but not during synchronized matching, causing we tried to
index already copied items (n to n+m).

BUG=526590
TEST=DisplayItemListTest.OutOfOrderNoCrash

Review URL: https://codereview.chromium.org/1307653007

git-svn-id: svn://svn.chromium.org/blink/trunk@201684 bbb929c8-8fbe-4397-9dbb-9b2b20218538

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

@@ -244,7 +244,7 @@ struct DisplayItemList::OutOfOrderIndexContext {
     DisplayItemIndicesByClientMap displayItemIndicesByClient;
 };
 
-DisplayItems::iterator DisplayItemList::findOutOfOrderCachedItem(DisplayItems::iterator currentIt, const DisplayItem::Id& id, OutOfOrderIndexContext& context)
+DisplayItems::iterator DisplayItemList::findOutOfOrderCachedItem(const DisplayItem::Id& id, OutOfOrderIndexContext& context)
 {
     ASSERT(clientCacheIsValid(id.client));
 
@@ -252,16 +252,12 @@ DisplayItems::iterator DisplayItemList::findOutOfOrderCachedItem(DisplayItems::i
     if (foundIndex != kNotFound)
         return m_currentDisplayItems.begin() + foundIndex;
 
-    return findOutOfOrderCachedItemForward(currentIt, id, context);
+    return findOutOfOrderCachedItemForward(id, context);
 }
 
 // Find forward for the item and index all skipped indexable items.
-DisplayItems::iterator DisplayItemList::findOutOfOrderCachedItemForward(DisplayItems::iterator currentIt, const DisplayItem::Id& id, OutOfOrderIndexContext& context)
+DisplayItems::iterator DisplayItemList::findOutOfOrderCachedItemForward(const DisplayItem::Id& id, OutOfOrderIndexContext& context)
 {
-    // Items before currentIt should have been copied. Skip indexing of them.
-    if (currentIt - context.nextItemToIndex > 0)
-        context.nextItemToIndex = currentIt;
-
     DisplayItems::iterator currentEnd = m_currentDisplayItems.end();
     for (; context.nextItemToIndex != currentEnd; ++context.nextItemToIndex) {
         const DisplayItem& item = *context.nextItemToIndex;
@@ -367,7 +363,7 @@ void DisplayItemList::commitNewDisplayItems(DisplayListDiff*)
             ASSERT(newDisplayItem.isCached());
             ASSERT(clientCacheIsValid(newDisplayItem.client()) || (RuntimeEnabledFeatures::slimmingPaintV2Enabled() && !paintOffsetWasInvalidated(newDisplayItem.client())));
             if (!isSynchronized) {
-                currentIt = findOutOfOrderCachedItem(currentIt, newDisplayItemId, outOfOrderIndexContext);
+                currentIt = findOutOfOrderCachedItem(newDisplayItemId, outOfOrderIndexContext);
 
                 if (currentIt == currentEnd) {
 #ifndef NDEBUG
@@ -406,6 +402,9 @@ void DisplayItemList::commitNewDisplayItems(DisplayListDiff*)
             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;
     }
 
 #if ENABLE(ASSERT)

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

@@ -178,8 +178,8 @@ private:
     static void addItemToIndexIfNeeded(const DisplayItem&, size_t index, DisplayItemIndicesByClientMap&);
 
     struct OutOfOrderIndexContext;
-    DisplayItems::iterator findOutOfOrderCachedItem(DisplayItems::iterator currentIt, const DisplayItem::Id&, OutOfOrderIndexContext&);
-    DisplayItems::iterator findOutOfOrderCachedItemForward(DisplayItems::iterator currentIt, const DisplayItem::Id&, OutOfOrderIndexContext&);
+    DisplayItems::iterator findOutOfOrderCachedItem(const DisplayItem::Id&, OutOfOrderIndexContext&);
+    DisplayItems::iterator findOutOfOrderCachedItemForward(const DisplayItem::Id&, OutOfOrderIndexContext&);
     void copyCachedSubsequence(DisplayItems::iterator& currentIt, DisplayItems& updatedList);
 
 #if ENABLE(ASSERT)

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

@@ -516,6 +516,31 @@ TEST_F(DisplayItemListTest, CachedSubsequenceSwapOrder)
         TestDisplayItem(container1, DisplayItem::EndSubsequence));
 }
 
+TEST_F(DisplayItemListTest, OutOfOrderNoCrash)
+{
+    TestDisplayItemClient client("client");
+    GraphicsContext context(&displayItemList());
+
+    const DisplayItem::Type type1 = DisplayItem::DrawingFirst;
+    const DisplayItem::Type type2 = static_cast<DisplayItem::Type>(DisplayItem::DrawingFirst + 1);
+    const DisplayItem::Type type3 = static_cast<DisplayItem::Type>(DisplayItem::DrawingFirst + 2);
+    const DisplayItem::Type type4 = static_cast<DisplayItem::Type>(DisplayItem::DrawingFirst + 3);
+
+    drawRect(context, client, type1, FloatRect(100, 100, 100, 100));
+    drawRect(context, client, type2, FloatRect(100, 100, 50, 200));
+    drawRect(context, client, type3, FloatRect(100, 100, 50, 200));
+    drawRect(context, client, type4, FloatRect(100, 100, 100, 100));
+
+    displayItemList().commitNewDisplayItems();
+
+    drawRect(context, client, type2, FloatRect(100, 100, 50, 200));
+    drawRect(context, client, type3, FloatRect(100, 100, 50, 200));
+    drawRect(context, client, type1, FloatRect(100, 100, 100, 100));
+    drawRect(context, client, type4, FloatRect(100, 100, 100, 100));
+
+    displayItemList().commitNewDisplayItems();
+}
+
 TEST_F(DisplayItemListTest, CachedNestedSubsequenceUpdate)
 {
     TestDisplayItemClient container1("container1");