Mac: Don't hang gpu process if a tab that shares its renderer process with other tabs and that has accelerated content and outstanding paints.

The problem was that the renderer process busy-waits on the GPU process to flush all gpu commands on close, and the GPU process waits with processing commands from the renderer until a paint ack arrives from the browser. But since the window is already closed, no paint acks are sent any more.

The fix is to let the browser tell the GPU process when a window is closed, and then let the GPU process not wait for paint acks if the corresponding window has already been closed.

Closed windows are identified by (renderer process id, render view routing id). Identifying closed windows by either surface id or gpu channel stub routing id does not work, because they are both created on the GPU side and sent to the browser asynchronously, so it's possible that a browser tab is closed before the ID arrives from the GPU process – in that case, it can't send the "window closed" message even though the GPU process is already in a state where it needs this event.

BUG=67170
TEST=
1.) Go to http://www.chromeexperiments.com/detail/body-browser/?f=webgl , click "Launch Experiment", wait until everything is loaded, close popup. %cpu of gpu process and renderer process should go to 0
2.) Go to http://www.chromeexperiments.com/detail/body-browser/?f=webgl , click "Launch Experiment", wait until the bar on the left is loaded but the body isn't yet, close popup. %cpu of gpu process and renderer process should go to 0, but it might take a few seconds until the %cpu in the renderer go down (the site decides to parse the XHR data that gets flushed on widget close)
3.) Go to http://www.chromeexperiments.com/detail/nine-point-five/?f=webgl , click "Launch Experiment", wait until everything is loaded, close popup. %cpu of gpu process and renderer process should go to 0
4.) Go to http://www.chromeexperiments.com/detail/nine-point-five/?f=webgl , click "Launch Experiment", close popup immediately after the background color changed to light grey. %cpu of gpu process and renderer process should go to 0
5.) Go to http://www.chromeexperiments.com/detail/nine-point-five/?f=webgl , click "Launch Experiment", close popup immediately. %cpu of gpu process and renderer process should go to 0

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

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

10 files changed, 113 insertions, 17 deletions

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 6317917..14b4d82 100644
--- a/chrome/browser/renderer_host/render_widget_host_view_mac.mm
+++ b/chrome/browser/renderer_host/render_widget_host_view_mac.mm
@@ -948,9 +948,50 @@ void RenderWidgetHostViewMac::DestroyFakePluginWindowHandle(
   // taken if a plugin is removed, but the RWHVMac itself stays alive.
 }
 
+namespace {
+class DidDestroyAcceleratedSurfaceSender : public Task {
+ public:
+  DidDestroyAcceleratedSurfaceSender(
+      int renderer_id,
+      int32 renderer_route_id)
+      : renderer_id_(renderer_id),
+        renderer_route_id_(renderer_route_id) {
+  }
+
+  void Run() {
+    GpuProcessHost::Get()->Send(
+        new GpuMsg_DidDestroyAcceleratedSurface(
+            renderer_id_, renderer_route_id_));
+  }
+
+ private:
+  int renderer_id_;
+  int32 renderer_route_id_;
+
+  DISALLOW_COPY_AND_ASSIGN(DidDestroyAcceleratedSurfaceSender);
+};
+}  // anonymous namespace
+
 // This is called by AcceleratedPluginView's -dealloc.
 void RenderWidgetHostViewMac::DeallocFakePluginWindowHandle(
     gfx::PluginWindowHandle window) {
+  // When a browser window with a GPUProcessor is closed, the render process
+  // will attempt to finish all GL commands. It will busy-wait on the GPU
+  // process until the command queue is empty. If a paint is pending, the GPU
+  // process won't process any GL commands until the browser sends a paint ack,
+  // but since the browser window is already closed, it will never arrive.
+  // To break this infinite loop, the browser tells the GPU process that the
+  // surface became invalid, which causes the GPU process to not wait for paint
+  // acks.
+  if (render_widget_host_ &&
+      plugin_container_manager_.IsRootContainer(window)) {
+    BrowserThread::PostTask(
+        BrowserThread::IO, FROM_HERE,
+        new DidDestroyAcceleratedSurfaceSender(
+            render_widget_host_->process()->id(),
+            render_widget_host_->routing_id()));
+  }
+
   plugin_container_manager_.DestroyFakePluginWindowHandle(window);
 }
 
@@ -1083,8 +1124,8 @@ void RenderWidgetHostViewMac::AcknowledgeSwapBuffers(
     int renderer_id,
     int32 route_id,
     uint64 swap_buffers_count) {
-  // Called on the display link. Hand actual work off to the UI thread, which
-  // will then redispatch the message to the IPC thread.
+  // Called on the display link thread. Hand actual work off to the IO thread,
+  // because |GpuProcessHost::Get()| can only be called there.
   // Currently, this is never called for plugins.
   if (render_widget_host_) {
     DCHECK_EQ(render_widget_host_->process()->id(), renderer_id);
diff --git a/chrome/common/gpu_messages_internal.h b/chrome/common/gpu_messages_internal.h
index 3814702..22f2f9d 100644
--- a/chrome/common/gpu_messages_internal.h
+++ b/chrome/common/gpu_messages_internal.h
@@ -59,6 +59,13 @@ IPC_MESSAGE_CONTROL3(GpuMsg_AcceleratedSurfaceBuffersSwappedACK,
                      int /* renderer_id */,
                      int32 /* route_id */,
                      uint64 /* swap_buffers_count */)
+
+// Tells the GPU process that the IOSurface of the buffer belonging to
+// |renderer_route_id| a given id was destroyed, either by the user closing the
+// tab hosting the surface, or by the renderer navigating to a new page.
+IPC_MESSAGE_CONTROL2(GpuMsg_DidDestroyAcceleratedSurface,
+                     int /* renderer_id */,
+                     int32 /* renderer_route_id */)
 #endif
 
 // Tells the GPU process to crash.
diff --git a/chrome/gpu/gpu_channel.cc b/chrome/gpu/gpu_channel.cc
index 7741391..8fb5024 100644
--- a/chrome/gpu/gpu_channel.cc
+++ b/chrome/gpu/gpu_channel.cc
@@ -79,6 +79,19 @@ void GpuChannel::AcceleratedSurfaceBuffersSwapped(
     return;
   stub->AcceleratedSurfaceBuffersSwapped(swap_buffers_count);
 }
+
+void GpuChannel::DidDestroySurface(int32 renderer_route_id) {
+  // Since acclerated views are created in the renderer process and then sent
+  // to the browser process during GPU channel construction, it is possible that
+  // this is called before a GpuCommandBufferStub for |renderer_route_id| was
+  // put into |stubs_|. Hence, do not walk |stubs_| here but instead remember
+  // all |renderer_route_id|s this was called for and use them later.
+  destroyed_renderer_routes_.insert(renderer_route_id);
+}
+
+bool GpuChannel::IsRenderViewGone(int32 renderer_route_id) {
+  return destroyed_renderer_routes_.count(renderer_route_id) > 0;
+}
 #endif
 
 void GpuChannel::OnControlMessageReceived(const IPC::Message& msg) {
diff --git a/chrome/gpu/gpu_channel.h b/chrome/gpu/gpu_channel.h
index 53c0d70..e449dc4 100644
--- a/chrome/gpu/gpu_channel.h
+++ b/chrome/gpu/gpu_channel.h
@@ -6,6 +6,7 @@
 #define CHROME_GPU_GPU_CHANNEL_H_
 #pragma once
 
+#include <set>
 #include <string>
 #include <vector>
 
@@ -55,6 +56,9 @@ class GpuChannel : public IPC::Channel::Listener,
 #if defined(OS_MACOSX)
   virtual void AcceleratedSurfaceBuffersSwapped(
       int32 route_id, uint64 swap_buffers_count);
+  void DidDestroySurface(int32 renderer_route_id);
+
+  bool IsRenderViewGone(int32 renderer_route_id);
 #endif
 
  private:
@@ -94,7 +98,11 @@ class GpuChannel : public IPC::Channel::Listener,
 #if defined(ENABLE_GPU)
   typedef IDMap<GpuCommandBufferStub, IDMapOwnPointer> StubMap;
   StubMap stubs_;
-#endif
+
+#if defined(OS_MACOSX)
+  std::set<int32> destroyed_renderer_routes_;
+#endif  // defined (OS_MACOSX)
+#endif  // defined (ENABLE_GPU)
 
   bool log_messages_;  // True if we should log sent and received messages.
 
diff --git a/chrome/gpu/gpu_command_buffer_stub.cc b/chrome/gpu/gpu_command_buffer_stub.cc
index 6ad18bc..bf3b2d0 100644
--- a/chrome/gpu/gpu_command_buffer_stub.cc
+++ b/chrome/gpu/gpu_command_buffer_stub.cc
@@ -281,6 +281,11 @@ void GpuCommandBufferStub::OnAsyncGetState() {
 
 void GpuCommandBufferStub::OnFlush(int32 put_offset,
                                    gpu::CommandBuffer::State* state) {
+#if defined(OS_MACOSX)
+  // See comment in |DidDestroySurface()| in gpu_processor_mac.cc.
+  if (channel_->IsRenderViewGone(render_view_id_))
+    processor_->DidDestroySurface();
+#endif
   *state = command_buffer_->Flush(put_offset);
 }
 
@@ -353,9 +358,7 @@ void GpuCommandBufferStub::SwapBuffersCallback() {
   params.window = handle_;
   params.surface_id = processor_->GetSurfaceId();
   params.route_id = route_id();
-#if defined(OS_MACOSX)
   params.swap_buffers_count = processor_->swap_buffers_count();
-#endif  // defined(OS_MACOSX)
   gpu_thread->Send(new GpuHostMsg_AcceleratedSurfaceBuffersSwapped(params));
 }
 
diff --git a/chrome/gpu/gpu_command_buffer_stub.h b/chrome/gpu/gpu_command_buffer_stub.h
index e61bdd0..66d1759 100644
--- a/chrome/gpu/gpu_command_buffer_stub.h
+++ b/chrome/gpu/gpu_command_buffer_stub.h
@@ -49,8 +49,13 @@ class GpuCommandBufferStub
   // Get the GLContext associated with this object.
   gpu::GPUProcessor* processor() const { return processor_.get(); }
 
+  // Identifies the various GpuCommandBufferStubs in the GPU process belonging
+  // to the same renderer process.
   int32 route_id() const { return route_id_; }
 
+  // Identifies the various render views in the renderer process.
+  int32 renderer_route_id() const { return renderer_id_; }
+
 #if defined(OS_WIN)
   // Called only by the compositor window's window proc
   void OnCompositorWindowPainted();
diff --git a/chrome/gpu/gpu_thread.cc b/chrome/gpu/gpu_thread.cc
index d506dda..b8c532b 100644
--- a/chrome/gpu/gpu_thread.cc
+++ b/chrome/gpu/gpu_thread.cc
@@ -55,22 +55,18 @@ void GpuThread::RemoveChannel(int renderer_id) {
 void GpuThread::OnControlMessageReceived(const IPC::Message& msg) {
   bool msg_is_ok = true;
   IPC_BEGIN_MESSAGE_MAP_EX(GpuThread, msg, msg_is_ok)
-    IPC_MESSAGE_HANDLER(GpuMsg_EstablishChannel,
-                        OnEstablishChannel)
-    IPC_MESSAGE_HANDLER(GpuMsg_CloseChannel,
-                        OnCloseChannel)
-    IPC_MESSAGE_HANDLER(GpuMsg_Synchronize,
-                        OnSynchronize)
-    IPC_MESSAGE_HANDLER(GpuMsg_CollectGraphicsInfo,
-                        OnCollectGraphicsInfo)
+    IPC_MESSAGE_HANDLER(GpuMsg_EstablishChannel, OnEstablishChannel)
+    IPC_MESSAGE_HANDLER(GpuMsg_CloseChannel, OnCloseChannel)
+    IPC_MESSAGE_HANDLER(GpuMsg_Synchronize, OnSynchronize)
+    IPC_MESSAGE_HANDLER(GpuMsg_CollectGraphicsInfo, OnCollectGraphicsInfo)
 #if defined(OS_MACOSX)
     IPC_MESSAGE_HANDLER(GpuMsg_AcceleratedSurfaceBuffersSwappedACK,
                         OnAcceleratedSurfaceBuffersSwappedACK)
+    IPC_MESSAGE_HANDLER(GpuMsg_DidDestroyAcceleratedSurface,
+                        OnDidDestroyAcceleratedSurface)
 #endif
-    IPC_MESSAGE_HANDLER(GpuMsg_Crash,
-                        OnCrash)
-    IPC_MESSAGE_HANDLER(GpuMsg_Hang,
-                        OnHang)
+    IPC_MESSAGE_HANDLER(GpuMsg_Crash, OnCrash)
+    IPC_MESSAGE_HANDLER(GpuMsg_Hang, OnHang)
   IPC_END_MESSAGE_MAP_EX()
 }
 
@@ -132,6 +128,14 @@ void GpuThread::OnAcceleratedSurfaceBuffersSwappedACK(
   scoped_refptr<GpuChannel> channel = iter->second;
   channel->AcceleratedSurfaceBuffersSwapped(route_id, swap_buffers_count);
 }
+void GpuThread::OnDidDestroyAcceleratedSurface(
+    int renderer_id, int32 renderer_route_id) {
+  GpuChannelMap::const_iterator iter = gpu_channels_.find(renderer_id);
+  if (iter == gpu_channels_.end())
+    return;
+  scoped_refptr<GpuChannel> channel = iter->second;
+  channel->DidDestroySurface(renderer_route_id);
+}
 #endif
 
 void GpuThread::OnCrash() {
diff --git a/chrome/gpu/gpu_thread.h b/chrome/gpu/gpu_thread.h
index 9aba2fd..2f2a1f9 100644
--- a/chrome/gpu/gpu_thread.h
+++ b/chrome/gpu/gpu_thread.h
@@ -43,6 +43,7 @@ class GpuThread : public ChildThread {
 #if defined(OS_MACOSX)
   void OnAcceleratedSurfaceBuffersSwappedACK(
       int renderer_id, int32 route_id, uint64 swap_buffers_count);
+  void OnDidDestroyAcceleratedSurface(int renderer_id, int32 renderer_route_id);
 #endif
   void OnCrash();
   void OnHang();
diff --git a/gpu/command_buffer/service/gpu_processor.h b/gpu/command_buffer/service/gpu_processor.h
index 9d30f980..a143bad 100644
--- a/gpu/command_buffer/service/gpu_processor.h
+++ b/gpu/command_buffer/service/gpu_processor.h
@@ -97,6 +97,8 @@ class GPUProcessor : public CommandBufferEngine {
   uint64 swap_buffers_count() const;
   void set_acknowledged_swap_buffers_count(
       uint64 acknowledged_swap_buffers_count);
+
+  void DidDestroySurface();
 #endif
 
   // Sets a callback that is called when a glResizeCHROMIUM command
diff --git a/gpu/command_buffer/service/gpu_processor_mac.cc b/gpu/command_buffer/service/gpu_processor_mac.cc
index 63a894d..4d9ba18 100644
--- a/gpu/command_buffer/service/gpu_processor_mac.cc
+++ b/gpu/command_buffer/service/gpu_processor_mac.cc
@@ -108,6 +108,18 @@ void GPUProcessor::set_acknowledged_swap_buffers_count(
   acknowledged_swap_buffers_count_ = acknowledged_swap_buffers_count;
 }
 
+void GPUProcessor::DidDestroySurface() {
+  // When a browser window with a GPUProcessor is closed, the render process
+  // will attempt to finish all GL commands, it will busy-wait on the GPU
+  // process until the command queue is empty. If a paint is pending, the GPU
+  // process won't process any GL commands until the browser sends a paint ack,
+  // but since the browser window is already closed, it will never arrive.
+  // To break this infinite loop, the browser tells the GPU process that the
+  // surface became invalid, which causes the GPU process to not wait for paint
+  // acks.
+  surface_.reset();
+}
+
 void GPUProcessor::WillSwapBuffers() {
   DCHECK(decoder_.get());
   DCHECK(decoder_->GetGLContext());