Merge repaintRects and paintInvalidationObjects in text-based-repaint test results

third_party/WebKit/Source/platform/graphics/GraphicsLayer.cpp

 /*
  * Copyright (C) 2009 Apple Inc. All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions
  * are met:
  * 1. Redistributions of source code must retain the above copyright
  *    notice, this list of conditions and the following disclaimer.
  * 2. Redistributions in binary form must reproduce the above copyright
  *    notice, this list of conditions and the following disclaimer in the
@@ skipping 53 matching lines @@
 #include <utility>
 
 #ifndef NDEBUG
 #include <stdio.h>
 #endif
 
 namespace blink {
 
 static bool s_drawDebugRedFill = true;
 
-// TODO(wangxianzhu): Remove this when we no longer invalidate rects.
-struct PaintInvalidationTrackingInfo {
+struct PaintInvalidationInfo {
     DISALLOW_NEW_EXCEPT_PLACEMENT_NEW();
-    Vector<FloatRect> invalidationRects;
-    Vector<String> invalidationObjects;
+    String object;

[pdr., 2016/04/12 00:20:06]

Why pass a string instead of a DisplayItemClient? Is it just for the case of
##ALL##? If so, what if we just dumped all of the invalidations instead of
outputting ##ALL##?

[Xianzhu, 2016/04/12 01:14:26]

Another case is LinkHighlightImpl which is not a DisplayItemClient.

Now I changed the parameter of trackPaintInvalidation to DisplayItemClient, but
we still can't just use DisplayItemClient here because invalidated objects may
have died when we dump the results.

+    FloatRect rect;
+    const char* reason;

[pdr., 2016/04/12 00:20:06]

Could we use PaintInvalidationReason directly here? If not, why don't we use a
String instead of a char array?

[Xianzhu, 2016/04/12 01:14:26]

Changed to PaintInvalidationReason.

Used const char* because reason is always a literal and I wanted to use
non-pre-defined reason e.g. for LinkHightlight. Now use PaintInvalidationFull.

 };
 
-typedef HashMap<const GraphicsLayer*, PaintInvalidationTrackingInfo> PaintInvalidationTrackingMap;
+typedef HashMap<const GraphicsLayer*, Vector<PaintInvalidationInfo>> PaintInvalidationTrackingMap;
 static PaintInvalidationTrackingMap& paintInvalidationTrackingMap()
 {
     DEFINE_STATIC_LOCAL(PaintInvalidationTrackingMap, map, ());
     return map;
 }
 
 PassOwnPtr<GraphicsLayer> GraphicsLayer::create(GraphicsLayerClient* client)
 {
     OwnPtr<GraphicsLayer> layer = adoptPtr(new GraphicsLayer(client));
     return layer.release();
@@ skipping 21 matching lines @@
     , m_parent(0)
     , m_maskLayer(0)
     , m_contentsClippingMaskLayer(0)
     , m_replicaLayer(0)
     , m_replicatedLayer(0)
     , m_paintCount(0)
     , m_contentsLayer(0)
     , m_contentsLayerId(0)
     , m_scrollableArea(nullptr)
     , m_3dRenderingContext(0)
+    , m_lastSetNeedsDisplayInRectDisplayItemClient(nullptr)
 {
 #if ENABLE(ASSERT)
     if (m_client)
         m_client->verifyNotPainting();
 #endif
 
     m_contentLayerDelegate = adoptPtr(new ContentLayerDelegate(this));
     m_layer = adoptPtr(Platform::current()->compositorSupport()->createContentLayer(m_contentLayerDelegate.get()));
     m_layer->layer()->setDrawsContent(m_drawsContent && m_contentsVisible);
     m_layer->layer()->setLayerClient(this);
@@ skipping 397 matching lines @@
 
 void GraphicsLayer::resetTrackedPaintInvalidations()
 {
     paintInvalidationTrackingMap().remove(this);
 }
 
 bool GraphicsLayer::hasTrackedPaintInvalidations() const
 {
     PaintInvalidationTrackingMap::iterator it = paintInvalidationTrackingMap().find(this);
     if (it != paintInvalidationTrackingMap().end())
-        return !it->value.invalidationRects.isEmpty();
+        return !it->value.isEmpty();
     return false;
 }
 
-void GraphicsLayer::trackPaintInvalidationRect(const FloatRect& rect)
+void GraphicsLayer::trackPaintInvalidation(const String& object, const FloatRect& rect, const char* reason)
 {
-    if (rect.isEmpty())
-        return;
-
     // The caller must check isTrackingPaintInvalidations() before calling this method
     // to avoid constructing the rect unnecessarily.
     ASSERT(isTrackingPaintInvalidations());
 
-    paintInvalidationTrackingMap().add(this, PaintInvalidationTrackingInfo()).storedValue->value.invalidationRects.append(rect);
+    PaintInvalidationInfo info = { object, rect, reason };
+    paintInvalidationTrackingMap().add(this, Vector<PaintInvalidationInfo>()).storedValue->value.append(info);
 }
 
-void GraphicsLayer::trackPaintInvalidationObject(const String& objectDebugString)
+static bool comparePaintInvalidationInfo(const PaintInvalidationInfo& a, const PaintInvalidationInfo& b)
 {
-    if (objectDebugString.isEmpty())
-        return;
-
-    // The caller must check isTrackingPaintInvalidations() before calling this method
-    // because constructing the debug string will be costly.
-    ASSERT(isTrackingPaintInvalidations());
-
-    paintInvalidationTrackingMap().add(this, PaintInvalidationTrackingInfo()).storedValue->value.invalidationObjects.append(objectDebugString);
-}
-
-static bool compareFloatRects(const FloatRect& a, const FloatRect& b)
-{
-    if (a.x() != b.x())
-        return a.x() > b.x();
-    if (a.y() != b.y())
-        return a.y() > b.y();
-    if (a.width() != b.width())
-        return a.width() > b.width();
-    return a.height() > b.height();
+    return codePointCompareLessThan(a.object, b.object);
 }
 
 template <typename T>
 static PassRefPtr<JSONArray> pointAsJSONArray(const T& point)
 {
     RefPtr<JSONArray> array = JSONArray::create();
     array->pushNumber(point.x());
     array->pushNumber(point.y());
     return array;
 }
@@ skipping 131 matching lines @@
         json->setArray("transform", transformAsJSONArray(m_transform));
 
     if (m_replicaLayer)
         json->setObject("replicaLayer", m_replicaLayer->layerTreeAsJSON(flags, renderingContextMap));
 
     if (m_replicatedLayer)
         json->setString("replicatedLayer", flags & LayerTreeIncludesDebugInfo ? pointerAsString(m_replicatedLayer) : "");
 
     PaintInvalidationTrackingMap::iterator it = paintInvalidationTrackingMap().find(this);
     if (it != paintInvalidationTrackingMap().end()) {
-        if (flags & LayerTreeIncludesPaintInvalidationRects) {
-            Vector<FloatRect>& rects = it->value.invalidationRects;
-            if (!rects.isEmpty()) {
-                std::sort(rects.begin(), rects.end(), &compareFloatRects);
-                RefPtr<JSONArray> rectsJSON = JSONArray::create();
-                for (auto& rect : rects) {
-                    if (rect.isEmpty())
-                        continue;
-                    rectsJSON->pushArray(rectAsJSONArray(rect));
+        if (flags & LayerTreeIncludesPaintInvalidations) {
+            Vector<PaintInvalidationInfo>& infos = it->value;
+            if (!infos.isEmpty()) {
+                std::stable_sort(infos.begin(), infos.end(), &comparePaintInvalidationInfo);
+                RefPtr<JSONArray> paintInvalidationsJSON = JSONArray::create();
+                for (auto& info : infos) {
+                    RefPtr<JSONObject> infoJSON = JSONObject::create();
+                    infoJSON->setString("object", info.object);
+                    infoJSON->setArray("rect", rectAsJSONArray(info.rect));
+                    infoJSON->setString("reason", info.reason);
+                    paintInvalidationsJSON->pushObject(infoJSON);
                 }
-                json->setArray("repaintRects", rectsJSON);
-            }
-        }
-
-        if (flags & LayerTreeIncludesPaintInvalidationObjects) {
-            Vector<String>& clients = it->value.invalidationObjects;
-            if (!clients.isEmpty()) {
-                RefPtr<JSONArray> clientsJSON = JSONArray::create();
-                for (auto& clientString : clients)
-                    clientsJSON->pushString(clientString);
-                json->setArray("paintInvalidationClients", clientsJSON);
+                json->setArray("paintInvalidations", paintInvalidationsJSON);
             }
         }
     }
 
     if ((flags & LayerTreeIncludesPaintingPhases) && m_paintingPhase) {
         RefPtr<JSONArray> paintingPhasesJSON = JSONArray::create();
         if (m_paintingPhase & GraphicsLayerPaintBackground)
             paintingPhasesJSON->pushString("GraphicsLayerPaintBackground");
         if (m_paintingPhase & GraphicsLayerPaintForeground)
             paintingPhasesJSON->pushString("GraphicsLayerPaintForeground");
@@ skipping 272 matching lines @@
         return;
     m_isRootForIsolatedGroup = isolated;
     platformLayer()->setIsRootForIsolatedGroup(isolated);
 }
 
 void GraphicsLayer::setContentsNeedsDisplay()
 {
     if (WebLayer* contentsLayer = contentsLayerIfRegistered()) {
         contentsLayer->invalidate();
         if (isTrackingPaintInvalidations())
-            trackPaintInvalidationRect(m_contentsRect);
+            trackPaintInvalidation("GraphicsLayerContents", m_contentsRect, "");
     }
 }
 
 void GraphicsLayer::setNeedsDisplay()
 {
     if (!drawsContent())
         return;
 
     // TODO(chrishtr): stop invalidating the rects once FrameView::paintRecursively does so.
     m_layer->layer()->invalidate();
-    if (isTrackingPaintInvalidations())
-        trackPaintInvalidationRect(FloatRect(FloatPoint(), m_size));
     for (size_t i = 0; i < m_linkHighlights.size(); ++i)
         m_linkHighlights[i]->invalidate();
+    getPaintController().invalidateAll();
 
-    getPaintController().invalidateAll();
     if (isTrackingPaintInvalidations())
-        trackPaintInvalidationObject("##ALL##");
+        trackPaintInvalidation("##ALL##", FloatRect(FloatPoint(), m_size), "");

[pdr., 2016/04/12 00:20:06]

I only see this output in 4 tests, but there are 16 callers to
setNeedsDisplay(). Is there a bug here, or do we just have poor test coverage?

[Xianzhu, 2016/04/12 01:14:26]

We may have test coverage for the other setNeedsDisplay(), but just not covered
by repaint tests.

 }
 
-void GraphicsLayer::setNeedsDisplayInRect(const IntRect& rect, PaintInvalidationReason invalidationReason)
+void GraphicsLayer::setNeedsDisplayInRect(const IntRect& rect, PaintInvalidationReason invalidationReason, const DisplayItemClient& client)
 {
     if (!drawsContent())
         return;
 
     m_layer->layer()->invalidateRect(rect);
     if (firstPaintInvalidationTrackingEnabled())
         m_debugInfo.appendAnnotatedInvalidateRect(rect, invalidationReason);
-    if (isTrackingPaintInvalidations())
-        trackPaintInvalidationRect(rect);
     for (size_t i = 0; i < m_linkHighlights.size(); ++i)
         m_linkHighlights[i]->invalidate();
+
+    if (isTrackingPaintInvalidations()) {
+        m_lastSetNeedsDisplayInRectDisplayItemClient = &client;
+        trackPaintInvalidation(client.debugName(), rect, paintInvalidationReasonToString(invalidationReason));
+    }
 }
 
-void GraphicsLayer::invalidateDisplayItemClient(const DisplayItemClient& displayItemClient, PaintInvalidationReason)
+void GraphicsLayer::invalidateDisplayItemClient(const DisplayItemClient& displayItemClient, PaintInvalidationReason invalidationReason)
 {
     if (!drawsContent())
         return;
 
     getPaintController().invalidate(displayItemClient);
-    if (isTrackingPaintInvalidations())
-        trackPaintInvalidationObject(displayItemClient.debugName());
+
+    if (isTrackingPaintInvalidations() && m_lastSetNeedsDisplayInRectDisplayItemClient != &displayItemClient)
+        trackPaintInvalidation(displayItemClient.debugName(), FloatRect(), paintInvalidationReasonToString(invalidationReason));
 }
 
 void GraphicsLayer::setContentsRect(const IntRect& rect)
 {
     if (rect == m_contentsRect)
         return;
 
     m_contentsRect = rect;
     updateContentsRect();
 }
@@ skipping 136 matching lines @@
 {
     if (!layer) {
         fprintf(stderr, "Cannot showGraphicsLayerTree for (nil).\n");
         return;
     }
 
     String output = layer->layerTreeAsText(blink::LayerTreeIncludesDebugInfo);
     fprintf(stderr, "%s\n", output.utf8().data());
 }
 #endif