Remove use of ShareTarget from payload tracker.

PiperOrigin-RevId: 631211496

sharing/nearby_sharing_service_impl.cc

@@ -2584,9 +2584,8 @@ void NearbySharingServiceImpl::OnPayloadPathsRegistered(
 
   info->set_payload_tracker(std::make_shared<PayloadTracker>(
       context_, share_target, attachment_info_map_,
-      [this](ShareTarget share_target, TransferMetadata transfer_metadata) {
-        OnPayloadTransferUpdate(share_target, transfer_metadata);
-      }));
+      absl::bind_front(&NearbySharingServiceImpl::OnPayloadTransferUpdate,
+                       this)));
 
   // Register status listener for all payloads.
   for (int64_t attachment_id : share_target.GetAttachmentIds()) {
@@ -2870,7 +2869,8 @@ void NearbySharingServiceImpl::OnCreatePayloads(
     bool success) {
   OutgoingShareTargetInfo* info = GetOutgoingShareTargetInfo(share_target.id);
   bool has_payloads = info && (!info->text_payloads().empty() ||
-                               !info->file_payloads().empty());
+                               !info->file_payloads().empty() ||
+                               !info->wifi_credentials_payloads().empty());
   if (!success || !has_payloads) {
     NL_LOG(WARNING) << __func__
                     << ": Failed to send file to remote ShareTarget. Failed to "
@@ -3703,9 +3703,8 @@ void NearbySharingServiceImpl::OnReceiveConnectionResponse(
 
       info->set_payload_tracker(std::make_unique<PayloadTracker>(
           context_, share_target, attachment_info_map_,
-          [this](ShareTarget share_target, TransferMetadata transfer_metadata) {
-            OnPayloadTransferUpdate(share_target, transfer_metadata);
-          }));
+          absl::bind_front(&NearbySharingServiceImpl::OnPayloadTransferUpdate,
+                           this)));
 
       if (NearbyFlags::GetInstance().GetBoolFlag(
               config_package_nearby::nearby_sharing_feature::
@@ -4018,84 +4017,95 @@ std::optional<ShareTarget> NearbySharingServiceImpl::CreateShareTarget(
 }
 
 void NearbySharingServiceImpl::OnPayloadTransferUpdate(
-    ShareTarget share_target, TransferMetadata metadata) {
+    int64_t share_target_id, TransferMetadata metadata) {
+  ShareTargetInfo* info = GetShareTargetInfo(share_target_id);
+  if (!info) {
+    // ShareTarget already disconnected.
+    NL_LOG(WARNING)
+        << "Received payload update after share target disconnected: "
+        << share_target_id;
+    return;
+  }
   bool is_in_progress =
       metadata.status() == TransferMetadata::Status::kInProgress;
 
-  if (is_in_progress && share_target.is_incoming &&
-      is_waiting_to_record_accept_to_transfer_start_metric_) {
-    is_waiting_to_record_accept_to_transfer_start_metric_ = false;
-  }
-
   // kInProgress status is logged extensively elsewhere so avoid the spam.
   if (!is_in_progress) {
     NL_VLOG(1) << __func__ << ": Nearby Share service: "
                << "Payload transfer update for share target with ID "
-               << share_target.id << ": "
+               << share_target_id << ": "
                << TransferMetadata::StatusToString(metadata.status());
   }
 
-  // Update file paths during progress. It may impact transfer speed.
-  // TODO: b/289290115 - Revisit UpdateFilePath to enhance transfer speed for
-  // MacOS.
-  if (update_file_paths_in_progress_ && share_target.is_incoming) {
-    UpdateFilePath(share_target);
-  }
-
-  if (metadata.status() == TransferMetadata::Status::kComplete &&
-      share_target.is_incoming) {
-    if (!OnIncomingPayloadsComplete(share_target)) {
-      metadata = TransferMetadataBuilder()
-                     .set_status(TransferMetadata::Status::kIncompletePayloads)
-                     .build();
-
-      // Reset file paths for file attachments.
-      for (auto& file : share_target.file_attachments)
-        file.set_file_path(std::nullopt);
-
-      // Reset body of text attachments.
-      for (auto& text : share_target.text_attachments)
-        text.set_text_body(std::string());
-
-      // Reset password of Wi-Fi credentials attachments.
-      for (auto& wifi_credentials : share_target.wifi_credentials_attachments) {
-        wifi_credentials.set_password(std::string());
-        wifi_credentials.set_is_hidden(false);
-      }
+  bool payload_incomplete = false;
+  if (info->IsIncoming()) {
+    if (is_in_progress &&
+        is_waiting_to_record_accept_to_transfer_start_metric_) {
+      is_waiting_to_record_accept_to_transfer_start_metric_ = false;
     }
 
-    if (IsBackgroundScanningFeatureEnabled()) {
-      fast_initiation_scanner_cooldown_timer_->Stop();
-      fast_initiation_scanner_cooldown_timer_->Start(
-          absl::ToInt64Milliseconds(kFastInitiationScannerCooldown), 0,
-          [this]() {
-            fast_initiation_scanner_cooldown_timer_->Stop();
-            InvalidateFastInitiationScanning();
-          });
-    }
-  } else if (metadata.status() == TransferMetadata::Status::kCancelled &&
-             share_target.is_incoming) {
-    NL_VLOG(1) << __func__ << ": Update file paths for cancelled transfer";
-    if (!update_file_paths_in_progress_) {
+    ShareTarget share_target = info->share_target();
+    // Update file paths during progress. It may impact transfer speed.
+    // TODO: b/289290115 - Revisit UpdateFilePath to enhance transfer speed for
+    // MacOS.
+    if (update_file_paths_in_progress_) {
       UpdateFilePath(share_target);
     }
+
+    if (metadata.status() == TransferMetadata::Status::kComplete) {
+      if (!OnIncomingPayloadsComplete(share_target)) {
+        payload_incomplete = true;
+
+        // Reset file paths for file attachments.
+        for (auto& file : share_target.file_attachments)
+          file.set_file_path(std::nullopt);
+
+        // Reset body of text attachments.
+        for (auto& text : share_target.text_attachments)
+          text.set_text_body(std::string());
+
+        // Reset password of Wi-Fi credentials attachments.
+        for (auto& wifi_credentials :
+             share_target.wifi_credentials_attachments) {
+          wifi_credentials.set_password(std::string());
+          wifi_credentials.set_is_hidden(false);
+        }
+      }
+
+      if (IsBackgroundScanningFeatureEnabled()) {
+        fast_initiation_scanner_cooldown_timer_->Stop();
+        fast_initiation_scanner_cooldown_timer_->Start(
+            absl::ToInt64Milliseconds(kFastInitiationScannerCooldown), 0,
+            [this]() {
+              fast_initiation_scanner_cooldown_timer_->Stop();
+              InvalidateFastInitiationScanning();
+            });
+      }
+    } else if (metadata.status() == TransferMetadata::Status::kCancelled) {
+      NL_VLOG(1) << __func__ << ": Update file paths for cancelled transfer";
+      if (!update_file_paths_in_progress_) {
+        UpdateFilePath(share_target);
+      }
+    }
+    info->set_share_target(share_target);
   }
 
   // Make sure to call this before calling Disconnect, or we risk losing some
   // transfer updates in the receive case due to the Disconnect call cleaning up
   // share targets.
-  ShareTargetInfo* info = GetShareTargetInfo(share_target.id);
-  if (info) {
-    info->set_share_target(share_target);
-    info->UpdateTransferMetadata(metadata);
-  }
+  info->UpdateTransferMetadata(
+      payload_incomplete
+          ? TransferMetadataBuilder()
+                .set_status(TransferMetadata::Status::kIncompletePayloads)
+                .build()
+          : metadata);
 
-  // Cancellation has its own disconnection strategy, possibly adding a delay
-  // before disconnection to provide the other party time to process the
-  // cancellation.
+  // Cancellation has its own disconnection strategy, possibly adding a
+  // delay before disconnection to provide the other party time to process
+  // the cancellation.
   if (TransferMetadata::IsFinalStatus(metadata.status()) &&
       metadata.status() != TransferMetadata::Status::kCancelled) {
-    Disconnect(share_target.id, metadata);
+    Disconnect(share_target_id, metadata);
   }
 }
 

sharing/nearby_sharing_service_impl.h

@@ -402,7 +402,7 @@ class NearbySharingServiceImpl
       std::optional<NearbyShareDecryptedPublicCertificate> certificate,
       bool is_incoming);
 
-  void OnPayloadTransferUpdate(ShareTarget share_target,
+  void OnPayloadTransferUpdate(int64_t share_target_id,
                                TransferMetadata metadata);
   bool OnIncomingPayloadsComplete(ShareTarget& share_target);
   void RemoveIncomingPayloads(ShareTarget share_target);

sharing/nearby_sharing_service_impl_test.cc

@@ -87,6 +87,7 @@
 #include "sharing/text_attachment.h"
 #include "sharing/transfer_metadata.h"
 #include "sharing/transfer_update_callback.h"
+#include "sharing/wifi_credentials_attachment.h"
 #include "google/protobuf/repeated_ptr_field.h"
 
 namespace nearby {
@@ -345,6 +346,15 @@ std::vector<std::unique_ptr<Attachment>> CreateFileAttachments(
   return attachments;
 }
 
+std::vector<std::unique_ptr<Attachment>> CreateWifiCredentialAttachments(
+    std::string ssid, std::string password) {
+  std::vector<std::unique_ptr<Attachment>> attachments;
+  attachments.push_back(std::make_unique<WifiCredentialsAttachment>(
+      std::move(ssid), service::proto::WifiCredentialsMetadata::WPA_PSK,
+      std::move(password)));
+  return attachments;
+}
+
 class NearbySharingServiceImplTest : public testing::Test {
  public:
   NearbySharingServiceImplTest() = default;
@@ -3662,6 +3672,76 @@ TEST_F(NearbySharingServiceImplTest, SendFilesSuccess) {
   UnregisterSendSurface(&transfer_callback, &discovery_callback);
 }
 
+TEST_F(NearbySharingServiceImplTest, SendWifiCredentialsSuccess) {
+  NearbyFlags::GetInstance().OverrideBoolFlagValue(
+      config_package_nearby::nearby_sharing_feature::
+          kEnableTransferCancellationOptimization,
+      true);
+  MockTransferUpdateCallback transfer_callback;
+  MockShareTargetDiscoveredCallback discovery_callback;
+  ShareTarget target =
+      SetUpOutgoingShareTarget(transfer_callback, discovery_callback);
+
+  absl::Notification introduction_notification;
+  ExpectTransferUpdates(transfer_callback, target,
+                        {TransferMetadata::Status::kConnecting,
+                         TransferMetadata::Status::kAwaitingLocalConfirmation,
+                         TransferMetadata::Status::kAwaitingRemoteAcceptance},
+                        [&]() { introduction_notification.Notify(); });
+
+  EXPECT_EQ(SendAttachments(target, CreateWifiCredentialAttachments(
+                                        "GoogleGuest", "password")),
+            NearbySharingServiceImpl::StatusCodes::kOk);
+  EXPECT_TRUE(
+      introduction_notification.WaitForNotificationWithTimeout(kWaitTimeout));
+
+  // Verify data sent to the remote device so far.
+  EXPECT_TRUE(ExpectPairedKeyEncryptionFrame());
+  EXPECT_TRUE(ExpectPairedKeyResultFrame());
+  std::optional<IntroductionFrame> intro = ExpectIntroductionFrame();
+
+  ASSERT_TRUE(intro.has_value());
+  ASSERT_EQ(intro->wifi_credentials_metadata_size(), 1);
+  auto meta = intro->wifi_credentials_metadata(0);
+
+  EXPECT_EQ(meta.ssid(), "GoogleGuest");
+  EXPECT_EQ(meta.security_type(),
+            service::proto::WifiCredentialsMetadata::WPA_PSK);
+
+  // Expect the wifi credential payload to be sent in the end.
+  absl::Notification payload_notification;
+  fake_nearby_connections_manager_->set_send_payload_callback(
+      [&](std::unique_ptr<nearby::sharing::Payload> payload,
+          std::weak_ptr<NearbyConnectionsManager::PayloadStatusListener>
+              listener) {
+        ASSERT_TRUE(payload->content.is_bytes());
+        std::vector<uint8_t> bytes = payload->content.bytes_payload.bytes;
+        nearby::sharing::service::proto::WifiCredentials wifi_credentials;
+        ASSERT_TRUE(
+            wifi_credentials.ParseFromArray(bytes.data(), bytes.size()));
+        EXPECT_EQ(wifi_credentials.password(), "password");
+        EXPECT_FALSE(wifi_credentials.hidden_ssid());
+
+        payload_notification.Notify();
+      });
+
+  // We're now waiting for the remote device to respond with the accept
+  // result.
+  absl::Notification accept_notification;
+  ExpectTransferUpdates(transfer_callback, target,
+                        {TransferMetadata::Status::kInProgress},
+                        [&]() { accept_notification.Notify(); });
+
+  // Kick off send process by accepting the transfer from the remote device.
+  SendConnectionResponse(ConnectionResponseFrame::ACCEPT);
+
+  EXPECT_TRUE(accept_notification.WaitForNotificationWithTimeout(kWaitTimeout));
+  EXPECT_TRUE(
+      payload_notification.WaitForNotificationWithTimeout(kWaitTimeout));
+
+  UnregisterSendSurface(&transfer_callback, &discovery_callback);
+}
+
 TEST_F(NearbySharingServiceImplTest, CancelSenderInitiator) {
   MockTransferUpdateCallback transfer_callback;
   MockShareTargetDiscoveredCallback discovery_callback;

sharing/payload_tracker.cc

@@ -43,9 +43,9 @@ namespace sharing {
 PayloadTracker::PayloadTracker(
     Context* context, const ShareTarget& share_target,
     const absl::flat_hash_map<int64_t, AttachmentInfo>& attachment_info_map,
-    std::function<void(ShareTarget, TransferMetadata)> update_callback)
+    std::function<void(int64_t, TransferMetadata)> update_callback)
     : context_(context),
-      share_target_(share_target),
+      share_target_id_(share_target.id),
       update_callback_(std::move(update_callback)) {
   total_transfer_size_ = 0;
   confirmed_transfer_size_ = 0;
@@ -152,7 +152,7 @@ void PayloadTracker::OnTransferUpdate(const State& state) {
   if (IsComplete()) {
     NL_VLOG(1) << __func__ << ": All payloads are complete.";
     update_callback_(
-        share_target_,
+        share_target_id_,
         TransferMetadataBuilder()
             .set_status(TransferMetadata::Status::kComplete)
             .set_progress(100)
@@ -165,7 +165,7 @@ void PayloadTracker::OnTransferUpdate(const State& state) {
   if (IsCancelled(state)) {
     NL_VLOG(1) << __func__ << ": Payloads cancelled.";
     update_callback_(
-        share_target_,
+        share_target_id_,
         TransferMetadataBuilder()
             .set_status(TransferMetadata::Status::kCancelled)
             .set_total_attachments_count(payload_state_.size())
@@ -177,7 +177,7 @@ void PayloadTracker::OnTransferUpdate(const State& state) {
   if (HasFailed(state)) {
     NL_VLOG(1) << __func__ << ": Payloads failed.";
     update_callback_(
-        share_target_,
+        share_target_id_,
         TransferMetadataBuilder()
             .set_status(TransferMetadata::Status::kFailed)
             .set_total_attachments_count(payload_state_.size())
@@ -240,7 +240,7 @@ void PayloadTracker::OnTransferUpdate(const State& state) {
   last_update_timestamp_ = current_time;
 
   update_callback_(
-      share_target_,
+      share_target_id_,
       TransferMetadataBuilder()
           .set_status(TransferMetadata::Status::kInProgress)
           .set_progress(percent)

sharing/payload_tracker.h

@@ -42,7 +42,7 @@ class PayloadTracker : public NearbyConnectionsManager::PayloadStatusListener {
   PayloadTracker(
       Context* context, const ShareTarget& share_target,
       const absl::flat_hash_map<int64_t, AttachmentInfo>& attachment_info_map,
-      std::function<void(ShareTarget, TransferMetadata)> update_callback);
+      std::function<void(int64_t, TransferMetadata)> update_callback);
   ~PayloadTracker() override;
 
   // NearbyConnectionsManager::PayloadStatusListener:
@@ -71,8 +71,8 @@ class PayloadTracker : public NearbyConnectionsManager::PayloadStatusListener {
   double CalculateProgressPercent(const State& state) const;
 
   Context* context_;
-  ShareTarget share_target_;
-  std::function<void(ShareTarget, TransferMetadata)> update_callback_;
+  const int64_t share_target_id_;
+  std::function<void(int64_t, TransferMetadata)> update_callback_;
 
   // Map of payload id to state of payload.
   std::map<int64_t, State> payload_state_;

sharing/payload_tracker_test.cc

@@ -58,7 +58,7 @@ class PayloadTrackerTest : public ::testing::Test {
                                  std::move(attachment_info));
     payload_tracker_ = std::make_unique<PayloadTracker>(
         context(), share_target_, attachment_info_map_,
-        [&](ShareTarget share_target, TransferMetadata transfer_metadata) {
+        [&](int64_t share_target_id, TransferMetadata transfer_metadata) {
           current_percentage_ = transfer_metadata.progress();
         });
   }