Commit 8c54d542 authored by Bo Liu's avatar Bo Liu Committed by Commit Bot

android: Avoid duplicating OnCrashDumpProcessed

CrashMetricsReporter::CrashDumpProcessed can be called multiple times
for the same process; see the comment in
ChildProcessCrashObserver::OnChildExit.

Previously we avoid duplicating collecting stats by checking that if
the dump is missing, meaning it has already been crashed before.

This mostly still works except for NotifyObserver call which is still
called under kMissingDump, so fix that.

Change-Id: I2145a0df8c604739f86fb9ab9cecf845168dcbfe
Reviewed-on: https://chromium-review.googlesource.com/1148558Reviewed-by: default avatarRobert Sesek <rsesek@chromium.org>
Reviewed-by: default avatarSiddhartha S <ssid@chromium.org>
Commit-Queue: Bo <boliu@chromium.org>
Cr-Commit-Position: refs/heads/master@{#577930}
parent cb16c9cb
...@@ -129,34 +129,6 @@ void NoOpUploader::TryToUploadCrashDump(const base::FilePath& crash_dump_path) { ...@@ -129,34 +129,6 @@ void NoOpUploader::TryToUploadCrashDump(const base::FilePath& crash_dump_path) {
base::Unretained(test_harness_))); base::Unretained(test_harness_)));
} }
TEST_F(CrashDumpManagerTest, NoDumpCreated) {
base::HistogramTester histogram_tester;
CrashDumpManager* manager = CrashDumpManager::GetInstance();
CrashMetricsReporterObserver observer;
crash_reporter::CrashMetricsReporter::GetInstance()->AddObserver(&observer);
crash_reporter::ChildExitObserver::TerminationInfo termination_info;
termination_info.process_host_id = 1;
termination_info.pid = base::kNullProcessHandle;
termination_info.process_type = content::PROCESS_TYPE_RENDERER;
termination_info.app_state =
base::android::APPLICATION_STATE_HAS_RUNNING_ACTIVITIES;
termination_info.normal_termination = false;
termination_info.binding_state = base::android::ChildBindingState::STRONG;
termination_info.was_killed_intentionally_by_browser = false;
termination_info.was_oom_protected_status = true;
base::PostTaskWithTraits(
FROM_HERE, {base::MayBlock(), base::TaskPriority::BACKGROUND},
base::BindOnce(&CrashDumpManager::ProcessMinidumpFileFromChild,
base::Unretained(manager), base::FilePath(),
termination_info));
observer.WaitForProcessed();
histogram_tester.ExpectTotalCount("Tab.RendererDetailedExitStatus", 0);
EXPECT_EQ(0, dumps_uploaded_);
}
TEST_F(CrashDumpManagerTest, NonOomCrash) { TEST_F(CrashDumpManagerTest, NonOomCrash) {
base::HistogramTester histogram_tester; base::HistogramTester histogram_tester;
......
...@@ -112,10 +112,9 @@ void CrashMetricsReporter::CrashDumpProcessed( ...@@ -112,10 +112,9 @@ void CrashMetricsReporter::CrashDumpProcessed(
const ChildExitObserver::TerminationInfo& info, const ChildExitObserver::TerminationInfo& info,
breakpad::CrashDumpManager::CrashDumpStatus status) { breakpad::CrashDumpManager::CrashDumpStatus status) {
ReportedCrashTypeSet reported_counts; ReportedCrashTypeSet reported_counts;
if (status == breakpad::CrashDumpManager::CrashDumpStatus::kMissingDump) { // Avoid duplicating processing for the same process.
NotifyObservers(info.process_host_id, reported_counts); if (status == breakpad::CrashDumpManager::CrashDumpStatus::kMissingDump)
return; return;
}
bool has_valid_dump = false; bool has_valid_dump = false;
switch (status) { switch (status) {
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment