Issue trace events to gather repaint information.

Source/core/rendering/RenderObject.cpp

Index: Source/core/rendering/RenderObject.cpp
diff --git a/Source/core/rendering/RenderObject.cpp b/Source/core/rendering/RenderObject.cpp
index 3c67d938d7e0ea3c3051fbeddc0cb52c6dd3cdfe..14bf2b684f35df0aa9b69718da0e78dcf232f2c4 100644
--- a/Source/core/rendering/RenderObject.cpp
+++ b/Source/core/rendering/RenderObject.cpp
@@ -82,6 +82,7 @@
 #include "platform/graphics/GraphicsContext.h"
 #include "wtf/RefCountedLeakCounter.h"
 #include "wtf/text/StringBuilder.h"
+#include "wtf/text/WTFString.h"
 #include <algorithm>
 #ifndef NDEBUG
 #include <stdio.h>
@@ -1378,6 +1379,10 @@ RenderLayerModelObject* RenderObject::containerForRepaint() const
 
 void RenderObject::repaintUsingContainer(const RenderLayerModelObject* repaintContainer, const IntRect& r) const
 {
+    TRACE_EVENT2(TRACE_DISABLED_BY_DEFAULT("blink.repaint"), "RenderObject::repaintUsingContainer()",
+        "object", TRACE_STR_COPY(this->debugName().ascii().data()),
+        "rect", TRACE_STR_COPY(String::format("%d,%d %dx%d", r.x(), r.y(), r.width(), r.height()).ascii().data()));
+
     if (!repaintContainer) {
         view()->repaintViewRectangle(r);
         return;
@@ -1449,6 +1454,9 @@ void RenderObject::repaint() const
     if (view->document().printing())
         return; // Don't repaint if we're printing.
 
+    TRACE_EVENT1(TRACE_DISABLED_BY_DEFAULT("blink.repaint"), "RenderObject::repaint()",
+        "object", TRACE_STR_COPY(this->debugName().ascii().data()));
+
     // FIXME: really, we're in the repaint phase here, and the following queries are legal.
     // Until those states are fully fledged, I'll just disable the ASSERTS.
     DisableCompositingQueryAsserts disabler;
@@ -1466,6 +1474,9 @@ void RenderObject::repaintRectangle(const LayoutRect& r) const
     if (view->document().printing())
         return; // Don't repaint if we're printing.
 
+    TRACE_EVENT1(TRACE_DISABLED_BY_DEFAULT("blink.repaint"), "RenderObject::repaintRectangle()",

[Julien - ping for review, 2014/03/21 18:35:34]

Is there a lot of values in tracing callers of repaintUsingContainer considering
the InvalidationReason?

[dsinclair, 2014/03/24 15:04:31]

The invalidationReasons are only coming from repaintAfterlayoutIfNeeded. Having
these events lets us know where repaintUsingContainer is being called from if it
doesn't go through repaintAfterLayoutIfNeeded.

+        "object", TRACE_STR_COPY(this->debugName().ascii().data()));
+
     LayoutRect dirtyRect(r);
 
     if (!RuntimeEnabledFeatures::repaintAfterLayoutEnabled()) {
@@ -1484,6 +1495,26 @@ IntRect RenderObject::pixelSnappedAbsoluteClippedOverflowRect() const
     return pixelSnappedIntRect(absoluteClippedOverflowRect());
 }
 
+const char* RenderObject::repaintReasonToString(const RepaintReason reason) const
+{
+    switch (reason) {
+    case RepaintNone:
+        return "none";
+    case RepaintSelfLayout:
+        return "self layout";
+    case RepaintBorderFitLines:
+        return "border fit lines";
+    case RepaintBorderRadius:
+        return "border radius";
+    case RepaintOutlineBoxChange:
+        return "outline box change";
+    case RepaintBoundsChange:
+        return "bounds change";
+    default:
+        return "unknown";
+    }
+}
+
 bool RenderObject::repaintAfterLayoutIfNeeded(const RenderLayerModelObject* repaintContainer, bool wasSelfLayout,
     const LayoutRect& oldBounds, const LayoutRect& oldOutlineBox,
     const LayoutRect* newBoundsPtr, const LayoutRect* newOutlineBoxRectPtr)
@@ -1492,41 +1523,54 @@ bool RenderObject::repaintAfterLayoutIfNeeded(const RenderLayerModelObject* repa
     if (v->document().printing())
         return false; // Don't repaint if we're printing.
 
+    TRACE_EVENT1(TRACE_DISABLED_BY_DEFAULT("blink.repaint"), "RenderObject::repaintAfterLayoutIfNeeded",
+        "object", TRACE_STR_COPY(this->debugName().ascii().data()));
+
     // This ASSERT fails due to animations.  See https://bugs.webkit.org/show_bug.cgi?id=37048
     // ASSERT(!newBoundsPtr || *newBoundsPtr == clippedOverflowRectForRepaint(repaintContainer));
     LayoutRect newBounds = newBoundsPtr ? *newBoundsPtr : clippedOverflowRectForRepaint(repaintContainer);
     LayoutRect newOutlineBox;
 
-    bool fullRepaint = wasSelfLayout;
+    RepaintReason repaintReason = RepaintNone;

[Julien - ping for review, 2014/03/21 18:35:34]

I hate RepaintNone as a default reason. I think this should just be
IncrementalInvalidation (btw, let's ditch repaint from our vocabulary).

[dsinclair, 2014/03/24 15:04:31]

Done.

+    if (wasSelfLayout)
+        repaintReason = RepaintSelfLayout;
+
     // Presumably a background or a border exists if border-fit:lines was specified.
-    if (!fullRepaint && style()->borderFit() == BorderFitLines)
-        fullRepaint = true;
-    if (!fullRepaint && style()->hasBorderRadius()) {
+    if (repaintReason == RepaintNone && style()->borderFit() == BorderFitLines)
+        repaintReason = RepaintBorderFitLines;
+
+    if (repaintReason == RepaintNone&& style()->hasBorderRadius()) {
         // If a border-radius exists and width/height is smaller than
         // radius width/height, we cannot use delta-repaint.
         RoundedRect oldRoundedRect = style()->getRoundedBorderFor(oldBounds);
         RoundedRect newRoundedRect = style()->getRoundedBorderFor(newBounds);
-        fullRepaint = oldRoundedRect.radii() != newRoundedRect.radii();
+        if (oldRoundedRect.radii() != newRoundedRect.radii())
+            repaintReason = RepaintBorderRadius;
     }
-    if (!fullRepaint) {
+
+    if (repaintReason == RepaintNone) {
         // This ASSERT fails due to animations.  See https://bugs.webkit.org/show_bug.cgi?id=37048
         // ASSERT(!newOutlineBoxRectPtr || *newOutlineBoxRectPtr == outlineBoundsForRepaint(repaintContainer));
         newOutlineBox = newOutlineBoxRectPtr ? *newOutlineBoxRectPtr : outlineBoundsForRepaint(repaintContainer);
 
         if ((hasOutline() && newOutlineBox.location() != oldOutlineBox.location())
             || (mustRepaintBackgroundOrBorder() && (newBounds != oldBounds || (hasOutline() && newOutlineBox != oldOutlineBox))))
-            fullRepaint = true;
+            repaintReason = RepaintOutlineBoxChange;
     }
 
     // If there is no intersection between the old and the new bounds, invalidating
     // the difference is more expensive than just doing a full repaint.
-    if (!fullRepaint && !newBounds.intersects(oldBounds))
-        fullRepaint = true;
+    if (repaintReason == RepaintNone && !newBounds.intersects(oldBounds))
+        repaintReason = RepaintBoundsChange;

[Julien - ping for review, 2014/03/21 18:35:34]

Sir, this is a change in behavior! We don't want to do incremental invalidations
if newBounds and oldBounds don't intersect. This is because once you don't
intersect, incremental invalidations can be more expensive than just a full
invalidation. This made some of our test progress and I think this is correct in
general.

[dsinclair, 2014/03/24 15:04:31]

I'm not sure I understand, there should be no behaviour change from this CL.
It's possible I named the enum badly, but I'm doing the same work as it was
previously. If the new bounds does not intersect the old bounds we will still
trigger the full repainting on line 1569 as repaintReason != RepaintNone.

 
     if (!repaintContainer)
         repaintContainer = v;
 
-    if (fullRepaint) {
+    if (repaintReason != RepaintNone) {
+        TRACE_EVENT_INSTANT2(TRACE_DISABLED_BY_DEFAULT("blink.repaint"), "repaint_reason::full",
+            "object", TRACE_STR_COPY(this->debugName().ascii().data()),
+            "reason", repaintReasonToString(repaintReason));
+
         repaintUsingContainer(repaintContainer, pixelSnappedIntRect(oldBounds));
         if (newBounds != oldBounds)
             repaintUsingContainer(repaintContainer, pixelSnappedIntRect(newBounds));
@@ -1536,6 +1580,11 @@ bool RenderObject::repaintAfterLayoutIfNeeded(const RenderLayerModelObject* repa
     if (newBounds == oldBounds && newOutlineBox == oldOutlineBox)
         return false;
 
+    repaintReason = RepaintBoundsChange;
+    TRACE_EVENT_INSTANT2(TRACE_DISABLED_BY_DEFAULT("blink.repaint"), "repaint_reason::delta",
+        "object", TRACE_STR_COPY(this->debugName().ascii().data()),
+        "reason", repaintReasonToString(repaintReason));
+
     LayoutUnit deltaLeft = newBounds.x() - oldBounds.x();
     if (deltaLeft > 0)
         repaintUsingContainer(repaintContainer, pixelSnappedIntRect(oldBounds.x(), oldBounds.y(), deltaLeft, oldBounds.height()));