Changes in DisplayItemClient for spv2 paint invalidation

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

 // Copyright 2015 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.
 
 #ifndef DisplayItemClient_h
 #define DisplayItemClient_h
 
 #include "platform/PlatformExport.h"
 #include "platform/geometry/LayoutRect.h"
+#include "platform/graphics/PaintInvalidationReason.h"
 #include "wtf/Assertions.h"
 #include "wtf/text/WTFString.h"
 
 #define CHECK_DISPLAY_ITEM_CLIENT_ALIVENESS 1
 // TODO(wangxianzhu): Restore the following line after we fix crbug.com/609218.
 // #define CHECK_DISPLAY_ITEM_CLIENT_ALIVENESS DCHECK_IS_ON()
 
 namespace blink {
 
 // Holds a unique cache generation id of display items and paint controllers.
 //
 // A paint controller sets its cache generation to DisplayItemCacheGeneration::next()
 // at the end of each commitNewDisplayItems, and updates the cache generation of each
 // client with cached drawings by calling DisplayItemClient::setDisplayItemsCached().
 // A display item is treated as validly cached in a paint controller if its cache generation
 // matches the paint controller's cache generation.
 //
 // SPv1 only: If a display item is painted on multiple paint controllers, because cache
 // generations are unique, the client's cache generation matches the last paint controller
 // only. The client will be treated as invalid on other paint controllers regardless if
 // it's validly cached by these paint controllers. The situation is very rare (about 0.07%
 // clients were painted on multiple paint controllers) so the performance penalty is trivial.
 class PLATFORM_EXPORT DisplayItemCacheGeneration {
     DISALLOW_NEW();
 public:
-    DisplayItemCacheGeneration() : m_value(kInvalidGeneration) { }
+    DisplayItemCacheGeneration() { invalidate(); }
 
-    void invalidate() { m_value = kInvalidGeneration; }
-    static DisplayItemCacheGeneration next() { return DisplayItemCacheGeneration(s_nextGeneration++); }
-    bool matches(const DisplayItemCacheGeneration& other)
+    void invalidate(PaintInvalidationReason reason = PaintInvalidationFull) { m_value = static_cast<Generation>(reason); }
+
+    static DisplayItemCacheGeneration next()
     {
-        return m_value != kInvalidGeneration && other.m_value != kInvalidGeneration && m_value == other.m_value;
+        // In case the value overflowed in the previous call.
+        if (s_nextGeneration < kFirstValidGeneration)

[pdr., 2016/06/27 18:55:04]

I did some back-of-the-envelope math and this would take over a month if every
frame does a full invalidation. Instead of supporting overflowing, what if we
just crash instead?

[Xianzhu, 2016/06/27 20:02:19]

There is analysis about cache generation overflow in
https://cs.chromium.org/chromium/src/third_party/WebKit/Source/platform/graph....
Do you think the analysis makes sense?

+            s_nextGeneration = kFirstValidGeneration;
+        return DisplayItemCacheGeneration(s_nextGeneration++);
+    }
+
+    bool matches(const DisplayItemCacheGeneration& other) const
+    {
+        return m_value >= kFirstValidGeneration && other.m_value >= kFirstValidGeneration && m_value == other.m_value;
+    }
+
+    PaintInvalidationReason getPaintInvalidationReason() const
+    {
+        return m_value < kFirstValidGeneration ? static_cast<PaintInvalidationReason>(m_value) : PaintInvalidationNone;
     }
 
 private:
     typedef uint32_t Generation;
-    DisplayItemCacheGeneration(Generation value) : m_value(value) { }
+    explicit DisplayItemCacheGeneration(Generation value) : m_value(value) { }
 
-    static const Generation kInvalidGeneration = 0;
+    static const Generation kFirstValidGeneration = static_cast<Generation>(PaintInvalidationReasonMax) + 1;
     static Generation s_nextGeneration;
     Generation m_value;
 };
 
-// The interface for objects that can be associated with display items.
+// The class for objects that can be associated with display items.
 // A DisplayItemClient object should live at least longer than the document cycle
 // in which its display items are created during painting.
 // After the document cycle, a pointer/reference to DisplayItemClient should be
 // no longer dereferenced unless we can make sure the client is still valid.
 class PLATFORM_EXPORT DisplayItemClient {
 public:
 #if CHECK_DISPLAY_ITEM_CLIENT_ALIVENESS
     DisplayItemClient();
     virtual ~DisplayItemClient();
 
     // 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() 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 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;
 
     // The visual rect of this DisplayItemClient, in object space of the object that owns the GraphicsLayer, i.e.
     // offset by offsetFromLayoutObjectWithSubpixelAccumulation().
     virtual LayoutRect visualRect() const = 0;
 
-    virtual bool displayItemsAreCached(DisplayItemCacheGeneration) const = 0;
-    virtual void setDisplayItemsCached(DisplayItemCacheGeneration) const = 0;
-    virtual void setDisplayItemsUncached() const = 0;
+    bool displayItemsAreCached(DisplayItemCacheGeneration cacheGeneration) const { return m_cacheGeneration.matches(cacheGeneration); }
+    void setDisplayItemsCached(DisplayItemCacheGeneration cacheGeneration) const { m_cacheGeneration = cacheGeneration; }
+    void setDisplayItemsUncached(PaintInvalidationReason reason = PaintInvalidationFull) const
+    {
+        m_cacheGeneration.invalidate(reason);
+#if CHECK_DISPLAY_ITEM_CLIENT_ALIVENESS
+        // Clear should-keep-alive of DisplayItemClients in a subsequence if this
+        // object is a subsequence.
+        endShouldKeepAliveAllClients(this);
+#endif
+    }
+
+    PaintInvalidationReason getPaintInvalidationReason() const { return m_cacheGeneration.getPaintInvalidationReason(); }
+
+private:
+    mutable DisplayItemCacheGeneration m_cacheGeneration;

[pdr., 2016/06/27 18:55:04]

It's confusing to me that we're using cache generation for both a generation and
for bits for the paint invalidation reason. Can this be hidden behind a struct
with just a single uint32 member and functions to hide the details?

[Xianzhu, 2016/06/27 20:02:19]

I'm not sure if I understand your comment. I think DisplayItemCacheGeneration is
the struct to hide the details but it may need a better name.

In the new patch set I renamed DisplayItemCacheGeneration to
CacheGenerationOrInvalidationReason and moved it into the private section of
DisplayItemClient.

 };
 
-#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(); \
-        ON_DISPLAY_ITEM_CLIENT_INVALIDATION(); \
-    } \
-    mutable DisplayItemCacheGeneration m_cacheGeneration;
-
-#define DISPLAY_ITEM_CACHE_STATUS_UNCACHEABLE_IMPLEMENTATION \
-    bool displayItemsAreCached(DisplayItemCacheGeneration) const final { return false; } \
-    void setDisplayItemsCached(DisplayItemCacheGeneration) const final { } \
-    void setDisplayItemsUncached() const final { }
-
 inline bool operator==(const DisplayItemClient& client1, const DisplayItemClient& client2) { return &client1 == &client2; }
 inline bool operator!=(const DisplayItemClient& client1, const DisplayItemClient& client2) { return &client1 != &client2; }
 
 } // namespace blink
 
 #endif // DisplayItemClient_h