Windows: BrowserChildProcessHostImpl reports OnProcessCrashed if the child process crashes before…

content/browser/browser_child_process_host_impl.cc

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "content/browser/browser_child_process_host_impl.h"
 
 #include "base/base_switches.h"
 #include "base/bind.h"
 #include "base/command_line.h"
 #include "base/files/file_path.h"
@@ skipping 68 matching lines @@
   child_process_host_->AddFilter(new TraceMessageFilter);
   child_process_host_->AddFilter(new ProfilerMessageFilter(type));
   child_process_host_->AddFilter(new HistogramMessageFilter());
 
   g_child_process_list.Get().push_back(this);
   GetContentClient()->browser()->BrowserChildProcessHostCreated(this);
 }
 
 BrowserChildProcessHostImpl::~BrowserChildProcessHostImpl() {
   g_child_process_list.Get().remove(this);
+
+#if defined(OS_WIN)
+  // The WaitableEvent takes ownership of its handle so release it here to
+  // avoid a double free.
+  if (process_waitable_event_.get())
+    process_waitable_event_->Release();

[cpu_(ooo_6.6-7.5), 2013/03/04 22:46:59]

call GetWatchedEvent to obtain this?

[apatrick_chromium, 2013/03/05 00:22:41]

Done.

+#endif
 }
 
 // static
 void BrowserChildProcessHostImpl::TerminateAll() {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::IO));
   // Make a copy since the BrowserChildProcessHost dtor mutates the original
   // list.
   BrowserChildProcessList copy = g_child_process_list.Get();
   for (BrowserChildProcessList::iterator it = copy.begin();
        it != copy.end(); ++it) {
@@ skipping 100 matching lines @@
 }
 
 bool BrowserChildProcessHostImpl::OnMessageReceived(
     const IPC::Message& message) {
   return delegate_->OnMessageReceived(message);
 }
 
 void BrowserChildProcessHostImpl::OnChannelConnected(int32 peer_pid) {
   Notify(NOTIFICATION_CHILD_PROCESS_HOST_CONNECTED);
   delegate_->OnChannelConnected(peer_pid);
+
+#if defined(OS_WIN)
+  // From this point onward, the exit of the child process is detected by an
+  // error on the IPC channel.
+  early_exit_watcher_.StopWatching();

[cpu_(ooo_6.6-7.5), 2013/03/04 22:46:59]

maybe this ahead of the delegate_ call?

[apatrick_chromium, 2013/03/05 00:22:41]

Done.

+#endif
 }
 
 void BrowserChildProcessHostImpl::OnChannelError() {
   delegate_->OnChannelError();
 }
 
 bool BrowserChildProcessHostImpl::CanShutdown() {
   return delegate_->CanShutdown();
 }
 
@@ skipping 38 matching lines @@
 
 bool BrowserChildProcessHostImpl::Send(IPC::Message* message) {
   return child_process_host_->Send(message);
 }
 
 void BrowserChildProcessHostImpl::OnProcessLaunched() {
   if (!child_process_->GetHandle()) {
     delete delegate_;  // Will delete us
     return;
   }
+
+#if defined(OS_WIN)
+  // Start a WaitableEventWatcher that will invoke OnProcessExitedEarly if the
+  // child process exits. This watcher is stopped once the IPC channel is
+  // connected and the exit of the child process is detecter by an error on the
+  // IPC channel thereafter.
+  process_waitable_event_.reset(

[cpu_(ooo_6.6-7.5), 2013/03/04 22:46:59]

do we really need the process_waitable_event_ as a member?

can we just new it and pass it to the watcher?

[apatrick_chromium, 2013/03/05 00:22:41]

Done.

+      new base::WaitableEvent(child_process_->GetHandle()));
+  early_exit_watcher_.StartWatching(
+      process_waitable_event_.get(),
+      base::Bind(&BrowserChildProcessHostImpl::OnProcessExitedEarly,
+                 base::Unretained(this)));
+#endif
+
   data_.handle = child_process_->GetHandle();
   delegate_->OnProcessLaunched();
 }
 
+void BrowserChildProcessHostImpl::OnProcessExitedEarly(
+    base::WaitableEvent* event) {
+  OnChildDisconnected();
+}
+
 }  // namespace content