Add extra parameter to BrowserChildProcessCrashed to pass the exit_code at...

Add extra parameter to BrowserChildProcessCrashed to pass the exit_code at time of crash/termination.

Wire this into all the existing callers.

Add a new child process watcher in chrome to be notified on child process crashes. This will confirm whether users with RESULT_CODE_INVALID_SANDBOX_STATE are crashing on background start (--no-startup-window) or not. This also adds required framework to paint a sad tab UI from this class in the future.

BUG=472324, 453541
TEST=Apply patch in http://pastebin.com/uEDxngBa and verify histogram ChildProcess.InvalidSandboxStateCrash.NoStartupWindow is recorded.

Review URL: https://codereview.chromium.org/1067733006

Cr-Commit-Position: refs/heads/master@{#327380}

chrome/browser/browser_process_impl.cc

@@ -27,6 +27,7 @@
 #include "base/time/default_tick_clock.h"
 #include "base/trace_event/trace_event.h"
 #include "chrome/browser/chrome_browser_main.h"
+#include "chrome/browser/chrome_child_process_watcher.h"
 #include "chrome/browser/chrome_content_browser_client.h"
 #include "chrome/browser/chrome_notification_types.h"
 #include "chrome/browser/component_updater/chrome_component_updater_configurator.h"
@@ -273,6 +274,8 @@ void BrowserProcessImpl::StartTearDown() {
   // the IO thread.
   promo_resource_service_.reset();
 
+  child_process_watcher_.reset();
+
 #if !defined(OS_ANDROID)
   // Debugger must be cleaned up before IO thread and NotificationService.
   remote_debugging_server_.reset();
@@ -1061,6 +1064,8 @@ void BrowserProcessImpl::PreMainMessageLoopRun() {
   storage_monitor::StorageMonitor::Create();
 #endif
 
+  child_process_watcher_.reset(new ChromeChildProcessWatcher());
+
   platform_part_->PreMainMessageLoopRun();
 }
 

chrome/browser/browser_process_impl.h

@@ -21,6 +21,7 @@
 #include "base/timer/timer.h"
 #include "chrome/browser/browser_process.h"
 
+class ChromeChildProcessWatcher;
 class ChromeDeviceClient;
 class ChromeNetLog;
 class ChromeResourceDispatcherHostDelegate;
@@ -301,6 +302,8 @@ class BrowserProcessImpl : public BrowserProcess,
 
   scoped_ptr<gcm::GCMDriver> gcm_driver_;
 
+  scoped_ptr<ChromeChildProcessWatcher> child_process_watcher_;
+
 #if !defined(OS_ANDROID)
   scoped_ptr<ChromeDeviceClient> device_client_;
 #endif

chrome/browser/chrome_child_process_watcher.cc

+// Copyright 2015 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 "chrome/browser/chrome_child_process_watcher.h"
+
+#include "base/command_line.h"
+#include "base/metrics/histogram.h"
+#include "chrome/common/chrome_result_codes.h"
+#include "chrome/common/chrome_switches.h"
+#include "content/public/browser/browser_child_process_observer.h"
+#include "content/public/browser/child_process_data.h"
+
+namespace {
+
+void AnalyzeCrash(int exit_code) {
+  if (exit_code == chrome::RESULT_CODE_INVALID_SANDBOX_STATE) {
+    base::CommandLine* command_line = base::CommandLine::ForCurrentProcess();
+    const bool no_startup_window =
+        command_line->HasSwitch(switches::kNoStartupWindow);
+    UMA_HISTOGRAM_BOOLEAN(
+        "ChildProcess.InvalidSandboxStateCrash.NoStartupWindow",
+        no_startup_window);
+  }
+}
+
+}  // namespace
+
+ChromeChildProcessWatcher::ChromeChildProcessWatcher() {
+  BrowserChildProcessObserver::Add(this);
+}
+
+ChromeChildProcessWatcher::~ChromeChildProcessWatcher() {
+  BrowserChildProcessObserver::Remove(this);
+}
+
+void ChromeChildProcessWatcher::BrowserChildProcessCrashed(
+    const content::ChildProcessData& data,
+    int exit_code) {
+  AnalyzeCrash(exit_code);
+}

chrome/browser/chrome_child_process_watcher.h

+// Copyright 2015 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.
+
+#ifndef CHROME_BROWSER_CHROME_CHILD_PROCESS_WATCHER_H_
+#define CHROME_BROWSER_CHROME_CHILD_PROCESS_WATCHER_H_
+
+#include "base/basictypes.h"
+#include "content/public/browser/browser_child_process_observer.h"
+
+// ChromeChildProcessWatcher watches for crashed child processes.
+class ChromeChildProcessWatcher : public content::BrowserChildProcessObserver {
+ public:
+  ChromeChildProcessWatcher();
+  ~ChromeChildProcessWatcher() override;
+
+ private:
+  // content::BrowserChildProcessObserver:
+  void BrowserChildProcessCrashed(const content::ChildProcessData& data,
+                                  int exit_code) override;
+
+  DISALLOW_COPY_AND_ASSIGN(ChromeChildProcessWatcher);
+};
+
+#endif  // CHROME_BROWSER_CHROME_CHILD_PROCESS_WATCHER_H_

chrome/browser/metrics/chrome_stability_metrics_provider.cc

@@ -236,7 +236,8 @@ void ChromeStabilityMetricsProvider::Observe(
 }
 
 void ChromeStabilityMetricsProvider::BrowserChildProcessCrashed(
-    const content::ChildProcessData& data) {
+    const content::ChildProcessData& data,
+    int exit_code) {
 #if defined(ENABLE_PLUGINS)
   // Exclude plugin crashes from the count below because we report them via
   // a separate UMA metric.

chrome/browser/metrics/chrome_stability_metrics_provider.h

@@ -48,7 +48,8 @@ class ChromeStabilityMetricsProvider
 
   // content::BrowserChildProcessObserver:
   void BrowserChildProcessCrashed(
-      const content::ChildProcessData& data) override;
+      const content::ChildProcessData& data,
+      int exit_code) override;
 
   // Logs the initiation of a page load and uses |web_contents| to do
   // additional logging of the type of page loaded.

chrome/browser/metrics/plugin_metrics_provider.cc

@@ -353,7 +353,8 @@ void PluginMetricsProvider::BrowserChildProcessHostConnected(
 }
 
 void PluginMetricsProvider::BrowserChildProcessCrashed(
-    const content::ChildProcessData& data) {
+    const content::ChildProcessData& data,
+    int exit_code) {
   GetChildProcessStats(data).process_crashes++;
   RecordCurrentStateWithDelay(kRecordStateDelayMs);
 }

chrome/browser/metrics/plugin_metrics_provider.h

@@ -90,8 +90,8 @@ class PluginMetricsProvider : public metrics::MetricsProvider,
   // content::BrowserChildProcessObserver:
   void BrowserChildProcessHostConnected(
       const content::ChildProcessData& data) override;
-  void BrowserChildProcessCrashed(
-      const content::ChildProcessData& data) override;
+  void BrowserChildProcessCrashed(const content::ChildProcessData& data,
+                                  int exit_code) override;
   void BrowserChildProcessInstanceCreated(
       const content::ChildProcessData& data) override;
 

chrome/chrome_browser.gypi

@@ -268,6 +268,8 @@
       'browser/chrome_browser_main_mac.mm',
       'browser/chrome_browser_main_win.cc',
       'browser/chrome_browser_main_win.h',
+      'browser/chrome_child_process_watcher.cc',
+      'browser/chrome_child_process_watcher.h',
       'browser/chrome_content_browser_client.cc',
       'browser/chrome_content_browser_client.h',
       'browser/chrome_content_browser_client_parts.h',

chromecast/browser/metrics/cast_stability_metrics_provider.cc

@@ -140,7 +140,8 @@ void CastStabilityMetricsProvider::Observe(
 }
 
 void CastStabilityMetricsProvider::BrowserChildProcessCrashed(
-    const content::ChildProcessData& data) {
+    const content::ChildProcessData& data,
+    int exit_code) {
   IncrementPrefValue(prefs::kStabilityChildProcessCrashCount);
 }
 

chromecast/browser/metrics/cast_stability_metrics_provider.h

@@ -57,7 +57,8 @@ class CastStabilityMetricsProvider
 
   // content::BrowserChildProcessObserver implementation:
   void BrowserChildProcessCrashed(
-      const content::ChildProcessData& data) override;
+      const content::ChildProcessData& data,
+      int exit_code) override;
 
   // Records a renderer process crash.
   void LogRendererCrash(content::RenderProcessHost* host,

components/crash/browser/crash_dump_manager_android.cc

@@ -126,7 +126,8 @@ void CrashDumpManager::BrowserChildProcessHostDisconnected(
 }
 
 void CrashDumpManager::BrowserChildProcessCrashed(
-    const content::ChildProcessData& data) {
+    const content::ChildProcessData& data,
+    int exit_code) {
   OnChildExit(data.id, data.handle);
 }
 

components/crash/browser/crash_dump_manager_android.h

@@ -55,7 +55,8 @@ class CrashDumpManager : public content::BrowserChildProcessObserver,
   void BrowserChildProcessHostDisconnected(
       const content::ChildProcessData& data) override;
   void BrowserChildProcessCrashed(
-      const content::ChildProcessData& data) override;
+      const content::ChildProcessData& data,
+      int exit_code) override;
 
   // NotificationObserver implementation:
   void Observe(int type,

content/browser/bootstrap_sandbox_mac.cc

@@ -31,7 +31,9 @@ class BootstrapSandboxPolicy : public BrowserChildProcessObserver {
   // BrowserChildProcessObserver:
   void BrowserChildProcessHostDisconnected(
       const ChildProcessData& data) override;
-  void BrowserChildProcessCrashed(const ChildProcessData& data) override;
+  void BrowserChildProcessCrashed(
+      const ChildProcessData& data,
+      int exit_code) override;
 
  private:
   friend struct DefaultSingletonTraits<BootstrapSandboxPolicy>;
@@ -53,7 +55,8 @@ void BootstrapSandboxPolicy::BrowserChildProcessHostDisconnected(
 }
 
 void BootstrapSandboxPolicy::BrowserChildProcessCrashed(
-      const ChildProcessData& data) {
+      const ChildProcessData& data,
+      int exit_code) {
   sandbox()->ChildDied(data.handle);
 }
 

content/browser/browser_child_process_host_impl.cc

@@ -53,9 +53,9 @@ void NotifyProcessHostDisconnected(const ChildProcessData& data) {
                     BrowserChildProcessHostDisconnected(data));
 }
 
-void NotifyProcessCrashed(const ChildProcessData& data) {
+void NotifyProcessCrashed(const ChildProcessData& data, int exit_code) {
   FOR_EACH_OBSERVER(BrowserChildProcessObserver, g_observers.Get(),
-                    BrowserChildProcessCrashed(data));
+                    BrowserChildProcessCrashed(data, exit_code));
 }
 
 }  // namespace
@@ -282,8 +282,9 @@ void BrowserChildProcessHostImpl::OnChildDisconnected() {
       case base::TERMINATION_STATUS_PROCESS_CRASHED:
       case base::TERMINATION_STATUS_ABNORMAL_TERMINATION: {
         delegate_->OnProcessCrashed(exit_code);
-        BrowserThread::PostTask(BrowserThread::UI, FROM_HERE,
-                                base::Bind(&NotifyProcessCrashed, data_));
+        BrowserThread::PostTask(
+            BrowserThread::UI, FROM_HERE,
+            base::Bind(&NotifyProcessCrashed, data_, exit_code));
         UMA_HISTOGRAM_ENUMERATION("ChildProcess.Crashed2",
                                   data_.process_type,
                                   PROCESS_TYPE_MAX);

content/browser/mach_broker_mac.h

@@ -69,7 +69,8 @@ class CONTENT_EXPORT MachBroker : public base::ProcessMetrics::PortProvider,
   // Implement |BrowserChildProcessObserver|.
   void BrowserChildProcessHostDisconnected(
       const ChildProcessData& data) override;
-  void BrowserChildProcessCrashed(const ChildProcessData& data) override;
+  void BrowserChildProcessCrashed(const ChildProcessData& data,
+      int exit_code) override;
 
   // Implement |NotificationObserver|.
   void Observe(int type,

content/browser/mach_broker_mac.mm

@@ -217,7 +217,8 @@ void MachBroker::BrowserChildProcessHostDisconnected(
   InvalidateChildProcessId(data.id);
 }
 
-void MachBroker::BrowserChildProcessCrashed(const ChildProcessData& data) {
+void MachBroker::BrowserChildProcessCrashed(const ChildProcessData& data,
+    int exit_code) {
   InvalidateChildProcessId(data.id);
 }
 

content/public/browser/browser_child_process_observer.h

@@ -24,7 +24,9 @@ class CONTENT_EXPORT BrowserChildProcessObserver {
       const ChildProcessData& data) {}
 
   // Called when a child process disappears unexpectedly as a result of a crash.
-  virtual void BrowserChildProcessCrashed(const ChildProcessData& data) {}
+  // |exit_code| contains the exit code from the process.
+  virtual void BrowserChildProcessCrashed(const ChildProcessData& data,
+                                          int exit_code) {}
 
   // Called when an instance of a particular child is created in a page. If one
   // page contains several regions rendered by the same child, this will be

content/public/browser/child_process_data.h

@@ -30,9 +30,15 @@ struct ChildProcessData {
   // current process.
   base::ProcessHandle handle;
 
+  // The exit code for the process. This is only valid for processes that have
+  // already exited e.g. when receiving a BrowserChildProcessCrashed callback.
+  int exit_code;
+
   explicit ChildProcessData(int process_type)
-      : process_type(process_type), id(0), handle(base::kNullProcessHandle) {
-  }
+      : process_type(process_type),
+        id(0),
+        handle(base::kNullProcessHandle),
+        exit_code(0) {}
 };
 
 }  // namespace content

tools/metrics/histograms/histograms.xml

@@ -2772,6 +2772,17 @@ Therefore, the affected-histogram name has to have at least one dot in it.
   </summary>
 </histogram>
 
+<histogram name="ChildProcess.InvalidSandboxStateCrash.NoStartupWindow"
+    enum="BooleanPresent">
+  <owner>wfh@chromium.org</owner>
+  <summary>
+    Whether the browser command line had the switch --no-startup-window when a
+    child process crashed due to invalid sandbox state. Recorded when a child
+    process crashes if the exit code from the child process is
+    RESULT_CODE_INVALID_SANDBOX_STATE.
+  </summary>
+</histogram>
+
 <histogram name="ChildProcess.Killed" enum="ProcessType">
   <obsolete>
     Deprecated 3/2013. Renamed to ChildProcess.Killed2.