[Nearby] : Make discovery manager handle early updates

Previously discovery manager assumed transfer updates would only come
after a share target was selected. This is not true and can happen when
registering the foreground surface if an existing transfer is in
progress or recently completed.

Fixed: 1128188
Change-Id: I5de66fd895117e37d8f45c76a346e6edabdc564d
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2419662
Commit-Queue: James Vecore <vecore@google.com>
Reviewed-by: Ryan Hansberry <hansberry@chromium.org>
Cr-Commit-Position: refs/heads/master@{#808934}

chrome/browser/nearby_sharing/nearby_per_session_discovery_manager.cc

@@ -53,7 +53,15 @@ NearbyPerSessionDiscoveryManager::~NearbyPerSessionDiscoveryManager() {
 void NearbyPerSessionDiscoveryManager::OnTransferUpdate(
     const ShareTarget& share_target,
     const TransferMetadata& transfer_metadata) {
-  DCHECK(transfer_update_listener_.is_bound());
+  if (!transfer_update_listener_.is_bound()) {
+    // This can happen when registering the send surface and an existing
+    // transfer is happening or recently happened.
+    NS_LOG(VERBOSE) << __func__
+                    << ": transfer_update_listener_ is not is_bound(), cannot "
+                       "forward transfer updates";
+    return;
+  }
+
   NS_LOG(VERBOSE) << __func__ << ": Nearby per-session discovery manager: "
                   << "Transfer update for share target with ID "
                   << share_target.id << ": "
@@ -62,8 +70,12 @@ void NearbyPerSessionDiscoveryManager::OnTransferUpdate(
 
   base::Optional<nearby_share::mojom::TransferStatus> status =
       GetTransferStatus(transfer_metadata);
-  if (!status)
+
+  if (!status) {
+    NS_LOG(VERBOSE) << __func__ << ": Nearby per-session discovery manager: "
+                    << " skipping status update, no mojo mapping defined yet.";
     return;
+  }
 
   transfer_update_listener_->OnTransferUpdate(*status,
                                               transfer_metadata.token());
@@ -126,6 +138,7 @@ void NearbyPerSessionDiscoveryManager::SelectShareTarget(
   // Bind update listener before calling the sharing service to get all updates.
   mojo::PendingReceiver<nearby_share::mojom::TransferUpdateListener> receiver =
       transfer_update_listener_.BindNewPipeAndPassReceiver();
+  transfer_update_listener_.reset_on_disconnect();
 
   NearbySharingService::StatusCodes status =
       nearby_sharing_service_->SendAttachments(iter->second,

chrome/browser/nearby_sharing/nearby_per_session_discovery_manager_unittest.cc

@@ -411,3 +411,35 @@ TEST_F(NearbyPerSessionDiscoveryManagerTest, OnTransferUpdate_WaitLocal) {
   EXPECT_CALL(sharing_service(), UnregisterSendSurface(&manager(), &manager()))
       .WillOnce(testing::Return(NearbySharingService::StatusCodes::kOk));
 }
+
+TEST_F(NearbyPerSessionDiscoveryManagerTest, TransferUpdateWithoutListener) {
+  MockStartDiscoveryCallback callback;
+  EXPECT_CALL(callback, Run(/*success=*/true));
+
+  EXPECT_CALL(
+      sharing_service(),
+      RegisterSendSurface(&manager(), &manager(),
+                          NearbySharingService::SendSurfaceState::kForeground))
+      .WillOnce(testing::Return(NearbySharingService::StatusCodes::kOk));
+
+  EXPECT_CALL(sharing_service(), UnregisterSendSurface(&manager(), &manager()))
+      .Times(0);  // Should not be called!
+
+  MockShareTargetListener listener;
+  manager().StartDiscovery(listener.Bind(), callback.Get());
+
+  // It is possible that during registration if a transfer is in progress
+  // already we might get a transfer update before selecting the share target.
+  // This used to cause a crash due to transfer_update_listener_ not being
+  // bound.
+  ShareTarget share_target;
+  manager().OnTransferUpdate(
+      share_target, TransferMetadataBuilder()
+                        .set_status(TransferMetadata::Status::kComplete)
+                        .build());
+
+  // However, we do expect UnregisterSendSurface to be called from the
+  // destructor.
+  EXPECT_CALL(sharing_service(), UnregisterSendSurface(&manager(), &manager()))
+      .WillOnce(testing::Return(NearbySharingService::StatusCodes::kOk));
+}