Wait properly for renderer crashes

This replaces a Sleep in automation with a wait for renderer crash. It turns out that our IPC on POSIX had one loophole that caused it not to notice very early crashes, so I also fixed that.

The problem was that when the child process died before connecting to the parent's IPC channel, the parent wouldn't notice the crash because the child end of the IPC pipe was kept open for too long. This change makes the code close the child end of the pipe right after forking the child.

This might also help with automation not noticing the browser crash during initial launch, or at least should be a good step toward fixing that problem.

BUG=38497,90489

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

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

1 files changed, 32 insertions, 18 deletions

diff --git a/ipc/ipc_channel_posix.cc b/ipc/ipc_channel_posix.cc
index a88506c..27c62f5 100644
--- a/ipc/ipc_channel_posix.cc
+++ b/ipc/ipc_channel_posix.cc
@@ -102,15 +102,9 @@ class PipeMap {
 
   // Remove the mapping for the given channel id. No error is signaled if the
   // channel_id doesn't exist
-  void RemoveAndClose(const std::string& channel_id) {
+  void Remove(const std::string& channel_id) {
     base::AutoLock locked(lock_);
-
-    ChannelToFDMap::iterator i = map_.find(channel_id);
-    if (i != map_.end()) {
-      if (HANDLE_EINTR(close(i->second)) < 0)
-        PLOG(ERROR) << "close " << channel_id;
-      map_.erase(i);
-    }
+    map_.erase(channel_id);
   }
 
   // Insert a mapping from @channel_id to @fd. It's a fatal error to insert a
@@ -403,7 +397,7 @@ bool Channel::ChannelImpl::CreatePipe(
         // Case 3 from comment above.
         // We only allow one connection.
         local_pipe = HANDLE_EINTR(dup(local_pipe));
-        PipeMap::GetInstance()->RemoveAndClose(pipe_name_);
+        PipeMap::GetInstance()->Remove(pipe_name_);
       } else {
         // Case 4a from comment above.
         // Guard against inappropriate reuse of the initial IPC channel.  If
@@ -426,6 +420,7 @@ bool Channel::ChannelImpl::CreatePipe(
         LOG(ERROR) << "Server already exists for " << pipe_name_;
         return false;
       }
+      base::AutoLock lock(client_pipe_lock_);
       if (!SocketPair(&local_pipe, &client_pipe_))
         return false;
       PipeMap::GetInstance()->Insert(pipe_name_, client_pipe_);
@@ -529,10 +524,7 @@ bool Channel::ChannelImpl::ProcessIncomingMessages() {
     }
     DCHECK(bytes_read);
 
-    if (client_pipe_ != -1) {
-      PipeMap::GetInstance()->RemoveAndClose(pipe_name_);
-      client_pipe_ = -1;
-    }
+    CloseClientFileDescriptor();
 
     // a pointer to an array of |num_wire_fds| file descriptors from the read
     const int* wire_fds = NULL;
@@ -916,10 +908,31 @@ bool Channel::ChannelImpl::Send(Message* message) {
   return true;
 }
 
-int Channel::ChannelImpl::GetClientFileDescriptor() const {
+int Channel::ChannelImpl::GetClientFileDescriptor() {
+  base::AutoLock lock(client_pipe_lock_);
   return client_pipe_;
 }
 
+int Channel::ChannelImpl::TakeClientFileDescriptor() {
+  base::AutoLock lock(client_pipe_lock_);
+  int fd = client_pipe_;
+  if (client_pipe_ != -1) {
+    PipeMap::GetInstance()->Remove(pipe_name_);
+    client_pipe_ = -1;
+  }
+  return fd;
+}
+
+void Channel::ChannelImpl::CloseClientFileDescriptor() {
+  base::AutoLock lock(client_pipe_lock_);
+  if (client_pipe_ != -1) {
+    PipeMap::GetInstance()->Remove(pipe_name_);
+    if (HANDLE_EINTR(close(client_pipe_)) < 0)
+      PLOG(ERROR) << "close " << pipe_name_;
+    client_pipe_ = -1;
+  }
+}
+
 bool Channel::ChannelImpl::AcceptsConnections() const {
   return server_listen_pipe_ != -1;
 }
@@ -1173,10 +1186,7 @@ void Channel::ChannelImpl::Close() {
     server_listen_connection_watcher_.StopWatchingFileDescriptor();
   }
 
-  if (client_pipe_ != -1) {
-    PipeMap::GetInstance()->RemoveAndClose(pipe_name_);
-    client_pipe_ = -1;
-  }
+  CloseClientFileDescriptor();
 }
 
 //------------------------------------------------------------------------------
@@ -1210,6 +1220,10 @@ int Channel::GetClientFileDescriptor() const {
   return channel_impl_->GetClientFileDescriptor();
 }
 
+int Channel::TakeClientFileDescriptor() {
+  return channel_impl_->TakeClientFileDescriptor();
+}
+
 bool Channel::AcceptsConnections() const {
   return channel_impl_->AcceptsConnections();
 }