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

content/browser/renderer_host/render_widget_host_view_mac.mm

 // Copyright (c) 2012 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/browser/renderer_host/render_widget_host_view_mac.h"
 
 #import <objc/runtime.h>
 #include <QuartzCore/QuartzCore.h>
 
 #include "base/basictypes.h"
@@ skipping 419 matching lines @@
                   initWithRenderWidgetHostViewMac:this] autorelease];
 
   if (GetCoreAnimationStatus() == CORE_ANIMATION_ENABLED) {
     EnableCoreAnimation();
   }
 
   render_widget_host_->SetView(this);
 }
 
 RenderWidgetHostViewMac::~RenderWidgetHostViewMac() {
+  // If a caller has set this, then when the caller tries to re-set it sometime
+  // in the future, we will crash.
+  CHECK(!about_to_validate_and_paint_);

[piman, 2014/02/18 21:47:04]

nit: DCHECK.

[ccameron, 2014/02/18 22:33:10]

Done.

+
   // This is being called from |cocoa_view_|'s destructor, so invalidate the
   // pointer.
   cocoa_view_ = nil;
 
   UnlockMouse();
 
   // Make sure that the layer doesn't reach into the now-invalid object.
   DestroyCompositedIOSurfaceAndLayer(kDestroyContext);
   software_layer_.reset();
 
@@ skipping 117 matching lines @@
   // Creating the CompositingIOSurfaceLayer may attempt to draw in setLayer,
   // which, if it fails, will promptly tear down everything that was just
   // created. If that happened, return failure.
   return compositing_iosurface_context_ &&
          compositing_iosurface_ &&
          (compositing_iosurface_layer_ || !use_core_animation_);
 }
 
 void RenderWidgetHostViewMac::DestroyCompositedIOSurfaceAndLayer(
     DestroyContextBehavior destroy_context_behavior) {
+  // Any pending frames will not be displayed, so ack them now.
+  SendPendingSwapAck();
+
   ScopedCAActionDisabler disabler;
 
   compositing_iosurface_.reset();
   if (compositing_iosurface_layer_) {
     [software_layer_ setNeedsDisplay];
     [compositing_iosurface_layer_ removeFromSuperlayer];
     [compositing_iosurface_layer_ disableCompositing];
     compositing_iosurface_layer_.reset();
   }
   switch (destroy_context_behavior) {
@@ skipping 154 matching lines @@
   if (!use_core_animation_)
     [[cocoa_view_ window] disableScreenUpdatesUntilFlush];
 
   [compositing_iosurface_layer_ setNeedsDisplay];
 }
 
 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();
   software_frame_manager_->SetVisibility(false);
 
   // There can be a transparent flash as this view is removed and the next is
   // added, because of OSX windowing races between displaying the contents of
   // the NSView and its corresponding OpenGL context.
   // disableScreenUpdatesUntilFlush prevents the transparent flash by avoiding
   // screen updates until the next tab draws.
@@ skipping 553 matching lines @@
       if (frame_subscriber_->ShouldCaptureFrame(present_time,
                                                 &frame, &callback)) {
         gfx::ScopedCGLSetCurrentContext scoped_set_current_context(
             compositing_iosurface_context_->cgl_context());
         compositing_iosurface_->SetIOSurfaceWithContextCurrent(
             compositing_iosurface_context_, surface_handle, size,
             surface_scale_factor);
         compositing_iosurface_->CopyToVideoFrame(
             gfx::Rect(size), frame,
             base::Bind(callback, present_time));
+        SendPendingSwapAck();
         return;
       }
     }
 
     // TODO(shess) If the view does not have a window, or the window
     // does not have backing, the IOSurface will log "invalid drawable"
     // in -setView:.  It is not clear how this code is reached with such
     // a case, so record some info into breakpad (some subset of
     // browsers are likely to crash later for unrelated reasons).
     // http://crbug.com/148882
@@ skipping 134 matching lines @@
                             overlay_view_rect.height() -
                             overlay_view_offset_.y());
     if (!overlay_view_->compositing_iosurface_->DrawIOSurface(
             compositing_iosurface_context_, overlay_view_rect,
             overlay_view_->scale_factor(), true)) {
       return false;
     }
   }
 
   SendPendingLatencyInfoToHost();
+  SendPendingSwapAck();
   return true;
 }
 
 void RenderWidgetHostViewMac::GotAcceleratedCompositingError() {
   DestroyCompositedIOSurfaceAndLayer(kDestroyContext);
   // The existing GL contexts may be in a bad state, so don't re-use any of the
   // existing ones anymore, rather, allocate new ones.
   CompositingIOSurfaceContext::MarkExistingContextsAsNotShareable();
   // Request that a new frame be generated.
   if (render_widget_host_)
@@ skipping 157 matching lines @@
   return true;
 }
 
 void RenderWidgetHostViewMac::AcceleratedSurfaceBuffersSwapped(
     const GpuHostMsg_AcceleratedSurfaceBuffersSwapped_Params& params,
     int gpu_host_id) {
   TRACE_EVENT0("browser",
       "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(
     const GpuHostMsg_AcceleratedSurfacePostSubBuffer_Params& params,
     int gpu_host_id) {
   TRACE_EVENT0("browser",
       "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() {
   if (compositing_iosurface_)
     compositing_iosurface_->UnrefIOSurface();
 }
 
 void RenderWidgetHostViewMac::AcceleratedSurfaceRelease() {
   DestroyCompositedIOSurfaceAndLayer(kDestroyContext);
 }
@@ skipping 353 matching lines @@
 
 void RenderWidgetHostViewMac::TickPendingLatencyInfoDelay() {
   // Keep calling setNeedsDisplay in a loop until enough display calls come in.
   base::MessageLoop::current()->PostTask(FROM_HERE,
       base::Bind(&RenderWidgetHostViewMac::TickPendingLatencyInfoDelay,
                  pending_latency_info_delay_weak_ptr_factory_.GetWeakPtr()));
   [software_layer_ setNeedsDisplay];
   [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 ---------------------------------------------------
 
 @implementation RenderWidgetHostViewCocoa
 @synthesize selectedRange = selectedRange_;
 @synthesize suppressNextEscapeKeyUp = suppressNextEscapeKeyUp_;
 @synthesize markedRange = markedRange_;
 
 - (id)initWithRenderWidgetHostViewMac:(RenderWidgetHostViewMac*)r {
@@ skipping 2028 matching lines @@
   return YES;
 }
 
 - (BOOL)isOpaque {
   if (renderWidgetHostView_->use_core_animation_)
     return YES;
   return [super isOpaque];
 }
 
 @end