Don't record reports as complete if there is no upload thread

This allows clients to use the database to handle uploads themselves,
e.g. on Android, where Crashpad does not yet provide an uploader.

The handler does not launch an upload thread when no url is supplied.
Previously, the handler would move these reports to
completed and record the upload as skipped with kUploadsDisabled.
With this change, these reports would remain pending until pruned,
with no metrics recorded for them in regard to their upload.

Bug: crashpad:30
Change-Id: I4167ab1531634b10e91d03229018ae6aab4103aa
Reviewed-on: https://chromium-review.googlesource.com/1010970
Reviewed-by: Robert Sesek <rsesek@chromium.org>
Commit-Queue: Joshua Peraza <jperaza@chromium.org>
This commit is contained in:
Joshua Peraza 2018-04-12 19:37:06 -07:00 committed by Commit Bot
parent dd4ba4c8a1
commit c80bf96001
6 changed files with 10 additions and 17 deletions

View File

@ -85,12 +85,12 @@ TEST(CrashpadClient, SimulateCrash) {
std::vector<CrashReportDatabase::Report> reports;
ASSERT_EQ(database->GetPendingReports(&reports),
CrashReportDatabase::kNoError);
EXPECT_EQ(reports.size(), 0u);
EXPECT_EQ(reports.size(), 1u);
reports.clear();
ASSERT_EQ(database->GetCompletedReports(&reports),
CrashReportDatabase::kNoError);
EXPECT_EQ(reports.size(), 1u);
EXPECT_EQ(reports.size(), 0u);
}
}
@ -147,11 +147,12 @@ class StartHandlerAtCrashTest : public MultiprocessExec {
std::vector<CrashReportDatabase::Report> reports;
ASSERT_EQ(database->GetPendingReports(&reports),
CrashReportDatabase::kNoError);
EXPECT_EQ(reports.size(), 0u);
EXPECT_EQ(reports.size(), 1u);
reports.clear();
ASSERT_EQ(database->GetCompletedReports(&reports),
CrashReportDatabase::kNoError);
EXPECT_EQ(reports.size(), 1u);
EXPECT_EQ(reports.size(), 0u);
}
DISALLOW_COPY_AND_ASSIGN(StartHandlerAtCrashTest);
@ -213,11 +214,12 @@ class StartHandlerForClientTest {
std::vector<CrashReportDatabase::Report> reports;
ASSERT_EQ(database->GetPendingReports(&reports),
CrashReportDatabase::kNoError);
EXPECT_EQ(reports.size(), 0u);
EXPECT_EQ(reports.size(), 1u);
reports.clear();
ASSERT_EQ(database->GetCompletedReports(&reports),
CrashReportDatabase::kNoError);
EXPECT_EQ(reports.size(), 1u);
EXPECT_EQ(reports.size(), 0u);
}
bool InstallHandler() {

View File

@ -93,7 +93,7 @@ void CrashWithExtendedHandler::ValidateGeneratedDump() {
ASSERT_TRUE(database);
std::vector<CrashReportDatabase::Report> reports;
ASSERT_EQ(database->GetCompletedReports(&reports),
ASSERT_EQ(database->GetPendingReports(&reports),
CrashReportDatabase::kNoError);
ASSERT_EQ(reports.size(), 1u);

View File

@ -140,9 +140,6 @@ bool CrashReportExceptionHandler::HandleExceptionWithConnection(
if (upload_thread_) {
upload_thread_->ReportPending(uuid);
} else {
database_->SkipReportUpload(
uuid, Metrics::CrashSkippedReason::kUploadsDisabled);
}
}

View File

@ -189,9 +189,6 @@ kern_return_t CrashReportExceptionHandler::CatchMachException(
if (upload_thread_) {
upload_thread_->ReportPending(uuid);
} else {
database_->SkipReportUpload(
uuid, Metrics::CrashSkippedReason::kUploadsDisabled);
}
}

View File

@ -126,9 +126,6 @@ unsigned int CrashReportExceptionHandler::ExceptionHandlerServerException(
if (upload_thread_) {
upload_thread_->ReportPending(uuid);
} else {
database_->SkipReportUpload(
uuid, Metrics::CrashSkippedReason::kUploadsDisabled);
}
}

View File

@ -155,7 +155,7 @@ def GetDumpFromProgram(
out = subprocess.check_output([
os.path.join(out_dir, 'crashpad_database_util.exe'),
'--database=' + test_database,
'--show-completed-reports',
'--show-pending-reports',
'--show-all-report-info',
])
for line in out.splitlines():