Defer CGLFlushDrawable until OSX-requested drawRect to avoid spinning when window is obscured.

Sadly, this fix regresses some black flashing bugs:
 -flicker-test2.html occasionally flashes while resizing the window.

I think this regression could be fixed if we always draw with the OpenGL context once the context is created. But that would be a third drawing path and some significant code, so let's consider it in a follow up.

BUG=127709
TEST=open https://cvs.khronos.org/svn/repos/registry/trunk/public/webgl/sdk/demos/google/particles/index.html ; open Activity Monitor and observe the chrome process CPU usage ; resize chrome window so it's smaller than the Activity Monitor window (or increase size of Activity Monitor); move Activity Monitor on top of chrome window to fully obscure it ; verify that the CPU usage of chrome does not spike upwards while the chrome window is obscured.

Review URL: https://chromiumcodereview.appspot.com/10382213

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

5 files changed, 79 insertions, 28 deletions

diff --git a/content/browser/renderer_host/render_widget_host_impl.cc b/content/browser/renderer_host/render_widget_host_impl.cc
index 6c4a78e..2ffb094 100644
--- a/content/browser/renderer_host/render_widget_host_impl.cc
+++ b/content/browser/renderer_host/render_widget_host_impl.cc
@@ -615,6 +615,10 @@ BackingStore* RenderWidgetHostImpl::GetBackingStore(bool force_create) {
   do {
     TRACE_EVENT0("renderer_host", "GetBackingStore::WaitForUpdate");
 
+#if defined(OS_MACOSX)
+    view_->AboutToWaitForBackingStoreMsg();
+#endif
+
     // When we have asked the RenderWidget to resize, and we are still waiting
     // on a response, block for a little while to see if we can't get a response
     // before returning the old (incorrectly sized) backing store.
diff --git a/content/browser/renderer_host/render_widget_host_view_mac.h b/content/browser/renderer_host/render_widget_host_view_mac.h
index 52d6e90..d9ae91f 100644
--- a/content/browser/renderer_host/render_widget_host_view_mac.h
+++ b/content/browser/renderer_host/render_widget_host_view_mac.h
@@ -283,6 +283,7 @@ class RenderWidgetHostViewMac : public content::RenderWidgetHostViewBase {
       int gpu_host_id) OVERRIDE;
   virtual void AcceleratedSurfaceSuspend() OVERRIDE;
   virtual bool HasAcceleratedSurface(const gfx::Size& desired_size) OVERRIDE;
+  virtual void AboutToWaitForBackingStoreMsg() OVERRIDE;
   virtual void GetScreenInfo(WebKit::WebScreenInfo* results) OVERRIDE;
   virtual gfx::Rect GetRootWindowBounds() OVERRIDE;
   virtual gfx::GLSurfaceHandle GetCompositingSurface() OVERRIDE;
@@ -319,8 +320,12 @@ class RenderWidgetHostViewMac : public content::RenderWidgetHostViewBase {
   const std::string& selected_text() const { return selected_text_; }
 
   // Call setNeedsDisplay on the cocoa_view_. The IOSurface will be drawn during
-  // the next drawRect.
-  void CompositorSwapBuffers(uint64 surface_handle);
+  // the next drawRect. Return true if the Ack should be sent, false if it
+  // should be deferred until drawRect.
+  bool CompositorSwapBuffers(uint64 surface_handle);
+  // Ack pending SwapBuffers requests, if any, to unblock the GPU process. Has
+  // no effect if there are no pending requests.
+  void AckPendingSwapBuffers();
 
   // These member variables should be private, but the associated ObjC class
   // needs access to them and can't be made a friend.
@@ -420,6 +425,10 @@ class RenderWidgetHostViewMac : public content::RenderWidgetHostViewBase {
   scoped_nsobject<NSWindow> pepper_fullscreen_window_;
   scoped_nsobject<FullscreenWindowManager> fullscreen_window_manager_;
 
+  // List of pending swaps for deferred acking:
+  //   pairs of (route_id, gpu_host_id).
+  std::list<std::pair<int32, int32> > pending_swap_buffers_acks_;
+
   DISALLOW_COPY_AND_ASSIGN(RenderWidgetHostViewMac);
 };
 
diff --git a/content/browser/renderer_host/render_widget_host_view_mac.mm b/content/browser/renderer_host/render_widget_host_view_mac.mm
index 9ba529f..662bf89 100644
--- a/content/browser/renderer_host/render_widget_host_view_mac.mm
+++ b/content/browser/renderer_host/render_widget_host_view_mac.mm
@@ -276,6 +276,7 @@ RenderWidgetHostViewMac::RenderWidgetHostViewMac(RenderWidgetHost* widget)
 }
 
 RenderWidgetHostViewMac::~RenderWidgetHostViewMac() {
+  AckPendingSwapBuffers();
   UnlockMouse();
 }
 
@@ -369,6 +370,10 @@ void RenderWidgetHostViewMac::WasHidden() {
   if (is_hidden_)
     return;
 
+  // Send ACKs for any pending SwapBuffers (if any) since we won't be displaying
+  // them and the GPU process is waiting.
+  AckPendingSwapBuffers();
+
   // If we receive any more paint messages while we are hidden, we want to
   // ignore them so we don't re-allocate the backing store.  We will paint
   // everything again when we become selected again.
@@ -661,6 +666,8 @@ void RenderWidgetHostViewMac::RenderViewGone(base::TerminationStatus status,
 }
 
 void RenderWidgetHostViewMac::Destroy() {
+  AckPendingSwapBuffers();
+
   // On Windows, popups are implemented with a popup window style, so that when
   // an event comes in that would "cancel" it, it receives the OnCancelMode
   // message and can kill itself. Alas, on the Mac, views cannot capture events
@@ -943,22 +950,47 @@ void RenderWidgetHostViewMac::AcceleratedSurfaceSetTransportDIB(
                                                    transport_dib);
 }
 
-void RenderWidgetHostViewMac::CompositorSwapBuffers(uint64 surface_handle) {
+bool RenderWidgetHostViewMac::CompositorSwapBuffers(uint64 surface_handle) {
   if (is_hidden_)
-    return;
+    return true;
 
   if (!compositing_iosurface_.get())
     compositing_iosurface_.reset(CompositingIOSurfaceMac::Create());
 
   if (!compositing_iosurface_.get())
-    return;
+    return true;
 
   compositing_iosurface_->SetIOSurface(surface_handle);
+
+  GotAcceleratedFrame();
+
   // No need to draw the surface if we are inside a drawRect. It will be done
   // later.
-  if (!about_to_validate_and_paint_)
-    compositing_iosurface_->DrawIOSurface(cocoa_view_);
-  GotAcceleratedFrame();
+  if (!about_to_validate_and_paint_) {
+    // Trigger a drawRect, but don't invalidate the whole window because it
+    // is expensive to clear it with transparency to expose the GL underneath.
+    [cocoa_view_ setNeedsDisplayInRect:NSMakeRect(0, 0, 1, 1)];
+
+    // While resizing, OSX fails to call drawRect on the NSView unless the
+    // window size has changed. That means we won't see animations update if the
+    // user has the mouse button held down, but is not currently changing the
+    // size of the window. To work around that, display here while resizing.
+    if ([cocoa_view_ inLiveResize])
+      [cocoa_view_ displayIfNeeded];
+  }
+  return false;
+}
+
+void RenderWidgetHostViewMac::AckPendingSwapBuffers() {
+  TRACE_EVENT0("browser", "RenderWidgetHostViewMac::AckPendingSwapBuffers");
+  while (!pending_swap_buffers_acks_.empty()) {
+    if (pending_swap_buffers_acks_.front().first != 0) {
+      RenderWidgetHostImpl::AcknowledgeSwapBuffers(
+          pending_swap_buffers_acks_.front().first,
+          pending_swap_buffers_acks_.front().second);
+    }
+    pending_swap_buffers_acks_.erase(pending_swap_buffers_acks_.begin());
+  }
 }
 
 void RenderWidgetHostViewMac::AcceleratedSurfaceBuffersSwapped(
@@ -966,13 +998,17 @@ void RenderWidgetHostViewMac::AcceleratedSurfaceBuffersSwapped(
     int gpu_host_id) {
   TRACE_EVENT0("browser",
       "RenderWidgetHostViewMac::AcceleratedSurfaceBuffersSwapped");
-  CHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
+  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
+
+  pending_swap_buffers_acks_.push_back(std::make_pair(params.route_id,
+                                                      gpu_host_id));
 
   // Compositor window is always gfx::kNullPluginWindow.
   // TODO(jbates) http://crbug.com/105344 This will be removed when there are no
   // plugin windows.
   if (params.window == gfx::kNullPluginWindow) {
-    CompositorSwapBuffers(params.surface_handle);
+    if (CompositorSwapBuffers(params.surface_handle))
+      AckPendingSwapBuffers();
   } else {
     // Deprecated accelerated plugin code path.
     AcceleratedPluginView* view = ViewForPluginWindowHandle(params.window);
@@ -986,11 +1022,7 @@ void RenderWidgetHostViewMac::AcceleratedSurfaceBuffersSwapped(
         [view setHidden:NO];
       [view drawView];
     }
-  }
-
-  if (params.route_id != 0) {
-    RenderWidgetHostImpl::AcknowledgeSwapBuffers(params.route_id,
-                                                 gpu_host_id);
+    AckPendingSwapBuffers();
   }
 }
 
@@ -999,13 +1031,17 @@ void RenderWidgetHostViewMac::AcceleratedSurfacePostSubBuffer(
     int gpu_host_id) {
   TRACE_EVENT0("browser",
       "RenderWidgetHostViewMac::AcceleratedSurfacePostSubBuffer");
-  CHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
+  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
+
+  pending_swap_buffers_acks_.push_back(std::make_pair(params.route_id,
+                                                      gpu_host_id));
 
   // Compositor window is always gfx::kNullPluginWindow.
   // TODO(jbates) http://crbug.com/105344 This will be removed when there are no
   // plugin windows.
   if (params.window == gfx::kNullPluginWindow) {
-    CompositorSwapBuffers(params.surface_handle);
+    if (CompositorSwapBuffers(params.surface_handle))
+      AckPendingSwapBuffers();
   } else {
     // Deprecated accelerated plugin code path.
     AcceleratedPluginView* view = ViewForPluginWindowHandle(params.window);
@@ -1021,11 +1057,7 @@ void RenderWidgetHostViewMac::AcceleratedSurfacePostSubBuffer(
         [view setHidden:NO];
       [view drawView];
     }
-  }
-
-  if (params.route_id != 0) {
-    RenderWidgetHostImpl::AcknowledgePostSubBuffer(
-        params.route_id, gpu_host_id);
+    AckPendingSwapBuffers();
   }
 }
 
@@ -1043,6 +1075,10 @@ bool RenderWidgetHostViewMac::HasAcceleratedSurface(
           compositing_iosurface_->io_surface_size() == desired_size);
 }
 
+void RenderWidgetHostViewMac::AboutToWaitForBackingStoreMsg() {
+  AckPendingSwapBuffers();
+}
+
 void RenderWidgetHostViewMac::OnAcceleratedCompositingStateChange() {
 }
 
@@ -1154,10 +1190,8 @@ void RenderWidgetHostViewMac::GotAcceleratedFrame() {
 
     // Need to wipe the software view with transparency to expose the GL
     // underlay. Invalidate the whole window to do that.
-    if (!about_to_validate_and_paint_) {
+    if (!about_to_validate_and_paint_)
       [cocoa_view_ setNeedsDisplay:YES];
-      [cocoa_view_ displayIfNeeded];
-    }
 
     // Delete software backingstore.
     BackingStoreManager::RemoveBackingStore(render_widget_host_);
@@ -1168,6 +1202,8 @@ void RenderWidgetHostViewMac::GotSoftwareFrame() {
   if (last_frame_was_accelerated_) {
     last_frame_was_accelerated_ = false;
 
+    AckPendingSwapBuffers();
+
     // Forget IOSurface since we are drawing a software frame now.
     if (compositing_iosurface_.get() &&
         compositing_iosurface_->HasIOSurface()) {
@@ -1895,9 +1931,6 @@ void RenderWidgetHostViewMac::SetTextInputActive(bool active) {
 
   if (renderWidgetHostView_->last_frame_was_accelerated_ &&
       renderWidgetHostView_->compositing_iosurface_.get()) {
-    // Note that this code path is only executed when there's window damage
-    // (when the window is foregrounded, for example). Normally, GPU frames
-    // arrive and are drawn during AcceleratedSurfaceBuffersSwapped.
     {
       TRACE_EVENT0("browser", "NSRectFill");
       // Draw transparency to expose the GL underlay. NSRectFill is extremely
@@ -1909,6 +1942,7 @@ void RenderWidgetHostViewMac::SetTextInputActive(bool active) {
     }
 
     renderWidgetHostView_->compositing_iosurface_->DrawIOSurface(self);
+    renderWidgetHostView_->AckPendingSwapBuffers();
     return;
   }
 
diff --git a/content/browser/renderer_host/test_render_view_host.cc b/content/browser/renderer_host/test_render_view_host.cc
index fa00b6a..3bb458c 100644
--- a/content/browser/renderer_host/test_render_view_host.cc
+++ b/content/browser/renderer_host/test_render_view_host.cc
@@ -125,6 +125,9 @@ bool TestRenderWidgetHostView::HasAcceleratedSurface(
 
 #if defined(OS_MACOSX)
 
+void TestRenderWidgetHostView::AboutToWaitForBackingStoreMsg() {
+}
+
 gfx::Rect TestRenderWidgetHostView::GetViewCocoaBounds() const {
   return gfx::Rect();
 }
diff --git a/content/browser/renderer_host/test_render_view_host.h b/content/browser/renderer_host/test_render_view_host.h
index 8a016fe..28a25a8 100644
--- a/content/browser/renderer_host/test_render_view_host.h
+++ b/content/browser/renderer_host/test_render_view_host.h
@@ -111,6 +111,7 @@ class TestRenderWidgetHostView : public RenderWidgetHostViewBase {
   virtual void AcceleratedSurfaceSuspend() OVERRIDE;
   virtual bool HasAcceleratedSurface(const gfx::Size& desired_size) OVERRIDE;
 #if defined(OS_MACOSX)
+  virtual void AboutToWaitForBackingStoreMsg() OVERRIDE;
   virtual gfx::Rect GetViewCocoaBounds() const OVERRIDE;
   virtual void PluginFocusChanged(bool focused, int plugin_id) OVERRIDE;
   virtual void StartPluginIme() OVERRIDE;