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

content/browser/renderer_host/compositing_iosurface_layer_mac.mm

Index: content/browser/renderer_host/compositing_iosurface_layer_mac.mm
diff --git a/content/browser/renderer_host/compositing_iosurface_layer_mac.mm b/content/browser/renderer_host/compositing_iosurface_layer_mac.mm
index 63f9ca36748f7cbd7712eb8667026160e59ace79..856ff62d81db10ada181eabb2a6b70f99fbf4e1b 100644
--- a/content/browser/renderer_host/compositing_iosurface_layer_mac.mm
+++ b/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 pump the run loop waiting for a frame.

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

nit: I don't think we actually pump the loop - we just wait synchronously on the
renderer message (dispatching just a select few incoming ones).

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

Changed to waitForResizedFrameInContext.

+- (void)pumpRunLoopForResizedFrameInContext:(CGLContextObj)glContext;
+
+@end
+
 @implementation CompositingIOSurfaceLayer
 
 @synthesize context = context_;
@@ -83,6 +91,41 @@
     [self setNeedsDisplay];
 }
 
+- (void)pumpRunLoopForResizedFrameInContext:(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 pumpRunLoopForResizedFrameInContext:glContext];
 
   // If a transition to software mode has occurred, this layer should be
   // removed from the heirarchy now, so don't draw anything.