Drop frontbuffers with ui-use-gpu-process, synchronized with browser, decoupled from backbuffer dro…

content/browser/renderer_host/render_widget_host_view_aura.cc

Index: content/browser/renderer_host/render_widget_host_view_aura.cc
diff --git a/content/browser/renderer_host/render_widget_host_view_aura.cc b/content/browser/renderer_host/render_widget_host_view_aura.cc
index c3405a6197858354e26d9f73adfeb68a1c8df457..ac525f459a737da6e67f87d7a58a2fb29d58a394 100644
--- a/content/browser/renderer_host/render_widget_host_view_aura.cc
+++ b/content/browser/renderer_host/render_widget_host_view_aura.cc
@@ -9,7 +9,6 @@
 #include "base/command_line.h"
 #include "base/debug/trace_event.h"
 #include "base/logging.h"
-#include "base/memory/weak_ptr.h"
 #include "base/message_loop.h"
 #include "base/string_number_conversions.h"
 #include "content/browser/renderer_host/backing_store_skia.h"
@@ -134,6 +133,13 @@ bool ShouldSendPinchGesture() {
   return pinch_allowed;
 }
 
+bool ShouldReleaseFrontSurface() {
+  static bool release_front_surface_allowed =
+      CommandLine::ForCurrentProcess()->HasSwitch(
+          switches::kEnableUIReleaseFrontSurface);
+  return release_front_surface_allowed;
+}
+
 }  // namespace
 
 // We have to implement the WindowObserver interface on a separate object
@@ -215,6 +221,11 @@ RenderWidgetHostViewAura::RenderWidgetHostViewAura(RenderWidgetHost* host)
       can_compose_inline_(true),
       has_composition_text_(false),
       current_surface_(0),
+      current_surface_is_protected_(true),
+      current_surface_in_use_by_compositor_(true),
+      pending_thumbnail_tasks_(0),
+      visibility_state_id_(0),
+      route_id_(0),
       paint_canvas_(NULL),
       synthetic_move_sent_(false),
       needs_update_texture_(false) {
@@ -286,11 +297,35 @@ RenderWidgetHost* RenderWidgetHostViewAura::GetRenderWidgetHost() const {
 }
 
 void RenderWidgetHostViewAura::DidBecomeSelected() {
+  if (host_->IsVisible())
+    return;
+
   host_->WasRestored();
+
+  ++visibility_state_id_;

[piman, 2012/06/12 20:59:48]

Why not do this in AdjustSurfaceProtection() before (and only if) sending the
message? The state id needs to increase any time the state changes - even if
it's not for visibility reason. BTW, I think it'd be better named
"protection_state_id" or something like that since that's really what it's
linked to.

[mmocny, 2012/06/12 21:25:51]

I need to increase state id before the Adjust function since the visibility
state id is tested for changes inside SetSurfaceNotInUseByCompositor to make
sure we dont have visibility ABA issues while delaying reset for compositing
callback.

I am assuming we will only flip protection as a result of visibility changes
(since right now, that is the only way we set/reset current_surface_), so it
really is a visibility state id (although visibility state id changes more often
than protection does, it always flips when we need it to).

On 2012/06/12 20:59:48, piman wrote:

+
+  if (!current_surface_ && host_->is_accelerated_compositing_active() &&
+      !released_front_lock_.get())
+    released_front_lock_ = window_->GetRootWindow()->GetCompositorLock();
+
+  AdjustSurfaceProtection();
 }
 
 void RenderWidgetHostViewAura::WasHidden() {
+  if (!host_->IsVisible())
+    return;
+
   host_->WasHidden();
+
+  ++visibility_state_id_;
+  released_front_lock_ = NULL;
+
+  if (ShouldReleaseFrontSurface()) {
+    current_surface_ = 0;
+    UpdateExternalTexture();
+  }
+
+  AdjustSurfaceProtection();
 }
 
 void RenderWidgetHostViewAura::SetSize(const gfx::Size& size) {
@@ -453,6 +488,7 @@ void RenderWidgetHostViewAura::CopyFromCompositingSurface(
     skia::PlatformCanvas* output,
     base::Callback<void(bool)> callback) {
   base::ScopedClosureRunner scoped_callback_runner(base::Bind(callback, false));
+
   ui::Compositor* compositor = GetCompositor();
   if (!compositor)
     return;
@@ -478,11 +514,25 @@ void RenderWidgetHostViewAura::CopyFromCompositingSurface(
   unsigned char* addr = static_cast<unsigned char*>(
       output->getTopDevice()->accessBitmap(true).getPixels());
   scoped_callback_runner.Release();
+  // Wrap the callback with an internal handler so that we can inject our
+  // own completion handlers (where we can call AdjustSurfaceProtection).
+  base::Callback<void(bool)> wrapper_callback = base::Bind(
+      &RenderWidgetHostViewAura::CopyFromCompositingSurfaceFinished,
+      AsWeakPtr(),
+      callback);
   gl_helper->CopyTextureTo(container->texture_id(),
                            container->size(),
                            size_in_pixel,
                            addr,
-                           callback);
+                           wrapper_callback);
+  ++pending_thumbnail_tasks_;

[piman, 2012/06/12 20:59:48]

do this before kicking the copy, in case the callback gets called synchronously
from the CopyTextureTo

+}
+
+void RenderWidgetHostViewAura::CopyFromCompositingSurfaceFinished(
+    base::Callback<void(bool)> callback, bool result) {
+  --pending_thumbnail_tasks_;
+  AdjustSurfaceProtection();
+  callback.Run(result);
 }
 
 void RenderWidgetHostViewAura::OnAcceleratedCompositingStateChange() {
@@ -499,12 +549,14 @@ void RenderWidgetHostViewAura::UpdateExternalTexture() {
   needs_update_texture_ = false;
   if (current_surface_ != 0 &&
       host_->is_accelerated_compositing_active()) {
-
+    DCHECK(image_transport_clients_.find(current_surface_) !=
+           image_transport_clients_.end());
     ImageTransportClient* container =
         image_transport_clients_[current_surface_];
     if (container)
       container->Update();
     window_->SetExternalTexture(container);
+    current_surface_in_use_by_compositor_ = true;
 
     if (!container) {
       resize_locks_.clear();
@@ -541,6 +593,19 @@ void RenderWidgetHostViewAura::UpdateExternalTexture() {
     }
   } else {
     window_->SetExternalTexture(NULL);
+    ui::Compositor* compositor = GetCompositor();
+    if (!compositor->DrawPending()) {
+      current_surface_in_use_by_compositor_ = false;
+      AdjustSurfaceProtection();
+    } else {
+      on_compositing_ended_callbacks_.push_back(
+          base::Bind(&RenderWidgetHostViewAura::
+                         SetSurfaceNotInUseByCompositor,
+                     AsWeakPtr(),
+                     visibility_state_id_));
+      if (!compositor->HasObserver(this))
+        compositor->AddObserver(this);
+    }
     resize_locks_.clear();
   }
 }
@@ -548,36 +613,61 @@ void RenderWidgetHostViewAura::UpdateExternalTexture() {
 void RenderWidgetHostViewAura::AcceleratedSurfaceBuffersSwapped(
     const GpuHostMsg_AcceleratedSurfaceBuffersSwapped_Params& params_in_pixel,
     int gpu_host_id) {
+  // If visibility state changed, then this swap is stale. We must still ACK but
+  // do not update current_surface_ since it may have been discarded.
+  if (params_in_pixel.visibility_state_id &&
+      params_in_pixel.visibility_state_id != visibility_state_id_) {
+    DCHECK(!current_surface_);
+    if (params_in_pixel.require_ack)

[piman, 2012/06/12 20:59:48]

nit: per sytle, needs braces if the body is >1 line

+      RenderWidgetHostImpl::AcknowledgeSwapBuffers(params_in_pixel.route_id,
+          gpu_host_id);
+    return;
+  }
   current_surface_ = params_in_pixel.surface_handle;
+  // If we don't require an ACK that means the content is not a fresh updated
+  // new frame, rather we are just resetting our handle to some old content that
+  // we still hadn't discarded. Although we could display imedietally, by not
+  // resetting the compositor lock here, we give us some time to get a fresh
+  // frame which means fewer content flashes.
+  if (params_in_pixel.require_ack)
+    released_front_lock_ = NULL;
+
+  DCHECK(current_surface_);
   UpdateExternalTexture();
 
   ui::Compositor* compositor = GetCompositor();
   if (!compositor) {
     // We have no compositor, so we have no way to display the surface.
     // Must still send the ACK.
-    RenderWidgetHostImpl::AcknowledgeSwapBuffers(params_in_pixel.route_id,
-        gpu_host_id);
+    if (params_in_pixel.require_ack)
+      RenderWidgetHostImpl::AcknowledgeSwapBuffers(params_in_pixel.route_id,
+          gpu_host_id);
   } else {
+    DCHECK(image_transport_clients_.find(params_in_pixel.surface_handle) !=
+           image_transport_clients_.end());
     gfx::Size surface_size_in_pixel =
         image_transport_clients_[params_in_pixel.surface_handle]->size();
     gfx::Size surface_size = content::ConvertSizeToDIP(this,
                                                        surface_size_in_pixel);
     window_->SchedulePaintInRect(gfx::Rect(surface_size));
 
-    if (!resize_locks_.empty() && !compositor->DrawPending()) {
-      // If we are waiting for the resize, fast-track the ACK.
-      // However only do so if we're not between the Draw() and the
-      // OnCompositingEnded(), because out-of-order execution in the GPU process
-      // might corrupt the "front buffer" for the currently issued frame.
-      RenderWidgetHostImpl::AcknowledgeSwapBuffers(
-          params_in_pixel.route_id, gpu_host_id);
-    } else {
-      // Add sending an ACK to the list of things to do OnCompositingEnded
-      on_compositing_ended_callbacks_.push_back(
-          base::Bind(&RenderWidgetHostImpl::AcknowledgeSwapBuffers,
-                     params_in_pixel.route_id, gpu_host_id));
-      if (!compositor->HasObserver(this))
-        compositor->AddObserver(this);
+    if (params_in_pixel.require_ack) {
+      if (!resize_locks_.empty() && !compositor->DrawPending()) {
+        // If we are waiting for the resize, fast-track the ACK.
+        // However only do so if we're not between the Draw() and the
+        // OnCompositingEnded(), because out-of-order execution in the GPU
+        // process might corrupt the "front buffer" for the currently issued
+        // frame.
+        RenderWidgetHostImpl::AcknowledgeSwapBuffers(
+            params_in_pixel.route_id, gpu_host_id);
+      } else {
+        // Add sending an ACK to the list of things to do OnCompositingEnded
+        on_compositing_ended_callbacks_.push_back(
+            base::Bind(&RenderWidgetHostImpl::AcknowledgeSwapBuffers,
+                       params_in_pixel.route_id, gpu_host_id));
+        if (!compositor->HasObserver(this))
+          compositor->AddObserver(this);
+      }
     }
   }
 }
@@ -585,7 +675,18 @@ void RenderWidgetHostViewAura::AcceleratedSurfaceBuffersSwapped(
 void RenderWidgetHostViewAura::AcceleratedSurfacePostSubBuffer(
     const GpuHostMsg_AcceleratedSurfacePostSubBuffer_Params& params_in_pixel,
     int gpu_host_id) {
+  // If visible state changed, then this PSB is stale. We must still ACK but
+  // do not update current_surface_.
+  if (params_in_pixel.visibility_state_id &&
+      params_in_pixel.visibility_state_id != visibility_state_id_) {
+    DCHECK(!current_surface_);
+    RenderWidgetHostImpl::AcknowledgePostSubBuffer(params_in_pixel.route_id,
+        gpu_host_id);
+    return;
+  }
   current_surface_ = params_in_pixel.surface_handle;
+  released_front_lock_ = NULL;
+  DCHECK(current_surface_);
   UpdateExternalTexture();
 
   ui::Compositor* compositor = GetCompositor();
@@ -595,6 +696,8 @@ void RenderWidgetHostViewAura::AcceleratedSurfacePostSubBuffer(
     RenderWidgetHostImpl::AcknowledgePostSubBuffer(
         params_in_pixel.route_id, gpu_host_id);
   } else {
+    DCHECK(image_transport_clients_.find(params_in_pixel.surface_handle) !=
+           image_transport_clients_.end());
     gfx::Size surface_size_in_pixel =
         image_transport_clients_[params_in_pixel.surface_handle]->size();
 
@@ -641,7 +744,8 @@ void RenderWidgetHostViewAura::AcceleratedSurfaceNew(
       int32 width_in_pixel,
       int32 height_in_pixel,
       uint64* surface_handle,
-      TransportDIB::Handle* shm_handle) {
+      TransportDIB::Handle* shm_handle,
+      int32 route_id) {
   ImageTransportFactory* factory = ImageTransportFactory::GetInstance();
   scoped_refptr<ImageTransportClient> surface(factory->CreateTransportClient(
         gfx::Size(width_in_pixel, height_in_pixel), surface_handle));
@@ -652,10 +756,14 @@ void RenderWidgetHostViewAura::AcceleratedSurfaceNew(
   *shm_handle = surface->Handle();
 
   image_transport_clients_[*surface_handle] = surface;
+
+  route_id_ = route_id;
 }
 
 void RenderWidgetHostViewAura::AcceleratedSurfaceRelease(
     uint64 surface_handle) {
+  DCHECK(image_transport_clients_.find(surface_handle) !=
+         image_transport_clients_.end());
   if (current_surface_ == surface_handle) {
     current_surface_ = 0;
     UpdateExternalTexture();
@@ -663,6 +771,38 @@ void RenderWidgetHostViewAura::AcceleratedSurfaceRelease(
   image_transport_clients_.erase(surface_handle);
 }
 
+void RenderWidgetHostViewAura::SetSurfaceNotInUseByCompositor(
+    uint32 expected_visibility_state_id) {
+  if (visibility_state_id_ != expected_visibility_state_id)
+    return;
+  current_surface_in_use_by_compositor_ = false;
+  AdjustSurfaceProtection();
+}
+
+void RenderWidgetHostViewAura::AdjustSurfaceProtection() {
+  // If the current surface is non null, it is protected.
+  // If we are visible, it is protected.
+  // If we are not visible and current surface is null, still stay protected
+  // until we finish thumbnailing and compositing.
+  bool surface_is_protected =
+      current_surface_ ||
+      host_->IsVisible() ||
+      (current_surface_is_protected_ &&
+          (pending_thumbnail_tasks_ || GetCompositor()->DrawPending()));
+  if (current_surface_is_protected_ == surface_is_protected)
+    return;
+  current_surface_is_protected_ = surface_is_protected;
+
+  if (!route_id_ || !shared_surface_handle_.parent_gpu_process_id)
+    return;
+
+  RenderWidgetHostImpl::SendFrontSurfaceIsProtected(
+      surface_is_protected,
+      visibility_state_id_,
+      route_id_,
+      shared_surface_handle_.parent_gpu_process_id);
+}
+
 void RenderWidgetHostViewAura::SetBackground(const SkBitmap& background) {
   content::RenderWidgetHostViewBase::SetBackground(background);
   host_->SetBackground(background);
@@ -1203,6 +1343,10 @@ void RenderWidgetHostViewAura::OnCompositingAborted(
 void RenderWidgetHostViewAura::OnLostResources(ui::Compositor* compositor) {
   image_transport_clients_.clear();
   current_surface_ = 0;
+  visibility_state_id_ = 0;
+  current_surface_is_protected_ = true;
+  current_surface_in_use_by_compositor_ = true;
+  route_id_ = 0;
   UpdateExternalTexture();
   locks_pending_draw_.clear();