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

content/browser/renderer_host/render_widget_host_view_aura.h

Index: content/browser/renderer_host/render_widget_host_view_aura.h
diff --git a/content/browser/renderer_host/render_widget_host_view_aura.h b/content/browser/renderer_host/render_widget_host_view_aura.h
index cecf5416aed596d0fdf53d80c4820b6adac56450..591c21b4b9497a6aa175f6e142daf9a3fdfb896c 100644
--- a/content/browser/renderer_host/render_widget_host_view_aura.h
+++ b/content/browser/renderer_host/render_widget_host_view_aura.h
@@ -23,7 +23,6 @@
 #include "webkit/glue/webcursor.h"
 
 namespace aura {
-class CompositorLock;
 class WindowTracker;
 }
 
@@ -32,6 +31,7 @@ class Canvas;
 }
 
 namespace ui {
+class CompositorLock;
 class InputMethod;
 class Texture;
 }
@@ -206,10 +206,11 @@ class RenderWidgetHostViewAura
 
   // Overridden from ui::CompositorObserver:
   virtual void OnCompositingDidCommit(ui::Compositor* compositor) OVERRIDE;
-  virtual void OnCompositingWillStart(ui::Compositor* compositor) OVERRIDE;
   virtual void OnCompositingStarted(ui::Compositor* compositor) OVERRIDE;
   virtual void OnCompositingEnded(ui::Compositor* compositor) OVERRIDE;
   virtual void OnCompositingAborted(ui::Compositor* compositor) OVERRIDE;
+  virtual void OnCompositingLockStateChanged(
+      ui::Compositor* compositor) OVERRIDE;
 
   // Overridden from ImageTransportFactoryObserver:
   virtual void OnLostResources() OVERRIDE;
@@ -217,6 +218,7 @@ class RenderWidgetHostViewAura
   virtual ~RenderWidgetHostViewAura();
 
   void UpdateCursorIfOverSelf();
+  bool ShouldFastACK(uint64 surface_id);
   void UpdateExternalTexture();
   ui::InputMethod* GetInputMethod() const;
 
@@ -241,12 +243,12 @@ class RenderWidgetHostViewAura
 
   // Run the compositing callbacks.
   void RunCompositingDidCommitCallbacks(ui::Compositor* compositor);
-  void RunCompositingWillStartCallbacks(ui::Compositor* compositor);
 
   // Insert a sync point into the compositor's command stream and acknowledge
   // that we have presented the accelerated surface buffer.
   static void InsertSyncPointAndACK(int32 route_id,
                                     int gpu_host_id,
+                                    bool presented,
                                     ui::Compositor* compositor);
 
   // Called when window_ is removed from the window tree.
@@ -330,9 +332,6 @@ class RenderWidgetHostViewAura
   std::vector< base::Callback<void(ui::Compositor*)> >
       on_compositing_did_commit_callbacks_;
 
-  std::vector< base::Callback<void(ui::Compositor*)> >
-      on_compositing_will_start_callbacks_;
-
   std::map<uint64, scoped_refptr<ui::Texture> >
       image_transport_clients_;
 
@@ -379,19 +378,32 @@ class RenderWidgetHostViewAura
 
   // Used to prevent further resizes while a resize is pending.
   class ResizeLock;
+  typedef std::vector<linked_ptr<ResizeLock> > ResizeLockList;
+
   // These locks are the ones waiting for a texture of the right size to come
   // back from the renderer/GPU process.
-  std::vector<linked_ptr<ResizeLock> > resize_locks_;
-  // These locks are the ones waiting for a frame to be drawn.
-  std::vector<linked_ptr<ResizeLock> > locks_pending_draw_;
+  ResizeLockList resize_locks_;
+  // These locks are the ones waiting for a frame to be committed.
+  ResizeLockList locks_pending_commit_;
 
   // This lock is for waiting for a front surface to become available to draw.
-  scoped_refptr<aura::CompositorLock> released_front_lock_;
+  scoped_refptr<ui::CompositorLock> released_front_lock_;
 
   // Used to track the state of the window we're created from. Only used when
   // created fullscreen.
   scoped_ptr<aura::WindowTracker> host_tracker_;
 
+  enum CanLockCompositorState {
+    YES,
+    // We locked, so at some point we'll need to kick a frame.
+    YES_DID_LOCK,
+    // No. A lock timed out, we need to kick a new frame before locking again.

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

Since the lock timed out, we just "kicked" a frame. Shouldn't that mean we just
need to wait for the commit before locking again? In which case, shouldn't this
be merged into the NO_PENDING_COMMIT state?

I'm probably missing something, it's been a while since I reviewed your original
change and I may not have paged in all the constraints.

Looking at the code, I believe the only transition out of this state is to
NO_PENDING_COMMIT when we receive a swap/post from the renderer. If the
intention is to never lock from this RWHVA until we did receive a frame from the
renderer, I think we should rename the state into something like
"NO_PENDING_RENDERER_FRAME" and update the comment (otherwise I'm super confused
if "frame" refers to a UI frame or a renderer frame).

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

Fast ACKs complicate the logic. When we fast ACK, we don't
UpdateExternalTexture. So if we just fast ACKed, the first commit after the lock
times out won't show an updated tab contents (it will gutter because the UI
window size changed). By delaying new compositor locks until we display an
updated renderer frame (no fast ACK), the renderer appears to keep up better
(en.wikipedia.org/wiki/Cat). It may cost us a bit because we miss a fast ACK,
but I find it feels much more responsive.

An alternative (where we always fast ACK) is that the window size changes, but
the tab contents may not change until the user stops resizing. It may
technically be faster, but it feels slower.
I hope the reasoning above explains why I have two states (fast ACKs). I'll
rename to NO_PENDING_FRAME ---> NO_PENDING_RENDERER_FRAME. That's clearer. FWIW,
there are one or two places where we just jump to NO_PENDING_COMMIT (e.g. when
we defer SetSurfaceNotInUseByCompositor until a commit occurs).

+    NO_PENDING_FRAME,
+    // No. We've got a frame, but it hasn't been committed.
+    NO_PENDING_COMMIT,
+  };
+  CanLockCompositorState can_lock_compositor_;
+
   DISALLOW_COPY_AND_ASSIGN(RenderWidgetHostViewAura);
 };