Don't ignore chrome crashes when autotesting.
If the file "/mnt/stateful_partition/etc/collect_chrome_crashes" exists on
the target, allow chrome crashes to be reported by crash reporter. The
intention is that the autotest system will create this file. This is a
relatively temporary fix to help track down chrome problems, until the
crashes can be handled by crash reporter in the field as well.
BUG=chromium-os:17987, chromium-os:17898
TEST=Ran unit tests.
Change-Id: I68a584a0b861669117d8e97f5687b4c8fc876011
Reviewed-on: http://gerrit.chromium.org/gerrit/4861
Reviewed-by: Ken Mixter <kmixter@chromium.org>
Tested-by: Ken Mixter <kmixter@chromium.org>
diff --git a/crash_reporter/user_collector.cc b/crash_reporter/user_collector.cc
index 5d4a1b1..da42655 100644
--- a/crash_reporter/user_collector.cc
+++ b/crash_reporter/user_collector.cc
@@ -40,6 +40,8 @@
static const char kCorePipeLimit[] = "4";
static const char kCoreToMinidumpConverterPath[] = "/usr/bin/core2md";
static const char kLeaveCoreFile[] = "/root/.leave_core";
+static const char kCollectChromeFile[] =
+ "/mnt/stateful_partition/etc/collect_chrome_crashes";
static const char kDefaultLogConfig[] = "/etc/crash_reporter_logs.conf";
@@ -398,7 +400,7 @@
exec,
minidump_path.value());
- if (!file_util::PathExists(FilePath(kLeaveCoreFile))) {
+ if (!IsDeveloperImage()) {
file_util::Delete(core_path, false);
} else {
LOG(INFO) << "Leaving core file at " << core_path.value()
@@ -416,9 +418,32 @@
return re.FullMatch(crash_attributes, pid, signal, kernel_supplied_name);
}
+bool UserCollector::IsDeveloperImage() {
+ // If we're testing crash reporter itself, we don't want to special-case
+ // for developer images.
+ if (IsCrashTestInProgress())
+ return false;
+ return file_util::PathExists(FilePath(kLeaveCoreFile));
+}
+
+bool UserCollector::ShouldIgnoreChromeCrashes() {
+ // If we're testing crash reporter itself, we don't want to allow an
+ // override for chrome crashes. And, let's be conservative and only
+ // allow an override for developer images.
+ if (!IsCrashTestInProgress() && IsDeveloperImage()) {
+ // Check if there's an override to indicate we should indeed collect
+ // chrome crashes. This allows the crashes to still be tracked when
+ // they occur in autotests. See "crosbug.com/17987".
+ if (file_util::PathExists(FilePath(kCollectChromeFile)))
+ return false;
+ }
+ // We default to ignoring chrome crashes.
+ return true;
+}
+
bool UserCollector::ShouldDump(bool has_owner_consent,
bool is_developer,
- bool is_crash_test_in_progress,
+ bool ignore_chrome_crashes,
const std::string &exec,
std::string *reason) {
reason->clear();
@@ -426,7 +451,8 @@
// Treat Chrome crashes as if the user opted-out. We stop counting Chrome
// crashes towards user crashes, so user crashes really mean non-Chrome
// user-space crashes.
- if (exec == "chrome" || exec == "supplied_chrome") {
+ if ((exec == "chrome" || exec == "supplied_chrome") &&
+ ignore_chrome_crashes) {
*reason = "ignoring - chrome crash";
return false;
}
@@ -434,7 +460,7 @@
// For developer builds, we always want to keep the crash reports unless
// we're testing the crash facilities themselves. This overrides
// feedback. Crash sending still obeys consent.
- if (is_developer && !is_crash_test_in_progress) {
+ if (is_developer) {
*reason = "developer build - not testing - always dumping";
return true;
}
@@ -485,8 +511,8 @@
std::string reason;
bool dump = ShouldDump(is_feedback_allowed_function_(),
- file_util::PathExists(FilePath(kLeaveCoreFile)),
- IsCrashTestInProgress(),
+ IsDeveloperImage(),
+ ShouldIgnoreChromeCrashes(),
exec,
&reason);
diff --git a/crash_reporter/user_collector.h b/crash_reporter/user_collector.h
index a231dd6..55fee8d 100644
--- a/crash_reporter/user_collector.h
+++ b/crash_reporter/user_collector.h
@@ -117,9 +117,15 @@
bool ParseCrashAttributes(const std::string &crash_attributes,
pid_t *pid, int *signal,
std::string *kernel_supplied_name);
+
+ // Returns true if we should consider ourselves to be running on a
+ // developer image.
+ bool IsDeveloperImage();
+ // Returns true if chrome crashes should be ignored.
+ bool ShouldIgnoreChromeCrashes();
bool ShouldDump(bool has_owner_consent,
bool is_developer,
- bool is_crash_test_in_progress,
+ bool ignore_chrome_crashes,
const std::string &exec,
std::string *reason);
diff --git a/crash_reporter/user_collector_test.cc b/crash_reporter/user_collector_test.cc
index 1a5c8c9..b18783f 100644
--- a/crash_reporter/user_collector_test.cc
+++ b/crash_reporter/user_collector_test.cc
@@ -124,30 +124,37 @@
TEST_F(UserCollectorTest, ShouldDumpDeveloperImageOverridesConsent) {
std::string reason;
- EXPECT_TRUE(collector_.ShouldDump(false, true, false,
+ EXPECT_TRUE(collector_.ShouldDump(false, true, true,
"chrome-wm", &reason));
EXPECT_EQ("developer build - not testing - always dumping", reason);
// When running a crash test, behave as normal.
- EXPECT_FALSE(collector_.ShouldDump(false, true, true,
+ EXPECT_FALSE(collector_.ShouldDump(false, false, true,
"chrome-wm", &reason));
EXPECT_EQ("ignoring - no consent", reason);
}
TEST_F(UserCollectorTest, ShouldDumpChromeOverridesDeveloperImage) {
std::string reason;
- EXPECT_FALSE(collector_.ShouldDump(false, true, false,
+ // When running a crash test, behave as normal.
+ EXPECT_FALSE(collector_.ShouldDump(false, false, true,
"chrome", &reason));
EXPECT_EQ("ignoring - chrome crash", reason);
+
+ // When in developer mode, test that chrome crashes are not ignored.
+ EXPECT_TRUE(collector_.ShouldDump(false, true, false,
+ "chrome", &reason));
+ EXPECT_EQ("developer build - not testing - always dumping",
+ reason);
}
TEST_F(UserCollectorTest, ShouldDumpUseConsentProductionImage) {
std::string result;
- EXPECT_FALSE(collector_.ShouldDump(false, false, false,
+ EXPECT_FALSE(collector_.ShouldDump(false, false, true,
"chrome-wm", &result));
EXPECT_EQ("ignoring - no consent", result);
- EXPECT_TRUE(collector_.ShouldDump(true, false, false,
+ EXPECT_TRUE(collector_.ShouldDump(true, false, true,
"chrome-wm", &result));
EXPECT_EQ("handling", result);
}