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

content/renderer/render_widget.cc

 // Copyright (c) 2011 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.
 
 #include "content/renderer/render_widget.h"
 
 #include "base/command_line.h"
 #include "base/debug/trace_event.h"
 #include "base/logging.h"
 #include "base/memory/scoped_ptr.h"
@@ skipping 655 matching lines @@
   DoDeferredUpdateAndSendInputAck();
 }
 
 void RenderWidget::DoDeferredUpdateAndSendInputAck() {
   DoDeferredUpdate();
 
   if (pending_input_event_ack_.get())
     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");
 
   if (!webwidget_)
     return;
   if (update_reply_pending()) {
     TRACE_EVENT0("renderer", "EarlyOut_UpdateReplyPending");
     return;
   }
   if (is_accelerated_compositing_active_ &&
@@ skipping 41 matching lines @@
                                  base::TimeDelta::FromMilliseconds(60),
                                  30);
 
     // Calculate filtered time per frame:
     float frame_time_elapsed = static_cast<float>(delay.InSecondsF());
     filtered_time_per_frame_ =
         0.9f * filtered_time_per_frame_ + 0.1f * frame_time_elapsed;
   }
   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_;
 
   // A plugin may be able to do an optimized paint. First check this, in which
   // case we can skip all of the bitmap generation and regular paint code.
   // This optimization allows PPAPI plugins that declare themselves on top of
   // the page (like a traditional windowed plugin) to be able to animate (think
   // movie playing) without repeatedly re-painting the page underneath, or
   // copying the plugin backing store (since we can send the plugin's backing
   // store directly to the browser).
   //
   // This optimization only works when the entire invalid region is contained
   // within the plugin. There is a related optimization in PaintRect for the
   // case where there may be multiple invalid regions.
   TransportDIB::Id dib_id = TransportDIB::Id();
   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());
+
+      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(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());
+
+      dib_id = current_paint_buf_->id();
     }
-
-    // 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

-
-    // 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);
-
-    for (size_t i = 0; i < copy_rects.size(); ++i)
-      PaintRect(copy_rects[i], bounds.origin(), canvas.get());
-
-    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_;
   params.resizer_rect = resizer_rect_;
   params.plugin_window_moves.swap(plugin_window_moves_);
   params.flags = next_paint_flags_;
   params.scroll_offset = GetScrollOffset();
 
   update_reply_pending_ = true;
   Send(new ViewHostMsg_UpdateRect(routing_id_, params));
@@ skipping 544 matching lines @@
 
 void RenderWidget::CleanupWindowInPluginMoves(gfx::PluginWindowHandle window) {
   for (WebPluginGeometryVector::iterator i = plugin_window_moves_.begin();
        i != plugin_window_moves_.end(); ++i) {
     if (i->window == window) {
       plugin_window_moves_.erase(i);
       break;
     }
   }
 }