Aura: Resize locks with --ui-enable-threaded-compositing

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 96fcbde81ecf931a3606b4727d99f13dd7b71915..6dfe2d592cee79a9a43fe260c0d14a47ac1960f9 100644
--- a/content/browser/renderer_host/render_widget_host_view_aura.cc
+++ b/content/browser/renderer_host/render_widget_host_view_aura.cc
@@ -197,11 +197,16 @@ class RenderWidgetHostViewAura::WindowObserver : public aura::WindowObserver {
 
 class RenderWidgetHostViewAura::ResizeLock {
  public:
-  ResizeLock(aura::RootWindow* root_window, const gfx::Size new_size)
+  ResizeLock(aura::RootWindow* root_window,
+             const gfx::Size new_size,
+             bool defer_compositor_lock)
       : root_window_(root_window),
         new_size_(new_size),
-        compositor_lock_(root_window_->GetCompositorLock()),
-        weak_ptr_factory_(this) {
+        compositor_lock_(defer_compositor_lock ?
+                         NULL :
+                         root_window_->compositor()->GetCompositorLock()),
+        weak_ptr_factory_(this),
+        defer_compositor_lock_(defer_compositor_lock) {
     root_window_->HoldMouseMoves();
 
     BrowserThread::PostDelayedTask(
@@ -216,6 +221,7 @@ class RenderWidgetHostViewAura::ResizeLock {
   }
 
   void UnlockCompositor() {
+    defer_compositor_lock_ = false;
     compositor_lock_ = NULL;
   }
 
@@ -231,11 +237,19 @@ class RenderWidgetHostViewAura::ResizeLock {
     return new_size_;
   }
 
+  void GrabDeferredLock() {
+    if (defer_compositor_lock_) {

[piman, 2012/10/17 21:26:32]

&& root_window_

[jonathan.backer, 2012/10/18 20:20:35]

Can do. It's more robust. But if root_window_ = NULL, we already
UnlockCompositor() and defer_compositor_lock = false.

+      compositor_lock_ = root_window_->compositor()->GetCompositorLock();
+      defer_compositor_lock_ = false;
+    }
+  }
+
  private:
   aura::RootWindow* root_window_;
   gfx::Size new_size_;
-  scoped_refptr<aura::CompositorLock> compositor_lock_;
+  scoped_refptr<ui::CompositorLock> compositor_lock_;
   base::WeakPtrFactory<ResizeLock> weak_ptr_factory_;
+  bool defer_compositor_lock_;
 
   DISALLOW_COPY_AND_ASSIGN(ResizeLock);
 };
@@ -262,7 +276,8 @@ RenderWidgetHostViewAura::RenderWidgetHostViewAura(RenderWidgetHost* host)
       surface_route_id_(0),
       paint_canvas_(NULL),
       synthetic_move_sent_(false),
-      accelerated_compositing_state_changed_(false) {
+      accelerated_compositing_state_changed_(false),
+      can_lock_compositor_(YES) {
   host_->SetView(this);
   window_observer_.reset(new WindowObserver(this));
   window_->AddObserver(window_observer_.get());
@@ -342,7 +357,7 @@ void RenderWidgetHostViewAura::WasShown() {
 
   if (!current_surface_ && host_->is_accelerated_compositing_active() &&
       !released_front_lock_.get()) {
-    released_front_lock_ = window_->GetRootWindow()->GetCompositorLock();
+    released_front_lock_ = GetCompositor()->GetCompositorLock();
   }
 
   AdjustSurfaceProtection();
@@ -387,9 +402,21 @@ void RenderWidgetHostViewAura::SetBounds(const gfx::Rect& rect) {
   if (window_->bounds().size() != rect.size() &&
       host_->is_accelerated_compositing_active()) {
     aura::RootWindow* root_window = window_->GetRootWindow();
-    if (root_window) {
+    ui::Compositor* compositor = GetCompositor();

[piman, 2012/10/17 21:26:32]

nit: ui::Compositor* compositor = root_window ? root_window->compositor() :
NULL;
That avoids traversing the tree a second time.

[jonathan.backer, 2012/10/18 20:20:35]

Done.

+    if (root_window && compositor) {
+      // Listen to changes in the compositor lock state.
+      if (!compositor->HasObserver(this))
+        compositor->AddObserver(this);
+
+      bool defer_compositor_lock =
+         can_lock_compositor_ == NO_PENDING_FRAME ||
+         can_lock_compositor_ == NO_PENDING_COMMIT;
+
+      if (can_lock_compositor_ == YES)
+        can_lock_compositor_ = YES_DID_LOCK;
+
       resize_locks_.push_back(make_linked_ptr(
-          new ResizeLock(root_window, rect.size())));
+          new ResizeLock(root_window, rect.size(), defer_compositor_lock)));
     }
   }
   window_->SetBounds(rect);
@@ -693,6 +720,28 @@ void RenderWidgetHostViewAura::OnAcceleratedCompositingStateChange() {
   accelerated_compositing_state_changed_ = true;
 }
 
+bool RenderWidgetHostViewAura::ShouldFastACK(uint64 surface_id) {
+  ui::Texture* container = image_transport_clients_[surface_id];
+  DCHECK(container);
+
+  if (can_lock_compositor_ == NO_PENDING_FRAME ||
+      can_lock_compositor_ == NO_PENDING_COMMIT ||
+      resize_locks_.empty())
+    return false;
+
+  gfx::Size container_size = ConvertSizeToDIP(this, container->size());
+  ResizeLockList::iterator it = resize_locks_.begin();
+  while (it != resize_locks_.end()) {
+    if ((*it)->expected_size() == container_size)
+      break;
+    ++it;
+  }
+
+  // We could be getting an unexpected frame due to an animation
+  // (i.e. we start resizing but we get an old size frame first).
+  return it == resize_locks_.end() || ++it != resize_locks_.end();
+}
+
 void RenderWidgetHostViewAura::UpdateExternalTexture() {
   // Delay processing accelerated compositing state change till here where we
   // act upon the state change. (Clear the external texture if switching to
@@ -714,7 +763,6 @@ void RenderWidgetHostViewAura::UpdateExternalTexture() {
     if (!container) {
       resize_locks_.clear();
     } else {
-      typedef std::vector<linked_ptr<ResizeLock> > ResizeLockList;
       ResizeLockList::iterator it = resize_locks_.begin();
       while (it != resize_locks_.end()) {
         gfx::Size container_size = ConvertSizeToDIP(this,
@@ -730,8 +778,8 @@ void RenderWidgetHostViewAura::UpdateExternalTexture() {
           // Delay the release of the lock until we've kicked a frame with the
           // new texture, to avoid resizing the UI before we have a chance to
           // draw a "good" frame.
-          locks_pending_draw_.insert(
-              locks_pending_draw_.begin(), resize_locks_.begin(), it);
+          locks_pending_commit_.insert(
+              locks_pending_commit_.begin(), resize_locks_.begin(), it);
           // However since we got the size we were looking for, unlock the
           // compositor.
           for (ResizeLockList::iterator it2 = resize_locks_.begin();
@@ -748,12 +796,12 @@ void RenderWidgetHostViewAura::UpdateExternalTexture() {
     window_->SetExternalTexture(NULL);
     if (ShouldReleaseFrontSurface() &&
         host_->is_accelerated_compositing_active()) {
-      // The current surface may have pipelined gl commands, so always wait for
-      // the next composite to start.  If the current surface is still null,
-      // then we really know its no longer in use.
+      // We need to wait for a commit to clear to guarantee that all we
+      // will not issue any more GL referencing the previous surface.
       ui::Compositor* compositor = GetCompositor();
       if (compositor) {
-        on_compositing_will_start_callbacks_.push_back(
+        can_lock_compositor_ = NO_PENDING_COMMIT;
+        on_compositing_did_commit_callbacks_.push_back(
             base::Bind(&RenderWidgetHostViewAura::
                            SetSurfaceNotInUseByCompositor,
                        AsWeakPtr()));
@@ -775,62 +823,44 @@ void RenderWidgetHostViewAura::AcceleratedSurfaceBuffersSwapped(
       params_in_pixel.protection_state_id != protection_state_id_) {
     DCHECK(!current_surface_);
     if (!params_in_pixel.skip_ack)
-      InsertSyncPointAndACK(params_in_pixel.route_id, gpu_host_id, NULL);
+      InsertSyncPointAndACK(params_in_pixel.route_id, gpu_host_id, false, NULL);
     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 immediately, 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.skip_ack)
-    released_front_lock_ = NULL;
-
-  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.
+  if (!compositor || ShouldFastACK(params_in_pixel.surface_handle)) {

[piman, 2012/10/17 21:26:32]

I think we still want to update the layer tree (UpdateExternalTexture) if we
don't have a compositor (we're outside of the window tree) but everything else
allows it. While I'm not 100% sure of all the conditions why we may be outside
the tree (I think prerendering and stuff), once we get back into the tree, we'd
want to have the up-to-date frame, and we may not even get a notification to
force one through the pipeline.

[jonathan.backer, 2012/10/18 20:20:35]

Done.

     if (!params_in_pixel.skip_ack)
-      InsertSyncPointAndACK(params_in_pixel.route_id, gpu_host_id, NULL);
+      InsertSyncPointAndACK(params_in_pixel.route_id, gpu_host_id, false,
+                            compositor);
   } else {
+    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 immediately,
+    // 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.skip_ack)
+      released_front_lock_ = NULL;
+
+    UpdateExternalTexture();
+
     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 = ConvertSizeToDIP(this,
-                                                       surface_size_in_pixel);
+    gfx::Size surface_size = ConvertSizeToDIP(this, surface_size_in_pixel);
     window_->SchedulePaintInRect(gfx::Rect(surface_size));
 
     if (!params_in_pixel.skip_ack) {
-      if (!resize_locks_.empty()) {
-        // If we are waiting for the resize, fast-track the ACK.
-        if (compositor->IsThreaded()) {
-          // We need the compositor thread to pick up the active buffer before
-          // ACKing.
-          on_compositing_did_commit_callbacks_.push_back(
-              base::Bind(&RenderWidgetHostViewAura::InsertSyncPointAndACK,
-                         params_in_pixel.route_id,
-                         gpu_host_id));
-          if (!compositor->HasObserver(this))
-            compositor->AddObserver(this);
-        } else {
-          // The compositor will pickup the active buffer during a draw, so we
-          // can ACK immediately.
-          InsertSyncPointAndACK(params_in_pixel.route_id, gpu_host_id,
-                                compositor);
-        }
-      } else {
-        // Add sending an ACK to the list of things to do OnCompositingWillStart
-        on_compositing_will_start_callbacks_.push_back(
-            base::Bind(&RenderWidgetHostViewAura::InsertSyncPointAndACK,
-                       params_in_pixel.route_id,
-                       gpu_host_id));
-        if (!compositor->HasObserver(this))
-          compositor->AddObserver(this);
-      }
+      // Add sending an ACK to the list of things to do OnCompositingDidCommit
+      can_lock_compositor_ = NO_PENDING_COMMIT;
+      on_compositing_did_commit_callbacks_.push_back(
+          base::Bind(&RenderWidgetHostViewAura::InsertSyncPointAndACK,
+                     params_in_pixel.route_id,
+                     gpu_host_id,
+                     true));
+      if (!compositor->HasObserver(this))
+        compositor->AddObserver(this);
     }
   }
 }
@@ -844,20 +874,19 @@ void RenderWidgetHostViewAura::AcceleratedSurfacePostSubBuffer(
   if (params_in_pixel.protection_state_id &&
       params_in_pixel.protection_state_id != protection_state_id_) {
     DCHECK(!current_surface_);
-    InsertSyncPointAndACK(params_in_pixel.route_id, gpu_host_id, NULL);
+    InsertSyncPointAndACK(params_in_pixel.route_id, gpu_host_id, false, NULL);
     return;
   }
-  current_surface_ = params_in_pixel.surface_handle;
-  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
-    InsertSyncPointAndACK(params_in_pixel.route_id, gpu_host_id, NULL);
+  if (!compositor || ShouldFastACK(params_in_pixel.surface_handle)) {
+    InsertSyncPointAndACK(params_in_pixel.route_id, gpu_host_id, false,
+                          compositor);
   } else {
+    current_surface_ = params_in_pixel.surface_handle;
+    released_front_lock_ = NULL;
+    DCHECK(current_surface_);
+    UpdateExternalTexture();
     DCHECK(image_transport_clients_.find(params_in_pixel.surface_handle) !=
            image_transport_clients_.end());
     gfx::Size surface_size_in_pixel =
@@ -879,32 +908,15 @@ void RenderWidgetHostViewAura::AcceleratedSurfacePostSubBuffer(
 
     window_->SchedulePaintInRect(rect_to_paint);
 
-    if (!resize_locks_.empty()) {
-      // If we are waiting for the resize, fast-track the ACK.
-      if (compositor->IsThreaded()) {
-        // We need the compositor thread to pick up the active buffer before
-        // ACKing.
-        on_compositing_did_commit_callbacks_.push_back(
-            base::Bind(&RenderWidgetHostViewAura::InsertSyncPointAndACK,
-                       params_in_pixel.route_id,
-                       gpu_host_id));
-        if (!compositor->HasObserver(this))
-          compositor->AddObserver(this);
-      } else {
-        // The compositor will pickup the active buffer during a draw, so we
-        // can ACK immediately.
-        InsertSyncPointAndACK(params_in_pixel.route_id, gpu_host_id,
-                              compositor);
-      }
-    } else {
-      // Add sending an ACK to the list of things to do OnCompositingWillStart
-      on_compositing_will_start_callbacks_.push_back(
-          base::Bind(&RenderWidgetHostViewAura::InsertSyncPointAndACK,
-                     params_in_pixel.route_id,
-                     gpu_host_id));
-      if (!compositor->HasObserver(this))
-        compositor->AddObserver(this);
-    }
+    // Add sending an ACK to the list of things to do OnCompositingDidCommit
+    can_lock_compositor_ = NO_PENDING_COMMIT;
+    on_compositing_did_commit_callbacks_.push_back(
+        base::Bind(&RenderWidgetHostViewAura::InsertSyncPointAndACK,
+                   params_in_pixel.route_id,
+                   gpu_host_id,
+                   true));
+    if (!compositor->HasObserver(this))
+      compositor->AddObserver(this);
   }
 }
 
@@ -1637,17 +1649,18 @@ void RenderWidgetHostViewAura::OnLostActive() {
 
 void RenderWidgetHostViewAura::OnCompositingDidCommit(
     ui::Compositor* compositor) {
+  if (can_lock_compositor_ == NO_PENDING_COMMIT) {
+    can_lock_compositor_ = YES;
+    for (ResizeLockList::iterator it = resize_locks_.begin();
+        it != resize_locks_.end(); ++it)
+      (*it)->GrabDeferredLock();

[piman, 2012/10/17 21:26:32]

should we set can_lock_compositor_ to YES_DID_LOCK if we did grab the lock?

[jonathan.backer, 2012/10/18 20:20:35]

Done.

+  }
   RunCompositingDidCommitCallbacks(compositor);
-}
-
-void RenderWidgetHostViewAura::OnCompositingWillStart(
-    ui::Compositor* compositor) {
-  RunCompositingWillStartCallbacks(compositor);
+  locks_pending_commit_.clear();
 }
 
 void RenderWidgetHostViewAura::OnCompositingStarted(
     ui::Compositor* compositor) {
-  locks_pending_draw_.clear();
 }
 
 void RenderWidgetHostViewAura::OnCompositingEnded(
@@ -1658,6 +1671,13 @@ void RenderWidgetHostViewAura::OnCompositingAborted(
     ui::Compositor* compositor) {
 }
 
+void RenderWidgetHostViewAura::OnCompositingLockStateChanged(
+    ui::Compositor* compositor) {
+  if (!compositor->IsLocked() && can_lock_compositor_ == YES_DID_LOCK) {

[piman, 2012/10/17 21:26:32]

It's worth a comment to point out that this happens when the lock timed out.

[jonathan.backer, 2012/10/18 20:20:35]

Done.

+    can_lock_compositor_ = NO_PENDING_FRAME;
+  }
+}
+
 ////////////////////////////////////////////////////////////////////////////////
 // RenderWidgetHostViewAura, ImageTransportFactoryObserver implementation:
 
@@ -1669,7 +1689,7 @@ void RenderWidgetHostViewAura::OnLostResources() {
   current_surface_in_use_by_compositor_ = true;
   surface_route_id_ = 0;
   UpdateExternalTexture();
-  locks_pending_draw_.clear();
+  locks_pending_commit_.clear();
 
   DCHECK(!shared_surface_handle_.is_null());
   ImageTransportFactory* factory = ImageTransportFactory::GetInstance();
@@ -1815,19 +1835,10 @@ void RenderWidgetHostViewAura::RunCompositingDidCommitCallbacks(
   on_compositing_did_commit_callbacks_.clear();
 }
 
-void RenderWidgetHostViewAura::RunCompositingWillStartCallbacks(
-    ui::Compositor* compositor) {
-  for (std::vector< base::Callback<void(ui::Compositor*)> >::const_iterator
-      it = on_compositing_will_start_callbacks_.begin();
-      it != on_compositing_will_start_callbacks_.end(); ++it) {
-    it->Run(compositor);
-  }
-  on_compositing_will_start_callbacks_.clear();
-}
-
 // static
 void RenderWidgetHostViewAura::InsertSyncPointAndACK(
-     int32 route_id, int gpu_host_id, ui::Compositor* compositor) {
+    int32 route_id, int gpu_host_id, bool presented,
+    ui::Compositor* compositor) {
   uint32 sync_point = 0;
   // If we have no compositor, so we must still send the ACK. A zero
   // sync point will not be waited for in the GPU process.
@@ -1837,7 +1848,7 @@ void RenderWidgetHostViewAura::InsertSyncPointAndACK(
   }
 
   RenderWidgetHostImpl::AcknowledgeBufferPresent(
-      route_id, gpu_host_id, sync_point);
+      route_id, gpu_host_id, presented, sync_point);
 }
 
 void RenderWidgetHostViewAura::RemovingFromRootWindow() {
@@ -1849,8 +1860,7 @@ void RenderWidgetHostViewAura::RemovingFromRootWindow() {
   // composited data.
   ui::Compositor* compositor = GetCompositor();
   RunCompositingDidCommitCallbacks(compositor);
-  RunCompositingWillStartCallbacks(compositor);
-  locks_pending_draw_.clear();
+  locks_pending_commit_.clear();
   if (compositor && compositor->HasObserver(this))
     compositor->RemoveObserver(this);
   DetachFromInputMethod();