Enable breakpad building by default on Android.

After https://chromiumcodereview.appspot.com/10407058 we can compile
breakpad by default and still have it disabled for non-official builds.

Changes the Android build to allow compiling breakpad but not use
it by not creating the crash fd and not passing it to the renderer
process unless breakpad is enabled.

Changes linux and Android to use a switch for enabling breakpad
since that's a lot easier to test with on android then an environment
variable.

BUG=105778,170530


Review URL: https://chromiumcodereview.appspot.com/11969025

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

14 files changed, 52 insertions, 47 deletions

diff --git a/build/common.gypi b/build/common.gypi
index 21412d4..7681ae4 100644
--- a/build/common.gypi
+++ b/build/common.gypi
@@ -1181,16 +1181,6 @@
         # Always use the chromium skia.
         'use_system_skia%': '0',
 
-        # Configure crash reporting and build options based on release type.
-        'conditions': [
-          ['buildtype=="Official"', {
-            # Only report crash dumps for Official builds.
-            'linux_breakpad%': 1,
-          }, {
-            'linux_breakpad%': 0,
-          }],
-        ],
-
         # When building as part of the Android system, use system libraries
         # where possible to reduce ROM size.
         # TODO(steveblock): Investigate using the system version of sqlite.
diff --git a/chrome/app/breakpad_linux.cc b/chrome/app/breakpad_linux.cc
index b4cca7b..0643993 100644
--- a/chrome/app/breakpad_linux.cc
+++ b/chrome/app/breakpad_linux.cc
@@ -27,6 +27,7 @@
 #include "base/file_path.h"
 #include "base/linux_util.h"
 #include "base/path_service.h"
+#include "base/platform_file.h"
 #include "base/posix/eintr_wrapper.h"
 #include "base/posix/global_descriptors.h"
 #include "base/process_util.h"
@@ -50,6 +51,7 @@
 
 #include "base/android/build_info.h"
 #include "base/android/path_utils.h"
+#include "chrome/common/descriptors_android.h"
 #include "third_party/lss/linux_syscall_support.h"
 #else
 #include "sandbox/linux/seccomp-legacy/linux_syscall_support.h"
@@ -1468,10 +1470,20 @@ void InitCrashReporter() {
 }
 
 #if defined(OS_ANDROID)
-void InitNonBrowserCrashReporterForAndroid(int minidump_fd) {
+void InitNonBrowserCrashReporterForAndroid() {
   const CommandLine* command_line = CommandLine::ForCurrentProcess();
-  if (command_line->HasSwitch(switches::kEnableCrashReporter))
-    EnableNonBrowserCrashDumping(minidump_fd);
+  if (command_line->HasSwitch(switches::kEnableCrashReporter)) {
+    // On Android we need to provide a FD to the file where the minidump is
+    // generated as the renderer and browser run with different UIDs
+    // (preventing the browser from inspecting the renderer process).
+    int minidump_fd = base::GlobalDescriptors::GetInstance()->
+        MaybeGet(kAndroidMinidumpDescriptor);
+    if (minidump_fd == base::kInvalidPlatformFileValue) {
+      NOTREACHED() << "Could not find minidump FD, crash reporting disabled.";
+    } else {
+      EnableNonBrowserCrashDumping(minidump_fd);
+    }
+  }
 }
 #endif  // OS_ANDROID
 
diff --git a/chrome/app/breakpad_linux.h b/chrome/app/breakpad_linux.h
index f589479..8f46926 100644
--- a/chrome/app/breakpad_linux.h
+++ b/chrome/app/breakpad_linux.h
@@ -11,7 +11,7 @@
 
 extern void InitCrashReporter();
 #if defined(OS_ANDROID)
-extern void InitNonBrowserCrashReporterForAndroid(int minidump_fd);
+extern void InitNonBrowserCrashReporterForAndroid();
 #endif
 bool IsCrashReporterEnabled();
 
diff --git a/chrome/app/chrome_main_delegate.cc b/chrome/app/chrome_main_delegate.cc
index 03ff32e..08735fc 100644
--- a/chrome/app/chrome_main_delegate.cc
+++ b/chrome/app/chrome_main_delegate.cc
@@ -645,16 +645,7 @@ void ChromeMainDelegate::PreSandboxStartup() {
   // need to call InitCrashReporter() in RunZygote().
   if (!process_type.empty() && process_type != switches::kZygoteProcess) {
 #if defined(OS_ANDROID)
-    // On Android we need to provide a FD to the file where the minidump is
-    // generated as the renderer and browser run with different UIDs
-    // (preventing the browser from inspecting the renderer process).
-    int minidump_fd = base::GlobalDescriptors::GetInstance()->
-        MaybeGet(kAndroidMinidumpDescriptor);
-    if (minidump_fd == base::kInvalidPlatformFileValue) {
-      NOTREACHED() << "Could not find minidump FD, crash reporting disabled.";
-    } else {
-      InitNonBrowserCrashReporterForAndroid(minidump_fd);
-    }
+    InitNonBrowserCrashReporterForAndroid();
 #else
     InitCrashReporter();
 #endif
diff --git a/chrome/browser/chrome_browser_main_android.cc b/chrome/browser/chrome_browser_main_android.cc
index 42dd90f..8220fac 100644
--- a/chrome/browser/chrome_browser_main_android.cc
+++ b/chrome/browser/chrome_browser_main_android.cc
@@ -4,10 +4,11 @@
 
 #include "chrome/browser/chrome_browser_main_android.h"
 
+#include "base/command_line.h"
 #include "base/path_service.h"
 #include "chrome/app/breakpad_linux.h"
 #include "chrome/browser/android/crash_dump_manager.h"
-#include "chrome/common/env_vars.h"
+#include "chrome/common/chrome_switches.h"
 #include "content/public/browser/android/compositor.h"
 #include "content/public/common/main_function_params.h"
 #include "net/android/network_change_notifier_factory_android.h"
@@ -33,9 +34,11 @@ void ChromeBrowserMainPartsAndroid::PreProfileInit() {
 #else
   bool breakpad_enabled = false;
 #endif
+
   // Allow Breakpad to be enabled in Chromium builds for testing purposes.
   if (!breakpad_enabled)
-    breakpad_enabled = getenv(env_vars::kEnableBreakpad) != NULL;
+    breakpad_enabled = CommandLine::ForCurrentProcess()->HasSwitch(
+          switches::kEnableCrashReporterForTesting);
 
   if (breakpad_enabled) {
     InitCrashReporter();
diff --git a/chrome/browser/chrome_browser_main_linux.cc b/chrome/browser/chrome_browser_main_linux.cc
index ac48ad2..ec39b18 100644
--- a/chrome/browser/chrome_browser_main_linux.cc
+++ b/chrome/browser/chrome_browser_main_linux.cc
@@ -14,9 +14,11 @@
 #if defined(USE_LINUX_BREAKPAD)
 #include <stdlib.h>
 
+#include "base/command_line.h"
 #include "base/linux_util.h"
 #include "chrome/app/breakpad_linux.h"
 #include "chrome/browser/prefs/pref_service.h"
+#include "chrome/common/chrome_switches.h"
 #include "chrome/common/env_vars.h"
 #include "chrome/common/pref_names.h"
 #include "content/public/browser/browser_thread.h"
@@ -43,10 +45,10 @@ 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.
-  // Environment variables may override the decision, but only if the
+  // 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_env_var = true;
+  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
@@ -78,23 +80,24 @@ bool IsCrashReportingEnabled(const PrefService* local_state) {
         local_state->FindPreference(prefs::kMetricsReportingEnabled);
     CHECK(metrics_reporting_enabled);
     breakpad_enabled = local_state->GetBoolean(prefs::kMetricsReportingEnabled);
-    use_env_var = metrics_reporting_enabled->IsUserModifiable();
+    use_switch = metrics_reporting_enabled->IsUserModifiable();
 #endif  // defined(OS_CHROMEOS)
   }
 
-  if (use_env_var) {
+  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 CHROME_ENABLE_BREAKPAD
+    // 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 = getenv(env_vars::kEnableBreakpad) != NULL;
+      breakpad_enabled = CommandLine::ForCurrentProcess()->HasSwitch(
+            switches::kEnableCrashReporterForTesting);
   }
 
   return breakpad_enabled;
diff --git a/chrome/browser/chrome_content_browser_client.cc b/chrome/browser/chrome_content_browser_client.cc
index d27c0b2..4e6c2cb 100644
--- a/chrome/browser/chrome_content_browser_client.cc
+++ b/chrome/browser/chrome_content_browser_client.cc
@@ -1840,13 +1840,15 @@ void ChromeContentBrowserClient::GetAdditionalMappedFilesForChildProcess(
                                          FileDescriptor(f, true)));
 
 #if defined(USE_LINUX_BREAKPAD)
-  f = CrashDumpManager::GetInstance()->CreateMinidumpFile(child_process_id);
-  if (f == base::kInvalidPlatformFileValue) {
-    LOG(ERROR) << "Failed to create file for minidump, crash reporting will be "
-        "disabled for this process.";
-  } else {
-    mappings->push_back(FileDescriptorInfo(kAndroidMinidumpDescriptor,
-                                           FileDescriptor(f, true)));
+  if (IsCrashReporterEnabled()) {
+    f = CrashDumpManager::GetInstance()->CreateMinidumpFile(child_process_id);
+    if (f == base::kInvalidPlatformFileValue) {
+      LOG(ERROR) << "Failed to create file for minidump, crash reporting will "
+                 "be disabled for this process.";
+    } else {
+      mappings->push_back(FileDescriptorInfo(kAndroidMinidumpDescriptor,
+                                             FileDescriptor(f, true)));
+    }
   }
 #endif  // defined(USE_LINUX_BREAKPAD)
 
diff --git a/chrome/common/chrome_switches.cc b/chrome/common/chrome_switches.cc
index 76fc60a..4246316 100644
--- a/chrome/common/chrome_switches.cc
+++ b/chrome/common/chrome_switches.cc
@@ -1548,6 +1548,11 @@ const char kOobeSkipPostLogin[]             = "oobe-skip-postlogin";
 // files needed to make this decision.
 const char kEnableCrashReporter[]           = "enable-crash-reporter";
 
+// 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)
 // Specifies which password store to use (detect, default, gnome, kwallet).
 const char kPasswordStore[]                 = "password-store";
diff --git a/chrome/common/chrome_switches.h b/chrome/common/chrome_switches.h
index 520abcf..acec940 100644
--- a/chrome/common/chrome_switches.h
+++ b/chrome/common/chrome_switches.h
@@ -431,6 +431,7 @@ extern const char kOobeSkipPostLogin[];
 
 #if defined(OS_POSIX)
 extern const char kEnableCrashReporter[];
+extern const char kEnableCrashReporterForTesting[];
 #if !defined(OS_MACOSX) && !defined(OS_CHROMEOS)
 extern const char kPasswordStore[];
 #endif
diff --git a/chrome/common/env_vars.cc b/chrome/common/env_vars.cc
index c678126..06e10f8 100644
--- a/chrome/common/env_vars.cc
+++ b/chrome/common/env_vars.cc
@@ -6,11 +6,6 @@
 
 namespace env_vars {
 
-// Enable Breakpad crash reporting.  This is used for automated
-// testing of Breakpad in Chromium builds where Breakpad is compiled
-// in by default but not usually enabled.
-const char kEnableBreakpad[] = "CHROME_ENABLE_BREAKPAD";
-
 // We call running in unattended mode (for automated testing) "headless".
 // This mode can be enabled using this variable or by the kNoErrorDialogs
 // switch.
diff --git a/chrome/common/env_vars.h b/chrome/common/env_vars.h
index 19ea5b0..0b3ba6f 100644
--- a/chrome/common/env_vars.h
+++ b/chrome/common/env_vars.h
@@ -9,7 +9,6 @@
 
 namespace env_vars {
 
-extern const char kEnableBreakpad[];
 extern const char kHeadless[];
 extern const char kLogFileName[];
 extern const char kSessionLogDir[];
diff --git a/ppapi/native_client/tests/breakpad_crash_test/crash_dump_tester.py b/ppapi/native_client/tests/breakpad_crash_test/crash_dump_tester.py
index cc3d14b..0963887 100755
--- a/ppapi/native_client/tests/breakpad_crash_test/crash_dump_tester.py
+++ b/ppapi/native_client/tests/breakpad_crash_test/crash_dump_tester.py
@@ -141,8 +141,7 @@ def Main(cleanup_funcs):
     # dumps to a temporary directory.
     home_dir = temp_dir
     os.environ['HOME'] = home_dir
-    # On Linux, we also need to set CHROME_ENABLE_BREAKPAD.
-    os.environ['CHROME_ENABLE_BREAKPAD'] = '1'
+    options.enable_crash_reporter = True
 
   result = browser_tester.Run(options.url, options)
 
diff --git a/ppapi/native_client/tools/browser_tester/browser_tester.py b/ppapi/native_client/tools/browser_tester/browser_tester.py
index 3b740a3..061a138 100755
--- a/ppapi/native_client/tools/browser_tester/browser_tester.py
+++ b/ppapi/native_client/tools/browser_tester/browser_tester.py
@@ -136,6 +136,9 @@ def BuildArgParser():
                     action='store_true',
                     help='Do not signal a failure if the browser process '
                     'crashes')
+  parser.add_option('--enable_crash_reporter', dest='enable_crash_reporter',
+                    action='store_true', default=False,
+                    help='Force crash reporting on.')
 
   return parser
 
diff --git a/ppapi/native_client/tools/browser_tester/browsertester/browserlauncher.py b/ppapi/native_client/tools/browser_tester/browsertester/browserlauncher.py
index d8bf9ac..1b25f203 100755
--- a/ppapi/native_client/tools/browser_tester/browsertester/browserlauncher.py
+++ b/ppapi/native_client/tools/browser_tester/browsertester/browserlauncher.py
@@ -305,6 +305,8 @@ class ChromeLauncher(BrowserLauncher):
       cmd.append('--load-extension=%s' %
                  ','.join(self.options.browser_extensions))
       cmd.append('--enable-experimental-extension-apis')
+    if self.options.enable_crash_reporter:
+      cmd.append('--enable-crash-reporter-for-testing')
     if self.options.tool == 'memcheck':
       cmd = ['src/third_party/valgrind/memcheck.sh',
              '-v',