Histogram times surrounding render crashes

content/browser/renderer_host/render_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.
 
 // Represents the browser side of the browser <--> renderer communication
 // channel. There will be one RenderProcessHost per renderer process.
 
 #include "content/browser/renderer_host/render_process_host_impl.h"
 
 #if defined(OS_WIN)
@@ skipping 764 matching lines @@
 
   // Test if there's an unload listener.
   // NOTE: It's possible that an onunload listener may be installed
   // while we're shutting down, so there's a small race here.  Given that
   // the window is small, it's unlikely that the web page has much
   // state that will be lost by not calling its unload handlers properly.
   if (!SuddenTerminationAllowed())
     return false;
 
   // Store the handle before it gets changed.
   base::ProcessHandle handle = GetHandle();

[rvargas (doing something else), 2012/03/22 22:16:30]

nit: most of the time we don't keep an extra local variable for stuff like this.
I don't mind it that much, and it was already there... just saying.

[jar (doing other things), 2012/03/22 22:42:58]

Done.

-  ProcessDied(handle, base::TERMINATION_STATUS_NORMAL_TERMINATION, 0, false);
+  RendererClosedDetails details(handle);
+  details.status = base::TERMINATION_STATUS_NORMAL_TERMINATION;

[rvargas (doing something else), 2012/03/22 22:16:30]

these are the default values

[jar (doing other things), 2012/03/22 22:42:58]

Changed to a DCHECK to help with readability (not have to look at class
defaults).

+  details.exit_code = 0;
+  details.was_alive = false;
+  ProcessDied(&details);
   fast_shutdown_started_ = true;
   return true;
 }
 
 void RenderProcessHostImpl::DumpHandles() {
 #if defined(OS_WIN)
   Send(new ChildProcessMsg_DumpHandles());
   return;
 #endif
 
@@ skipping 143 matching lines @@
   tracked_objects::ThreadData::Status status =
       tracked_objects::ThreadData::status();
   Send(new ChildProcessMsg_SetProfilerStatus(status));
 }
 
 void RenderProcessHostImpl::OnChannelError() {
   if (!channel_.get())
     return;
 
   // Store the handle before it gets changed.
   base::ProcessHandle handle = GetHandle();

[rvargas (doing something else), 2012/03/22 22:16:30]

I'd also remove this one. In fact, can we add a TODO somewhere about removing
the handle from details? If the next line closes the handle, we should not keep
a stale value around.

[jar (doing other things), 2012/03/22 22:42:58]

Done.

-
+  RendererClosedDetails details(handle);
   // 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) :
+  details.status = child_process_launcher_.get() ?
+      child_process_launcher_->GetChildTerminationStatus(&details.exit_code) :
       base::TERMINATION_STATUS_NORMAL_TERMINATION;
 
 #if defined(OS_WIN)
   if (!run_renderer_in_process()) {
-    if (status == base::TERMINATION_STATUS_STILL_RUNNING) {
+    if (details.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);
-}
+  details.was_alive = false;
+  ProcessDied(&details);
+  }
 
 // 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);
+  RendererClosedDetails details(handle);
+  details.status = base::GetTerminationStatus(waitable_event->Release(),
+                                              &details.exit_code);
   delete waitable_event;
-  ProcessDied(handle, status, exit_code, true);
+  details.was_alive = true;
+  ProcessDied(&details);
 #endif
 }
 
 content::BrowserContext* RenderProcessHostImpl::GetBrowserContext() const {
   return browser_context_;
 }
 
 int RenderProcessHostImpl::GetID() const {
   return id_;
 }
@@ skipping 213 matching lines @@
   // Now pick a random suitable renderer, if we have any.
   if (!suitable_renderers.empty()) {
     int suitable_count = static_cast<int>(suitable_renderers.size());
     int random_index = base::RandInt(0, suitable_count - 1);
     return suitable_renderers[random_index];
   }
 
   return NULL;
 }
 
-void RenderProcessHostImpl::ProcessDied(base::ProcessHandle handle,
-                                           base::TerminationStatus status,
-                                           int exit_code,
-                                           bool was_alive) {
+void RenderProcessHostImpl::ProcessDied(RendererClosedDetails* details) {
   // 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();
   gpu_message_filter_ = NULL;
 
   IDMap<RenderWidgetHost>::iterator iter(&render_widget_hosts_);
   while (!iter.IsAtEnd()) {
     RenderWidgetHostImpl::From(iter.GetCurrentValue())->OnMessageReceived(
         ViewHostMsg_RenderViewGone(iter.GetCurrentKey(),
-                                   static_cast<int>(status),
-                                   exit_code));
+                                   static_cast<int>(details->status),
+                                   details->exit_code));
     iter.Advance();
   }
 
   ClearTransportDIBCache();
 
   // this object is not deleted at this point and may be reused later.
   // TODO(darin): clean this up
 }
 
 void RenderProcessHostImpl::OnShutdownRequest() {
@@ skipping 84 matching lines @@
 void RenderProcessHostImpl::OnRevealFolderInOS(const FilePath& path) {
   // Only honor the request if appropriate persmissions are granted.
   if (ChildProcessSecurityPolicyImpl::GetInstance()->CanReadFile(GetID(),
                                                                  path))
     content::GetContentClient()->browser()->OpenItem(path);
 }
 
 void RenderProcessHostImpl::OnSavedPageAsMHTML(int job_id, int64 data_size) {
   MHTMLGenerationManager::GetInstance()->MHTMLGenerated(job_id, data_size);
 }