Compile in Breakpad by default on Linux

chrome/browser/chrome_browser_main_linux.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.
 
 #include "chrome/browser/chrome_browser_main_linux.h"
 
 #include "chrome/browser/system_monitor/media_transfer_protocol_device_observer_linux.h"
 #include "device/media_transfer_protocol/media_transfer_protocol_manager.h"
 
 #if !defined(OS_CHROMEOS)
@@ skipping 25 matching lines @@
 #if !defined(OS_CHROMEOS)
 void GetLinuxDistroCallback() {
   base::GetLinuxDistro();  // Initialize base::linux_distro if needed.
 }
 #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.
-  // 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);
   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);
   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)
 
 }  // namespace
 
 ChromeBrowserMainPartsLinux::ChromeBrowserMainPartsLinux(
     const content::MainFunctionParams& parameters)
     : ChromeBrowserMainPartsPosix(parameters),
       did_pre_profile_init_(false) {
@@ skipping 44 matching lines @@
   // Release it now. Otherwise the FILE thread would be gone when we try to
   // release it in the dtor and Valgrind would report a leak on almost ever
   // single browser_test.
   removable_device_notifications_linux_ = NULL;
 #endif
 
   media_transfer_protocol_device_observer_.reset();
 
   ChromeBrowserMainPartsPosix::PostMainMessageLoopRun();
 }