Remove the code to wait on disconnected child processes to get the exit code. This was done in r101435 to fix a problem where we were seeing processes "quit" but the result code wasn't ready. It looks like r68831 caused this, since it made the channel be disconnected before the process exits.

Review URL: https://chromiumcodereview.appspot.com/10702048

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

8 files changed, 24 insertions, 159 deletions

diff --git a/content/browser/browser_child_process_host_impl.cc b/content/browser/browser_child_process_host_impl.cc
index 0d81c92..00076d9 100644
--- a/content/browser/browser_child_process_host_impl.cc
+++ b/content/browser/browser_child_process_host_impl.cc
@@ -28,9 +28,7 @@
 #include "content/public/common/content_switches.h"
 #include "content/public/common/result_codes.h"
 
-#if defined(OS_WIN)
-#include "base/synchronization/waitable_event.h"
-#elif defined(OS_MACOSX)
+#if defined(OS_MACOSX)
 #include "content/browser/mach_broker_mac.h"
 #endif
 
@@ -81,11 +79,7 @@ BrowserChildProcessHostImpl::BrowserChildProcessHostImpl(
     content::ProcessType type,
     BrowserChildProcessHostDelegate* delegate)
     : data_(type),
-      delegate_(delegate),
-#if !defined(OS_WIN)
-      ALLOW_THIS_IN_INITIALIZER_LIST(task_factory_(this)),
-#endif
-      disconnect_was_alive_(false) {
+      delegate_(delegate) {
   data_.id = ChildProcessHostImpl::GenerateChildProcessUniqueId();
 
   child_process_host_.reset(ChildProcessHost::Create(this));
@@ -211,13 +205,6 @@ bool BrowserChildProcessHostImpl::CanShutdown() {
   return delegate_->CanShutdown();
 }
 
-// Normally a ChildProcessHostDelegate deletes itself from this callback, but at
-// this layer and below we need to have the final child process exit code to
-// properly bucket crashes vs kills. On Windows we can do this if we wait until
-// the process handle is signaled; on the rest of the platforms, we schedule a
-// delayed task to wait for an exit code. However, this means that this method
-// may be called twice: once from the actual channel error and once from
-// OnWaitableEventSignaled() or the delayed task.
 void BrowserChildProcessHostImpl::OnChildDisconnected() {
   DCHECK(data_.handle != base::kNullProcessHandle);
   int exit_code;
@@ -231,11 +218,6 @@ void BrowserChildProcessHostImpl::OnChildDisconnected() {
       UMA_HISTOGRAM_ENUMERATION("ChildProcess.Crashed",
                                 data_.type,
                                 content::PROCESS_TYPE_MAX);
-      if (disconnect_was_alive_) {
-        UMA_HISTOGRAM_ENUMERATION("ChildProcess.CrashedWasAlive",
-                                  data_.type,
-                                  content::PROCESS_TYPE_MAX);
-      }
       break;
     }
     case base::TERMINATION_STATUS_PROCESS_WAS_KILLED: {
@@ -244,42 +226,13 @@ void BrowserChildProcessHostImpl::OnChildDisconnected() {
       UMA_HISTOGRAM_ENUMERATION("ChildProcess.Killed",
                                 data_.type,
                                 content::PROCESS_TYPE_MAX);
-      if (disconnect_was_alive_) {
-        UMA_HISTOGRAM_ENUMERATION("ChildProcess.KilledWasAlive",
-                                  data_.type,
-                                  content::PROCESS_TYPE_MAX);
-      }
       break;
     }
     case base::TERMINATION_STATUS_STILL_RUNNING: {
-      // Exit code not yet available. Ensure we don't wait forever for an exit
-      // code.
-      if (disconnect_was_alive_) {
-        UMA_HISTOGRAM_ENUMERATION("ChildProcess.DisconnectedAlive",
-                                  data_.type,
-                                  content::PROCESS_TYPE_MAX);
-        break;
-      }
-      disconnect_was_alive_ = true;
-#if defined(OS_WIN)
-      child_watcher_.StartWatching(
-          new base::WaitableEvent(data_.handle), this);
-#else
-      // On non-Windows platforms, give the child process some time to die after
-      // disconnecting the channel so that the exit code and termination status
-      // become available. This is best effort -- if the process doesn't die
-      // within the time limit, this object gets destroyed.
-      const base::TimeDelta kExitCodeWait =
-          base::TimeDelta::FromMilliseconds(250);
-      MessageLoop::current()->PostDelayedTask(
-          FROM_HERE,
-          base::Bind(&BrowserChildProcessHostImpl::OnChildDisconnected,
-                     task_factory_.GetWeakPtr()),
-          kExitCodeWait);
-#endif
-      return;
+      UMA_HISTOGRAM_ENUMERATION("ChildProcess.DisconnectedAlive",
+                                data_.type,
+                                content::PROCESS_TYPE_MAX);
     }
-
     default:
       break;
   }
@@ -291,24 +244,6 @@ void BrowserChildProcessHostImpl::OnChildDisconnected() {
   delete delegate_;  // Will delete us
 }
 
-// The child process handle has been signaled so the exit code is finally
-// available. Unfortunately STILL_ACTIVE (0x103) is a valid exit code in
-// which case we should not call OnChildDisconnected() or else we will be
-// waiting forever.
-void BrowserChildProcessHostImpl::OnWaitableEventSignaled(
-    base::WaitableEvent* waitable_event) {
-#if defined (OS_WIN)
-  unsigned long exit_code = 0;
-  GetExitCodeProcess(waitable_event->Release(), &exit_code);
-  delete waitable_event;
-  if (exit_code == STILL_ACTIVE) {
-    delete delegate_;  // Will delete us
-  } else {
-    BrowserChildProcessHostImpl::OnChildDisconnected();
-  }
-#endif
-}
-
 bool BrowserChildProcessHostImpl::Send(IPC::Message* message) {
   return child_process_host_->Send(message);
 }
diff --git a/content/browser/browser_child_process_host_impl.h b/content/browser/browser_child_process_host_impl.h
index 877f5e4..1fc742a 100644
--- a/content/browser/browser_child_process_host_impl.h
+++ b/content/browser/browser_child_process_host_impl.h
@@ -10,9 +10,7 @@
 
 #include "base/compiler_specific.h"
 #include "base/memory/scoped_ptr.h"
-#include "base/memory/weak_ptr.h"
 #include "base/process.h"
-#include "base/synchronization/waitable_event_watcher.h"
 #include "content/browser/child_process_launcher.h"
 #include "content/public/browser/browser_child_process_host.h"
 #include "content/public/browser/child_process_data.h"
@@ -28,8 +26,7 @@ class BrowserChildProcessHostIterator;
 class CONTENT_EXPORT BrowserChildProcessHostImpl :
     public content::BrowserChildProcessHost,
     public NON_EXPORTED_BASE(content::ChildProcessHostDelegate),
-    public ChildProcessLauncher::Client,
-    public base::WaitableEventWatcher::Delegate {
+    public ChildProcessLauncher::Client {
  public:
   BrowserChildProcessHostImpl(
       content::ProcessType type,
@@ -56,8 +53,6 @@ class CONTENT_EXPORT BrowserChildProcessHostImpl :
   virtual void SetName(const string16& name) OVERRIDE;
   virtual void SetHandle(base::ProcessHandle handle) OVERRIDE;
 
-  bool disconnect_was_alive() const { return disconnect_was_alive_; }
-
   // Returns the handle of the child process. This can be called only after
   // OnProcessLaunched is called or it will be invalid and may crash.
   base::ProcessHandle GetHandle() const;
@@ -92,21 +87,11 @@ class CONTENT_EXPORT BrowserChildProcessHostImpl :
   // ChildProcessLauncher::Client implementation.
   virtual void OnProcessLaunched() OVERRIDE;
 
-  // public base::WaitableEventWatcher::Delegate implementation:
-  virtual void OnWaitableEventSignaled(
-      base::WaitableEvent* waitable_event) OVERRIDE;
-
   content::ChildProcessData data_;
   content::BrowserChildProcessHostDelegate* delegate_;
   scoped_ptr<content::ChildProcessHost> child_process_host_;
 
   scoped_ptr<ChildProcessLauncher> child_process_;
-#if defined(OS_WIN)
-  base::WaitableEventWatcher child_watcher_;
-#else
-  base::WeakPtrFactory<BrowserChildProcessHostImpl> task_factory_;
-#endif
-  bool disconnect_was_alive_;
 };
 
 #endif  // CONTENT_BROWSER_BROWSER_CHILD_PROCESS_HOST_IMPL_H_
diff --git a/content/browser/download/download_manager_impl.cc b/content/browser/download/download_manager_impl.cc
index a34d25e..e0f441b 100644
--- a/content/browser/download/download_manager_impl.cc
+++ b/content/browser/download/download_manager_impl.cc
@@ -12,6 +12,7 @@
 #include "base/file_util.h"
 #include "base/i18n/case_conversion.h"
 #include "base/logging.h"
+#include "base/message_loop.h"
 #include "base/stl_util.h"
 #include "base/stringprintf.h"
 #include "base/synchronization/lock.h"
diff --git a/content/browser/gpu/gpu_process_host.cc b/content/browser/gpu/gpu_process_host.cc
index f716102..cc9176b 100644
--- a/content/browser/gpu/gpu_process_host.cc
+++ b/content/browser/gpu/gpu_process_host.cc
@@ -235,10 +235,6 @@ bool GpuProcessHost::HostIsValid(GpuProcessHost* host) {
   if (!host)
     return false;
 
-  // Check if the GPU process has died and the host is about to be destroyed.
-  if (host->process_->disconnect_was_alive())
-    return false;
-
   // The Gpu process is invalid if it's not using software, the card is
   // blacklisted, and we can kill it and start over.
   if (CommandLine::ForCurrentProcess()->HasSwitch(switches::kSingleProcess) ||
diff --git a/content/browser/renderer_host/render_process_host_impl.cc b/content/browser/renderer_host/render_process_host_impl.cc
index 2335831..bfea883 100644
--- a/content/browser/renderer_host/render_process_host_impl.cc
+++ b/content/browser/renderer_host/render_process_host_impl.cc
@@ -116,7 +116,6 @@
 #include "webkit/plugins/plugin_switches.h"
 
 #if defined(OS_WIN)
-#include "base/synchronization/waitable_event.h"
 #include "content/common/font_cache_dispatcher_win.h"
 #endif
 
@@ -855,9 +854,7 @@ bool RenderProcessHostImpl::FastShutdownIfPossible() {
   if (!SuddenTerminationAllowed())
     return false;
 
-  // Store the handle before it gets changed.
-  base::ProcessHandle handle = GetHandle();
-  ProcessDied(handle, base::TERMINATION_STATUS_NORMAL_TERMINATION, 0, false);
+  ProcessDied();
   fast_shutdown_started_ = true;
   return true;
 }
@@ -1027,44 +1024,7 @@ void RenderProcessHostImpl::OnChannelConnected(int32 peer_pid) {
 }
 
 void RenderProcessHostImpl::OnChannelError() {
-  if (!channel_.get())
-    return;
-
-  // Store the handle before it gets changed.
-  base::ProcessHandle handle = GetHandle();
-
-  // child_process_launcher_ can be NULL in single process mode or if fast
-  // termination happened.
-  int exit_code = 0;
-  base::TerminationStatus status =
-      child_process_launcher_.get() ?
-      child_process_launcher_->GetChildTerminationStatus(&exit_code) :
-      base::TERMINATION_STATUS_NORMAL_TERMINATION;
-
-#if defined(OS_WIN)
-  if (!run_renderer_in_process()) {
-    if (status == base::TERMINATION_STATUS_STILL_RUNNING) {
-      HANDLE process = child_process_launcher_->GetHandle();
-      child_process_watcher_.StartWatching(
-          new base::WaitableEvent(process), this);
-      return;
-    }
-  }
-#endif
-  ProcessDied(handle, status, exit_code, false);
-}
-
-// Called when the renderer process handle has been signaled.
-void RenderProcessHostImpl::OnWaitableEventSignaled(
-    base::WaitableEvent* waitable_event) {
-#if defined (OS_WIN)
-  base::ProcessHandle handle = GetHandle();
-  int exit_code = 0;
-  base::TerminationStatus status =
-      base::GetTerminationStatus(waitable_event->Release(), &exit_code);
-  delete waitable_event;
-  ProcessDied(handle, status, exit_code, true);
-#endif
+  ProcessDied();
 }
 
 BrowserContext* RenderProcessHostImpl::GetBrowserContext() const {
@@ -1378,17 +1338,22 @@ void RenderProcessHostImpl::RegisterProcessHostForSite(
   map->RegisterProcess(site, process);
 }
 
-void RenderProcessHostImpl::ProcessDied(base::ProcessHandle handle,
-                                        base::TerminationStatus status,
-                                        int exit_code,
-                                        bool was_alive) {
+void RenderProcessHostImpl::ProcessDied() {
   // Our child process has died.  If we didn't expect it, it's a crash.
   // In any case, we need to let everyone know it's gone.
   // The OnChannelError notification can fire multiple times due to nested sync
   // calls to a renderer. If we don't have a valid channel here it means we
   // already handled the error.
 
-  RendererClosedDetails details(handle, status, exit_code, was_alive);
+  // child_process_launcher_ can be NULL in single process mode or if fast
+  // termination happened.
+  int exit_code = 0;
+  base::TerminationStatus status =
+      child_process_launcher_.get() ?
+      child_process_launcher_->GetChildTerminationStatus(&exit_code) :
+      base::TERMINATION_STATUS_NORMAL_TERMINATION;
+
+  RendererClosedDetails details(GetHandle(), status, exit_code);
   NotificationService::current()->Notify(
       NOTIFICATION_RENDERER_PROCESS_CLOSED,
       Source<RenderProcessHost>(this),
diff --git a/content/browser/renderer_host/render_process_host_impl.h b/content/browser/renderer_host/render_process_host_impl.h
index 4f1cda80..99bd801 100644
--- a/content/browser/renderer_host/render_process_host_impl.h
+++ b/content/browser/renderer_host/render_process_host_impl.h
@@ -12,7 +12,6 @@
 
 #include "base/memory/scoped_ptr.h"
 #include "base/process.h"
-#include "base/synchronization/waitable_event_watcher.h"
 #include "base/timer.h"
 #include "content/browser/child_process_launcher.h"
 #include "content/common/content_export.h"
@@ -24,10 +23,6 @@ class CommandLine;
 class GpuMessageFilter;
 class RenderWidgetHelper;
 
-namespace base {
-class WaitableEvent;
-}
-
 namespace content {
 class RendererMainThread;
 class RenderWidgetHost;
@@ -49,8 +44,7 @@ class RenderWidgetHostImpl;
 // communicate through the two process objects.
 class CONTENT_EXPORT RenderProcessHostImpl
     : public RenderProcessHost,
-      public ChildProcessLauncher::Client,
-      public base::WaitableEventWatcher::Delegate {
+      public ChildProcessLauncher::Client {
  public:
   RenderProcessHostImpl(BrowserContext* browser_context, bool is_guest);
   virtual ~RenderProcessHostImpl();
@@ -107,10 +101,6 @@ class CONTENT_EXPORT RenderProcessHostImpl
   // ChildProcessLauncher::Client implementation.
   virtual void OnProcessLaunched() OVERRIDE;
 
-  // base::WaitableEventWatcher::Delegate implementation.
-  virtual void OnWaitableEventSignaled(
-      base::WaitableEvent* waitable_event) OVERRIDE;
-
   // Call this function when it is evident that the child process is actively
   // performing some operation, for example if we just received an IPC message.
   void mark_child_process_activity_time() {
@@ -210,12 +200,8 @@ class CONTENT_EXPORT RenderProcessHostImpl
   // Callers can reduce the RenderProcess' priority.
   void SetBackgrounded(bool backgrounded);
 
-  // Handle termination of our process. |was_alive| indicates that when we
-  // tried to retrieve the exit code the process had not finished yet.
-  void ProcessDied(base::ProcessHandle handle,
-                   base::TerminationStatus status,
-                   int exit_code,
-                   bool was_alive);
+  // Handle termination of our process.
+  void ProcessDied();
 
   // The count of currently visible widgets.  Since the host can be a container
   // for multiple widgets, it uses this count to determine when it should be
@@ -268,11 +254,6 @@ class CONTENT_EXPORT RenderProcessHostImpl
   // because the queued messages may have dependencies on the init messages.
   std::queue<IPC::Message*> queued_messages_;
 
-#if defined(OS_WIN)
-  // Used to wait until the renderer dies to get an accurrate exit code.
-  base::WaitableEventWatcher child_process_watcher_;
-#endif
-
   // The globally-unique identifier for this RPH.
   int id_;
 
diff --git a/content/browser/renderer_host/render_widget_host_view_mac.mm b/content/browser/renderer_host/render_widget_host_view_mac.mm
index e6bfb67..56ef1609 100644
--- a/content/browser/renderer_host/render_widget_host_view_mac.mm
+++ b/content/browser/renderer_host/render_widget_host_view_mac.mm
@@ -15,6 +15,7 @@
 #include "base/mac/scoped_cftyperef.h"
 #import "base/mac/scoped_nsautorelease_pool.h"
 #import "base/memory/scoped_nsobject.h"
+#include "base/message_loop.h"
 #include "base/metrics/histogram.h"
 #include "base/string_util.h"
 #include "base/sys_info.h"
diff --git a/content/browser/web_contents/web_contents_view_mac.mm b/content/browser/web_contents/web_contents_view_mac.mm
index 3c0f0b8..6d21cd90 100644
--- a/content/browser/web_contents/web_contents_view_mac.mm
+++ b/content/browser/web_contents/web_contents_view_mac.mm
@@ -9,6 +9,7 @@
 #include <string>
 
 #import "base/mac/scoped_sending_event.h"
+#include "base/message_loop.h"
 #import "base/message_pump_mac.h"
 #include "content/browser/renderer_host/popup_menu_helper_mac.h"
 #include "content/browser/renderer_host/render_view_host_factory.h"