Do not send a frame swap ack from the browser until the frame is drawn

Code to do this was recently removed because it was no longer being
used in r241225 (https://codereview.chromium.org/116103002). This is
now necessary again to throttle renderers when using CoreAnimation.

Frames are acked only when they are drawn. Both the CoreAnimation and
the non-CoreAnimation paths are changed to behave this way. By virtue
of the fact that the non-CoreAnimation path draws immediately, this
should have no functional effect on that path.

This differs from the mechanism deleted in the aforementioned patch in
two ways.

First, it uses a scoped_ptr of a struct instead of a vector of pairs to
store the information about the swap being returned. This should improve
readability in that the ack has struct names instead of just first and
second in the pair, and in that the scoped_ptr does not suggest support
for multiple pending swaps (which does not exist).

Second, it does makes RWHVMac ack frames more aggressively when
inside a draw call, rather than adding a call from RWHImpl to the view,
to effect the ack (which is what the old way does).

BUG=340133

Review URL: https://codereview.chromium.org/165703002

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@251918 0039d316-1c4b-4281-b951-d872f2087c98

content/browser/renderer_host/compositing_iosurface_layer_mac.mm

@@ -16,6 +16,14 @@
 #include "ui/gfx/size_conversions.h"
 #include "ui/gl/gpu_switching_manager.h"
 
+@interface CompositingIOSurfaceLayer()
+
+// Private method to wait for a frame of the right size if we're in an active
+// resize. This may potentially dispatch select messages from the run loop.
+- (void)waitForResizedFrameInContext:(CGLContextObj)glContext;
+
+@end
+
 @implementation CompositingIOSurfaceLayer
 
 @synthesize context = context_;
@@ -83,6 +91,41 @@
     [self setNeedsDisplay];
 }
 
+- (void)waitForResizedFrameInContext:(CGLContextObj)glContext {
+  // Cache a copy of renderWidgetHostView_ because it may be reset if
+  // a software frame is received in GetBackingStore.
+  content::RenderWidgetHostViewMac* cached_view = renderWidgetHostView_;
+  if (!cached_view->render_widget_host_ ||
+      cached_view->render_widget_host_->is_hidden()) {
+    return;
+  }
+
+  // Note that GetBackingStore can potentially spawn a nested run loop, which
+  // may change the current GL context, or, because the GL contexts are
+  // shared, may change the currently-bound FBO. Ensure that, when the run
+  // loop returns, the original GL context remain current, and the original
+  // FBO remain bound.
+  // TODO(ccameron): This is far too fragile a mechanism to rely on. Find
+  // a way to avoid doing this.
+  GLuint previous_framebuffer = 0;
+  glGetIntegerv(GL_FRAMEBUFFER_BINDING,
+                reinterpret_cast<GLint*>(&previous_framebuffer));
+  {
+    // If a resize is in progress then GetBackingStore request a frame of the
+    // current window size and block until a frame of the right size comes in.
+    // This makes the window content not lag behind the resize (at the cost of
+    // blocking on the browser's main thread).
+    gfx::ScopedCGLSetCurrentContext scoped_set_current_context(NULL);
+    cached_view->about_to_validate_and_paint_ = true;
+    (void)cached_view->render_widget_host_->GetBackingStore(true);
+    cached_view->about_to_validate_and_paint_ = false;
+  }
+  CHECK_EQ(CGLGetCurrentContext(), glContext)
+      << "original GL context failed to re-bind after nested run loop, "
+      << "browser crash is imminent.";
+  glBindFramebuffer(GL_FRAMEBUFFER, previous_framebuffer);
+}
+
 // The remaining methods implement the CAOpenGLLayer interface.
 
 - (CGLPixelFormatObj)copyCGLPixelFormatForDisplayMask:(uint32_t)mask {
@@ -124,36 +167,12 @@
     return;
   }
 
-  // Cache a copy of renderWidgetHostView_ because it may be reset if
-  // a software frame is received in GetBackingStore.
-  content::RenderWidgetHostViewMac* cached_view = renderWidgetHostView_;
+  // Acknowledge the frame before we potentially wait for a frame of the right
+  // size.
+  renderWidgetHostView_->SendPendingSwapAck();
 
-  // If a resize is in progress then GetBackingStore request a frame of the
-  // current window size and block until a frame of the right size comes in.
-  // This makes the window content not lag behind the resize (at the cost of
-  // blocking on the browser's main thread).
-  if (cached_view->render_widget_host_) {
-    // Note that GetBackingStore can potentially spawn a nested run loop, which
-    // may change the current GL context, or, because the GL contexts are
-    // shared, may change the currently-bound FBO. Ensure that, when the run
-    // loop returns, the original GL context remain current, and the original
-    // FBO remain bound.
-    // TODO(ccameron): This is far too fragile a mechanism to rely on. Find
-    // a way to avoid doing this.
-    GLuint previous_framebuffer = 0;
-    glGetIntegerv(GL_FRAMEBUFFER_BINDING,
-                  reinterpret_cast<GLint*>(&previous_framebuffer));
-    {
-      gfx::ScopedCGLSetCurrentContext scoped_set_current_context(NULL);
-      cached_view->about_to_validate_and_paint_ = true;
-      (void)cached_view->render_widget_host_->GetBackingStore(true);
-      cached_view->about_to_validate_and_paint_ = false;
-    }
-    CHECK_EQ(CGLGetCurrentContext(), glContext)
-        << "original GL context failed to re-bind after nested run loop, "
-        << "browser crash is imminent.";
-    glBindFramebuffer(GL_FRAMEBUFFER, previous_framebuffer);
-  }
+  // Wait for a frame of the right size to come in, if needed.
+  [self waitForResizedFrameInContext:glContext];
 
   // If a transition to software mode has occurred, this layer should be
   // removed from the heirarchy now, so don't draw anything.

content/browser/renderer_host/render_widget_host_view_mac.h

@@ -398,8 +398,11 @@ class RenderWidgetHostViewMac : public RenderWidgetHostViewBase,
   // someone (other than superview) has retained |cocoa_view_|.
   RenderWidgetHostImpl* render_widget_host_;
 
-  // This is true when we are currently painting and thus should handle extra
-  // paint requests by expanding the invalid rect rather than actually painting.
+  // This is true when we are currently painting. In the legacy renderer, this
+  // means we should handle extra paint requests by expanding the invalid rect
+  // rather than actually painting. In hardware compositing it means that we
+  // are inside a draw callback and should not wait for frames to draw before
+  // acknowledging them.
   bool about_to_validate_and_paint_;
 
   // This is true when we have already scheduled a call to
@@ -483,10 +486,24 @@ class RenderWidgetHostViewMac : public RenderWidgetHostViewBase,
   void SendPendingLatencyInfoToHost();
   void TickPendingLatencyInfoDelay();
 
+  void SendPendingSwapAck();
+
  private:
   friend class RenderWidgetHostView;
   friend class RenderWidgetHostViewMacTest;
 
+  struct PendingSwapAck {
+    PendingSwapAck(int32 route_id, int gpu_host_id, int32 renderer_id)
+        : route_id(route_id),
+          gpu_host_id(gpu_host_id),
+          renderer_id(renderer_id) {}
+    int32 route_id;
+    int gpu_host_id;
+    int32 renderer_id;
+  };
+  scoped_ptr<PendingSwapAck> pending_swap_ack_;
+  void AddPendingSwapAck(int32 route_id, int gpu_host_id, int32 renderer_id);
+
   // The view will associate itself with the given widget. The native view must
   // be hooked up immediately to the view hierarchy, or else when it is
   // deleted it will delete this out from under the caller.

content/browser/renderer_host/render_widget_host_view_mac.mm

@@ -437,6 +437,10 @@ RenderWidgetHostViewMac::RenderWidgetHostViewMac(RenderWidgetHost* widget)
 }
 
 RenderWidgetHostViewMac::~RenderWidgetHostViewMac() {
+  // If a caller has set this, then when the caller tries to re-set it sometime
+  // in the future, we will crash.
+  DCHECK(!about_to_validate_and_paint_);
+
   // This is being called from |cocoa_view_|'s destructor, so invalidate the
   // pointer.
   cocoa_view_ = nil;
@@ -574,6 +578,9 @@ bool RenderWidgetHostViewMac::CreateCompositedIOSurfaceLayer() {
 
 void RenderWidgetHostViewMac::DestroyCompositedIOSurfaceAndLayer(
     DestroyContextBehavior destroy_context_behavior) {
+  // Any pending frames will not be displayed, so ack them now.
+  SendPendingSwapAck();
+
   ScopedCAActionDisabler disabler;
 
   compositing_iosurface_.reset();
@@ -748,6 +755,10 @@ void RenderWidgetHostViewMac::WasHidden() {
   if (render_widget_host_->is_hidden())
     return;
 
+  // Any pending frames will not be displayed until this is shown again. Ack
+  // them now.
+  SendPendingSwapAck();
+
   // If we have a renderer, then inform it that we are being hidden so it can
   // reduce its resource utilization.
   render_widget_host_->WasHidden();
@@ -1321,6 +1332,7 @@ void RenderWidgetHostViewMac::CompositorSwapBuffers(
         compositing_iosurface_->CopyToVideoFrame(
             gfx::Rect(size), frame,
             base::Bind(callback, present_time));
+        SendPendingSwapAck();
         return;
       }
     }
@@ -1440,6 +1452,7 @@ bool RenderWidgetHostViewMac::DrawIOSurfaceWithoutCoreAnimation() {
       underlay_view_->compositing_iosurface_ &&
       underlay_view_has_drawn_) {
     [underlay_view_->cocoa_view() setNeedsDisplay:YES];
+    SendPendingSwapAck();
     return true;
   }
 
@@ -1475,6 +1488,7 @@ bool RenderWidgetHostViewMac::DrawIOSurfaceWithoutCoreAnimation() {
   }
 
   SendPendingLatencyInfoToHost();
+  SendPendingSwapAck();
   return true;
 }
 
@@ -1652,19 +1666,14 @@ void RenderWidgetHostViewMac::AcceleratedSurfaceBuffersSwapped(
       "RenderWidgetHostViewMac::AcceleratedSurfaceBuffersSwapped");
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
 
+  AddPendingSwapAck(params.route_id,
+                    gpu_host_id,
+                    compositing_iosurface_ ?
+                        compositing_iosurface_->GetRendererID() : 0);
   CompositorSwapBuffers(params.surface_handle,
                         params.size,
                         params.scale_factor,
                         params.latency_info);
-
-  AcceleratedSurfaceMsg_BufferPresented_Params ack_params;
-  ack_params.sync_point = 0;
-  ack_params.renderer_id = compositing_iosurface_ ?
-      compositing_iosurface_->GetRendererID() : 0;
-  RenderWidgetHostImpl::AcknowledgeBufferPresent(params.route_id,
-                                                 gpu_host_id,
-                                                 ack_params);
-  render_widget_host_->AcknowledgeSwapBuffersToRenderer();
 }
 
 void RenderWidgetHostViewMac::AcceleratedSurfacePostSubBuffer(
@@ -1674,19 +1683,14 @@ void RenderWidgetHostViewMac::AcceleratedSurfacePostSubBuffer(
       "RenderWidgetHostViewMac::AcceleratedSurfacePostSubBuffer");
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
 
+  AddPendingSwapAck(params.route_id,
+                    gpu_host_id,
+                    compositing_iosurface_ ?
+                        compositing_iosurface_->GetRendererID() : 0);
   CompositorSwapBuffers(params.surface_handle,
                         params.surface_size,
                         params.surface_scale_factor,
                         params.latency_info);
-
-  AcceleratedSurfaceMsg_BufferPresented_Params ack_params;
-  ack_params.sync_point = 0;
-  ack_params.renderer_id = compositing_iosurface_ ?
-      compositing_iosurface_->GetRendererID() : 0;
-  RenderWidgetHostImpl::AcknowledgeBufferPresent(params.route_id,
-                                                 gpu_host_id,
-                                                 ack_params);
-  render_widget_host_->AcknowledgeSwapBuffersToRenderer();
 }
 
 void RenderWidgetHostViewMac::AcceleratedSurfaceSuspend() {
@@ -2060,6 +2064,32 @@ void RenderWidgetHostViewMac::TickPendingLatencyInfoDelay() {
   [compositing_iosurface_layer_ setNeedsDisplay];
 }
 
+void RenderWidgetHostViewMac::AddPendingSwapAck(
+    int32 route_id, int gpu_host_id, int32 renderer_id) {
+  DCHECK(!pending_swap_ack_);
+  pending_swap_ack_.reset(new PendingSwapAck(
+      route_id, gpu_host_id, renderer_id));
+  // If we're about to paint, ack this immediatley.
+  if (about_to_validate_and_paint_)
+    SendPendingSwapAck();
+}
+
+void RenderWidgetHostViewMac::SendPendingSwapAck() {
+  if (!pending_swap_ack_)
+    return;
+
+  AcceleratedSurfaceMsg_BufferPresented_Params ack_params;
+  ack_params.sync_point = 0;
+  ack_params.renderer_id = pending_swap_ack_->renderer_id;
+  RenderWidgetHostImpl::AcknowledgeBufferPresent(pending_swap_ack_->route_id,
+                                                 pending_swap_ack_->gpu_host_id,
+                                                 ack_params);
+  if (render_widget_host_)
+    render_widget_host_->AcknowledgeSwapBuffersToRenderer();
+
+  pending_swap_ack_.reset();
+}
+
 }  // namespace content
 
 // RenderWidgetHostViewCocoa ---------------------------------------------------