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

chrome/browser/renderer_host/render_widget_host_view_mac.mm

Index: chrome/browser/renderer_host/render_widget_host_view_mac.mm
===================================================================
--- chrome/browser/renderer_host/render_widget_host_view_mac.mm	(revision 67289)
+++ chrome/browser/renderer_host/render_widget_host_view_mac.mm	(working copy)
@@ -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,9 +172,22 @@
   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.
   NSSize cachedSize_;
@@ -186,6 +201,15 @@
                          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)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 @@
   // 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 @@
     renderWidgetHostView_ = r;
     pluginHandle_ = pluginHandle;
     cachedSize_ = NSZeroSize;
+    swapBuffersCount_ = 0;
+    acknowledgedSwapBuffersCount_ = 0;
+    rendererId_ = 0;
+    routeId_ = 0;
 
     [self setAutoresizingMask:NSViewMaxXMargin|NSViewMinYMargin];
 
@@ -290,9 +325,25 @@
   }
 
   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 @@
   NSSize size = [self frame].size;
   glViewport(0, 0, size.width, size.height);
 
+  CGLSetCurrentContext(0);

[Nico, 2010/11/24 23:43:29]

Why did you add this line?

[Ken Russell (switch to Gerrit), 2010/11/25 01:02:08]

OpenGL's rules are that a context can only be current on one thread at a time.
Generally, if a context is current on one thread, then attempts to make it
current on another thread result in an error. Because the context was being left
current on the display link thread, it seemed possible that the main thread
would not be able to correctly make it current for some of its operations. At
one point during the development of this CL I thought that that might be the
reason for the drastic slowdowns on Mac OS X. It turned out not to be, but I
think this is still needed for correctness.

   CGLUnlockContext(cglContext_);
   globalFrameDidChangeCGLLockCount_--;
 
@@ -938,7 +990,11 @@
 }
 
 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 @@
   // 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 @@
   }
 }
 
+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:

[apatrick_chromium, 2010/11/24 23:56:09]

indentation

[Ken Russell (switch to Gerrit), 2010/11/25 01:02:08]

Done.

+  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()) {