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 <[email protected]> Commit-Queue: Joshua Peraza <[email protected]>
sbahra · Apr 13, 2018 · c80bf96 · c80bf96
1 parent dd4ba4c
commit c80bf96
Show file tree

Hide file tree

Showing 6 changed files with 10 additions and 17 deletions.
diff --git a/client/crashpad_client_linux_test.cc b/client/crashpad_client_linux_test.cc
@@ -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() {

diff --git a/handler/crashpad_handler_test.cc b/handler/crashpad_handler_test.cc
@@ -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);
 

diff --git a/handler/linux/crash_report_exception_handler.cc b/handler/linux/crash_report_exception_handler.cc
@@ -140,9 +140,6 @@ bool CrashReportExceptionHandler::HandleExceptionWithConnection(
 
     if (upload_thread_) {
       upload_thread_->ReportPending(uuid);
-    } else {
-      database_->SkipReportUpload(
-          uuid, Metrics::CrashSkippedReason::kUploadsDisabled);
     }
   }
 

diff --git a/handler/mac/crash_report_exception_handler.cc b/handler/mac/crash_report_exception_handler.cc
@@ -189,9 +189,6 @@ kern_return_t CrashReportExceptionHandler::CatchMachException(
 
     if (upload_thread_) {
       upload_thread_->ReportPending(uuid);
-    } else {
-      database_->SkipReportUpload(
-          uuid, Metrics::CrashSkippedReason::kUploadsDisabled);
     }
   }
 

diff --git a/handler/win/crash_report_exception_handler.cc b/handler/win/crash_report_exception_handler.cc
@@ -126,9 +126,6 @@ unsigned int CrashReportExceptionHandler::ExceptionHandlerServerException(
 
     if (upload_thread_) {
       upload_thread_->ReportPending(uuid);
-    } else {
-      database_->SkipReportUpload(
-          uuid, Metrics::CrashSkippedReason::kUploadsDisabled);
     }
   }
 

diff --git a/snapshot/win/end_to_end_test.py b/snapshot/win/end_to_end_test.py
@@ -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():