Add flow control between renderer and GPU processes, and, on Mac OS X,

between GPU process and browser process. Thanks to Al Patrick for the
renderer<->GPU flow control mechanism.

This CL prevents the renderer from issuing more OpenGL work than the
GPU process can execute, and, on the Mac, prevents the combination of
the renderer and GPU processes from transmitting more frames via
IOSurfaces from the GPU to the browser process than can be handled by
the GPU.

This fix causes the renderer to block inside ggl::SwapBuffers() when
it gets too far ahead. Different strategies can be considered going
forward, including measuring frame rates in the GPU and renderer
processes and trying to match them via techniques such as delaying the
execution of some timers. However, despite the general undesirability
of blocking the render thread, this fix results in a significant
performance improvement.

With this fix integrated, a fill-limited test case from Chris Rogers
displays at 60 FPS instead of 15 FPS on a Mac Pro. Gregg Tavares'
aquarium from webglsamples.googlecode.com displays at 30 FPS instead
of 4 or 5 FPS on a MacBook Pro. The frame rates measured at the
JavaScript level now match the actual frame rate of the browser, where
previously they were much higher since they were issuing more work
than the browser could render.

A few other minor OpenGL bugs potentially impacting the correctness of
the Mac compositor are being fixed as well in this CL.

Verified that WebGL, CSS 3D and YouTube (Core Animation plugin)
content all work.

BUG=63539
TEST=none

Review URL: http://codereview.chromium.org/5317007

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

9 files changed, 175 insertions, 30 deletions

diff --git a/chrome/browser/gpu_process_host.cc b/chrome/browser/gpu_process_host.cc
index bf9c1c9..afa904c 100644
--- a/chrome/browser/gpu_process_host.cc
+++ b/chrome/browser/gpu_process_host.cc
@@ -308,11 +308,15 @@ class BuffersSwappedDispatcher : public Task {
       int32 renderer_id,
       int32 render_view_id,
       gfx::PluginWindowHandle window,
-      uint64 surface_id)
+      uint64 surface_id,
+      int32 route_id,
+      uint64 swap_buffers_count)
       : renderer_id_(renderer_id),
         render_view_id_(render_view_id),
         window_(window),
-        surface_id_(surface_id) {
+        surface_id_(surface_id),
+        route_id_(route_id),
+        swap_buffers_count_(swap_buffers_count) {
   }
 
   void Run() {
@@ -323,7 +327,14 @@ class BuffersSwappedDispatcher : public Task {
     RenderWidgetHostView* view = host->view();
     if (!view)
       return;
-    view->AcceleratedSurfaceBuffersSwapped(window_, surface_id_);
+    view->AcceleratedSurfaceBuffersSwapped(
+        // Parameters needed to swap the IOSurface.
+        window_,
+        surface_id_,
+        // Parameters needed to formulate an acknowledgment.
+        renderer_id_,
+        route_id_,
+        swap_buffers_count_);
   }
 
  private:
@@ -331,6 +342,8 @@ class BuffersSwappedDispatcher : public Task {
   int32 render_view_id_;
   gfx::PluginWindowHandle window_;
   uint64 surface_id_;
+  int32 route_id_;
+  uint64 swap_buffers_count_;
 
   DISALLOW_COPY_AND_ASSIGN(BuffersSwappedDispatcher);
 };
@@ -338,14 +351,20 @@ class BuffersSwappedDispatcher : public Task {
 }  // namespace
 
 void GpuProcessHost::OnAcceleratedSurfaceBuffersSwapped(
-    int32 renderer_id,
-    int32 render_view_id,
-    gfx::PluginWindowHandle window,
-    uint64 surface_id) {
+    const GpuHostMsg_AcceleratedSurfaceBuffersSwapped_Params& params) {
   BrowserThread::PostTask(
       BrowserThread::UI, FROM_HERE,
       new BuffersSwappedDispatcher(
-          renderer_id, render_view_id, window, surface_id));
+          // These are the parameters needed to look up the IOSurface
+          // on this side.
+          params.renderer_id,
+          params.render_view_id,
+          params.window,
+          params.surface_id,
+          // These are additional parameters needed to formulate an
+          // acknowledgment.
+          params.route_id,
+          params.swap_buffers_count));
 }
 #endif
 
diff --git a/chrome/browser/gpu_process_host.h b/chrome/browser/gpu_process_host.h
index 69969c6..4b40a25 100644
--- a/chrome/browser/gpu_process_host.h
+++ b/chrome/browser/gpu_process_host.h
@@ -15,6 +15,7 @@
 #include "gfx/native_widget_types.h"
 
 struct GpuHostMsg_AcceleratedSurfaceSetIOSurface_Params;
+struct GpuHostMsg_AcceleratedSurfaceBuffersSwapped_Params;
 class GPUInfo;
 class ResourceMessageFilter;
 
@@ -90,10 +91,8 @@ class GpuProcessHost : public BrowserChildProcessHost, public NonThreadSafe {
 #elif defined(OS_MACOSX)
   void OnAcceleratedSurfaceSetIOSurface(
       const GpuHostMsg_AcceleratedSurfaceSetIOSurface_Params& params);
-  void OnAcceleratedSurfaceBuffersSwapped(int32 renderer_id,
-                                          int32 render_view_id,
-                                          gfx::PluginWindowHandle window,
-                                          uint64 surface_id);
+  void OnAcceleratedSurfaceBuffersSwapped(
+      const GpuHostMsg_AcceleratedSurfaceBuffersSwapped_Params& params);
 #endif
 
   // Sends the response for establish channel request to the renderer.
diff --git a/chrome/browser/renderer_host/accelerated_surface_container_manager_mac.cc b/chrome/browser/renderer_host/accelerated_surface_container_manager_mac.cc
index 6b76b03..1ad09ba 100644
--- a/chrome/browser/renderer_host/accelerated_surface_container_manager_mac.cc
+++ b/chrome/browser/renderer_host/accelerated_surface_container_manager_mac.cc
@@ -100,7 +100,7 @@ void AcceleratedSurfaceContainerManagerMac::Draw(CGLContextObj context,
 
   glColorMask(true, true, true, true);
   // Should match the clear color of RenderWidgetHostViewMac.
-  glClearColor(255, 255, 255, 255);
+  glClearColor(1.0f, 1.0f, 1.0f, 1.0f);
   // TODO(thakis): Clearing the whole color buffer is wasteful, since most of
   // it is overwritten by the surface.
   glClear(GL_COLOR_BUFFER_BIT | GL_DEPTH_BUFFER_BIT);
diff --git a/chrome/browser/renderer_host/render_widget_host.cc b/chrome/browser/renderer_host/render_widget_host.cc
index 2617cbd..4482bbd 100644
--- a/chrome/browser/renderer_host/render_widget_host.cc
+++ b/chrome/browser/renderer_host/render_widget_host.cc
@@ -1062,7 +1062,11 @@ void RenderWidgetHost::OnAcceleratedSurfaceSetTransportDIB(
 void RenderWidgetHost::OnAcceleratedSurfaceBuffersSwapped(
     gfx::PluginWindowHandle window, uint64 surface_id) {
   if (view_) {
-    view_->AcceleratedSurfaceBuffersSwapped(window, surface_id);
+    // This code path could be updated to implement flow control for
+    // updating of accelerated plugins as well. However, if we add support
+    // for composited plugins then this is not necessary.
+    view_->AcceleratedSurfaceBuffersSwapped(window, surface_id,
+                                            0, 0, 0);
   }
 }
 #elif defined(OS_POSIX)
diff --git a/chrome/browser/renderer_host/render_widget_host_view.h b/chrome/browser/renderer_host/render_widget_host_view.h
index 2c99926..4ad2c41 100644
--- a/chrome/browser/renderer_host/render_widget_host_view.h
+++ b/chrome/browser/renderer_host/render_widget_host_view.h
@@ -225,8 +225,18 @@ class RenderWidgetHostView {
       int32 width,
       int32 height,
       TransportDIB::Handle transport_dib) = 0;
+  // |window| and |surface_id| indicate which accelerated surface's
+  // buffers swapped. |renderer_id|, |route_id| and
+  // |swap_buffers_count| are used to formulate a reply to the GPU
+  // process to prevent it from getting too far ahead. They may all be
+  // zero, in which case no flow control is enforced; this case is
+  // currently used for accelerated plugins.
   virtual void AcceleratedSurfaceBuffersSwapped(
-      gfx::PluginWindowHandle window, uint64 surface_id) = 0;
+      gfx::PluginWindowHandle window,
+      uint64 surface_id,
+      int renderer_id,
+      int32 route_id,
+      uint64 swap_buffers_count) = 0;
   virtual void GpuRenderingStateDidChange() = 0;
 #endif
 
diff --git a/chrome/browser/renderer_host/render_widget_host_view_mac.h b/chrome/browser/renderer_host/render_widget_host_view_mac.h
index 154c0c5..e539f14 100644
--- a/chrome/browser/renderer_host/render_widget_host_view_mac.h
+++ b/chrome/browser/renderer_host/render_widget_host_view_mac.h
@@ -243,8 +243,12 @@ class RenderWidgetHostViewMac : public RenderWidgetHostView {
       int32 width,
       int32 height,
       TransportDIB::Handle transport_dib);
-  virtual void AcceleratedSurfaceBuffersSwapped(gfx::PluginWindowHandle window,
-                                                uint64 surface_id);
+  virtual void AcceleratedSurfaceBuffersSwapped(
+      gfx::PluginWindowHandle window,
+      uint64 surface_id,
+      int32 renderer_id,
+      int32 route_id,
+      uint64 swap_buffers_count);
   virtual void GpuRenderingStateDidChange();
   void DrawAcceleratedSurfaceInstance(
       CGLContextObj context,
@@ -273,6 +277,12 @@ class RenderWidgetHostViewMac : public RenderWidgetHostView {
   // should be called after the software backing store has been painted to.
   void HandleDelayedGpuViewHiding();
 
+  // This is called from the display link thread, and provides the GPU
+  // process a notion of how quickly the browser is able to keep up with it.
+  void AcknowledgeSwapBuffers(int renderer_id,
+                              int32 route_id,
+                              uint64 swap_buffers_count);
+
   // These member variables should be private, but the associated ObjC class
   // needs access to them and can't be made a friend.
 
diff --git a/chrome/browser/renderer_host/render_widget_host_view_mac.mm b/chrome/browser/renderer_host/render_widget_host_view_mac.mm
index bb4b57b..307eb2e 100644
--- a/chrome/browser/renderer_host/render_widget_host_view_mac.mm
+++ b/chrome/browser/renderer_host/render_widget_host_view_mac.mm
@@ -24,6 +24,7 @@
 #include "chrome/browser/browser_trial.h"
 #import "chrome/browser/cocoa/rwhvm_editcommand_helper.h"
 #import "chrome/browser/cocoa/view_id_util.h"
+#include "chrome/browser/gpu_process_host.h"
 #include "chrome/browser/plugin_process_host.h"
 #include "chrome/browser/renderer_host/backing_store_mac.h"
 #include "chrome/browser/renderer_host/render_process_host.h"
@@ -33,6 +34,7 @@
 #include "chrome/common/chrome_switches.h"
 #include "chrome/common/native_web_keyboard_event.h"
 #include "chrome/common/edit_command.h"
+#include "chrome/common/gpu_messages.h"
 #include "chrome/common/plugin_messages.h"
 #include "chrome/common/render_messages.h"
 #include "skia/ext/platform_canvas.h"
@@ -170,8 +172,21 @@ void DisablePasswordInput() {
   RenderWidgetHostViewMac* renderWidgetHostView_;  // weak
   gfx::PluginWindowHandle pluginHandle_;  // weak
 
-  // True if the backing IO surface was updated since we last painted.
-  BOOL surfaceWasSwapped_;
+  // The number of swap buffers calls that have been requested by the
+  // GPU process, or a monotonically increasing number of calls to
+  // updateSwapBuffersCount:fromRenderer:routeId: if the update came
+  // from an accelerated plugin.
+  uint64 swapBuffersCount_;
+  // The number of swap buffers calls that have been processed by the
+  // display link thread. This is only used with the GPU process
+  // update path.
+  volatile uint64 acknowledgedSwapBuffersCount_;
+
+  // Auxiliary information needed to formulate an acknowledgment to
+  // the GPU process. These are constant after the first message.
+  // These are both zero for updates coming from a plugin process.
+  volatile int rendererId_;
+  volatile int32 routeId_;
 
   // Cocoa methods can only be called on the main thread, so have a copy of the
   // view's size, since it's required on the displaylink thread.
@@ -186,6 +201,15 @@ void DisablePasswordInput() {
                          pluginHandle:(gfx::PluginWindowHandle)pluginHandle;
 - (void)drawView;
 
+// Updates the number of swap buffers calls that have been requested.
+// This is currently called with non-zero values only in response to
+// updates from the GPU process. For accelerated plugins, all zeros
+// are passed, and the view takes this as a hint that no flow control
+// or acknowledgment of the swap buffers are desired.
+- (void)updateSwapBuffersCount:(uint64)count
+                  fromRenderer:(int)rendererId
+                       routeId:(int32)routeId;
+
 // NSViews autorelease subviews when they die. The RWHVMac gets destroyed when
 // RHWVCocoa gets dealloc'd, which means the AcceleratedPluginView child views
 // can be around a little longer than the RWHVMac. This is called when the
@@ -193,14 +217,10 @@ void DisablePasswordInput() {
 - (void)onRenderWidgetHostViewGone;
 
 // This _must_ be atomic, since it's accessed from several threads.
-@property BOOL surfaceWasSwapped;
-
-// This _must_ be atomic, since it's accessed from several threads.
 @property NSSize cachedSize;
 @end
 
 @implementation AcceleratedPluginView
-@synthesize surfaceWasSwapped = surfaceWasSwapped_;
 @synthesize cachedSize = cachedSize_;
 
 - (CVReturn)getFrameForTime:(const CVTimeStamp*)outputTime {
@@ -208,11 +228,22 @@ void DisablePasswordInput() {
   // called from a background thread.
   base::mac::ScopedNSAutoreleasePool pool;
 
-  if (![self surfaceWasSwapped])
+  bool sendAck = (rendererId_ != 0 || routeId_ != 0);
+  uint64 currentSwapBuffersCount = swapBuffersCount_;
+  if (currentSwapBuffersCount == acknowledgedSwapBuffersCount_) {
     return kCVReturnSuccess;
+  }
 
   [self drawView];
-  [self setSurfaceWasSwapped:NO];
+
+  acknowledgedSwapBuffersCount_ = currentSwapBuffersCount;
+  if (sendAck && renderWidgetHostView_) {
+    renderWidgetHostView_->AcknowledgeSwapBuffers(
+        rendererId_,
+        routeId_,
+        acknowledgedSwapBuffersCount_);
+  }
+
   return kCVReturnSuccess;
 }
 
@@ -235,6 +266,10 @@ static CVReturn DrawOneAcceleratedPluginCallback(
     renderWidgetHostView_ = r;
     pluginHandle_ = pluginHandle;
     cachedSize_ = NSZeroSize;
+    swapBuffersCount_ = 0;
+    acknowledgedSwapBuffersCount_ = 0;
+    rendererId_ = 0;
+    routeId_ = 0;
 
     [self setAutoresizingMask:NSViewMaxXMargin|NSViewMinYMargin];
 
@@ -290,9 +325,25 @@ static CVReturn DrawOneAcceleratedPluginCallback(
   }
 
   CGLFlushDrawable(cglContext_);
+  CGLSetCurrentContext(0);
   CGLUnlockContext(cglContext_);
 }
 
+- (void)updateSwapBuffersCount:(uint64)count
+                  fromRenderer:(int)rendererId
+                       routeId:(int32)routeId {
+  if (rendererId == 0 && routeId == 0) {
+    // This notification is coming from a plugin process, for which we
+    // don't have flow control implemented right now. Fake up a swap
+    // buffers count so that we can at least skip useless renders.
+    ++swapBuffersCount_;
+  } else {
+    rendererId_ = rendererId;
+    routeId_ = routeId;
+    swapBuffersCount_ = count;
+  }
+}
+
 - (void)onRenderWidgetHostViewGone {
   if (!renderWidgetHostView_)
     return;
@@ -340,6 +391,7 @@ static CVReturn DrawOneAcceleratedPluginCallback(
   NSSize size = [self frame].size;
   glViewport(0, 0, size.width, size.height);
 
+  CGLSetCurrentContext(0);
   CGLUnlockContext(cglContext_);
   globalFrameDidChangeCGLLockCount_--;
 
@@ -938,7 +990,11 @@ void RenderWidgetHostViewMac::AcceleratedSurfaceSetTransportDIB(
 }
 
 void RenderWidgetHostViewMac::AcceleratedSurfaceBuffersSwapped(
-    gfx::PluginWindowHandle window, uint64 surface_id) {
+    gfx::PluginWindowHandle window,
+    uint64 surface_id,
+    int renderer_id,
+    int32 route_id,
+    uint64 swap_buffers_count) {
   CHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
   PluginViewMap::iterator it = plugin_views_.find(window);
   DCHECK(plugin_views_.end() != it);
@@ -953,7 +1009,9 @@ void RenderWidgetHostViewMac::AcceleratedSurfaceBuffersSwapped(
   // The surface is hidden until its first paint, to not show gargabe.
   if (plugin_container_manager_.SurfaceShouldBeVisible(window))
     [view setHidden:NO];
-  [view setSurfaceWasSwapped:YES];
+  [view updateSwapBuffersCount:swap_buffers_count
+                  fromRenderer:renderer_id
+                       routeId:route_id];
 }
 
 void RenderWidgetHostViewMac::UpdateRootGpuViewVisibility(
@@ -988,6 +1046,43 @@ void RenderWidgetHostViewMac::HandleDelayedGpuViewHiding() {
   }
 }
 
+namespace {
+class BuffersSwappedAcknowledger : public Task {
+ public:
+  BuffersSwappedAcknowledger(
+      int renderer_id,
+      int32 route_id,
+      uint64 swap_buffers_count)
+      : renderer_id_(renderer_id),
+        route_id_(route_id),
+        swap_buffers_count_(swap_buffers_count) {
+  }
+
+  void Run() {
+    GpuProcessHost::Get()->Send(
+        new GpuMsg_AcceleratedSurfaceBuffersSwappedACK(
+            renderer_id_, route_id_, swap_buffers_count_));
+  }
+
+ private:
+  int renderer_id_;
+  int32 route_id_;
+  uint64 swap_buffers_count_;
+
+  DISALLOW_COPY_AND_ASSIGN(BuffersSwappedAcknowledger);
+};
+}  // anonymous namespace
+
+void RenderWidgetHostViewMac::AcknowledgeSwapBuffers(
+    int renderer_id,
+    int32 route_id,
+    uint64 swap_buffers_count) {
+  BrowserThread::PostTask(
+      BrowserThread::IO, FROM_HERE,
+      new BuffersSwappedAcknowledger(
+          renderer_id, route_id, swap_buffers_count));
+}
+
 void RenderWidgetHostViewMac::GpuRenderingStateDidChange() {
   CHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
   if (GetRenderWidgetHost()->is_gpu_rendering_active()) {
diff --git a/chrome/browser/renderer_host/test/test_render_view_host.cc b/chrome/browser/renderer_host/test/test_render_view_host.cc
index f3988e1..65d82e9 100644
--- a/chrome/browser/renderer_host/test/test_render_view_host.cc
+++ b/chrome/browser/renderer_host/test/test_render_view_host.cc
@@ -172,7 +172,11 @@ void TestRenderWidgetHostView::AcceleratedSurfaceSetTransportDIB(
 }
 
 void TestRenderWidgetHostView::AcceleratedSurfaceBuffersSwapped(
-    gfx::PluginWindowHandle window, uint64 surface_id) {
+    gfx::PluginWindowHandle window,
+    uint64 surface_id,
+    int renderer_id,
+    int32 route_id,
+    uint64 swap_buffers_count) {
 }
 
 void TestRenderWidgetHostView::GpuRenderingStateDidChange() {
diff --git a/chrome/browser/renderer_host/test/test_render_view_host.h b/chrome/browser/renderer_host/test/test_render_view_host.h
index 4f262f4..fe6daa7 100644
--- a/chrome/browser/renderer_host/test/test_render_view_host.h
+++ b/chrome/browser/renderer_host/test/test_render_view_host.h
@@ -117,8 +117,12 @@ class TestRenderWidgetHostView : public RenderWidgetHostView {
       int32 width,
       int32 height,
       TransportDIB::Handle transport_dib);
-  virtual void AcceleratedSurfaceBuffersSwapped(gfx::PluginWindowHandle window,
-                                                uint64 surface_id);
+  virtual void AcceleratedSurfaceBuffersSwapped(
+      gfx::PluginWindowHandle window,
+      uint64 surface_id,
+      int renderer_id,
+      int32 route_id,
+      uint64 swap_buffers_count);
   virtual void GpuRenderingStateDidChange();
 #endif
   virtual void SetVisuallyDeemphasized(bool deemphasized) { }