[webkit-reviews] review denied: [Bug 107099] Coordinated Graphics: Remove m_pendingSyncBackingStores in LayerTreeRenderer. : [Attachment 185396] Patch

[bugzilla-daemon at webkit.org]

Benjamin Poulain <benjamin at webkit.org> has denied Huang Dongsung
<luxtella at company100.net>'s request for review:
Bug 107099: Coordinated Graphics: Remove m_pendingSyncBackingStores in
LayerTreeRenderer.
https://bugs.webkit.org/show_bug.cgi?id=107099

Attachment 185396: Patch
https://bugs.webkit.org/attachment.cgi?id=185396&action=review

------- Additional Comments from Benjamin Poulain <benjamin at webkit.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=185396&action=review


> Source/WebKit2/UIProcess/CoordinatedGraphics/LayerTreeRenderer.cpp:420
> +    if (m_backingStores.get(graphicsLayer))

To test if a key exists in a HashMap/HashSet, the correct thing to do is
HashMap::contains().

> Source/WebKit2/UIProcess/CoordinatedGraphics/LayerTreeRenderer.cpp:435
> +    if (!m_backingStores.get(graphicsLayer))
>	   return;
> -    }
>  
> -    if (!layer->backingStore())
> -	   return; // The layer has no backing store (and no pending addition).

> -
> -    ASSERT(!m_pendingSyncBackingStores.contains(layer));
> -    m_pendingSyncBackingStores.add(layer, 0);
> +   
m_releasedCoordinatedBackingStores.append(m_backingStores.take(graphicsLayer));

> +    toGraphicsLayerTextureMapper(graphicsLayer)->setBackingStore(0);

This particular code should be using iterators. Otherwise you are doing get()
twice.