ui: Clean up damaged rects and clear them after painting.

ui/compositor/layer.cc

Index: ui/compositor/layer.cc
diff --git a/ui/compositor/layer.cc b/ui/compositor/layer.cc
index 3e61d40b95a2c02151bf44dd5418c2d74cc655c8..377e29e919ad2e7671b35d0204a001de283ca7c8 100644
--- a/ui/compositor/layer.cc
+++ b/ui/compositor/layer.cc
@@ -657,11 +657,7 @@ bool Layer::SchedulePaint(const gfx::Rect& invalid_rect) {
       type_ == LAYER_NINE_PATCH || (!delegate_ && !mailbox_.IsValid()))
     return false;
 
-  damaged_region_.op(invalid_rect.x(),
-                     invalid_rect.y(),
-                     invalid_rect.right(),
-                     invalid_rect.bottom(),
-                     SkRegion::kUnion_Op);
+  damaged_region_.Union(invalid_rect);
   ScheduleDraw();
   return true;
 }
@@ -673,20 +669,17 @@ void Layer::ScheduleDraw() {
 }
 
 void Layer::SendDamagedRects() {
-  if ((delegate_ || mailbox_.IsValid()) && !damaged_region_.isEmpty()) {
-    for (SkRegion::Iterator iter(damaged_region_); !iter.done(); iter.next()) {
-      const SkIRect& sk_damaged = iter.rect();
-      gfx::Rect damaged(
-          sk_damaged.x(),
-          sk_damaged.y(),
-          sk_damaged.width(),
-          sk_damaged.height());
-      cc_layer_->SetNeedsDisplayRect(damaged);
-    }
-    damaged_region_.setEmpty();
-  }
-  for (size_t i = 0; i < children_.size(); ++i)
-    children_[i]->SendDamagedRects();
+  if (damaged_region_.IsEmpty())
+    return;
+  if (!delegate_ && !mailbox_.IsValid())
+    return;
+
+  for (cc::Region::Iterator iter(damaged_region_); iter.has_rect(); iter.next())
+    cc_layer_->SetNeedsDisplayRect(iter.rect());
+}
+
+void Layer::ClearDamagedRects() {
+  damaged_region_.Clear();
 }
 
 void Layer::CompleteAllAnimations() {
@@ -749,6 +742,7 @@ void Layer::PaintContents(
       sk_canvas, device_scale_factor_));
   if (delegate_)
     delegate_->OnPaintLayer(PaintContext(canvas.get(), clip));
+  ClearDamagedRects();

[sky, 2015/04/21 23:56:54]

What happens if as part of painting something gets damaged? Should the clear
happen before calling to paint?

[piman, 2015/04/22 03:32:13]

Conversely, what happens if damage to the layer is added between SendDamageRects
and PaintContents?

OTOH, with display lists I wonder if we can remove the damage logic altogether.
It was added so that we could send damage and then cancel it because the
delegate would go away but we may have lost pixels. With display lists, assuming
we can keep them around, this wouldn't be a problem any more.

[danakj, 2015/04/22 16:32:21]

It's possible, I wanted to think of that as a bug, but we have a unit test
covering this behaviour (which was failing), so I guess it's something we have
explicitly supported in the past.

I've moved the Clear above, which also fixes the test.
Ya, this has always been kinda sketchy lookin'. I think the good news here is
that the ui embedder code has no chance to do anything between Layout() which
sends the rects and Paint() which uses/clears them. These both happen in the
same task (BeginMainFrame) that the compositor controls.

It means that invalidations during paint will show up /next/ frame, but that's
the behaviour we seem to want with a unit test guarding it.
The ui::Layer isn't the thing holding the cache, it would still need its
delegate around to give back DisplayItems. Without a delegate, the next
recording for the layer would be empty.

 }
 
 void Layer::PaintContentsToDisplayList(
@@ -765,6 +759,7 @@ void Layer::PaintContentsToDisplayList(
     delegate_->OnPaintLayer(
         PaintContext(display_list, device_scale_factor_, clip, invalidation));
   }
+  ClearDamagedRects();
 }
 
 bool Layer::FillsBoundsCompletely() const { return fills_bounds_completely_; }