Fixed bug that caused canvas updates to trigger two compositor paints.

content/renderer/render_widget.cc

Index: content/renderer/render_widget.cc
diff --git a/content/renderer/render_widget.cc b/content/renderer/render_widget.cc
index 5cc8d73f9645a050bec1c25ba4f490b61293572c..79478d47b133cb7420a037cfec43645a8a6d6bb2 100644
--- a/content/renderer/render_widget.cc
+++ b/content/renderer/render_widget.cc
@@ -673,6 +673,12 @@ void RenderWidget::DoDeferredUpdateAndSendInputAck() {
     Send(pending_input_event_ack_.release());
 }
 
+void RenderWidget::popPendingUpdate() {
+  // Save off the pending update now since painting may cause more
+  // invalidation.  Some WebCore rendering objects only layout when painted.
+  paint_aggregator_.PopPendingUpdate(&paint_aggregator_update_);
+}
+
 void RenderWidget::DoDeferredUpdate() {
   TRACE_EVENT0("renderer", "RenderWidget::DoDeferredUpdate");
 
@@ -734,13 +740,8 @@ void RenderWidget::DoDeferredUpdate() {
   }
   last_do_deferred_update_time_ = frame_begin_ticks;
 
-  // OK, save the pending update to a local since painting may cause more
-  // invalidation.  Some WebCore rendering objects only layout when painted.
-  PaintAggregator::PendingUpdate update;
-  paint_aggregator_.PopPendingUpdate(&update);
-
-  gfx::Rect scroll_damage = update.GetScrollDamage();
-  gfx::Rect bounds = update.GetPaintBounds().Union(scroll_damage);
+  gfx::Rect scroll_damage;
+  gfx::Rect bounds;
 
   // Compositing the page may disable accelerated compositing.
   bool accelerated_compositing_was_active = is_accelerated_compositing_active_;
@@ -760,63 +761,73 @@ void RenderWidget::DoDeferredUpdate() {
   TransportDIB* dib = NULL;
   std::vector<gfx::Rect> copy_rects;
   gfx::Rect optimized_copy_rect, optimized_copy_location;
-  if (update.scroll_rect.IsEmpty() &&
-      !is_accelerated_compositing_active_ &&
-      GetBitmapForOptimizedPluginPaint(bounds, &dib, &optimized_copy_location,
-                                       &optimized_copy_rect)) {
-    // Only update the part of the plugin that actually changed.
-    optimized_copy_rect = optimized_copy_rect.Intersect(bounds);
-    bounds = optimized_copy_location;
-    copy_rects.push_back(optimized_copy_rect);
-    dib_id = dib->id();
-  } else if (!is_accelerated_compositing_active_) {
-    // Compute a buffer for painting and cache it.
-    scoped_ptr<skia::PlatformCanvas> canvas(
-        RenderProcess::current()->GetDrawingCanvas(&current_paint_buf_,
-                                                   bounds));
-    if (!canvas.get()) {
-      NOTREACHED();
-      return;
-    }
+  if (!is_accelerated_compositing_active_) {
+    popPendingUpdate();
+    scroll_damage = paint_aggregator_update_.GetScrollDamage();
+    bounds = paint_aggregator_update_.GetPaintBounds().Union(scroll_damage);
+
+    if (paint_aggregator_update_.scroll_rect.IsEmpty() &&
+        GetBitmapForOptimizedPluginPaint(bounds, &dib, &optimized_copy_location,
+                                         &optimized_copy_rect)) {
+      // Only update the part of the plugin that actually changed.
+      optimized_copy_rect = optimized_copy_rect.Intersect(bounds);
+      bounds = optimized_copy_location;
+      copy_rects.push_back(optimized_copy_rect);
+      dib_id = dib->id();
+    } else {
+      // Compute a buffer for painting and cache it.
+      scoped_ptr<skia::PlatformCanvas> canvas(
+          RenderProcess::current()->GetDrawingCanvas(&current_paint_buf_,
+                                                     bounds));
+      if (!canvas.get()) {
+        NOTREACHED();
+        return;
+      }
 
-    // We may get back a smaller canvas than we asked for.
-    // TODO(darin): This seems like it could cause painting problems!
-    DCHECK_EQ(bounds.width(), canvas->getDevice()->width());
-    DCHECK_EQ(bounds.height(), canvas->getDevice()->height());
-    bounds.set_width(canvas->getDevice()->width());
-    bounds.set_height(canvas->getDevice()->height());
+      // We may get back a smaller canvas than we asked for.
+      // TODO(darin): This seems like it could cause painting problems!
+      DCHECK_EQ(bounds.width(), canvas->getDevice()->width());
+      DCHECK_EQ(bounds.height(), canvas->getDevice()->height());
+      bounds.set_width(canvas->getDevice()->width());
+      bounds.set_height(canvas->getDevice()->height());
 
-    HISTOGRAM_COUNTS_100("MPArch.RW_PaintRectCount", update.paint_rects.size());

[jbates, 2011/07/09 02:06:22]

None of this code block has changed - it's just moved inside the
if(!is_accelerated_compositing_active_) block

+      HISTOGRAM_COUNTS_100("MPArch.RW_PaintRectCount",
+                           paint_aggregator_update_.paint_rects.size());
 
-    // The scroll damage is just another rectangle to paint and copy.
-    copy_rects.swap(update.paint_rects);
-    if (!scroll_damage.IsEmpty())
-      copy_rects.push_back(scroll_damage);
+      // The scroll damage is just another rectangle to paint and copy.
+      copy_rects.swap(paint_aggregator_update_.paint_rects);
+      if (!scroll_damage.IsEmpty())
+        copy_rects.push_back(scroll_damage);
 
-    for (size_t i = 0; i < copy_rects.size(); ++i)
-      PaintRect(copy_rects[i], bounds.origin(), canvas.get());
+      for (size_t i = 0; i < copy_rects.size(); ++i)
+        PaintRect(copy_rects[i], bounds.origin(), canvas.get());
 
-    dib_id = current_paint_buf_->id();
+      dib_id = current_paint_buf_->id();
+    }
   } else {  // Accelerated compositing path
     // Begin painting.
     webwidget_->composite(false);
     if (using_asynchronous_swapbuffers_)
       num_swapbuffers_complete_pending_++;
+
+    // paint_aggregator_update_ was updated during composite.
+    scroll_damage = paint_aggregator_update_.GetScrollDamage();
+    bounds = paint_aggregator_update_.GetPaintBounds().Union(scroll_damage);
   }
 
   // sending an ack to browser process that the paint is complete...
   ViewHostMsg_UpdateRect_Params params;
   params.bitmap = dib_id;
   params.bitmap_rect = bounds;
-  params.dx = update.scroll_delta.x();
-  params.dy = update.scroll_delta.y();
+  params.dx = paint_aggregator_update_.scroll_delta.x();
+  params.dy = paint_aggregator_update_.scroll_delta.y();
   if (accelerated_compositing_was_active) {
     // If painting is done via the gpu process then we clear out all damage
     // rects to save the browser process from doing unecessary work.
     params.scroll_rect = gfx::Rect();
     params.copy_rects.clear();
   } else {
-    params.scroll_rect = update.scroll_rect;
+    params.scroll_rect = paint_aggregator_update_.scroll_rect;
     params.copy_rects.swap(copy_rects);  // TODO(darin): clip to bounds?
   }
   params.view_size = size_;