Move accept state management into IncomingShareSession.

PiperOrigin-RevId: 651900492

sharing/incoming_share_session.cc

@@ -181,9 +181,38 @@ bool IncomingShareSession::ProcessKeyVerificationResult(
   return true;
 }
 
-void IncomingShareSession::AcceptTransfer(
+bool IncomingShareSession::ReadyForTransfer(
+    std::function<void(std::optional<V1Frame> frame)> frame_read_callback) {
+  if (!IsConnected()) {
+    NL_LOG(WARNING) << __func__ << ": out of order API call.";
+    return false;
+  }
+  ready_for_accept_ = true;
+  set_disconnect_status(
+      TransferMetadata::Status::kUnexpectedDisconnection);
+
+  frames_reader()->ReadFrame(std::move(frame_read_callback));
+
+  if (!self_share()) {
+    TransferMetadataBuilder transfer_metadata_builder;
+    transfer_metadata_builder.set_status(
+        TransferMetadata::Status::kAwaitingLocalConfirmation);
+    transfer_metadata_builder.set_token(token());
+
+    UpdateTransferMetadata(transfer_metadata_builder.build());
+    return false;
+  }
+  return true;
+}
+
+bool IncomingShareSession::AcceptTransfer(
     Clock* clock, NearbyConnectionsManager& connections_manager,
     std::function<void(int64_t, TransferMetadata)> update_callback) {
+  if (!ready_for_accept_ || !IsConnected()) {
+    NL_LOG(WARNING) << __func__ << ": out of order API call.";
+    return false;
+  }
+  ready_for_accept_ = false;
   const absl::flat_hash_map<int64_t, int64_t>& payload_map =
       attachment_payload_map();
   set_payload_tracker(std::make_shared<PayloadTracker>(
@@ -224,6 +253,7 @@ void IncomingShareSession::AcceptTransfer(
   // Log analytics event of starting to receive payloads.
   analytics_recorder().NewReceiveAttachmentsStart(session_id(),
                                                   attachment_container());
+  return true;
 }
 
 bool IncomingShareSession::UpdateFilePayloadPaths(

sharing/incoming_share_session.h

@@ -72,8 +72,17 @@ class IncomingShareSession : public ShareSession {
   bool UpdateFilePayloadPaths(
       const NearbyConnectionsManager& connections_manager);
 
+  // Returns true if the transfer can begin and AcceptTransfer should be called
+  // immediately.
+  // Returns false if user needs to accept the transfer.
+  bool ReadyForTransfer(
+      std::function<
+          void(std::optional<nearby::sharing::service::proto::V1Frame> frame)>
+          frame_read_callback);
+
   // Accept the transfer and begin listening for payload transfer updates.
-  void AcceptTransfer(
+  // Returns false if session is not in a state to accept the transfer.
+  bool AcceptTransfer(
       Clock* clock, NearbyConnectionsManager& connections_manager,
       std::function<void(int64_t, TransferMetadata)> update_callback);
 
@@ -102,6 +111,7 @@ class IncomingShareSession : public ShareSession {
       transfer_update_callback_;
 
   bool bandwidth_upgrade_requested_ = false;
+  bool ready_for_accept_ = false;
 };
 
 }  // namespace nearby::sharing

sharing/incoming_share_session_test.cc

@@ -589,6 +589,68 @@ TEST_F(IncomingShareSessionTest, FinalizePayloadsMissingWifiPayloads) {
               IsFalse());
 }
 
+TEST_F(IncomingShareSessionTest, ReadyForTransferNotConnected) {
+  session_.set_session_id(1234);
+
+  FakeNearbyConnectionsManager connections_manager;
+  FakeClock clock;
+  EXPECT_THAT(session_.ReadyForTransfer([](std::optional<V1Frame> frame) {}),
+              IsFalse());
+}
+
+TEST_F(IncomingShareSessionTest, ReadyForTransferNotSelfShare) {
+  NearbySharingDecoderImpl nearby_sharing_decoder;
+  FakeNearbyConnection connection;
+  session_.set_session_id(1234);
+  EXPECT_TRUE(
+      session_.OnConnected(nearby_sharing_decoder, absl::Now(), &connection));
+
+  EXPECT_THAT(session_.ReadyForTransfer([](std::optional<V1Frame> frame) {}),
+              IsFalse());
+}
+
+TEST_F(IncomingShareSessionTest, ReadyForTransferSelfShare) {
+  ShareTarget share_target;
+  share_target.for_self_share = true;
+  IncomingShareSession session(task_runner_, analytics_recorder_,
+                               std::string("XYCA"), share_target,
+                               transfer_metadata_callback_.AsStdFunction());
+  NearbySharingDecoderImpl nearby_sharing_decoder;
+  FakeNearbyConnection connection;
+  session.set_session_id(1234);
+  EXPECT_TRUE(
+      session.OnConnected(nearby_sharing_decoder, absl::Now(), &connection));
+
+  EXPECT_THAT(session.ReadyForTransfer([](std::optional<V1Frame> frame) {}),
+              IsTrue());
+}
+
+TEST_F(IncomingShareSessionTest, AcceptTransferNotConnected) {
+  session_.set_session_id(1234);
+
+  FakeNearbyConnectionsManager connections_manager;
+  FakeClock clock;
+  EXPECT_THAT(session_.AcceptTransfer(&clock, connections_manager,
+                                      [](int64_t, TransferMetadata) {}),
+              IsFalse());
+}
+
+TEST_F(IncomingShareSessionTest, AcceptTransferNotReady) {
+  NearbySharingDecoderImpl nearby_sharing_decoder;
+  FakeNearbyConnection connection;
+  session_.set_session_id(1234);
+  EXPECT_TRUE(
+      session_.OnConnected(nearby_sharing_decoder, absl::Now(), &connection));
+  EXPECT_THAT(session_.ProcessIntroduction(introduction_frame_),
+              Eq(std::nullopt));
+
+  FakeNearbyConnectionsManager connections_manager;
+  FakeClock clock;
+  EXPECT_THAT(session_.AcceptTransfer(&clock, connections_manager,
+                                      [](int64_t, TransferMetadata) {}),
+              IsFalse());
+}
+
 TEST_F(IncomingShareSessionTest, AcceptTransferSuccess) {
   NearbySharingDecoderImpl nearby_sharing_decoder;
   FakeNearbyConnection connection;
@@ -597,6 +659,8 @@ TEST_F(IncomingShareSessionTest, AcceptTransferSuccess) {
       session_.OnConnected(nearby_sharing_decoder, absl::Now(), &connection));
   EXPECT_THAT(session_.ProcessIntroduction(introduction_frame_),
               Eq(std::nullopt));
+  EXPECT_THAT(session_.ReadyForTransfer([](std::optional<V1Frame> frame) {}),
+              IsFalse());
   EXPECT_CALL(transfer_metadata_callback_, Call(_, _))
       .WillOnce(Invoke([](const IncomingShareSession& session,
                           const TransferMetadata& metadata) {
@@ -605,13 +669,12 @@ TEST_F(IncomingShareSessionTest, AcceptTransferSuccess) {
       }));
   EXPECT_CALL(
       mock_event_logger_,
-      Log(Matcher<const SharingLog&>(AllOf((
-          HasCategory(EventCategory::RECEIVING_EVENT),
-          HasEventType(EventType::RESPOND_TO_INTRODUCTION),
-          Property(&SharingLog::respond_introduction,
-                   HasAction(ResponseToIntroduction::ACCEPT_INTRODUCTION)),
-          Property(&SharingLog::respond_introduction,
-                   HasSessionId(1234)))))));
+      Log(Matcher<const SharingLog&>(AllOf(
+          (HasCategory(EventCategory::RECEIVING_EVENT),
+           HasEventType(EventType::RESPOND_TO_INTRODUCTION),
+           Property(&SharingLog::respond_introduction,
+                    HasAction(ResponseToIntroduction::ACCEPT_INTRODUCTION)),
+           Property(&SharingLog::respond_introduction, HasSessionId(1234)))))));
   EXPECT_CALL(mock_event_logger_,
               Log(Matcher<const SharingLog&>(
                   AllOf((HasCategory(EventCategory::RECEIVING_EVENT),
@@ -621,8 +684,9 @@ TEST_F(IncomingShareSessionTest, AcceptTransferSuccess) {
 
   FakeNearbyConnectionsManager connections_manager;
   FakeClock clock;
-  session_.AcceptTransfer(&clock, connections_manager,
-                          [](int64_t, TransferMetadata) {});
+  EXPECT_THAT(session_.AcceptTransfer(&clock, connections_manager,
+                                      [](int64_t, TransferMetadata) {}),
+              IsTrue());
 
   for (auto it : session_.attachment_payload_map()) {
     EXPECT_THAT(

sharing/nearby_sharing_service_impl.cc

@@ -812,6 +812,24 @@ void NearbySharingServiceImpl::SendAttachments(
       });
 }
 
+bool NearbySharingServiceImpl::OutgoingSessionAccept(
+    OutgoingShareSession& session) {
+  if (!session.IsConnected()) {
+    NL_LOG(WARNING) << __func__
+                    << ": Accept invoked for unconnected share target";
+    return false;
+  }
+  if (!ReadyToAccept(session.self_share(),
+                     last_outgoing_metadata_.has_value()
+                         ? std::get<2>(*last_outgoing_metadata_).status()
+                         : TransferMetadata::Status::kUnknown)) {
+    NL_LOG(WARNING) << __func__ << ": out of order API call.";
+    return false;
+  }
+  SendPayloads(session);
+  return true;
+}
+
 void NearbySharingServiceImpl::Accept(
     int64_t share_target_id,
     std::function<void(StatusCodes status_codes)> status_codes_callback) {
@@ -819,48 +837,32 @@ void NearbySharingServiceImpl::Accept(
       "api_accept",
       [this, share_target_id,
        status_codes_callback = std::move(status_codes_callback)]() {
-        ShareSession* session = GetShareSession(share_target_id);
-        if (session == nullptr) {
-          NL_LOG(WARNING) << __func__
-                          << ": Accept invoked for unknown share target";
-          std::move(status_codes_callback)(StatusCodes::kInvalidArgument);
-          return;
-        }
-        if (!session->IsConnected()) {
-          NL_LOG(WARNING) << __func__
-                          << ": Accept invoked for unconnected share target";
-          std::move(status_codes_callback)(StatusCodes::kOutOfOrderApiCall);
-          return;
-        }
-
-        bool is_incoming = session->IsIncoming();
-        std::optional<
-            std::tuple<ShareTarget, AttachmentContainer, TransferMetadata>>
-            metadata =
-                is_incoming ? last_incoming_metadata_ : last_outgoing_metadata_;
-        if (!ReadyToAccept(session->self_share(),
-                           metadata.has_value()
-                               ? std::get<2>(*metadata).status()
-                               : TransferMetadata::Status::kUnknown)) {
-          NL_LOG(WARNING) << __func__ << ": out of order API call.";
-          std::move(status_codes_callback)(StatusCodes::kOutOfOrderApiCall);
-          return;
-        }
-
-        if (is_incoming) {
-          IncomingShareSession* incoming_session =
-              GetIncomingShareSession(share_target_id);
-          mutual_acceptance_timeout_alarm_.reset();
-
-          incoming_session->AcceptTransfer(
+        IncomingShareSession* incoming_session =
+            GetIncomingShareSession(share_target_id);
+        if (incoming_session != nullptr) {
+          // Incoming session.
+          bool accept_success = incoming_session->AcceptTransfer(
               context_->GetClock(), *nearby_connections_manager_,
               absl::bind_front(
                   &NearbySharingServiceImpl::OnPayloadTransferUpdate, this));
-          std::move(status_codes_callback)(StatusCodes::kOk);
+          std::move(status_codes_callback)(
+              accept_success ? StatusCodes::kOk
+                             : StatusCodes::kOutOfOrderApiCall);
           return;
         }
-
-        std::move(status_codes_callback)(SendPayloads(*session));
+        OutgoingShareSession* outgoing_session =
+            GetOutgoingShareSession(share_target_id);
+        if (outgoing_session != nullptr) {
+          // Outgoing session.
+          bool accept_success = OutgoingSessionAccept(*outgoing_session);
+          std::move(status_codes_callback)(
+              accept_success ? StatusCodes::kOk
+                             : StatusCodes::kOutOfOrderApiCall);
+          return;
+        }
+        NL_LOG(WARNING) << __func__
+                        << ": Accept invoked for unknown share target";
+        std::move(status_codes_callback)(StatusCodes::kInvalidArgument);
       });
 }
 
@@ -2409,16 +2411,9 @@ void NearbySharingServiceImpl::OnTransferStarted(bool is_incoming) {
   InvalidateSurfaceState();
 }
 
-NearbySharingService::StatusCodes NearbySharingServiceImpl::SendPayloads(
-    ShareSession& session) {
+void NearbySharingServiceImpl::SendPayloads(OutgoingShareSession& session) {
   NL_VLOG(1) << __func__ << ": Preparing to send payloads to "
              << session.share_target().id;
-  if (!session.IsConnected()) {
-    NL_LOG(WARNING) << __func__
-                    << ": Failed to send payload due to missing connection.";
-    return StatusCodes::kOutOfOrderApiCall;
-  }
-
   // Log analytics event of sending attachment start.
   analytics_recorder_->NewSendAttachmentsStart(
       session.session_id(), session.attachment_container(),
@@ -2430,9 +2425,15 @@ NearbySharingService::StatusCodes NearbySharingServiceImpl::SendPayloads(
           .set_token(session.token())
           .set_status(TransferMetadata::Status::kAwaitingRemoteAcceptance)
           .build());
-
-  ReceiveConnectionResponse(session);
-  return StatusCodes::kOk;
+  NL_VLOG(1) << __func__ << ": Receiving response frame from "
+             << session.share_target().id;
+  session.frames_reader()->ReadFrame(
+      nearby::sharing::service::proto::V1Frame::RESPONSE,
+      [this, share_target_id = session.share_target().id](
+          std::optional<nearby::sharing::service::proto::V1Frame> frame) {
+        OnReceiveConnectionResponse(share_target_id, std::move(frame));
+      },
+      kReadResponseFrameTimeout);
 }
 
 void NearbySharingServiceImpl::OnOutgoingConnection(
@@ -2487,12 +2488,6 @@ void NearbySharingServiceImpl::SendIntroduction(OutgoingShareSession& session) {
   NL_VLOG(1) << __func__ << ": Preparing to send introduction to "
              << session.share_target().id;
 
-  if (!session.IsConnected()) {
-    NL_LOG(WARNING) << __func__ << ": No NearbyConnection tied to "
-                    << session.share_target().id;
-    return;
-  }
-
   // Log analytics event of sending introduction.
   analytics_recorder_->NewSendIntroduction(
       session.session_id(), session.share_target(),
@@ -2500,12 +2495,10 @@ void NearbySharingServiceImpl::SendIntroduction(OutgoingShareSession& session) {
       /*concurrent_connections=*/GetConnectedShareTargetCount(),
       session.os_type());
 
-  NearbyConnection* connection = session.connection();
-
   if (foreground_send_surface_map_.empty() &&
       background_send_surface_map_.empty()) {
     NL_LOG(WARNING) << __func__ << ": No transfer callbacks, disconnecting.";
-    connection->Close();
+    session.connection()->Close();
     return;
   }
 
@@ -2634,18 +2627,14 @@ void NearbySharingServiceImpl::OnCreatePayloads(
       });
 }
 
-void NearbySharingServiceImpl::Fail(int64_t share_target_id,
+void NearbySharingServiceImpl::Fail(IncomingShareSession& session,
                                     TransferMetadata::Status status) {
-  ShareSession* session = GetShareSession(share_target_id);
-  if (!session || !session->IsConnected()) {
-    NL_LOG(WARNING) << __func__ << ": Fail invoked for unknown share target.";
-    return;
-  }
   RunOnNearbySharingServiceThreadDelayed(
       "incoming_rejection_delay", kIncomingRejectionDelay,
-      [this, share_target_id]() { CloseConnection(share_target_id); });
+      absl::bind_front(&NearbySharingServiceImpl::CloseConnection, this,
+                       session.share_target().id));
 
-  session->set_disconnect_status(status);
+  session.set_disconnect_status(status);
 
   // Send response to remote device.
   nearby::sharing::service::proto::ConnectionResponseFrame::Status
@@ -2672,8 +2661,8 @@ void NearbySharingServiceImpl::Fail(int64_t share_target_id,
       break;
   }
 
-  session->WriteResponseFrame(response_status);
-  session->UpdateTransferMetadata(
+  session.WriteResponseFrame(response_status);
+  session.UpdateTransferMetadata(
       TransferMetadataBuilder().set_status(status).build());
 }
 
@@ -2993,7 +2982,7 @@ void NearbySharingServiceImpl::OnReceivedIntroduction(
   std::optional<TransferMetadata::Status> status =
       session->ProcessIntroduction(*frame);
   if (status.has_value()) {
-    Fail(share_target_id, *status);
+    Fail(*session, *status);
     return;
   }
 
@@ -3023,7 +3012,7 @@ void NearbySharingServiceImpl::OnReceivedIntroduction(
       IsOutOfStorage(device_info_, download_path,
                      session->attachment_container().GetStorageSize());
   if (is_out_of_storage) {
-    Fail(share_target_id, TransferMetadata::Status::kNotEnoughSpace);
+    Fail(*session, TransferMetadata::Status::kNotEnoughSpace);
     NL_LOG(WARNING) << __func__
                     << ": Not enough space on the receiver. We have informed "
                     << share_target_id;
@@ -3033,21 +3022,6 @@ void NearbySharingServiceImpl::OnReceivedIntroduction(
   OnStorageCheckCompleted(*session);
 }
 
-void NearbySharingServiceImpl::ReceiveConnectionResponse(
-    ShareSession& session) {
-  NL_VLOG(1) << __func__ << ": Receiving response frame from "
-             << session.share_target().id;
-  NL_DCHECK(session.IsConnected());
-
-  session.frames_reader()->ReadFrame(
-      nearby::sharing::service::proto::V1Frame::RESPONSE,
-      [this, share_target_id = session.share_target().id](
-          std::optional<nearby::sharing::service::proto::V1Frame> frame) {
-        OnReceiveConnectionResponse(share_target_id, std::move(frame));
-      },
-      kReadResponseFrameTimeout);
-}
-
 void NearbySharingServiceImpl::OnReceiveConnectionResponse(
     int64_t share_target_id,
     std::optional<nearby::sharing::service::proto::V1Frame> frame) {
@@ -3163,39 +3137,24 @@ void NearbySharingServiceImpl::OnStorageCheckCompleted(
             << __func__
             << ": Incoming mutual acceptance timed out, closing connection for "
             << share_target_id;
-
-        Fail(share_target_id, TransferMetadata::Status::kTimedOut);
+        IncomingShareSession* session =
+            GetIncomingShareSession(share_target_id);
+        if (session != nullptr) {
+          Fail(*session, TransferMetadata::Status::kTimedOut);
+        }
       });
-
-  if (!session.self_share()) {
-    TransferMetadataBuilder transfer_metadata_builder;
-    transfer_metadata_builder.set_status(
-        TransferMetadata::Status::kAwaitingLocalConfirmation);
-    transfer_metadata_builder.set_token(session.token());
-
-    session.UpdateTransferMetadata(transfer_metadata_builder.build());
-  } else {
+  if (session.ReadyForTransfer(
+          absl::bind_front(&NearbySharingServiceImpl::OnFrameRead, this,
+                           session.share_target().id))) {
     // Don't need to send kAwaitingLocalConfirmation for auto accept of Self
     // share.
+    NL_LOG(INFO) << __func__ << ": Auto-accepting self share.";
+    session.AcceptTransfer(
+        context_->GetClock(), *nearby_connections_manager_,
+        absl::bind_front(&NearbySharingServiceImpl::OnPayloadTransferUpdate,
+                         this));
     OnTransferStarted(/*is_incoming=*/true);
   }
-
-  session.set_disconnect_status(
-      TransferMetadata::Status::kUnexpectedDisconnection);
-
-  if (session.self_share()) {
-    NL_LOG(INFO) << __func__ << ": Auto-accepting self share.";
-    Accept(session.share_target().id, [](StatusCodes status_codes) {
-      NL_LOG(INFO) << __func__ << ": Auto-accepting result: "
-                   << static_cast<int>(status_codes);
-    });
-  }
-
-  session.frames_reader()->ReadFrame(
-      [this, share_target_id = session.share_target().id](
-          std::optional<nearby::sharing::service::proto::V1Frame> frame) {
-        OnFrameRead(share_target_id, std::move(frame));
-      });
 }
 
 void NearbySharingServiceImpl::OnFrameRead(
@@ -3258,6 +3217,7 @@ void NearbySharingServiceImpl::HandleProgressUpdateFrame(
       NL_LOG(ERROR) << "Received ProgressUpdate Frame on unknown session";
       return;
     }
+    mutual_acceptance_timeout_alarm_.reset();
     NL_LOG(INFO) << __func__ << ": Received progress for ShareTarget "
                  << share_target_id << " : "
                  << progress_update_frame.progress();

sharing/nearby_sharing_service_impl.h

@@ -312,7 +312,7 @@ class NearbySharingServiceImpl
   void OnTransferComplete();
   void OnTransferStarted(bool is_incoming);
 
-  StatusCodes SendPayloads(ShareSession& session);
+  void SendPayloads(OutgoingShareSession& session);
 
   void OnOutgoingConnection(absl::Time connect_start_time,
                             NearbyConnection* connection,
@@ -325,7 +325,7 @@ class NearbySharingServiceImpl
   void OnCreatePayloads(std::vector<uint8_t> endpoint_info,
                         OutgoingShareSession& session, bool success);
 
-  void Fail(int64_t share_target_id, TransferMetadata::Status status);
+  void Fail(IncomingShareSession& session, TransferMetadata::Status status);
   void OnIncomingAdvertisementDecoded(
       absl::string_view endpoint_id, IncomingShareSession& session,
       std::unique_ptr<Advertisement> advertisement);
@@ -349,7 +349,6 @@ class NearbySharingServiceImpl
   void OnReceivedIntroduction(
       int64_t share_target_id,
       std::optional<nearby::sharing::service::proto::IntroductionFrame> frame);
-  void ReceiveConnectionResponse(ShareSession& session);
   void OnReceiveConnectionResponse(
       int64_t share_target_id,
       std::optional<nearby::sharing::service::proto::V1Frame> frame);
@@ -457,6 +456,8 @@ class NearbySharingServiceImpl
   // Send initial adapter state to observer for each supported adapter.
   void SendInitialAdapterState(NearbySharingService::Observer* observer);
 
+  bool OutgoingSessionAccept(OutgoingShareSession& session);
+
   // Used to run nearby sharing service APIs.
   std::unique_ptr<TaskRunner> service_thread_;
   Context* const context_;

sharing/share_session.cc

@@ -97,6 +97,14 @@ void ShareSession::UpdateTransferMetadata(
   InvokeTransferUpdateCallback(transfer_metadata);
 }
 
+std::weak_ptr<NearbyConnectionsManager::PayloadStatusListener>
+ShareSession::payload_tracker() const {
+  if (!payload_tracker_) {
+    return std::weak_ptr<NearbyConnectionsManager::PayloadStatusListener>();
+  }
+  return payload_tracker_->GetWeakPtr();
+}
+
 void ShareSession::set_disconnect_status(
     TransferMetadata::Status disconnect_status) {
   disconnect_status_ = disconnect_status;

sharing/share_session.h

@@ -78,9 +78,7 @@ class ShareSession {
   IncomingFramesReader* frames_reader() const { return frames_reader_.get(); }
 
   std::weak_ptr<NearbyConnectionsManager::PayloadStatusListener>
-  payload_tracker() const {
-    return payload_tracker_->GetWeakPtr();
-  }
+  payload_tracker() const;
 
   int64_t session_id() const { return session_id_; }