Fix a race where WebGL would attempt to use the GpuChannel before it was in the CONNECTED state.

I modified things so that GpuChannelHost goes into the CONNECTED state as soon as the SyncChannel is created. This is safe because any IPC messages that are send are just queued up in the named pipe.

TEST=try
BUG=none

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

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

6 files changed, 11 insertions, 20 deletions

diff --git a/chrome/renderer/ggl/ggl.cc b/chrome/renderer/ggl/ggl.cc
index f8946b9..db7eb1b 100644
--- a/chrome/renderer/ggl/ggl.cc
+++ b/chrome/renderer/ggl/ggl.cc
@@ -117,7 +117,7 @@ Context::~Context() {
 bool Context::Initialize(gfx::NativeViewId view, const gfx::Size& size) {
   DCHECK(size.width() >= 0 && size.height() >= 0);
 
-  if (!channel_->ready())
+  if (channel_->state() != GpuChannelHost::CONNECTED)
     return false;
 
   // Ensure the gles2 library is initialized first in a thread safe way.
diff --git a/chrome/renderer/gpu_channel_host.cc b/chrome/renderer/gpu_channel_host.cc
index 3562c0b..1f2eb21 100644
--- a/chrome/renderer/gpu_channel_host.cc
+++ b/chrome/renderer/gpu_channel_host.cc
@@ -20,6 +20,11 @@ void GpuChannelHost::Connect(const std::string& channel_name) {
       channel_name, IPC::Channel::MODE_CLIENT, this, NULL,
       ChildProcess::current()->io_message_loop(), true,
       ChildProcess::current()->GetShutDownEvent()));
+
+  // It is safe to send IPC messages before the channel completes the connection
+  // and receives the hello message from the GPU process. The messages get
+  // cached.
+  state_ = CONNECTED;
 }
 
 void GpuChannelHost::OnMessageReceived(const IPC::Message& message) {
@@ -30,7 +35,6 @@ void GpuChannelHost::OnMessageReceived(const IPC::Message& message) {
 }
 
 void GpuChannelHost::OnChannelConnected(int32 peer_pid) {
-  state_ = CONNECTED;
 }
 
 void GpuChannelHost::OnChannelError() {
diff --git a/chrome/renderer/gpu_channel_host.h b/chrome/renderer/gpu_channel_host.h
index d697fd1..e8216e4 100644
--- a/chrome/renderer/gpu_channel_host.h
+++ b/chrome/renderer/gpu_channel_host.h
@@ -43,9 +43,6 @@ class GpuChannelHost : public IPC::Channel::Listener,
 
   State state() const { return state_; }
 
-  // Returns whether the channel to the GPU process is ready.
-  bool ready() const { return channel_.get() != NULL; }
-
   // IPC::Channel::Listener implementation:
   virtual void OnMessageReceived(const IPC::Message& msg);
   virtual void OnChannelConnected(int32 peer_pid);
diff --git a/chrome/renderer/render_thread.cc b/chrome/renderer/render_thread.cc
index 95588c8..3573488 100644
--- a/chrome/renderer/render_thread.cc
+++ b/chrome/renderer/render_thread.cc
@@ -741,19 +741,9 @@ void RenderThread::EstablishGpuChannel() {
 }
 
 GpuChannelHost* RenderThread::EstablishGpuChannelSync() {
-  // We may need to retry the connection establishment if an existing
-  // connection has gone bad, which will not be detected in
-  // EstablishGpuChannel since we do not send duplicate
-  // ViewHostMsg_EstablishGpuChannel messages -- doing so breaks the
-  // preexisting connection in bad ways.
-  bool retry = true;
-  for (int i = 0; i < 2 && retry; ++i) {
-    EstablishGpuChannel();
-    retry = !Send(new ViewHostMsg_SynchronizeGpu());
-  }
-  // TODO(kbr): the GPU channel is still in the unconnected state at this point.
-  // Need to figure out whether it is really safe to return it.
-  return gpu_channel_.get();
+  EstablishGpuChannel();
+  Send(new ViewHostMsg_SynchronizeGpu());
+  return GetGpuChannel();
 }
 
 GpuChannelHost* RenderThread::GetGpuChannel() {
diff --git a/chrome/renderer/webgles2context_impl.cc b/chrome/renderer/webgles2context_impl.cc
index 89e13129..a14e0e3 100644
--- a/chrome/renderer/webgles2context_impl.cc
+++ b/chrome/renderer/webgles2context_impl.cc
@@ -36,7 +36,7 @@ bool WebGLES2ContextImpl::initialize(
   GpuChannelHost* host = render_thread->EstablishGpuChannelSync();
   if (!host)
     return false;
-  DCHECK(host->ready());
+  DCHECK(host->state() == GpuChannelHost::CONNECTED);
 
   // If a WebView is passed then create a context rendering directly
   // into the window used by the WebView, otherwise create an offscreen
diff --git a/chrome/renderer/webgraphicscontext3d_command_buffer_impl.cc b/chrome/renderer/webgraphicscontext3d_command_buffer_impl.cc
index 56c6392..61060b3 100644
--- a/chrome/renderer/webgraphicscontext3d_command_buffer_impl.cc
+++ b/chrome/renderer/webgraphicscontext3d_command_buffer_impl.cc
@@ -58,7 +58,7 @@ bool WebGraphicsContext3DCommandBufferImpl::initialize(
   GpuChannelHost* host = render_thread->EstablishGpuChannelSync();
   if (!host)
     return false;
-  DCHECK(host->ready());
+  DCHECK(host->state() == GpuChannelHost::CONNECTED);
   context_ = ggl::CreateOffscreenContext(host, parent_context, gfx::Size(1, 1));
   if (!context_)
     return false;