GPU process command buffer stubs are destroyed when channel is closed.

- This was didn't work by of a reference counting cycle.

GPU process no longer automatically terminates when the last channel is closed.
- The GPU process will be launched at renderer startup and keep running unless it crashes.

Command buffer stubs no longer hold a strong reference to their parent.

TEST=try
BUG=none

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

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

6 files changed, 59 insertions, 80 deletions

diff --git a/chrome/gpu/gpu_channel.cc b/chrome/gpu/gpu_channel.cc
index c0470f7..091410c 100644
--- a/chrome/gpu/gpu_channel.cc
+++ b/chrome/gpu/gpu_channel.cc
@@ -24,25 +24,12 @@
 #include "ipc/ipc_channel_posix.h"
 #endif
 
-namespace {
-class GpuReleaseTask : public Task {
- public:
-  void Run() {
-    ChildProcess::current()->ReleaseProcess();
-  }
-};
-
-// How long we wait before releasing the GPU process.
-const int kGpuReleaseTimeMS = 10000;
-}  // namespace anonymous
-
 GpuChannel::GpuChannel(int renderer_id)
     : renderer_id_(renderer_id)
 #if defined(OS_POSIX)
     , renderer_fd_(-1)
 #endif
 {
-  ChildProcess::current()->AddRefProcess();
   const CommandLine* command_line = CommandLine::ForCurrentProcess();
   log_messages_ = command_line->HasSwitch(switches::kLogPluginMessages);
 }
@@ -54,10 +41,6 @@ GpuChannel::~GpuChannel() {
     close(renderer_fd_);
   }
 #endif
-  ChildProcess::current()->io_message_loop()->PostDelayedTask(
-      FROM_HERE,
-      new GpuReleaseTask(),
-      kGpuReleaseTimeMS);
 }
 
 void GpuChannel::OnChannelConnected(int32 peer_pid) {
@@ -82,22 +65,7 @@ void GpuChannel::OnMessageReceived(const IPC::Message& message) {
 }
 
 void GpuChannel::OnChannelError() {
-  // Destroy channel. This will cause the channel to be recreated if another
-  // attempt is made to establish a connection from the corresponding renderer.
-  channel_.reset();
-
-  // Close renderer process handle.
-  renderer_process_.Close();
-
-#if defined(ENABLE_GPU)
-  // Destroy all the stubs on this channel.
-  for (StubMap::const_iterator iter = stubs_.begin();
-       iter != stubs_.end();
-       ++iter) {
-    router_.RemoveRoute(iter->second->route_id());
-  }
-  stubs_.clear();
-#endif
+  static_cast<GpuThread*>(ChildThread::current())->RemoveChannel(renderer_id_);
 }
 
 bool GpuChannel::Send(IPC::Message* message) {
@@ -164,10 +132,10 @@ void GpuChannel::OnCreateViewCommandBuffer(gfx::NativeViewId view_id,
 #endif
 
   *route_id = GenerateRouteID();
-  scoped_refptr<GpuCommandBufferStub> stub = new GpuCommandBufferStub(
-      this, handle, NULL, gfx::Size(), 0, *route_id);
-  router_.AddRoute(*route_id, stub);
-  stubs_[*route_id] = stub;
+  scoped_ptr<GpuCommandBufferStub> stub(new GpuCommandBufferStub(
+      this, handle, NULL, gfx::Size(), 0, *route_id));
+  router_.AddRoute(*route_id, stub.get());
+  stubs_.AddWithID(stub.release(), *route_id);
 #endif  // ENABLE_GPU
 }
 
@@ -177,22 +145,19 @@ void GpuChannel::OnCreateOffscreenCommandBuffer(int32 parent_route_id,
                                                 int32* route_id) {
 #if defined(ENABLE_GPU)
   *route_id = GenerateRouteID();
-  scoped_refptr<GpuCommandBufferStub> parent_stub;
-  if (parent_route_id != 0) {
-    StubMap::iterator it = stubs_.find(parent_route_id);
-    DCHECK(it != stubs_.end());
-    parent_stub = it->second;
-  }
+  GpuCommandBufferStub* parent_stub = NULL;
+  if (parent_route_id != 0)
+    parent_stub = stubs_.Lookup(parent_route_id);
 
-  scoped_refptr<GpuCommandBufferStub> stub = new GpuCommandBufferStub(
+  scoped_ptr<GpuCommandBufferStub> stub(new GpuCommandBufferStub(
       this,
       NULL,
-      parent_stub.get(),
+      parent_stub,
       size,
       parent_texture_id,
-      *route_id);
-  router_.AddRoute(*route_id, stub);
-  stubs_[*route_id] = stub;
+      *route_id));
+  router_.AddRoute(*route_id, stub.get());
+  stubs_.AddWithID(stub.release(), *route_id);
 #else
   *route_id = 0;
 #endif
@@ -200,10 +165,8 @@ void GpuChannel::OnCreateOffscreenCommandBuffer(int32 parent_route_id,
 
 void GpuChannel::OnDestroyCommandBuffer(int32 route_id) {
 #if defined(ENABLE_GPU)
-  StubMap::iterator it = stubs_.find(route_id);
-  DCHECK(it != stubs_.end());
-  stubs_.erase(it);
   router_.RemoveRoute(route_id);
+  stubs_.Remove(route_id);
 #endif
 }
 
diff --git a/chrome/gpu/gpu_channel.h b/chrome/gpu/gpu_channel.h
index 60dc906..5ce10e0 100644
--- a/chrome/gpu/gpu_channel.h
+++ b/chrome/gpu/gpu_channel.h
@@ -7,9 +7,9 @@
 
 #include <string>
 
-#include "base/hash_tables.h"
-#include "base/ref_counted.h"
+#include "base/id_map.h"
 #include "base/scoped_open_process.h"
+#include "base/scoped_ptr.h"
 #include "build/build_config.h"
 #include "chrome/common/message_router.h"
 #include "chrome/gpu/gpu_command_buffer_stub.h"
@@ -88,7 +88,7 @@ class GpuChannel : public IPC::Channel::Listener,
   MessageRouter router_;
 
 #if defined(ENABLE_GPU)
-  typedef base::hash_map<int32, scoped_refptr<GpuCommandBufferStub> > StubMap;
+  typedef IDMap<GpuCommandBufferStub, IDMapOwnPointer> StubMap;
   StubMap stubs_;
 #endif
 
diff --git a/chrome/gpu/gpu_command_buffer_stub.cc b/chrome/gpu/gpu_command_buffer_stub.cc
index e30a34c..1c372b8 100644
--- a/chrome/gpu/gpu_command_buffer_stub.cc
+++ b/chrome/gpu/gpu_command_buffer_stub.cc
@@ -21,7 +21,8 @@ GpuCommandBufferStub::GpuCommandBufferStub(GpuChannel* channel,
                                            int32 route_id)
     : channel_(channel),
       handle_(handle),
-      parent_(parent),
+      parent_(
+          parent ? parent->AsWeakPtr() : base::WeakPtr<GpuCommandBufferStub>()),
       initial_size_(size),
       parent_texture_id_(parent_texture_id),
       route_id_(route_id) {
diff --git a/chrome/gpu/gpu_command_buffer_stub.h b/chrome/gpu/gpu_command_buffer_stub.h
index 91d7029..7383840 100644
--- a/chrome/gpu/gpu_command_buffer_stub.h
+++ b/chrome/gpu/gpu_command_buffer_stub.h
@@ -8,7 +8,7 @@
 #if defined(ENABLE_GPU)
 
 #include "base/process.h"
-#include "base/ref_counted.h"
+#include "base/weak_ptr.h"
 #include "gfx/native_widget_types.h"
 #include "gfx/size.h"
 #include "gpu/command_buffer/service/command_buffer_service.h"
@@ -21,7 +21,7 @@ class GpuChannel;
 class GpuCommandBufferStub
     : public IPC::Channel::Listener,
       public IPC::Message::Sender,
-      public base::RefCountedThreadSafe<GpuCommandBufferStub> {
+      public base::SupportsWeakPtr<GpuCommandBufferStub> {
  public:
   GpuCommandBufferStub(GpuChannel* channel,
                        gfx::PluginWindowHandle handle,
@@ -54,9 +54,13 @@ class GpuCommandBufferStub
                            uint32* size);
   void OnResizeOffscreenFrameBuffer(const gfx::Size& size);
 
-  scoped_refptr<GpuChannel> channel_;
+  // The lifetime of objects of this class is managed by a GpuChannel. The
+  // GpuChannels destroy all the GpuCommandBufferStubs that they own when they
+  // are destroyed. So a raw pointer is safe.
+  GpuChannel* channel_;
+
   gfx::PluginWindowHandle handle_;
-  scoped_refptr<GpuCommandBufferStub> parent_;
+  base::WeakPtr<GpuCommandBufferStub> parent_;
   gfx::Size initial_size_;
   uint32 parent_texture_id_;
   int32 route_id_;
diff --git a/chrome/gpu/gpu_thread.cc b/chrome/gpu/gpu_thread.cc
index d076ba2..c5ac79f 100644
--- a/chrome/gpu/gpu_thread.cc
+++ b/chrome/gpu/gpu_thread.cc
@@ -7,7 +7,9 @@
 #include <string>
 #include <vector>
 
+#include "app/gfx/gl/gl_context.h"
 #include "base/command_line.h"
+#include "build/build_config.h"
 #include "chrome/common/child_process.h"
 #include "chrome/common/gpu_info.h"
 #include "chrome/common/gpu_messages.h"
@@ -70,6 +72,10 @@ GpuBackingStoreGLXContext* GpuThread::GetGLXContext() {
 }
 #endif
 
+void GpuThread::RemoveChannel(int renderer_id) {
+  gpu_channels_.erase(renderer_id);
+}
+
 void GpuThread::OnControlMessageReceived(const IPC::Message& msg) {
   bool msg_is_ok = true;
   IPC_BEGIN_MESSAGE_MAP_EX(GpuThread, msg, msg_is_ok)
@@ -84,33 +90,35 @@ void GpuThread::OnControlMessageReceived(const IPC::Message& msg) {
 
 void GpuThread::OnEstablishChannel(int renderer_id) {
   scoped_refptr<GpuChannel> channel;
+  IPC::ChannelHandle channel_handle;
 
-  GpuChannelMap::const_iterator iter = gpu_channels_.find(renderer_id);
-  if (iter == gpu_channels_.end()) {
-    channel = new GpuChannel(renderer_id);
-  } else {
-    channel = iter->second;
-  }
+  // Fail to establish a channel if some implementation of GL cannot be
+  // initialized.
+  if (gfx::GLContext::InitializeOneOff()) {
+    GpuChannelMap::const_iterator iter = gpu_channels_.find(renderer_id);
+    if (iter == gpu_channels_.end()) {
+      channel = new GpuChannel(renderer_id);
+    } else {
+      channel = iter->second;
+    }
 
-  DCHECK(channel != NULL);
+    DCHECK(channel != NULL);
 
-  if (channel->Init()) {
-    // TODO(apatrick): figure out when to remove channels from the map. They
-    // will never be destroyed otherwise.
-    gpu_channels_[renderer_id] = channel;
-  } else {
-    channel = NULL;
-  }
+    if (channel->Init()) {
+      gpu_channels_[renderer_id] = channel;
+    } else {
+      channel = NULL;
+    }
 
-  IPC::ChannelHandle channel_handle;
-  if (channel.get()) {
-    channel_handle.name = channel->GetChannelName();
+    if (channel.get()) {
+      channel_handle.name = channel->GetChannelName();
 #if defined(OS_POSIX)
-    // On POSIX, pass the renderer-side FD. Also mark it as auto-close so that
-    // it gets closed after it has been sent.
-    int renderer_fd = channel->DisownRendererFd();
-    channel_handle.socket = base::FileDescriptor(renderer_fd, true);
+      // On POSIX, pass the renderer-side FD. Also mark it as auto-close so that
+      // it gets closed after it has been sent.
+      int renderer_fd = channel->DisownRendererFd();
+      channel_handle.socket = base::FileDescriptor(renderer_fd, true);
 #endif
+    }
   }
 
   GPUInfo gpu_info;
diff --git a/chrome/gpu/gpu_thread.h b/chrome/gpu/gpu_thread.h
index fdef70b..7f63305 100644
--- a/chrome/gpu/gpu_thread.h
+++ b/chrome/gpu/gpu_thread.h
@@ -30,6 +30,9 @@ class GpuThread : public ChildThread {
   Display* display() const { return display_; }
 #endif
 
+  // Remove the channel for a particular renderer.
+  void RemoveChannel(int renderer_id);
+
  private:
   // ChildThread overrides.
   virtual void OnControlMessageReceived(const IPC::Message& msg);