[Nearby] Local device data updater returns null response on failure

Instead of returning a separate |success| parameter from the local
device data updater, return base::nullopt for the response if the update
fails.

Bug: b/157685298
Change-Id: I653a23cc8e6391ee016540bbb7d46ca58755db3a
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2310318
Commit-Queue: Josh Nohle <nohle@chromium.org>
Reviewed-by: James Vecore <vecore@google.com>
Cr-Commit-Position: refs/heads/master@{#790916}

chrome/browser/nearby_sharing/local_device_data/fake_nearby_share_device_data_updater.cc

@@ -11,13 +11,18 @@ FakeNearbyShareDeviceDataUpdater::FakeNearbyShareDeviceDataUpdater(
 FakeNearbyShareDeviceDataUpdater::~FakeNearbyShareDeviceDataUpdater() = default;
 
 void FakeNearbyShareDeviceDataUpdater::RunNextRequest(
-    bool success,
     const base::Optional<nearbyshare::proto::UpdateDeviceResponse>& response) {
-  FinishAttempt(success, response);
+  FinishAttempt(response);
 }
 
 void FakeNearbyShareDeviceDataUpdater::HandleNextRequest() {}
 
+FakeNearbyShareDeviceDataUpdaterFactory::
+    FakeNearbyShareDeviceDataUpdaterFactory() = default;
+
+FakeNearbyShareDeviceDataUpdaterFactory::
+    ~FakeNearbyShareDeviceDataUpdaterFactory() = default;
+
 std::unique_ptr<NearbyShareDeviceDataUpdater>
 FakeNearbyShareDeviceDataUpdaterFactory::CreateInstance(
     const std::string& device_id,

chrome/browser/nearby_sharing/local_device_data/fake_nearby_share_device_data_updater.h

@@ -23,9 +23,8 @@ class FakeNearbyShareDeviceDataUpdater : public NearbyShareDeviceDataUpdater {
   ~FakeNearbyShareDeviceDataUpdater() override;
 
   // Advances the request queue and invokes request callback with the input
-  // parameters |success| and |response|.
+  // parameter |response|.
   void RunNextRequest(
-      bool success,
       const base::Optional<nearbyshare::proto::UpdateDeviceResponse>& response);
 
   const std::string& device_id() const { return device_id_; }

chrome/browser/nearby_sharing/local_device_data/nearby_share_device_data_updater.cc

@@ -53,7 +53,6 @@ void NearbyShareDeviceDataUpdater::ProcessRequestQueue() {
 }
 
 void NearbyShareDeviceDataUpdater::FinishAttempt(
-    bool success,
     const base::Optional<nearbyshare::proto::UpdateDeviceResponse>& response) {
   DCHECK(is_request_in_progress_);
   DCHECK(!pending_requests_.empty());
@@ -61,7 +60,7 @@ void NearbyShareDeviceDataUpdater::FinishAttempt(
   Request current_request = std::move(pending_requests_.front());
   pending_requests_.pop();
 
-  std::move(current_request.callback).Run(success, response);
+  std::move(current_request.callback).Run(response);
 
   is_request_in_progress_ = false;
   ProcessRequestQueue();

chrome/browser/nearby_sharing/local_device_data/nearby_share_device_data_updater.h

@@ -25,9 +25,10 @@
 // the number of UpdateDevice RPC calls.
 class NearbyShareDeviceDataUpdater {
  public:
+  // If the request is unsuccessful, |response| is base::nullopt.
   using ResultCallback = base::OnceCallback<void(
-      bool,
-      const base::Optional<nearbyshare::proto::UpdateDeviceResponse>&)>;
+      const base::Optional<nearbyshare::proto::UpdateDeviceResponse>&
+          response)>;
 
   struct Request {
     Request(base::Optional<std::string> device_name,
@@ -79,8 +80,9 @@ class NearbyShareDeviceDataUpdater {
  protected:
   void ProcessRequestQueue();
   virtual void HandleNextRequest() = 0;
+
+  // If the request is unsuccessful, |response| is base::nullopt.
   void FinishAttempt(
-      bool success,
       const base::Optional<nearbyshare::proto::UpdateDeviceResponse>& response);
 
   std::string device_id_;

chrome/browser/nearby_sharing/local_device_data/nearby_share_device_data_updater_impl.cc

@@ -131,7 +131,7 @@ void NearbyShareDeviceDataUpdaterImpl::OnRpcSuccess(
   nearbyshare::proto::UpdateDeviceResponse response_copy(response);
   client_.reset();
   RecordResultMetrics(UpdaterResultCode::kSuccess);
-  FinishAttempt(/*success=*/true, response_copy);
+  FinishAttempt(response_copy);
 }
 
 void NearbyShareDeviceDataUpdaterImpl::OnRpcFailure(
@@ -139,11 +139,11 @@ void NearbyShareDeviceDataUpdaterImpl::OnRpcFailure(
   timer_.Stop();
   client_.reset();
   RecordResultMetrics(RequestErrorToUpdaterResultCode(error));
-  FinishAttempt(/*success=*/false, /*response=*/base::nullopt);
+  FinishAttempt(/*response=*/base::nullopt);
 }
 
 void NearbyShareDeviceDataUpdaterImpl::OnTimeout() {
   client_.reset();
   RecordResultMetrics(UpdaterResultCode::kTimeout);
-  FinishAttempt(/*success=*/false, /*response=*/base::nullopt);
+  FinishAttempt(/*response=*/base::nullopt);
 }

chrome/browser/nearby_sharing/local_device_data/nearby_share_device_data_updater_impl_unittest.cc

@@ -171,7 +171,6 @@ class NearbyShareDeviceDataUpdaterImplTest : public ::testing::Test {
                   client->update_device_requests()[0].request);
 
     // Send and verify the response.
-    size_t num_results = results_.size();
     size_t num_responses = responses_.size();
     switch (result) {
       case UpdateDeviceRequestResult::kSuccess:
@@ -186,14 +185,12 @@ class NearbyShareDeviceDataUpdaterImplTest : public ::testing::Test {
         FastForward(kTestTimeout);
         return;
     }
-    EXPECT_EQ(num_results + 1, results_.size());
     EXPECT_EQ(num_responses + 1, responses_.size());
 
-    bool success = result == UpdateDeviceRequestResult::kSuccess;
-    EXPECT_EQ(success, results_.back());
-    VerifyResponse(
-        success ? base::make_optional(TestResponse()) : base::nullopt,
-        responses_.back());
+    VerifyResponse(result == UpdateDeviceRequestResult::kSuccess
+                       ? base::make_optional(TestResponse())
+                       : base::nullopt,
+                   responses_.back());
   }
 
  private:
@@ -203,16 +200,13 @@ class NearbyShareDeviceDataUpdaterImplTest : public ::testing::Test {
   }
 
   // The callback passed into UpdateDeviceData().
-  void OnResult(bool success,
-                const base::Optional<nearbyshare::proto::UpdateDeviceResponse>&
+  void OnResult(const base::Optional<nearbyshare::proto::UpdateDeviceResponse>&
                     response) {
-    results_.push_back(success);
     responses_.push_back(response);
   }
 
   base::test::SingleThreadTaskEnvironment task_environment_{
       base::test::TaskEnvironment::TimeSource::MOCK_TIME};
-  std::vector<bool> results_;
   std::vector<base::Optional<nearbyshare::proto::UpdateDeviceResponse>>
       responses_;
   FakeNearbyShareClientFactory fake_client_factory_;