[forensics] Delete CrashReporter::File

thomasgales · Rebase bot · commit beded4d96790 · 2024-01-11T01:02:26.000Z
fuchsia.feedback/CrashReporter::File was marked removed in F12. F12 is no longer supported so this function can be deleted. Fixed: b/42073371 Test: `fx test //src/developer/forensics` Test: `fx shell crasher` @ daf95b989056649b Change-Id: Iaa3a705c0199e3be28eb9aea57e6dffdca67454f Reviewed-on: https://fuchsia-review.googlesource.com/c/fuchsia/+/968113 Reviewed-by: Christopher Johnson <crjohns@google.com> Reviewed-by: Alex Pankhurst <pankhurst@google.com> Reviewed-by: Chris Suter <csuter@google.com> Reviewed-by: James Sullivan <jfsulliv@google.com> API-Review: Chris Suter <csuter@google.com> Reviewed-by: Étienne J. Membrives <etiennej@google.com> Commit-Queue: Thomas Gales <tgales@google.com>
diff --git a/sdk/fidl/fuchsia.feedback/crash_reporter.fidl b/sdk/fidl/fuchsia.feedback/crash_reporter.fidl
@@ -9,24 +9,6 @@ using zx;
 /// Provides the ability to file crash reports.
 @discoverable
 closed protocol CrashReporter {
-    /// Files a crash `report`.
-    ///
-    /// This could mean generating a crash report in a local crash report database or uploading the
-    /// crash report to a remote crash server depending on the FIDL server's configuration.
-    ///
-    /// # Deprecation
-    ///
-    /// This method returns an immediate acknowledgement that the request was
-    /// received. In practice this acknowledgement isn't used and only returns
-    /// an error if the program name is blank. By F12, FileReport should be
-    /// used instead. FileReport will provide a useful status once the report
-    /// has been successfully or unsuccessfully filed.
-    // TODO(https://fxbug.dev/122422): delete File once F12 is out of support window.
-    @available(deprecated=11, removed=12, legacy=true, note="use FileReport")
-    strict File(resource struct {
-        report CrashReport;
-    }) -> () error zx.Status;
-
     /// Files a crash `report` and gives the final result of the operation.
     ///
     /// This could mean generating a crash report in a local crash report
diff --git a/sdk/fidl/fuchsia.feedback/fuchsia.feedback.api b/sdk/fidl/fuchsia.feedback/fuchsia.feedback.api
@@ -1,3 +1,3 @@
 {
-  "fidl/fuchsia.feedback": "83c2d9fdd458d45f3c04d35ef2b2fe48"
+  "fidl/fuchsia.feedback": "a7f935274ba8eb263147ea661b3727a9"
 }
diff --git a/src/developer/forensics/crash_reports/crash_reporter.cc b/src/developer/forensics/crash_reports/crash_reporter.cc
@@ -154,22 +154,6 @@ void CrashReporter::PersistAllCrashReports() {
   snapshot_collector_.Shutdown();
 }
 
-void CrashReporter::File(fuchsia::feedback::CrashReport report, FileCallback callback) {
-  if (!report.has_program_name()) {
-    FX_LOGS(ERROR) << "Input report missing required program name. Won't file.";
-    callback(::fpromise::error(ZX_ERR_INVALID_ARGS));
-    info_.LogCrashState(cobalt::CrashState::kDropped);
-    return;
-  }
-
-  // Execute the callback informing the client the report has been filed. The rest of the async flow
-  // can take quite some time and blocking clients would defeat the purpose of sharing the snapshot.
-  callback(::fpromise::ok());
-
-  File(std::move(report), /*is_hourly_snapshot=*/false,
-       [](const CrashReporter_FileReport_Result& result) {});
-}
-
 void CrashReporter::FileReport(fuchsia::feedback::CrashReport report, FileReportCallback callback) {
   if (!report.has_program_name()) {
     FX_LOGS(ERROR) << "Input report missing required program name. Won't file.";
diff --git a/src/developer/forensics/crash_reports/crash_reporter.h b/src/developer/forensics/crash_reports/crash_reporter.h
@@ -52,9 +52,6 @@ class CrashReporter : public fuchsia::feedback::CrashReporter {
   // reports.
   void PersistAllCrashReports();
 
-  // |fuchsia::feedback::CrashReporter|
-  void File(fuchsia::feedback::CrashReport report, FileCallback callback) override;
-
   // |fuchsia::feedback::CrashReporter|
   void FileReport(fuchsia::feedback::CrashReport report, FileReportCallback callback) override;
 
diff --git a/src/developer/forensics/crash_reports/tests/crash_reporter_unittest.cc b/src/developer/forensics/crash_reports/tests/crash_reporter_unittest.cc
@@ -465,30 +465,6 @@ TEST_F(CrashReporterTest, Succeed_OnInputCrashReport) {
   CheckAttachmentsOnServer({kDefaultAttachmentBundleKey});
 }
 
-// TODO(https://fxbug.dev/117123): delete when CrashReporter::File is removed.
-TEST_F(CrashReporterTest, Succeed_OnInputCrashReport_OldFile) {
-  SetUpDataProviderServer(
-      std::make_unique<stubs::DataProvider>(kFeedbackAnnotations, kDefaultAttachmentBundleKey));
-  SetUpCrashReporterDefaultConfig({kUploadSuccessful});
-
-  CrashReport report;
-  report.set_program_name(kProgramName);
-
-  // Run loop to start the clock.
-  RunLoopUntilIdle();
-  std::optional<::fpromise::result<void, zx_status_t>> out_result{std::nullopt};
-  crash_reporter_->File(std::move(report),
-                        [&out_result](::fpromise::result<void, zx_status_t> result) {
-                          out_result = std::move(result);
-                        });
-  RunLoopFor(kSnapshotSharedRequestWindow);
-  FX_CHECK(out_result.has_value());
-  ASSERT_TRUE(out_result->is_ok());
-
-  CheckAnnotationsOnServer();
-  CheckAttachmentsOnServer({kDefaultAttachmentBundleKey});
-}
-
 TEST_F(CrashReporterTest, EnforcesQuota) {
   SetUpDataProviderServer(
       std::make_unique<stubs::DataProvider>(kFeedbackAnnotations, kDefaultAttachmentBundleKey));
@@ -722,24 +698,6 @@ TEST_F(CrashReporterTest, Fail_OnInvalidInputCrashReport) {
   EXPECT_EQ(result.err(), FilingError::INVALID_ARGS_ERROR);
 }
 
-// TODO(https://fxbug.dev/117123): delete when CrashReporter::File is removed.
-TEST_F(CrashReporterTest, Fail_OnInvalidInputCrashReport_OldFile) {
-  SetUpDataProviderServer(std::make_unique<stubs::DataProviderReturnsEmptySnapshot>());
-  SetUpCrashReporterDefaultConfig();
-
-  // Run loop to start the clock.
-  RunLoopUntilIdle();
-  CrashReport report;
-  std::optional<::fpromise::result<void, zx_status_t>> out_result{std::nullopt};
-  crash_reporter_->File(std::move(report),
-                        [&out_result](::fpromise::result<void, zx_status_t> result) {
-                          out_result = std::move(result);
-                        });
-  RunLoopFor(kSnapshotSharedRequestWindow);
-  ASSERT_TRUE(out_result.has_value());
-  EXPECT_TRUE(out_result->is_error());
-}
-
 TEST_F(CrashReporterTest, Succeed_OnInputCrashReportWithIsFatalTrue) {
   SetUpDataProviderServer(
       std::make_unique<stubs::DataProvider>(kFeedbackAnnotations, kDefaultAttachmentBundleKey));
diff --git a/src/developer/forensics/exceptions/tests/crash_reporter_unittest.cc b/src/developer/forensics/exceptions/tests/crash_reporter_unittest.cc
@@ -45,14 +45,6 @@ constexpr zx::duration kDefaultTimeout{zx::duration::infinite()};
 
 class StubCrashReporter : public fuchsia::feedback::CrashReporter {
  public:
-  void File(fuchsia::feedback::CrashReport report, FileCallback callback) override {
-    reports_.push_back(std::move(report));
-
-    fuchsia::feedback::CrashReporter_File_Result result;
-    result.set_response({});
-    callback(std::move(result));
-  }
-
   void FileReport(fuchsia::feedback::CrashReport report, FileReportCallback callback) override {
     reports_.push_back(std::move(report));
 
diff --git a/src/developer/forensics/public/fakes/crash_reporter.cc b/src/developer/forensics/public/fakes/crash_reporter.cc
@@ -17,19 +17,6 @@ namespace fakes {
 
 using namespace fuchsia::feedback;
 
-void CrashReporter::File(CrashReport report, FileCallback callback) {
-  if (!report.has_program_name()) {
-    callback(CrashReporter_File_Result::WithErr(ZX_ERR_INVALID_ARGS));
-  } else {
-    callback(CrashReporter_File_Result::WithResponse(CrashReporter_File_Response()));
-  }
-
-  ++num_crash_reports_filed_;
-  if (querier_) {
-    querier_->UpdateAndNotify(num_crash_reports_filed_);
-  }
-}
-
 void CrashReporter::FileReport(CrashReport report, FileReportCallback callback) {
   if (!report.has_program_name()) {
     callback(fpromise::error(FilingError::INVALID_ARGS_ERROR));
diff --git a/src/developer/forensics/public/fakes/crash_reporter.h b/src/developer/forensics/public/fakes/crash_reporter.h
@@ -24,9 +24,6 @@ class CrashReporter : public fuchsia::feedback::CrashReporter {
       fidl::InterfaceRequest<fuchsia::feedback::testing::FakeCrashReporterQuerier> request);
   void ResetQuerier();
 
-  // |fuchsia::feedback::CrashReporter|
-  void File(fuchsia::feedback::CrashReport report, FileCallback callback) override;
-
   // |fuchsia::feedback::CrashReporter|
   void FileReport(fuchsia::feedback::CrashReport report, FileReportCallback callback) override;
 
diff --git a/src/developer/forensics/testing/stubs/crash_reporter.cc b/src/developer/forensics/testing/stubs/crash_reporter.cc
@@ -54,18 +54,6 @@ CrashReporter::~CrashReporter() {
       << ErrorMessage("is fatal", ToString(is_fatal_), ToString(expectations_.is_fatal));
 }
 
-void CrashReporter::File(fuchsia::feedback::CrashReport report, FileCallback callback) {
-  FileReport(std::move(report),
-             [callback = std::move(callback)](
-                 const fuchsia::feedback::CrashReporter_FileReport_Result& result) {
-               if (result.is_err()) {
-                 callback(fpromise::error(ZX_ERR_INTERNAL));
-               } else {
-                 callback(fpromise::ok());
-               }
-             });
-}
-
 void CrashReporter::FileReport(fuchsia::feedback::CrashReport report, FileReportCallback callback) {
   FX_CHECK(report.has_crash_signature());
   FX_CHECK(report.has_attachments());
@@ -98,21 +86,11 @@ void CrashReporter::FileReport(fuchsia::feedback::CrashReport report, FileReport
   callback(fpromise::ok(std::move(results)));
 }
 
-void CrashReporterAlwaysReturnsError::File(fuchsia::feedback::CrashReport report,
-                                           FileCallback callback) {
-  callback(::fpromise::error(ZX_ERR_INTERNAL));
-}
-
 void CrashReporterAlwaysReturnsError::FileReport(fuchsia::feedback::CrashReport report,
                                                  FileReportCallback callback) {
   callback(::fpromise::error(fuchsia::feedback::FilingError::INVALID_ARGS_ERROR));
 }
 
-void CrashReporterNoFileExpected::File(fuchsia::feedback::CrashReport report,
-                                       FileCallback callback) {
-  FX_CHECK(false) << "No call to File() expected";
-}
-
 void CrashReporterNoFileExpected::FileReport(fuchsia::feedback::CrashReport report,
                                              FileReportCallback callback) {
   FX_CHECK(false) << "No call to FileReport() expected";
diff --git a/src/developer/forensics/testing/stubs/crash_reporter.h b/src/developer/forensics/testing/stubs/crash_reporter.h
@@ -31,9 +31,6 @@ class CrashReporter : public CrashReporterBase {
 
   ~CrashReporter();
 
-  // |fuchsia::feedback::CrashReporter|
-  void File(fuchsia::feedback::CrashReport report, FileCallback callback) override;
-
   // |fuchsia::feedback::CrashReporter|
   void FileReport(fuchsia::feedback::CrashReport report, FileReportCallback callback) override;
 
@@ -48,27 +45,18 @@ class CrashReporter : public CrashReporterBase {
 
 class CrashReporterClosesConnection : public CrashReporterBase {
  public:
-  // |fuchsia::feedback::CrashReporter|
-  STUB_METHOD_CLOSES_CONNECTION(File, fuchsia::feedback::CrashReport, FileCallback)
-
   // |fuchsia::feedback::CrashReporter|
   STUB_METHOD_CLOSES_CONNECTION(FileReport, fuchsia::feedback::CrashReport, FileReportCallback)
 };
 
 class CrashReporterAlwaysReturnsError : public CrashReporterBase {
  public:
-  // |fuchsia::feedback::CrashReporter|
-  void File(fuchsia::feedback::CrashReport report, FileCallback callback) override;
-
   // |fuchsia::feedback::CrashReporter|
   void FileReport(fuchsia::feedback::CrashReport report, FileReportCallback callback) override;
 };
 
 class CrashReporterNoFileExpected : public CrashReporterBase {
  public:
-  // |fuchsia::feedback::CrashReporter|
-  void File(fuchsia::feedback::CrashReport report, FileCallback callback) override;
-
   // |fuchsia::feedback::CrashReporter|
   void FileReport(fuchsia::feedback::CrashReport report, FileReportCallback callback) override;
 };
diff --git a/src/diagnostics/detect/testing/realm-factory/src/realm_factory.rs b/src/diagnostics/detect/testing/realm-factory/src/realm_factory.rs
@@ -209,18 +209,6 @@ impl AsyncRequestHandler<fcrash::CrashReporterMarker> for MockComponent {
                     )
                     .context("failed to send response to client")?;
             }
-
-            fidl_fuchsia_feedback::CrashReporterRequest::File { report, responder } => {
-                let fcrash::CrashReport { program_name, crash_signature, .. } = report;
-                let program_name = program_name.unwrap_or("".to_string());
-                let crash_signature = crash_signature.unwrap_or("".to_string());
-                if let Some(sender) = &self.sender {
-                    sender.send_crash_report(&crash_signature, &program_name);
-                }
-                responder
-                    .send(Ok(()).map_err(|_| 0))
-                    .context("failed to send response to client")?;
-            }
         };
 
         Ok(())
diff --git a/src/performance/memory/sampler/src/crash_reporter.rs b/src/performance/memory/sampler/src/crash_reporter.rs
@@ -138,10 +138,6 @@ mod test {
 
     fn handle_crash_reporter_request(request: CrashReporterRequest) -> CrashReport {
         match request {
-            CrashReporterRequest::File { report, responder } => {
-                let _ = responder.send(Ok(()));
-                report
-            }
             CrashReporterRequest::FileReport { report, responder } => {
                 let _ = responder.send(Ok(&Default::default()));
                 report
diff --git a/src/storage/fshost/integration/src/mocks.rs b/src/storage/fshost/integration/src/mocks.rs
@@ -152,10 +152,6 @@ async fn run_crash_reporter(
 ) {
     while let Some(request) = stream.next().await {
         match request.unwrap() {
-            ffeedback::CrashReporterRequest::File { report, responder } => {
-                crash_reports_sink.send(report).await.unwrap();
-                responder.send(Ok(())).unwrap();
-            }
             ffeedback::CrashReporterRequest::FileReport { report, responder } => {
                 crash_reports_sink.send(report).await.unwrap();
                 responder.send(Ok(&FileReportResults::default())).unwrap();
diff --git a/src/sys/pkg/testing/mock-crash-reporter/BUILD.gn b/src/sys/pkg/testing/mock-crash-reporter/BUILD.gn
@@ -15,7 +15,6 @@ rustc_library("lib") {
     "//sdk/fidl/fuchsia.feedback:fuchsia.feedback_rust",
     "//src/lib/fidl/rust/fidl",
     "//src/lib/fuchsia-async",
-    "//src/lib/zircon/rust:fuchsia-zircon",
     "//third_party/rust_crates:futures",
   ]
 
diff --git a/src/sys/pkg/testing/mock-crash-reporter/src/lib.rs b/src/sys/pkg/testing/mock-crash-reporter/src/lib.rs
@@ -8,7 +8,6 @@ use {
         FilingError,
     },
     fuchsia_async::Task,
-    fuchsia_zircon::Status,
     futures::{
         channel::mpsc,
         future::{self, BoxFuture},
@@ -69,14 +68,6 @@ impl MockCrashReporterService {
     ) {
         while let Some(event) = stream.try_next().await.expect("received CrashReporter request") {
             match event {
-                fidl_fuchsia_feedback::CrashReporterRequest::File { report, responder } => {
-                    let res = self.call_hook.file_report(report).await;
-                    match res {
-                        Err(_) => responder.send(Err(Status::NOT_SUPPORTED.into_raw())).unwrap(),
-                        Ok(_) => responder.send(Ok(())).unwrap(),
-                    }
-                }
-
                 fidl_fuchsia_feedback::CrashReporterRequest::FileReport { report, responder } => {
                     let res = self.call_hook.file_report(report).await;
                     match res {

Original file line number	Diff line number	Diff line change
`@@ -1,3 +1,3 @@`
`1`	`1`	`{`
`2`		`- "fidl/fuchsia.feedback": "83c2d9fdd458d45f3c04d35ef2b2fe48"`
	`2`	`+ "fidl/fuchsia.feedback": "a7f935274ba8eb263147ea661b3727a9"`
`3`	`3`	`}`