Revert 128369 - Histogram times surrounding render crashes

Report exec time, kernal time, and user cpu time
when we have a crash.  Also report non-crashing 
durations for comparison.

r=rvargas
Review URL: https://chromiumcodereview.appspot.com/9769011

TBR=jar@chromium.org
Review URL: https://chromiumcodereview.appspot.com/9845018

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

5 files changed, 54 insertions, 112 deletions

diff --git a/chrome/browser/metrics/metrics_service.cc b/chrome/browser/metrics/metrics_service.cc
index 4ad8cb1..d5dfcc8 100644
--- a/chrome/browser/metrics/metrics_service.cc
+++ b/chrome/browser/metrics/metrics_service.cc
@@ -560,7 +560,8 @@ void MetricsService::Observe(int type,
                     details).ptr();
         content::RenderProcessHost* host =
             content::Source<content::RenderProcessHost>(source).ptr();
-        LogRendererCrash(host, *process_details);
+        LogRendererCrash(
+            host, process_details->status, process_details->was_alive);
       }
       break;
 
@@ -1368,71 +1369,34 @@ void MetricsService::LogLoadStarted() {
   // might be lost due to a crash :-(.
 }
 
-void MetricsService::LogRendererCrash(
-    content::RenderProcessHost* host,
-    const content::RenderProcessHost::RendererClosedDetails& details) {
+void MetricsService::LogRendererCrash(content::RenderProcessHost* host,
+                                      base::TerminationStatus status,
+                                      bool was_alive) {
   Profile* profile = Profile::FromBrowserContext(host->GetBrowserContext());
   ExtensionService* service = profile->GetExtensionService();
   bool was_extension_process =
       service && service->process_map()->Contains(host->GetID());
-  if (details.status == base::TERMINATION_STATUS_PROCESS_CRASHED ||
-      details.status == base::TERMINATION_STATUS_ABNORMAL_TERMINATION) {
-    if (was_extension_process) {
+  if (status == base::TERMINATION_STATUS_PROCESS_CRASHED ||
+      status == base::TERMINATION_STATUS_ABNORMAL_TERMINATION) {
+    if (was_extension_process)
       IncrementPrefValue(prefs::kStabilityExtensionRendererCrashCount);
-    } else {
+    else
       IncrementPrefValue(prefs::kStabilityRendererCrashCount);
 
-#if defined(OS_WIN)
-      if (details.have_process_times) {
-        if (details.status == base::TERMINATION_STATUS_PROCESS_CRASHED) {
-          UMA_HISTOGRAM_TIMES("BrowserRenderProcessHost.CrashedDuration",
-                              details.run_duration);
-          UMA_HISTOGRAM_TIMES("BrowserRenderProcessHost.CrashedKernelTime",
-                              details.kernel_duration);
-          UMA_HISTOGRAM_TIMES("BrowserRenderProcessHost.CrashedUserTime",
-                              details.user_duration);
-        } else {
-          DCHECK(details.status ==
-                 base::TERMINATION_STATUS_ABNORMAL_TERMINATION);
-          UMA_HISTOGRAM_TIMES("BrowserRenderProcessHost.AbnormalTermDuration",
-                              details.run_duration);
-          UMA_HISTOGRAM_TIMES("BrowserRenderProcessHost.AbnormalTermKernelTime",
-                              details.kernel_duration);
-          UMA_HISTOGRAM_TIMES("BrowserRenderProcessHost.AbnormalTermUserTime",
-                              details.user_duration);
-        }
-      }
-#endif  // OS_WIN
-    }
-
-    // TODO(jar): These histograms should be small enumerated histograms.
     UMA_HISTOGRAM_PERCENTAGE("BrowserRenderProcessHost.ChildCrashes",
                              was_extension_process ? 2 : 1);
-    if (details.was_alive) {
+    if (was_alive) {
       UMA_HISTOGRAM_PERCENTAGE("BrowserRenderProcessHost.ChildCrashesWasAlive",
                                was_extension_process ? 2 : 1);
     }
-  } else if (details.status == base::TERMINATION_STATUS_PROCESS_WAS_KILLED) {
+  } else if (status == base::TERMINATION_STATUS_PROCESS_WAS_KILLED) {
     UMA_HISTOGRAM_PERCENTAGE("BrowserRenderProcessHost.ChildKills",
                              was_extension_process ? 2 : 1);
-    if (details.was_alive) {
+    if (was_alive) {
       UMA_HISTOGRAM_PERCENTAGE("BrowserRenderProcessHost.ChildKillsWasAlive",
                                was_extension_process ? 2 : 1);
     }
   }
-
-#if defined(OS_WIN)
-  if (details.have_process_times && !was_extension_process &&
-      details.status != base::TERMINATION_STATUS_PROCESS_CRASHED &&
-      details.status != base::TERMINATION_STATUS_ABNORMAL_TERMINATION) {
-    UMA_HISTOGRAM_TIMES("BrowserRenderProcessHost.NormalTermDuration",
-                        details.run_duration);
-    UMA_HISTOGRAM_TIMES("BrowserRenderProcessHost.NormalTermKernelTime",
-                        details.kernel_duration);
-    UMA_HISTOGRAM_TIMES("BrowserRenderProcessHost.NormalTermUserTime",
-                        details.user_duration);
-  }
-#endif  // OS_WIN
 }
 
 void MetricsService::LogRendererHang() {
diff --git a/chrome/browser/metrics/metrics_service.h b/chrome/browser/metrics/metrics_service.h
index 64d5266..ff4df36 100644
--- a/chrome/browser/metrics/metrics_service.h
+++ b/chrome/browser/metrics/metrics_service.h
@@ -21,12 +21,10 @@
 #include "base/process_util.h"
 #include "chrome/browser/io_thread.h"
 #include "chrome/common/metrics/metrics_service_base.h"
-#include "content/public/browser/notification_observer.h"
-#include "content/public/browser/notification_registrar.h"
-#include "content/public/browser/render_process_host.h"
 #include "content/public/common/process_type.h"
 #include "content/public/common/url_fetcher_delegate.h"
-
+#include "content/public/browser/notification_observer.h"
+#include "content/public/browser/notification_registrar.h"
 
 #if defined(OS_CHROMEOS)
 #include "chrome/browser/chromeos/external_metrics.h"
@@ -271,9 +269,9 @@ class MetricsService : public content::NotificationObserver,
   void IncrementLongPrefsValue(const char* path);
 
   // Records a renderer process crash.
-  void LogRendererCrash(
-      content::RenderProcessHost* host,
-      const content::RenderProcessHost::RendererClosedDetails& process_details);
+  void LogRendererCrash(content::RenderProcessHost* host,
+                        base::TerminationStatus status,
+                        bool was_alive);
 
   // Records a renderer process hang.
   void LogRendererHang();
diff --git a/content/browser/renderer_host/render_process_host_impl.cc b/content/browser/renderer_host/render_process_host_impl.cc
index d8aa5ab..dece81c 100644
--- a/content/browser/renderer_host/render_process_host_impl.cc
+++ b/content/browser/renderer_host/render_process_host_impl.cc
@@ -784,11 +784,8 @@ bool RenderProcessHostImpl::FastShutdownIfPossible() {
     return false;
 
   // Store the handle before it gets changed.
-  RendererClosedDetails details(GetHandle());
-  DCHECK_EQ(details.status, base::TERMINATION_STATUS_NORMAL_TERMINATION);
-  DCHECK_EQ(details.exit_code, 0);
-  DCHECK_EQ(details.was_alive, false);
-  ProcessDied(&details);
+  base::ProcessHandle handle = GetHandle();
+  ProcessDied(handle, base::TERMINATION_STATUS_NORMAL_TERMINATION, 0, false);
   fast_shutdown_started_ = true;
   return true;
 }
@@ -972,16 +969,19 @@ void RenderProcessHostImpl::OnChannelError() {
     return;
 
   // Store the handle before it gets changed.
-  RendererClosedDetails details(GetHandle());
+  base::ProcessHandle handle = GetHandle();
+
   // child_process_launcher_ can be NULL in single process mode or if fast
   // termination happened.
-  details.status = child_process_launcher_.get() ?
-      child_process_launcher_->GetChildTerminationStatus(&details.exit_code) :
+  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 (details.status == base::TERMINATION_STATUS_STILL_RUNNING) {
+    if (status == base::TERMINATION_STATUS_STILL_RUNNING) {
       HANDLE process = child_process_launcher_->GetHandle();
       child_process_watcher_.StartWatching(
           new base::WaitableEvent(process), this);
@@ -989,20 +989,19 @@ void RenderProcessHostImpl::OnChannelError() {
     }
   }
 #endif
-  details.was_alive = false;
-  ProcessDied(&details);
-  }
+  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)
-  RendererClosedDetails details(GetHandle());
-  details.status = base::GetTerminationStatus(waitable_event->Release(),
-                                              &details.exit_code);
+  base::ProcessHandle handle = GetHandle();
+  int exit_code = 0;
+  base::TerminationStatus status =
+      base::GetTerminationStatus(waitable_event->Release(), &exit_code);
   delete waitable_event;
-  details.was_alive = true;
-  ProcessDied(&details);
+  ProcessDied(handle, status, exit_code, true);
 #endif
 }
 
@@ -1236,17 +1235,21 @@ content::RenderProcessHost*
   return NULL;
 }
 
-void RenderProcessHostImpl::ProcessDied(RendererClosedDetails* details) {
+void RenderProcessHostImpl::ProcessDied(base::ProcessHandle handle,
+                                           base::TerminationStatus status,
+                                           int exit_code,
+                                           bool was_alive) {
   // 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);
   content::NotificationService::current()->Notify(
       content::NOTIFICATION_RENDERER_PROCESS_CLOSED,
       content::Source<RenderProcessHost>(this),
-      content::Details<RendererClosedDetails>(details));
+      content::Details<RendererClosedDetails>(&details));
 
   child_process_launcher_.reset();
   channel_.reset();
@@ -1256,8 +1259,8 @@ void RenderProcessHostImpl::ProcessDied(RendererClosedDetails* details) {
   while (!iter.IsAtEnd()) {
     RenderWidgetHostImpl::From(iter.GetCurrentValue())->OnMessageReceived(
         ViewHostMsg_RenderViewGone(iter.GetCurrentKey(),
-                                   static_cast<int>(details->status),
-                                   details->exit_code));
+                                   static_cast<int>(status),
+                                   exit_code));
     iter.Advance();
   }
 
diff --git a/content/browser/renderer_host/render_process_host_impl.h b/content/browser/renderer_host/render_process_host_impl.h
index 6b9f826..e67224e 100644
--- a/content/browser/renderer_host/render_process_host_impl.h
+++ b/content/browser/renderer_host/render_process_host_impl.h
@@ -175,8 +175,12 @@ class CONTENT_EXPORT RenderProcessHostImpl
   // Callers can reduce the RenderProcess' priority.
   void SetBackgrounded(bool backgrounded);
 
-  // Handle termination of our process.
-  void ProcessDied(RendererClosedDetails* details);
+  // 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);
 
   // 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
diff --git a/content/public/browser/render_process_host.h b/content/public/browser/render_process_host.h
index b9a8ce5..dc64905 100644
--- a/content/public/browser/render_process_host.h
+++ b/content/public/browser/render_process_host.h
@@ -40,42 +40,15 @@ class CONTENT_EXPORT RenderProcessHost : public IPC::Message::Sender,
 
   // Details for RENDERER_PROCESS_CLOSED notifications.
   struct RendererClosedDetails {
-    explicit RendererClosedDetails(base::ProcessHandle handle) {
+    RendererClosedDetails(base::ProcessHandle handle,
+                          base::TerminationStatus status,
+                          int exit_code,
+                          bool was_alive) {
       this->handle = handle;
-      // default values should be updated by caller.
-      status = base::TERMINATION_STATUS_NORMAL_TERMINATION;
-      exit_code = 0;
-      was_alive = false;
-
-#if defined(OS_WIN)
-      have_process_times = false;
-      FILETIME win_creation_time;
-      FILETIME win_exit_time;
-      FILETIME win_kernel_time;
-      FILETIME win_user_time;
-      if (!GetProcessTimes(handle, &win_creation_time, &win_exit_time,
-                           &win_kernel_time, &win_user_time)) {
-        DWORD error = GetLastError();
-        DLOG(ERROR) << "Error getting process data" << error;
-        return;
-      }
-      user_duration = base::Time::FromFileTime(win_user_time) -
-          base::Time::Time();
-      kernel_duration = base::Time::FromFileTime(win_kernel_time) -
-          base::Time::Time();
-      run_duration = base::Time::FromFileTime(win_exit_time) -
-          base::Time::FromFileTime(win_creation_time);
-      have_process_times = true;
-#endif   // OS_WIN
+      this->status = status;
+      this->exit_code = exit_code;
+      this->was_alive = was_alive;
     }
-
-#if defined(OS_WIN)
-    base::TimeDelta kernel_duration;
-    base::TimeDelta user_duration;
-    base::TimeDelta run_duration;
-    bool have_process_times;
-#endif   // OS_WIN
-
     base::ProcessHandle handle;
     base::TerminationStatus status;
     int exit_code;