[Nearby] Handle outgoing transfer updates

This handles outgoing transfer updates from the PayloadTracker and
gracefully closes outgoing connections after 1 minute.

Bug: 1085067
Change-Id: Ia16fc5fdb8cfd1846b130385414064dc3bd2a8cd
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2379900Reviewed-by: Himanshu Jaju <himanshujaju@chromium.org>
Commit-Queue: Richard Knoll <knollr@chromium.org>
Cr-Commit-Position: refs/heads/master@{#802771}

chrome/browser/nearby_sharing/constants.h

@@ -15,6 +15,13 @@ constexpr base::TimeDelta kReadResponseFrameTimeout =
 constexpr base::TimeDelta kInitiateNearbyConnectionTimeout =
     base::TimeDelta::FromSeconds(60);
 
+// The delay before the sender will disconnect from the receiver after sending a
+// file. Note that the receiver is expected to immediately disconnect, so this
+// delay is a worst-effort disconnection. Disconnecting too early may interrupt
+// in flight packets, especially over WiFi LAN.
+constexpr base::TimeDelta kOutgoingDisconnectionDelay =
+    base::TimeDelta::FromSeconds(60);
+
 // The delay before the receiver will disconnect from the sender after rejecting
 // an incoming file. The sender is expected to disconnect immediately after
 // reading the rejection frame.

chrome/browser/nearby_sharing/fake_nearby_connections_manager.cc

@@ -73,7 +73,7 @@ void FakeNearbyConnectionsManager::Send(const std::string& endpoint_id,
                                         PayloadStatusListener* listener) {
   DCHECK(!is_shutdown());
   if (send_payload_callback_)
-    send_payload_callback_.Run(std::move(payload));
+    send_payload_callback_.Run(std::move(payload), listener);
 }
 
 void FakeNearbyConnectionsManager::RegisterPayloadStatusListener(

chrome/browser/nearby_sharing/fake_nearby_connections_manager.h

@@ -87,7 +87,8 @@ class FakeNearbyConnectionsManager
   }
   DataUsage connected_data_usage() const { return connected_data_usage_; }
   void set_send_payload_callback(
-      base::RepeatingCallback<void(PayloadPtr payload)> callback) {
+      base::RepeatingCallback<void(PayloadPtr, PayloadStatusListener*)>
+          callback) {
     send_payload_callback_ = std::move(callback);
   }
   const base::Optional<std::vector<uint8_t>>& adverting_endpoint_info() {
@@ -115,7 +116,8 @@ class FakeNearbyConnectionsManager
   std::map<std::string, std::vector<uint8_t>> endpoint_auth_tokens_;
   NearbyConnection* connection_ = nullptr;
   DataUsage connected_data_usage_ = DataUsage::kUnknown;
-  base::RepeatingCallback<void(PayloadPtr payload)> send_payload_callback_;
+  base::RepeatingCallback<void(PayloadPtr, PayloadStatusListener*)>
+      send_payload_callback_;
   base::Optional<std::vector<uint8_t>> adverting_endpoint_info_;
   std::set<std::string> disconnected_endpoints_;
 

chrome/browser/nearby_sharing/nearby_sharing_service_impl.cc

@@ -2691,11 +2691,9 @@ void NearbySharingServiceImpl::OnPayloadTransferUpdate(
     Disconnect(share_target, metadata);
   }
 
-  if (share_target.is_incoming) {
-    OnIncomingTransferUpdate(share_target, metadata);
-  } else {
-    // TODO(crbug.com/1085067): Add call for outgoing transfer.
-  }
+  ShareTargetInfo* info = GetShareTargetInfo(share_target);
+  if (info->transfer_update_callback())
+    info->transfer_update_callback()->OnTransferUpdate(share_target, metadata);
 }
 
 void NearbySharingServiceImpl::OnPayloadsFailed(ShareTarget share_target) {
@@ -2821,7 +2819,33 @@ void NearbySharingServiceImpl::Disconnect(const ShareTarget& share_target,
     return;
   }
 
-  // TODO(crbug.com/1085067): Implement outgoing disconnection.
+  // Disconnect after a timeout to make sure any pending payloads are sent.
+  auto timer = std::make_unique<base::CancelableOnceClosure>(base::BindOnce(
+      &NearbySharingServiceImpl::OnDisconnectingConnectionTimeout,
+      weak_ptr_factory_.GetWeakPtr(), *endpoint_id));
+  base::SequencedTaskRunnerHandle::Get()->PostDelayedTask(
+      FROM_HERE, timer->callback(), kOutgoingDisconnectionDelay);
+  disconnection_timeout_alarms_[*endpoint_id] = std::move(timer);
+
+  // Stop the disconnection timeout if the connection has been closed already.
+  if (share_target_info->connection()) {
+    share_target_info->connection()->SetDisconnectionListener(base::BindOnce(
+        &NearbySharingServiceImpl::OnDisconnectingConnectionDisconnected,
+        weak_ptr_factory_.GetWeakPtr(), share_target, *endpoint_id));
+  }
+}
+
+void NearbySharingServiceImpl::OnDisconnectingConnectionTimeout(
+    const std::string& endpoint_id) {
+  disconnection_timeout_alarms_.erase(endpoint_id);
+  nearby_connections_manager_->Disconnect(endpoint_id);
+}
+
+void NearbySharingServiceImpl::OnDisconnectingConnectionDisconnected(
+    const ShareTarget& share_target,
+    const std::string& endpoint_id) {
+  disconnection_timeout_alarms_.erase(endpoint_id);
+  UnregisterShareTarget(share_target);
 }
 
 ShareTargetInfo& NearbySharingServiceImpl::GetOrCreateShareTargetInfo(

chrome/browser/nearby_sharing/nearby_sharing_service_impl.h

@@ -264,6 +264,9 @@ class NearbySharingServiceImpl
   bool OnIncomingPayloadsComplete(ShareTarget& share_target);
   void OnPayloadsFailed(ShareTarget share_target);
   void Disconnect(const ShareTarget& share_target, TransferMetadata metadata);
+  void OnDisconnectingConnectionTimeout(const std::string& endpoint_id);
+  void OnDisconnectingConnectionDisconnected(const ShareTarget& share_target,
+                                             const std::string& endpoint_id);
 
   ShareTargetInfo& GetOrCreateShareTargetInfo(const ShareTarget& share_target,
                                               const std::string& endpoint_id);
@@ -349,6 +352,11 @@ class NearbySharingServiceImpl
   // not press accept within the timeout.
   base::CancelableOnceClosure mutual_acceptance_timeout_alarm_;
 
+  // A map of ShareTarget id to disconnection timeout callback. Used to only
+  // disconnect after a timeout to keep sending any pending payloads.
+  base::flat_map<std::string, std::unique_ptr<base::CancelableOnceClosure>>
+      disconnection_timeout_alarms_;
+
   // The current advertising power level. PowerLevel::kUnknown while not
   // advertising.
   PowerLevel advertising_power_level_ = PowerLevel::kUnknown;

chrome/browser/nearby_sharing/nearby_sharing_service_impl_unittest.cc

@@ -707,6 +707,83 @@ class NearbySharingServiceImplTest : public testing::Test {
     }
   }
 
+  void SetUpOutgoingConnectionUntilAccept(
+      MockTransferUpdateCallback& transfer_callback,
+      const ShareTarget& target) {
+    base::RunLoop introduction_run_loop;
+    ExpectTransferUpdates(transfer_callback, target,
+                          {TransferMetadata::Status::kConnecting,
+                           TransferMetadata::Status::kAwaitingLocalConfirmation,
+                           TransferMetadata::Status::kAwaitingRemoteAcceptance},
+                          introduction_run_loop.QuitClosure());
+
+    EXPECT_EQ(NearbySharingServiceImpl::StatusCodes::kOk,
+              service_->SendAttachments(target,
+                                        CreateTextAttachments({kTextPayload})));
+    introduction_run_loop.Run();
+
+    // Verify data sent to the remote device so far.
+    ExpectPairedKeyEncryptionFrame();
+    ExpectPairedKeyResultFrame();
+    ExpectIntroductionFrame();
+  }
+
+  struct PayloadInfo {
+    int64_t payload_id;
+    NearbyConnectionsManager::PayloadStatusListener* listener;
+  };
+
+  PayloadInfo AcceptAndSendPayload(
+      MockTransferUpdateCallback& transfer_callback,
+      const ShareTarget& target) {
+    PayloadInfo info = {};
+    base::RunLoop payload_run_loop;
+    fake_nearby_connections_manager_->set_send_payload_callback(
+        base::BindLambdaForTesting(
+            [&](NearbyConnectionsManager::PayloadPtr payload,
+                NearbyConnectionsManager::PayloadStatusListener* listener) {
+              ASSERT_TRUE(payload->content->is_bytes());
+              std::vector<uint8_t> bytes = payload->content->get_bytes()->bytes;
+              EXPECT_EQ(kTextPayload, std::string(bytes.begin(), bytes.end()));
+              info.payload_id = payload->id;
+              info.listener = listener;
+              payload_run_loop.Quit();
+            }));
+
+    // We're now waiting for the remote device to respond with the accept
+    // result.
+    base::RunLoop accept_run_loop;
+    ExpectTransferUpdates(transfer_callback, target,
+                          {TransferMetadata::Status::kInProgress},
+                          accept_run_loop.QuitClosure());
+
+    // Kick off send process by accepting the transfer from the remote device.
+    SendConnectionResponse(
+        sharing::mojom::ConnectionResponseFrame::Status::kAccept);
+
+    accept_run_loop.Run();
+    payload_run_loop.Run();
+    return info;
+  }
+
+  void FinishOutgoingTransfer(MockTransferUpdateCallback& transfer_callback,
+                              const ShareTarget& target,
+                              const PayloadInfo& info) {
+    // Simulate a successful transfer via Nearby Connections.
+    base::RunLoop success_run_loop;
+    ExpectTransferUpdates(transfer_callback, target,
+                          {TransferMetadata::Status::kComplete},
+                          success_run_loop.QuitClosure());
+    ASSERT_TRUE(info.listener);
+    info.listener->OnStatusUpdate(
+        location::nearby::connections::mojom::PayloadTransferUpdate::New(
+            info.payload_id,
+            location::nearby::connections::mojom::PayloadStatus::kSuccess,
+            /*total_bytes=*/strlen(kTextPayload),
+            /*bytes_transferred=*/strlen(kTextPayload)));
+    success_run_loop.Run();
+  }
+
  protected:
   FakeNearbyShareLocalDeviceDataManager* local_device_data_manager() {
     EXPECT_EQ(1u, local_device_data_manager_factory_.instances().size());
@@ -3101,30 +3178,48 @@ TEST_F(NearbySharingServiceImplTest, SendText_Success) {
   EXPECT_EQ(test_metadata_key.encrypted_key(),
             advertisement->encrypted_metadata_key());
 
-  // Expect the text payload to be sent in the end.
-  base::RunLoop payload_run_loop;
-  fake_nearby_connections_manager_->set_send_payload_callback(
-      base::BindLambdaForTesting(
-          [&](NearbyConnectionsManager::PayloadPtr payload) {
-            ASSERT_TRUE(payload->content->is_bytes());
-            std::vector<uint8_t> bytes = payload->content->get_bytes()->bytes;
-            std::string sent = std::string(bytes.begin(), bytes.end());
-            EXPECT_EQ(kTextPayload, sent);
-            payload_run_loop.Quit();
-          }));
+  PayloadInfo info = AcceptAndSendPayload(transfer_callback, target);
+  FinishOutgoingTransfer(transfer_callback, target, info);
 
-  // We're now waiting for the remote device to respond with the accept result.
-  base::RunLoop accept_run_loop;
-  ExpectTransferUpdates(transfer_callback, target,
-                        {TransferMetadata::Status::kInProgress},
-                        accept_run_loop.QuitClosure());
+  // We should not have called disconnect yet as we want to wait for 1 minute to
+  // make sure all outgoing packets have been sent properly.
+  EXPECT_TRUE(
+      fake_nearby_connections_manager_->connection_endpoint_info(kEndpointId));
 
-  // Kick off send process by accepting the transfer from the remote device.
-  SendConnectionResponse(
-      sharing::mojom::ConnectionResponseFrame::Status::kAccept);
+  // Forward time until we send the disconnect request to Nearby Connections.
+  task_environment_.FastForwardBy(kOutgoingDisconnectionDelay);
 
-  accept_run_loop.Run();
-  payload_run_loop.Run();
+  // Expect to be disconnected now.
+  EXPECT_FALSE(
+      fake_nearby_connections_manager_->connection_endpoint_info(kEndpointId));
+
+  service_->UnregisterSendSurface(&transfer_callback, &discovery_callback);
+}
+
+TEST_F(NearbySharingServiceImplTest, SendText_SuccessClosedConnection) {
+  MockTransferUpdateCallback transfer_callback;
+  MockShareTargetDiscoveredCallback discovery_callback;
+  ShareTarget target =
+      SetUpOutgoingShareTarget(transfer_callback, discovery_callback);
+  SetUpOutgoingConnectionUntilAccept(transfer_callback, target);
+  PayloadInfo info = AcceptAndSendPayload(transfer_callback, target);
+  FinishOutgoingTransfer(transfer_callback, target, info);
+
+  // We should not have called disconnect yet as we want to wait for 1 minute to
+  // make sure all outgoing packets have been sent properly.
+  EXPECT_TRUE(
+      fake_nearby_connections_manager_->connection_endpoint_info(kEndpointId));
+
+  // Call disconnect on the connection early before the timeout has passed.
+  connection_.Close();
+
+  // Expect that we haven't called disconnect again as the endpoint is already
+  // disconnected.
+  EXPECT_TRUE(
+      fake_nearby_connections_manager_->connection_endpoint_info(kEndpointId));
+
+  // Make sure the scheduled disconnect callback does nothing.
+  task_environment_.FastForwardBy(kOutgoingDisconnectionDelay);
 
   service_->UnregisterSendSurface(&transfer_callback, &discovery_callback);
 }
@@ -3167,7 +3262,8 @@ TEST_F(NearbySharingServiceImplTest, SendFiles_Success) {
   base::RunLoop payload_run_loop;
   fake_nearby_connections_manager_->set_send_payload_callback(
       base::BindLambdaForTesting(
-          [&](NearbyConnectionsManager::PayloadPtr payload) {
+          [&](NearbyConnectionsManager::PayloadPtr payload,
+              NearbyConnectionsManager::PayloadStatusListener* listener) {
             base::ScopedAllowBlockingForTesting allow_blocking;
 
             ASSERT_TRUE(payload->content->is_file());

chrome/browser/nearby_sharing/payload_tracker.cc

@@ -68,6 +68,7 @@ void PayloadTracker::OnTransferUpdate() {
     update_callback_.Run(share_target_,
                          TransferMetadataBuilder()
                              .set_status(TransferMetadata::Status::kComplete)
+                             .set_progress(100)
                              .build());
     return;
   }

chrome/browser/nearby_sharing/payload_tracker_unittest.cc

@@ -136,6 +136,7 @@ TEST_F(PayloadTrackerTest, PayloadsComplete_Successful) {
       Run(testing::_,
           MetadataMatcher(TransferMetadataBuilder()
                               .set_status(TransferMetadata::Status::kComplete)
+                              .set_progress(100)
                               .build())));
 
   for (int payload_id = 0; payload_id < kAttachmentCount; payload_id++) {