Compile in Breakpad by default on Linux

chrome/browser/chrome_browser_main_linux.cc

Index: chrome/browser/chrome_browser_main_linux.cc
diff --git a/chrome/browser/chrome_browser_main_linux.cc b/chrome/browser/chrome_browser_main_linux.cc
index 2bf9e3a8c3cc37cf4ef27ca3d03bd0f39a46c7ca..728d62d1e7536446196fd6250bbd357bbb88d5d8 100644
--- a/chrome/browser/chrome_browser_main_linux.cc
+++ b/chrome/browser/chrome_browser_main_linux.cc
@@ -43,9 +43,11 @@ 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.
-  // The kHeadless environment variable overrides the decision, but only if the
-  // crash service is under control of the user. It is used by QA testing
-  // infrastructure to switch on generation of crash reports.
+  // Environment variables may override the decision, but only 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_env_var = true;
+
 #if defined(OS_CHROMEOS)
   bool is_guest_session =
       CommandLine::ForCurrentProcess()->HasSwitch(switches::kGuestSession);
@@ -57,17 +59,32 @@ bool IsCrashReportingEnabled(const PrefService* local_state) {
                                             &reporting_enabled);
   bool breakpad_enabled =
       !(is_guest_session && is_stable_channel) && reporting_enabled;
-  if (!breakpad_enabled)
-    breakpad_enabled = getenv(env_vars::kHeadless) != NULL;
 #else
   const PrefService::Preference* metrics_reporting_enabled =
       local_state->FindPreference(prefs::kMetricsReportingEnabled);
   CHECK(metrics_reporting_enabled);
   bool breakpad_enabled =

[Lei Zhang, 2013/01/09 22:36:49]

I'm concerned that a Chromium build can have this pref set, for whatever reason.
With this CL, the Chromium build will start sending crash reports. We should set
this to false for Chromium builds.

For buildbots, it shouldn't matter, since they will enable Breakpad via the
mechanisms below.

[Mark Seaborn, 2013/01/10 00:48:28]

To clarify, is your concern about when Chromium is run as normal desktop app, or
about when Chromium code is run by tests on the bots that might enable metrics
reporting (e.g. to test UMA) but that doesn't intend to enable Breakpad?

I've made this code conditional.

[Lei Zhang, 2013/01/10 00:58:49]

My concern is about users running it on their desktop. I doubt bots will have
metrics enabled.

[Mark Seaborn, 2013/01/10 01:23:02]

Oh, I thought you'd say the opposite.  I'm not so worried about the case of
users running Chromium on their desktops, because the UMA/Breakpad option is not
supposed to be displayed in Chromium.  If crash reporting is turned on without
the user seeing the option, there are bigger problems, because crash reporting
is only supposed to happen if the user consents.

       local_state->GetBoolean(prefs::kMetricsReportingEnabled);
-  if (!breakpad_enabled && metrics_reporting_enabled->IsUserModifiable())
-    breakpad_enabled = getenv(env_vars::kHeadless) != NULL;
+  use_env_var = metrics_reporting_enabled->IsUserModifiable();
 #endif  // defined(OS_CHROMEOS)
+
+  if (use_env_var) {
+    // 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 CHROME_ENABLE_BREAKPAD
+    // instead.
+#if defined(GOOGLE_CHROME_BUILD)
+    if (!breakpad_enabled)
+      breakpad_enabled = getenv(env_vars::kHeadless) != NULL;
+#endif
+    if (!breakpad_enabled)
+      breakpad_enabled = getenv("CHROME_ENABLE_BREAKPAD") != NULL;

[Lei Zhang, 2013/01/09 22:36:49]

Likely some other pieces of C++ code will use this, so put it in
chrome/common/env_vars.h?

[Mark Seaborn, 2013/01/10 00:48:28]

OK, done.

+  }
+
   return breakpad_enabled;
 }
 #endif  // defined(USE_LINUX_BREAKPAD)