Delay UpdateRect until the SwapBuffers callback when accelerated compositing is on.

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 85 matching lines @@
       animation_task_posted_(false),
       invalidation_task_posted_(false) {
   RenderProcess::current()->AddRefProcess();
   DCHECK(RenderThread::Get());
   has_disable_gpu_vsync_switch_ = CommandLine::ForCurrentProcess()->HasSwitch(
       switches::kDisableGpuVsync);
 }
 
 RenderWidget::~RenderWidget() {
   DCHECK(!webwidget_) << "Leaking our WebWidget!";
+  while (!updates_pending_swap_.empty()) {

[jam, 2011/11/11 03:03:45]

STLDeleteContainerPointers?

[piman, 2011/11/16 01:00:55]

std::queue doesn't have iterators. I changed to a std::deque, and used
STLDeleteElements.

+    delete updates_pending_swap_.front();
+    updates_pending_swap_.pop();
+  }
   if (current_paint_buf_) {
     RenderProcess::current()->ReleaseTransportDIB(current_paint_buf_);
     current_paint_buf_ = NULL;
   }
   // If we are swapped out, we have released already.
   if (!is_swapped_out_)
     RenderProcess::current()->ReleaseProcess();
 }
 
 // static
@@ skipping 232 matching lines @@
     RenderProcess::current()->ReleaseProcess();
 }
 
 void RenderWidget::OnRequestMoveAck() {
   DCHECK(pending_window_rect_count_);
   pending_window_rect_count_--;
 }
 
 void RenderWidget::OnUpdateRectAck() {
   TRACE_EVENT0("renderer", "RenderWidget::OnUpdateRectAck");
-  DCHECK(update_reply_pending());
+  DCHECK(update_reply_pending_);
   update_reply_pending_ = false;
 
   // If we sent an UpdateRect message with a zero-sized bitmap, then we should
   // have no current paint buffer.
   if (current_paint_buf_) {
     RenderProcess::current()->ReleaseTransportDIB(current_paint_buf_);
     current_paint_buf_ = NULL;
   }
 
   // If swapbuffers is still pending, then defer the update until the
@@ skipping 11 matching lines @@
 }
 
 bool RenderWidget::SupportsAsynchronousSwapBuffers()
 {
   return false;
 }
 
 void RenderWidget::OnSwapBuffersAborted()
 {
   TRACE_EVENT0("renderer", "RenderWidget::OnSwapBuffersAborted");
+  while (!updates_pending_swap_.empty()) {
+    ViewHostMsg_UpdateRect* msg = updates_pending_swap_.front();
+    updates_pending_swap_.pop();
+    if (msg)

[jam, 2011/11/11 03:03:45]

here and below, when would updates_pending_swap_ not have a null pointer?

[piman, 2011/11/16 01:00:55]

Added a comment.

+      Send(msg);
+  }
   num_swapbuffers_complete_pending_ = 0;
   using_asynchronous_swapbuffers_ = false;
   // Schedule another frame so the compositor learns about it.
   scheduleComposite();
 }
 
 void RenderWidget::OnSwapBuffersPosted() {
   TRACE_EVENT0("renderer", "RenderWidget::OnSwapBuffersPosted");
-  if (using_asynchronous_swapbuffers_)
+
+  if (using_asynchronous_swapbuffers_) {
+    ViewHostMsg_UpdateRect* msg = NULL;
+    if (pending_update_params_.get()) {
+      msg = new ViewHostMsg_UpdateRect(routing_id_, *pending_update_params_);
+      pending_update_params_.reset();
+    }
+    updates_pending_swap_.push(msg);
     num_swapbuffers_complete_pending_++;
+  }
 }
 
 void RenderWidget::OnSwapBuffersComplete() {
   TRACE_EVENT0("renderer", "RenderWidget::OnSwapBuffersComplete");
   // When compositing deactivates, we reset the swapbuffers pending count.  The
   // swapbuffers acks may still arrive, however.
   if (num_swapbuffers_complete_pending_ == 0) {
     TRACE_EVENT0("renderer", "EarlyOut_ZeroSwapbuffersPending");
     return;
   }
+  DCHECK(!updates_pending_swap_.empty());
+  ViewHostMsg_UpdateRect* msg = updates_pending_swap_.front();
+  updates_pending_swap_.pop();
+  if (msg) {
+    Send(msg);
+  }
   num_swapbuffers_complete_pending_--;
 
   // If update reply is still pending, then defer the update until that reply
   // occurs.
   if (update_reply_pending_){
     TRACE_EVENT0("renderer", "EarlyOut_UpdateReplyPending");
     return;
   }
 
   // If we are not accelerated rendering, then this is a stale swapbuffers from
@@ skipping 283 matching lines @@
 
   if (pending_input_event_ack_.get())
     Send(pending_input_event_ack_.release());
 }
 
 void RenderWidget::DoDeferredUpdate() {
   TRACE_EVENT0("renderer", "RenderWidget::DoDeferredUpdate");
 
   if (!webwidget_)
     return;
-  if (update_reply_pending()) {
+  if (update_reply_pending_) {
     TRACE_EVENT0("renderer", "EarlyOut_UpdateReplyPending");
     return;
   }
   if (is_accelerated_compositing_active_ &&
       num_swapbuffers_complete_pending_ >= kMaxSwapBuffersPending) {
     TRACE_EVENT0("renderer", "EarlyOut_MaxSwapBuffersPending");
     return;
   }
 
   // Suppress updating when we are hidden.
@@ skipping 43 matching lines @@
   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);
 
-  // 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;
+  DCHECK(!pending_update_params_.get());
+  pending_update_params_.reset(new ViewHostMsg_UpdateRect_Params);
+  pending_update_params_->dx = update.scroll_delta.x();
+  pending_update_params_->dy = update.scroll_delta.y();
+  pending_update_params_->scroll_rect = update.scroll_rect;
+  pending_update_params_->view_size = size_;
+  pending_update_params_->resizer_rect = resizer_rect_;
+  pending_update_params_->plugin_window_moves.swap(plugin_window_moves_);
+  pending_update_params_->flags = next_paint_flags_;
+  pending_update_params_->scroll_offset = GetScrollOffset();
+  next_paint_flags_ = 0;
+
   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();
+    pending_update_params_->bitmap = dib->id();
+    pending_update_params_->bitmap_rect = optimized_copy_location;
+    pending_update_params_->copy_rects.push_back(optimized_copy_rect);
+    pending_update_params_->flags |= ViewHostMsg_UpdateRect_Flags::NEEDS_ACK;
   } 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;
     }
 
     // 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());
 
+    pending_update_params_->bitmap = current_paint_buf_->id();
+    pending_update_params_->bitmap_rect = bounds;
+    pending_update_params_->flags |= ViewHostMsg_UpdateRect_Flags::NEEDS_ACK;
+
+    std::vector<gfx::Rect>& copy_rects = pending_update_params_->copy_rects;
     // 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.
+    pending_update_params_->bitmap_rect = bounds;
+    pending_update_params_->scroll_rect = gfx::Rect();
     webwidget_->composite(false);
   }
 
-  // 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();
-  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.

[nduca, 2011/11/15 01:57:32]

Might want to preserve this comment up by the //begin painting comment

[piman, 2011/11/16 01:00:55]

Done.

-    params.scroll_rect = gfx::Rect();
-    params.copy_rects.clear();
-  } else {
-    params.scroll_rect = update.scroll_rect;
-    params.copy_rects.swap(copy_rects);  // TODO(darin): clip to bounds?
+  // If composite() called SwapBuffers, pending_update_params_ will be reset (in
+  // OnSwapBuffersPosted), meaning a message has been added to the
+  // updates_pending_swap_ queue, that will be sent later. Otherwise, we send
+  // the message now.
+  if (pending_update_params_.get()) {

[nduca, 2011/11/15 01:57:32]

Can we just put this in the non-composited half of the if above? I think the
comment about "composite calls onSwapBuffersPosted" fits better into the
composite() call...

[piman, 2011/11/16 01:00:55]

When we lose the context, composite() doesn't call swapbuffers. I want to make
sure we're not missing an UpdateRect, so I'm following an ownership pattern:
- pending_update_params_ is created meaning a message needs to be sent
- if we go through the OnSwapBuffersPosted, the swapbuffers calback mechanism
takes ownership of it, and will send the message either when the callback comes,
or when we lost the context.
- if we get here with pending_update_params_ not reset for any reason (software,
composite failed or didn't swap for any reason), that means we need to send it
now.

+    // sending an ack to browser process that the paint is complete...
+    update_reply_pending_ = ViewHostMsg_UpdateRect_Flags::needs_ack(
+        pending_update_params_->flags);
+    Send(new ViewHostMsg_UpdateRect(routing_id_, *pending_update_params_));
+    pending_update_params_.reset();
   }
-  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));
-  next_paint_flags_ = 0;
 
   UpdateTextInputState();
   UpdateSelectionBounds();
 
   // Let derived classes know we've painted.
   DidInitiatePaint();
 }
 
 ///////////////////////////////////////////////////////////////////////////////
 // WebWidgetClient
 
 void RenderWidget::didInvalidateRect(const WebRect& rect) {
   // The invalidated rect might be outside the bounds of the view.
   gfx::Rect view_rect(size_);
   gfx::Rect damaged_rect = view_rect.Intersect(rect);
   if (damaged_rect.IsEmpty())
     return;
 
   paint_aggregator_.InvalidateRect(damaged_rect);
 
   // We may not need to schedule another call to DoDeferredUpdate.
   if (invalidation_task_posted_)
     return;
   if (!paint_aggregator_.HasPendingUpdate())
     return;
-  if (update_reply_pending() ||
+  if (update_reply_pending_ ||
       num_swapbuffers_complete_pending_ >= kMaxSwapBuffersPending)
     return;
 
   // When GPU rendering, combine pending animations and invalidations into
   // a single update.
   if (is_accelerated_compositing_active_ && animation_task_posted_)
     return;
 
   // Perform updating asynchronously.  This serves two purposes:
   // 1) Ensures that we call WebView::Paint without a bunch of other junk
@@ skipping 17 matching lines @@
   if (damaged_rect.IsEmpty())
     return;
 
   paint_aggregator_.ScrollRect(dx, dy, damaged_rect);
 
   // We may not need to schedule another call to DoDeferredUpdate.
   if (invalidation_task_posted_)
     return;
   if (!paint_aggregator_.HasPendingUpdate())
     return;
-  if (update_reply_pending() ||
+  if (update_reply_pending_ ||
       num_swapbuffers_complete_pending_ >= kMaxSwapBuffersPending)
     return;
 
   // When GPU rendering, combine pending animations and invalidations into
   // a single update.
   if (is_accelerated_compositing_active_ && animation_task_posted_)
     return;
 
   // Perform updating asynchronously.  This serves two purposes:
   // 1) Ensures that we call WebView::Paint without a bunch of other junk
   //    on the call stack.
   // 2) Allows us to collect more damage rects before painting to help coalesce
   //    the work that we will need to do.
   invalidation_task_posted_ = true;
   MessageLoop::current()->PostTask(FROM_HERE, NewRunnableMethod(
       this, &RenderWidget::InvalidationCallback));
 }
 
 void RenderWidget::didActivateCompositor(int compositor_identifier) {
   TRACE_EVENT0("gpu", "RenderWidget::didActivateCompositor");
 
   CompositorThread* compositor_thread =
       RenderThreadImpl::current()->compositor_thread();
   if (compositor_thread)
     compositor_thread->AddCompositor(routing_id_, compositor_identifier);
 
+  if (!is_accelerated_compositing_active_) {
+    // When not in accelerated compositing mode, in certain cases (e.g. waiting
+    // for a resize or if no backing store) the RenderWidgetHost is blocking the
+    // browser's UI thread for some time, waiting for an UpdateRect. If we are
+    // going to switch to accelerated compositing, the GPU process may need
+    // round-trips to the browser's UI thread before finishing the frame,
+    // causing deadlocks if we delay the UpdateRect until we receive the
+    // OnSwapBuffersComplete.  So send a dummy message that will unblock the
+    // browser's UI thread.
+    Send(new ViewHostMsg_UpdateIsDelayed(routing_id_));
+  }
+
   is_accelerated_compositing_active_ = true;
   Send(new ViewHostMsg_DidActivateAcceleratedCompositing(
       routing_id_, is_accelerated_compositing_active_));
 
   // Note: asynchronous swapbuffer support currently only matters if
   // compositing scheduling happens on the RenderWidget.
   using_asynchronous_swapbuffers_ = SupportsAsynchronousSwapBuffers();
 }
 
 void RenderWidget::didDeactivateCompositor() {
@@ skipping 514 matching lines @@
   }
 }
 
 bool RenderWidget::WillHandleMouseEvent(const WebKit::WebMouseEvent& event) {
   return false;
 }
 
 bool RenderWidget::WebWidgetHandlesCompositorScheduling() const {
   return false;
 }