commit 91be72b59358b822a5fb46ba5cc325ba49ff2d1e
author Francois Rousseau <frousseau@google.com> Thu May 23 16:55:21 2019 +0000
committer CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org> Thu May 23 16:55:21 2019 +0000
tree 60ce854f2356bdced5d460b0b040d492cf5959bf
parent 0282a41e25bc1c71286135c6bf8499993a7eafe3

[crash] set up error handler for connection to feedback data provider

DX-1469 #comment

TESTED=`fx run-test crashpad_agent_tests -t crashpad_agent_unittest -- --gtest_filter=CrashpadAgentTest.AnalysisSucceedOnNoFeedbackDataProvider`

Change-Id: Iaa6aee148b816214902ea962a26542d4c3974b50

src/developer/crashpad_agent/crashpad_agent.cc

diff --git a/src/developer/crashpad_agent/crashpad_agent.cc b/src/developer/crashpad_agent/crashpad_agent.cc
index 66ebf96..5f41033 100644
--- a/src/developer/crashpad_agent/crashpad_agent.cc
+++ b/src/developer/crashpad_agent/crashpad_agent.cc
@@ -237,10 +237,16 @@
 
   const uint64_t id = next_feedback_data_provider_id_++;
 
-  // TODO(DX-1469): set up set_error_handler() and call complete_error() on
-  // the pending fit::bridge.
   feedback_data_providers_[id] =
       services_->Connect<fuchsia::feedback::DataProvider>();
+  feedback_data_providers_[id].set_error_handler(
+      [get_data_done](zx_status_t status) {
+        FX_PLOGS(ERROR, status)
+            << "Lost connection to fuchsia.feedback.DataProvider";
+        if (get_data_done->completer) {
+          get_data_done->completer.complete_error();
+        }
+      });
   feedback_data_providers_[id]->GetData(
       [this, id, get_data_done](
           fuchsia::feedback::DataProvider_GetData_Result out_result) {

src/developer/crashpad_agent/tests/crashpad_agent_unittest.cc

diff --git a/src/developer/crashpad_agent/tests/crashpad_agent_unittest.cc b/src/developer/crashpad_agent/tests/crashpad_agent_unittest.cc
index d1c50bc..d848863 100644
--- a/src/developer/crashpad_agent/tests/crashpad_agent_unittest.cc
+++ b/src/developer/crashpad_agent/tests/crashpad_agent_unittest.cc
@@ -120,10 +120,12 @@
     // We expect as attachments the ones returned by the feedback::DataProvider
     // and the extra ones specific to the crash analysis flow under test.
     std::vector<std::string> expected_attachments = expected_extra_attachments;
-    expected_attachments.insert(
-        expected_attachments.begin(),
-        stub_feedback_data_provider_->attachment_keys().begin(),
-        stub_feedback_data_provider_->attachment_keys().end());
+    if (stub_feedback_data_provider_) {
+      expected_attachments.insert(
+          expected_attachments.begin(),
+          stub_feedback_data_provider_->attachment_keys().begin(),
+          stub_feedback_data_provider_->attachment_keys().end());
+    }
 
     std::vector<std::string> attachments;
     const std::string report_attachments_dir =
@@ -185,9 +187,15 @@
   }
 
   uint64_t total_num_feedback_data_provider_bindings() {
+    if (!stub_feedback_data_provider_) {
+      return 0u;
+    }
     return stub_feedback_data_provider_->total_num_bindings();
   }
   size_t current_num_feedback_data_provider_bindings() {
+    if (!stub_feedback_data_provider_) {
+      return 0u;
+    }
     return stub_feedback_data_provider_->current_num_bindings();
   }
 
@@ -445,6 +453,8 @@
   ResetFeedbackDataProvider(
       std::make_unique<StubFeedbackDataProviderReturnsNoAttachment>());
   EXPECT_TRUE(RunOneCrashAnalysis().is_response());
+  // The only attachment should be the one from the crash analysis as no
+  // feedback data attachments will be retrieved.
   CheckAttachments({"kernel_panic_crash_log"});
 }
 
@@ -458,6 +468,18 @@
   ResetFeedbackDataProvider(
       std::make_unique<StubFeedbackDataProviderReturnsNoData>());
   EXPECT_TRUE(RunOneCrashAnalysis().is_response());
+  // The only attachment should be the one from the crash analysis as no
+  // feedback data will be retrieved.
+  CheckAttachments({"kernel_panic_crash_log"});
+}
+
+TEST_F(CrashpadAgentTest, AnalysisSucceedOnNoFeedbackDataProvider) {
+  // We pass a nullptr stub so there will be no fuchsia.feedback.DataProvider
+  // service to connect to.
+  ResetFeedbackDataProvider(nullptr);
+  EXPECT_TRUE(RunOneCrashAnalysis().is_response());
+  // The only attachment should be the one from the crash analysis as no
+  // feedback data will be retrieved.
   CheckAttachments({"kernel_panic_crash_log"});
 }
 
@@ -483,7 +505,7 @@
 
   EXPECT_EQ(total_num_feedback_data_provider_bindings(), num_calls);
   // The unbinding is asynchronous so we need to run the loop until all the
-  // outstanding connections are actually close in the stub.
+  // outstanding connections are actually closed in the stub.
   RunLoopUntil(
       [this] { return current_num_feedback_data_provider_bindings() == 0u; });
 }