Move init of breakpad in the browser process as early as possible

On Linux and Android, we delayed initialization until the browser
process was already up. That way, certain crash keys did not get
registered correctly.

Instead, use a similar logic than on mac and windows to determine
whether we can run breakpad.

In particular, this relies on the fact that the GetCollectStatsConsent()
setting reflects the policy enforced value.

R=mark@chromium.org, pastarmovj@chromium.org, rsesek@chromium.org
BUG=311877

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

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

14 files changed, 70 insertions, 120 deletions

diff --git a/base/base_switches.cc b/base/base_switches.cc
index d6b7d02..c6dce9b 100644
--- a/base/base_switches.cc
+++ b/base/base_switches.cc
@@ -54,4 +54,11 @@ const char kWaitForDebugger[]               = "wait-for-debugger";
 // Sends a pretty-printed version of tracing info to the console.
 const char kTraceToConsole[]                = "trace-to-console";
 
+#if defined(OS_POSIX)
+// Used for turning on Breakpad crash reporting in a debug environment where
+// crash reporting is typically compiled but disabled.
+const char kEnableCrashReporterForTesting[] =
+    "enable-crash-reporter-for-testing";
+#endif
+
 }  // namespace switches
diff --git a/base/base_switches.h b/base/base_switches.h
index 1d8a8a8..33b2b68 100644
--- a/base/base_switches.h
+++ b/base/base_switches.h
@@ -23,6 +23,10 @@ extern const char kVModule[];
 extern const char kWaitForDebugger[];
 extern const char kTraceToConsole[];
 
+#if defined(OS_POSIX)
+extern const char kEnableCrashReporterForTesting[];
+#endif
+
 }  // namespace switches
 
 #endif  // BASE_BASE_SWITCHES_H_
diff --git a/chrome/app/chrome_breakpad_client.cc b/chrome/app/chrome_breakpad_client.cc
index d8f9359..07de3fb 100644
--- a/chrome/app/chrome_breakpad_client.cc
+++ b/chrome/app/chrome_breakpad_client.cc
@@ -20,6 +20,7 @@
 #include "chrome/common/chrome_switches.h"
 #include "chrome/common/crash_keys.h"
 #include "chrome/common/env_vars.h"
+#include "chrome/installer/util/google_update_settings.h"
 
 #if defined(OS_WIN)
 #include <windows.h>
@@ -40,14 +41,15 @@
 #include "chrome/common/dump_without_crashing.h"
 #endif
 
-#if defined(OS_WIN) || defined(OS_MACOSX)
-#include "chrome/installer/util/google_update_settings.h"
-#endif
-
 #if defined(OS_ANDROID)
 #include "chrome/common/descriptors_android.h"
 #endif
 
+#if defined(OS_CHROMEOS)
+#include "chrome/common/chrome_version_info.h"
+#include "chromeos/chromeos_switches.h"
+#endif
+
 namespace chrome {
 
 namespace {
@@ -342,11 +344,27 @@ bool ChromeBreakpadClient::IsRunningUnattended() {
   return env->HasVar(env_vars::kHeadless);
 }
 
-#if defined(OS_WIN) || defined(OS_MACOSX)
 bool ChromeBreakpadClient::GetCollectStatsConsent() {
-  return GoogleUpdateSettings::GetCollectStatsConsent();
-}
+  // Convert #define to a variable so that we can use if() rather than
+  // #if below and so at least compile-test the Chrome code in
+  // Chromium builds.
+#if defined(GOOGLE_CHROME_BUILD)
+  bool is_chrome_build = true;
+#else
+  bool is_chrome_build = false;
+#endif
+
+#if defined(OS_CHROMEOS)
+  bool is_guest_session = CommandLine::ForCurrentProcess()->HasSwitch(
+      chromeos::switches::kGuestSession);
+  bool is_stable_channel =
+      chrome::VersionInfo::GetChannel() == chrome::VersionInfo::CHANNEL_STABLE;
+
+  if (is_guest_session && is_stable_channel)
+    return false;
 #endif
+  return is_chrome_build && GoogleUpdateSettings::GetCollectStatsConsent();
+}
 
 #if defined(OS_ANDROID)
 int ChromeBreakpadClient::GetAndroidMinidumpDescriptor() {
diff --git a/chrome/app/chrome_breakpad_client.h b/chrome/app/chrome_breakpad_client.h
index 78f54f6..10d0980 100644
--- a/chrome/app/chrome_breakpad_client.h
+++ b/chrome/app/chrome_breakpad_client.h
@@ -54,9 +54,9 @@ class ChromeBreakpadClient : public breakpad::BreakpadClient {
 
   virtual bool IsRunningUnattended() OVERRIDE;
 
-#if defined(OS_WIN) || defined(OS_MACOSX)
   virtual bool GetCollectStatsConsent() OVERRIDE;
 
+#if defined(OS_WIN) || defined(OS_MACOSX)
   virtual bool ReportingIsEnforcedByPolicy(bool* breakpad_enabled) OVERRIDE;
 #endif
 
diff --git a/chrome/app/chrome_main_delegate.cc b/chrome/app/chrome_main_delegate.cc
index 9731e98..80bde7e 100644
--- a/chrome/app/chrome_main_delegate.cc
+++ b/chrome/app/chrome_main_delegate.cc
@@ -702,17 +702,18 @@ void ChromeMainDelegate::PreSandboxStartup() {
   }
 
 #if defined(OS_POSIX) && !defined(OS_MACOSX)
-  // Needs to be called after we have chrome::DIR_USER_DATA.  BrowserMain
-  // sets this up for the browser process in a different manner. Zygotes
-  // need to call InitCrashReporter() in RunZygote().
-  if (!process_type.empty() && process_type != switches::kZygoteProcess) {
+  // Zygote needs to call InitCrashReporter() in RunZygote().
+  if (process_type != switches::kZygoteProcess) {
 #if defined(OS_ANDROID)
-    breakpad::InitNonBrowserCrashReporterForAndroid();
-#else
+    if (process_type.empty())
+      breakpad::InitCrashReporter();
+    else
+      breakpad::InitNonBrowserCrashReporterForAndroid();
+#else  // !defined(OS_ANDROID)
     breakpad::InitCrashReporter();
-#endif
+#endif  // defined(OS_ANDROID)
   }
-#endif
+#endif  // defined(OS_POSIX) && !defined(OS_MACOSX)
 
   // After all the platform Breakpads have been initialized, store the command
   // line for crash reporting.
diff --git a/chrome/browser/chrome_browser_main_android.cc b/chrome/browser/chrome_browser_main_android.cc
index 0acfb7e..61d6818 100644
--- a/chrome/browser/chrome_browser_main_android.cc
+++ b/chrome/browser/chrome_browser_main_android.cc
@@ -44,7 +44,6 @@ void ChromeBrowserMainPartsAndroid::PreProfileInit() {
           switches::kEnableCrashReporterForTesting);
 
   if (breakpad_enabled) {
-    breakpad::InitCrashReporter();
     base::FilePath crash_dump_dir;
     PathService::Get(chrome::DIR_CRASH_DUMPS, &crash_dump_dir);
     crash_dump_manager_.reset(new breakpad::CrashDumpManager(crash_dump_dir));
diff --git a/chrome/browser/chrome_browser_main_linux.cc b/chrome/browser/chrome_browser_main_linux.cc
index 835afa4..9ae5ec0 100644
--- a/chrome/browser/chrome_browser_main_linux.cc
+++ b/chrome/browser/chrome_browser_main_linux.cc
@@ -16,12 +16,7 @@
 #include "chrome/common/pref_names.h"
 #include "components/breakpad/app/breakpad_linux.h"
 
-#if defined(OS_CHROMEOS)
-#include "chrome/browser/chromeos/settings/cros_settings.h"
-#include "chrome/common/chrome_version_info.h"
-#include "chromeos/chromeos_switches.h"
-#include "chromeos/settings/cros_settings_names.h"
-#else
+#if !defined(OS_CHROMEOS)
 #include "chrome/browser/storage_monitor/storage_monitor_linux.h"
 #include "chrome/browser/sxs_linux.h"
 #include "content/public/browser/browser_thread.h"
@@ -35,68 +30,6 @@ void GetLinuxDistroCallback() {
 }
 #endif
 
-bool IsCrashReportingEnabled(const PrefService* local_state) {
-  // Check whether we should initialize the crash reporter. It may be disabled
-  // through configuration policy or user preference. It must be disabled for
-  // Guest mode on Chrome OS in Stable channel.
-  // Also allow crash reporting to be enabled with a command-line flag if the
-  // crash service is under control of the user. It is used by QA
-  // testing infrastructure to switch on generation of crash reports.
-  bool use_switch = true;
-
-  // Convert #define to a variable so that we can use if() rather than
-  // #if below and so at least compile-test the Chrome code in
-  // Chromium builds.
-#if defined(GOOGLE_CHROME_BUILD)
-  bool is_chrome_build = true;
-#else
-  bool is_chrome_build = false;
-#endif
-
-  // Check these settings in Chrome builds only, to reduce the chance
-  // that we accidentally upload crash dumps from Chromium builds.
-  bool breakpad_enabled = false;
-  if (is_chrome_build) {
-#if defined(OS_CHROMEOS)
-    bool is_guest_session = CommandLine::ForCurrentProcess()->HasSwitch(
-        chromeos::switches::kGuestSession);
-    bool is_stable_channel =
-        chrome::VersionInfo::GetChannel() ==
-        chrome::VersionInfo::CHANNEL_STABLE;
-    // TODO(pastarmovj): Consider the TrustedGet here.
-    bool reporting_enabled;
-    chromeos::CrosSettings::Get()->GetBoolean(chromeos::kStatsReportingPref,
-                                              &reporting_enabled);
-    breakpad_enabled =
-        !(is_guest_session && is_stable_channel) && reporting_enabled;
-#else
-    const PrefService::Preference* metrics_reporting_enabled =
-        local_state->FindPreference(prefs::kMetricsReportingEnabled);
-    CHECK(metrics_reporting_enabled);
-    breakpad_enabled = local_state->GetBoolean(prefs::kMetricsReportingEnabled);
-    use_switch = metrics_reporting_enabled->IsUserModifiable();
-#endif  // defined(OS_CHROMEOS)
-  }
-
-  if (use_switch) {
-    // Linux Breakpad interferes with the debug stack traces produced
-    // by EnableInProcessStackDumping(), used in browser_tests, so we
-    // do not allow CHROME_HEADLESS=1 to enable Breakpad in Chromium
-    // because the buildbots have CHROME_HEADLESS set.  However, we
-    // allow CHROME_HEADLESS to enable Breakpad in Chrome for
-    // compatibility with Breakpad/Chrome tests that may rely on this.
-    // TODO(mseaborn): Change tests to use --enable-crash-reporter-for-testing
-    // instead.
-    if (is_chrome_build && !breakpad_enabled)
-      breakpad_enabled = getenv(env_vars::kHeadless) != NULL;
-    if (!breakpad_enabled)
-      breakpad_enabled = CommandLine::ForCurrentProcess()->HasSwitch(
-            switches::kEnableCrashReporterForTesting);
-  }
-
-  return breakpad_enabled;
-}
-
 }  // namespace
 
 ChromeBrowserMainPartsLinux::ChromeBrowserMainPartsLinux(
@@ -121,9 +54,6 @@ void ChromeBrowserMainPartsLinux::PreProfileInit() {
       base::Bind(&sxs_linux::AddChannelMarkToUserDataDir));
 #endif
 
-  if (IsCrashReportingEnabled(local_state()))
-    breakpad::InitCrashReporter();
-
   ChromeBrowserMainPartsPosix::PreProfileInit();
 }
 
diff --git a/chrome/common/chrome_switches.cc b/chrome/common/chrome_switches.cc
index c4c1b86..6996107 100644
--- a/chrome/common/chrome_switches.cc
+++ b/chrome/common/chrome_switches.cc
@@ -1561,17 +1561,10 @@ const char kEnableCast[]                    = "enable-cast";
 const char kOpenAsh[]                       = "open-ash";
 #endif
 
-#if defined(OS_POSIX)
-// Used for turning on Breakpad crash reporting in a debug environment where
-// crash reporting is typically compiled but disabled.
-const char kEnableCrashReporterForTesting[] =
-    "enable-crash-reporter-for-testing";
-
-#if !defined(OS_MACOSX) && !defined(OS_CHROMEOS)
+#if defined(OS_POSIX) && !defined(OS_MACOSX) && !defined(OS_CHROMEOS)
 // Specifies which password store to use (detect, default, gnome, kwallet).
 const char kPasswordStore[]                 = "password-store";
 #endif
-#endif  // OS_POSIX
 
 #if defined(OS_LINUX) && !defined(OS_CHROMEOS)
 // Triggers migration of user data directory to another directory
diff --git a/chrome/common/chrome_switches.h b/chrome/common/chrome_switches.h
index e4ad28a..149507c 100644
--- a/chrome/common/chrome_switches.h
+++ b/chrome/common/chrome_switches.h
@@ -432,12 +432,9 @@ extern const char kEnableCast[];
 extern const char kOpenAsh[];
 #endif
 
-#if defined(OS_POSIX)
-extern const char kEnableCrashReporterForTesting[];
-#if !defined(OS_MACOSX) && !defined(OS_CHROMEOS)
+#if defined(OS_POSIX) && !defined(OS_MACOSX) && !defined(OS_CHROMEOS)
 extern const char kPasswordStore[];
 #endif
-#endif
 
 #if defined(OS_LINUX) && !defined(OS_CHROMEOS)
 extern const char kMigrateDataDirForSxS[];
diff --git a/components/breakpad/app/breakpad_client.cc b/components/breakpad/app/breakpad_client.cc
index 16f7dc7..c39d8f9 100644
--- a/components/breakpad/app/breakpad_client.cc
+++ b/components/breakpad/app/breakpad_client.cc
@@ -103,11 +103,11 @@ bool BreakpadClient::IsRunningUnattended() {
   return true;
 }
 
-#if defined(OS_WIN) || defined(OS_MACOSX)
 bool BreakpadClient::GetCollectStatsConsent() {
   return false;
 }
 
+#if defined(OS_WIN) || defined(OS_MACOSX)
 bool BreakpadClient::ReportingIsEnforcedByPolicy(bool* breakpad_enabled) {
   return false;
 }
diff --git a/components/breakpad/app/breakpad_client.h b/components/breakpad/app/breakpad_client.h
index 6a514d0..a92c0fb 100644
--- a/components/breakpad/app/breakpad_client.h
+++ b/components/breakpad/app/breakpad_client.h
@@ -119,10 +119,10 @@ class BreakpadClient {
   // Returns true if running in unattended mode (for automated testing).
   virtual bool IsRunningUnattended();
 
-#if defined(OS_WIN) || defined(OS_MACOSX)
   // Returns true if the user has given consent to collect stats.
   virtual bool GetCollectStatsConsent();
 
+#if defined(OS_WIN) || defined(OS_MACOSX)
   // Returns true if breakpad is enforced via management policies. In that
   // case, |breakpad_enabled| is set to the value enforced by policies.
   virtual bool ReportingIsEnforcedByPolicy(bool* breakpad_enabled);
diff --git a/components/breakpad/app/breakpad_linux.cc b/components/breakpad/app/breakpad_linux.cc
index 740213b..9106568 100644
--- a/components/breakpad/app/breakpad_linux.cc
+++ b/components/breakpad/app/breakpad_linux.cc
@@ -1455,6 +1455,19 @@ void InitCrashReporter() {
   const std::string process_type =
       parsed_command_line.GetSwitchValueASCII(switches::kProcessType);
   if (process_type.empty()) {
+    bool enable_breakpad = GetBreakpadClient()->GetCollectStatsConsent() ||
+                           GetBreakpadClient()->IsRunningUnattended();
+    enable_breakpad &=
+        !parsed_command_line.HasSwitch(switches::kDisableBreakpad);
+    if (!enable_breakpad) {
+      enable_breakpad = parsed_command_line.HasSwitch(
+          switches::kEnableCrashReporterForTesting);
+    }
+    if (!enable_breakpad) {
+      VLOG(1) << "Breakpad disabled";
+      return;
+    }
+
     EnableCrashDumping(GetBreakpadClient()->IsRunningUnattended());
   } else if (process_type == switches::kRendererProcess ||
              process_type == switches::kPluginProcess ||
diff --git a/content/shell/app/shell_main_delegate.cc b/content/shell/app/shell_main_delegate.cc
index f6f5247..7f1d570 100644
--- a/content/shell/app/shell_main_delegate.cc
+++ b/content/shell/app/shell_main_delegate.cc
@@ -206,9 +206,12 @@ void ShellMainDelegate::PreSandboxStartup() {
     std::string process_type =
         CommandLine::ForCurrentProcess()->GetSwitchValueASCII(
             switches::kProcessType);
-    if (!process_type.empty() && process_type != switches::kZygoteProcess) {
+    if (process_type != switches::kZygoteProcess) {
 #if defined(OS_ANDROID)
-      breakpad::InitNonBrowserCrashReporterForAndroid();
+      if (process_type.empty())
+        breakpad::InitCrashReporter();
+      else
+        breakpad::InitNonBrowserCrashReporterForAndroid();
 #else
       breakpad::InitCrashReporter();
 #endif
diff --git a/content/shell/browser/shell_browser_main_parts.cc b/content/shell/browser/shell_browser_main_parts.cc
index f78f066..8415ced 100644
--- a/content/shell/browser/shell_browser_main_parts.cc
+++ b/content/shell/browser/shell_browser_main_parts.cc
@@ -47,18 +47,6 @@
 #endif
 #endif
 
-#if defined(OS_MACOSX)
-#include "components/breakpad/app/breakpad_mac.h"
-#endif
-
-#if defined(OS_POSIX) && !defined(OS_MACOSX)
-#include "components/breakpad/app/breakpad_linux.h"
-#endif
-
-#if defined(OS_WIN)
-#include "components/breakpad/app/breakpad_win.h"
-#endif
-
 namespace content {
 
 namespace {
@@ -134,16 +122,13 @@ void ShellBrowserMainParts::PreEarlyInitialization() {
 }
 
 void ShellBrowserMainParts::PreMainMessageLoopRun() {
-#if defined(OS_POSIX) && !defined(OS_MACOSX)
+#if defined(OS_ANDROID)
   if (CommandLine::ForCurrentProcess()->HasSwitch(
           switches::kEnableCrashReporter)) {
-    breakpad::InitCrashReporter();
-#if defined(OS_ANDROID)
     base::FilePath crash_dumps_dir =
         CommandLine::ForCurrentProcess()->GetSwitchValuePath(
             switches::kCrashDumpsDir);
     crash_dump_manager_.reset(new breakpad::CrashDumpManager(crash_dumps_dir));
-#endif
   }
 #endif
   net_log_.reset(new ShellNetLog());