Add new termination status for failed launch

chrome/browser/metrics/chrome_stability_metrics_provider.cc

 // Copyright 2014 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/metrics/chrome_stability_metrics_provider.h"
 
 #include <vector>
 
 #include "base/logging.h"
 #include "base/metrics/histogram.h"
 #include "base/metrics/sparse_histogram.h"
 #include "base/prefs/pref_registry_simple.h"
 #include "base/prefs/pref_service.h"
 #include "chrome/browser/browser_process.h"
 #include "chrome/browser/chrome_notification_types.h"
 #include "chrome/common/chrome_constants.h"
+#include "chrome/common/chrome_result_codes.h"
 #include "chrome/common/pref_names.h"
 #include "components/metrics/proto/system_profile.pb.h"
 #include "content/public/browser/child_process_data.h"
 #include "content/public/browser/notification_service.h"
 #include "content/public/browser/render_process_host.h"
 #include "content/public/browser/user_metrics.h"
 #include "content/public/browser/web_contents.h"
 
 #if defined(ENABLE_EXTENSIONS)
 #include "extensions/browser/process_map.h"
@@ skipping 32 matching lines @@
 // Converts an exit code into something that can be inserted into our
 // histograms (which expect non-negative numbers less than MAX_INT).
 int MapCrashExitCodeForHistogram(int exit_code) {
 #if defined(OS_WIN)
   // Since |abs(STATUS_GUARD_PAGE_VIOLATION) == MAX_INT| it causes problems in
   // histograms.cc. Solve this by remapping it to a smaller value, which
   // hopefully doesn't conflict with other codes.
   if (exit_code == STATUS_GUARD_PAGE_VIOLATION)
     return 0x1FCF7EC3;  // Randomly picked number.
 #endif
-
+  if (exit_code == -1)
+    return chrome::RESULT_CODE_RENDERER_PROCESS_START_FAILURE;
   return std::abs(exit_code);
 }
 
 #if defined(OS_WIN)
 void CountBrowserCrashDumpAttempts() {
   enum Outcome {
     OUTCOME_SUCCESS,
     OUTCOME_FAILURE,
     OUTCOME_UNKNOWN,
     OUTCOME_MAX_VALUE
@@ skipping 192 matching lines @@
     content::RenderProcessHost* host,
     base::TerminationStatus status,
     int exit_code) {
   bool was_extension_process = false;
 #if defined(ENABLE_EXTENSIONS)
   was_extension_process =
       extensions::ProcessMap::Get(host->GetBrowserContext())->Contains(
           host->GetID());
 #endif
   if (status == base::TERMINATION_STATUS_PROCESS_CRASHED ||
-      status == base::TERMINATION_STATUS_ABNORMAL_TERMINATION) {
+      status == base::TERMINATION_STATUS_ABNORMAL_TERMINATION ||
+      (status == base::TERMINATION_STATUS_PROCESS_WAS_KILLED &&

[Alexei Svitkine (slow), 2015/08/28 16:45:50]

Probably worth adding a unit test for - e.g. call LogRendererCrash() with given
params and check that a crash is recorded.

[Will Harris, 2015/09/01 01:15:38]

Done.

+       exit_code == -1)) {

[Will Harris, 2015/08/28 16:41:11]

reviewer note: this error code comes from

RenderProcessHostImpl::OnProcessLaunchFailed()

[Alexei Svitkine (slow), 2015/08/28 16:45:50]

Can this be made a constant that both places can refer to?

Is that what you meant to do with RESULT_CODE_RENDERER_PROCESS_START_FAILURE?

[Will Harris, 2015/08/28 16:52:13]

render_process_host_impl.cc is in content so I'd have to make a new content
result code to return this value. These are content/public/common/result_codes.h
and there's not many of them.

I chose to put the result code in chrome/ as it's a "psuedo" resultcode i.e. the
process doesn't actually have a real exit code, it just represents the failure
to start so we can see it on chrome histograms.

tbh not entirely sure what is best here perhaps jam has an opinion as well.

[jam, 2015/08/28 17:18:35]

It seems a pseudo code can be in both modules. Since content already knows about
it, i agree better to have one place for the constant instead of duplicating it.

     if (was_extension_process) {
       IncrementPrefValue(prefs::kStabilityExtensionRendererCrashCount);
 
       UMA_HISTOGRAM_SPARSE_SLOWLY("CrashExitCodes.Extension",
                                   MapCrashExitCodeForHistogram(exit_code));
     } else {
       IncrementPrefValue(prefs::kStabilityRendererCrashCount);
 
       UMA_HISTOGRAM_SPARSE_SLOWLY("CrashExitCodes.Renderer",
                                   MapCrashExitCodeForHistogram(exit_code));
@@ skipping 16 matching lines @@
 #endif
   } else if (status == base::TERMINATION_STATUS_STILL_RUNNING) {
     UMA_HISTOGRAM_PERCENTAGE("BrowserRenderProcessHost.DisconnectedAlive",
                              was_extension_process ? 2 : 1);
   }
 }
 
 void ChromeStabilityMetricsProvider::LogRendererHang() {
   IncrementPrefValue(prefs::kStabilityRendererHangCount);
 }