diff --git a/sharing/incoming_share_session.cc b/sharing/incoming_share_session.cc index 6ff61784..425a766d 100644 --- a/sharing/incoming_share_session.cc +++ b/sharing/incoming_share_session.cc @@ -51,22 +51,23 @@ namespace nearby::sharing { namespace { using ::location::nearby::proto::sharing::OSType; +using ::location::nearby::proto::sharing::ResponseToIntroduction; using ::nearby::sharing::service::proto::ConnectionResponseFrame; using ::nearby::sharing::service::proto::IntroductionFrame; -using ::location::nearby::proto::sharing::ResponseToIntroduction; using ::nearby::sharing::service::proto::V1Frame; using ::nearby::sharing::service::proto::WifiCredentials; } // namespace IncomingShareSession::IncomingShareSession( - TaskRunner& service_thread, + Clock* clock, TaskRunner& service_thread, + NearbyConnectionsManager* connections_manager, analytics::AnalyticsRecorder& analytics_recorder, std::string endpoint_id, const ShareTarget& share_target, std::function transfer_update_callback) - : ShareSession(service_thread, analytics_recorder, std::move(endpoint_id), - share_target), + : ShareSession(clock, service_thread, connections_manager, + analytics_recorder, std::move(endpoint_id), share_target), transfer_update_callback_(std::move(transfer_update_callback)) {} IncomingShareSession::IncomingShareSession(IncomingShareSession&&) = default; @@ -209,7 +210,6 @@ bool IncomingShareSession::ReadyForTransfer( } bool IncomingShareSession::AcceptTransfer( - Clock* clock, std::function update_callback) { if (!ready_for_accept_ || !IsConnected()) { NL_LOG(WARNING) << __func__ << ": out of order API call."; @@ -219,7 +219,7 @@ bool IncomingShareSession::AcceptTransfer( const absl::flat_hash_map& payload_map = attachment_payload_map(); set_payload_tracker(std::make_shared( - clock, share_target().id, attachment_container(), payload_map, + &clock(), share_target().id, attachment_container(), payload_map, std::move(update_callback))); // Register status listener for all payloads. @@ -228,8 +228,8 @@ bool IncomingShareSession::AcceptTransfer( << ": Started listening for progress on payload: " << it->second << " for attachment: " << it->first; - connections_manager()->RegisterPayloadStatusListener(it->second, - payload_tracker()); + connections_manager().RegisterPayloadStatusListener(it->second, + payload_tracker()); NL_VLOG(1) << __func__ << ": Accepted incoming files from share target - " << share_target().id; @@ -277,7 +277,7 @@ bool IncomingShareSession::UpdateFilePayloadPaths() { } const Payload* incoming_payload = - connections_manager()->GetIncomingPayload(it->second); + connections_manager().GetIncomingPayload(it->second); if (!incoming_payload || !incoming_payload->content.is_file()) { NL_LOG(WARNING) << __func__ << ": No payload found for file - " << file.id(); @@ -309,7 +309,7 @@ bool IncomingShareSession::UpdatePayloadContents() { return false; } const Payload* incoming_payload = - connections_manager()->GetIncomingPayload(it->second); + connections_manager().GetIncomingPayload(it->second); if (!incoming_payload || !incoming_payload->content.is_bytes()) { NL_LOG(WARNING) << __func__ << ": No payload found for text - " << text.id(); @@ -344,7 +344,7 @@ bool IncomingShareSession::UpdatePayloadContents() { } const Payload* incoming_payload = - connections_manager()->GetIncomingPayload(it->second); + connections_manager().GetIncomingPayload(it->second); if (!incoming_payload || !incoming_payload->content.is_bytes()) { NL_LOG(WARNING) << __func__ << ": No payload found for WiFi credentials - " @@ -407,7 +407,7 @@ bool IncomingShareSession::TryUpgradeBandwidth() { if (!bandwidth_upgrade_requested_ && attachment_container().GetTotalAttachmentsSize() >= kAttachmentsSizeThresholdOverHighQualityMedium) { - connections_manager()->UpgradeBandwidth(endpoint_id()); + connections_manager().UpgradeBandwidth(endpoint_id()); bandwidth_upgrade_requested_ = true; return true; } @@ -439,8 +439,7 @@ void IncomingShareSession::SendFailureResponse( WriteResponseFrame(response_status); NL_DCHECK(TransferMetadata::IsFinalStatus(status)) << "SendFailureResponse should only be called with a final status"; - UpdateTransferMetadata( - TransferMetadataBuilder().set_status(status).build()); + UpdateTransferMetadata(TransferMetadataBuilder().set_status(status).build()); } std::pair IncomingShareSession::PayloadTransferUpdate( diff --git a/sharing/incoming_share_session.h b/sharing/incoming_share_session.h index 1bfaf9e7..2c9661fa 100644 --- a/sharing/incoming_share_session.h +++ b/sharing/incoming_share_session.h @@ -28,6 +28,7 @@ #include "internal/platform/task_runner.h" #include "sharing/analytics/analytics_recorder.h" #include "sharing/nearby_connection.h" +#include "sharing/nearby_connections_manager.h" #include "sharing/paired_key_verification_runner.h" #include "sharing/proto/wire_format.pb.h" #include "sharing/share_session.h" @@ -42,7 +43,8 @@ namespace nearby::sharing { class IncomingShareSession : public ShareSession { public: IncomingShareSession( - TaskRunner& service_thread, + Clock* clock, TaskRunner& service_thread, + NearbyConnectionsManager* connections_manager, analytics::AnalyticsRecorder& analytics_recorder, std::string endpoint_id, const ShareTarget& share_target, std::function @@ -82,7 +84,6 @@ class IncomingShareSession : public ShareSession { // Accept the transfer and begin listening for payload transfer updates. // Returns false if session is not in a state to accept the transfer. bool AcceptTransfer( - Clock* clock, std::function update_callback); // Returns the file paths of all file payloads. diff --git a/sharing/incoming_share_session_test.cc b/sharing/incoming_share_session_test.cc index a01b70bc..bd0cbb74 100644 --- a/sharing/incoming_share_session_test.cc +++ b/sharing/incoming_share_session_test.cc @@ -116,8 +116,9 @@ std::unique_ptr CreateWifiCredentialsPayload( class IncomingShareSessionTest : public ::testing::Test { protected: IncomingShareSessionTest() - : session_(task_runner_, analytics_recorder_, std::string(kEndpointId), - share_target_, transfer_metadata_callback_.AsStdFunction()) { + : session_(&clock_, task_runner_, &connections_manager_, + analytics_recorder_, std::string(kEndpointId), share_target_, + transfer_metadata_callback_.AsStdFunction()) { NL_CHECK( proto2::TextFormat::ParseFromString(R"pb( file_metadata { @@ -183,8 +184,7 @@ class IncomingShareSessionTest : public ::testing::Test { }; TEST_F(IncomingShareSessionTest, ProcessIntroductionNoSupportedPayload) { - EXPECT_TRUE( - session_.OnConnected(absl::Now(), &connections_manager_, &connection_)); + EXPECT_TRUE(session_.OnConnected(absl::Now(), &connection_)); IntroductionFrame frame; EXPECT_THAT(session_.ProcessIntroduction(frame), @@ -193,8 +193,7 @@ TEST_F(IncomingShareSessionTest, ProcessIntroductionNoSupportedPayload) { } TEST_F(IncomingShareSessionTest, ProcessIntroductionEmptyFile) { - EXPECT_TRUE( - session_.OnConnected(absl::Now(), &connections_manager_, &connection_)); + EXPECT_TRUE(session_.OnConnected(absl::Now(), &connection_)); IntroductionFrame frame; frame.mutable_file_metadata(); @@ -204,8 +203,7 @@ TEST_F(IncomingShareSessionTest, ProcessIntroductionEmptyFile) { } TEST_F(IncomingShareSessionTest, ProcessIntroductionFilesTooLarge) { - EXPECT_TRUE( - session_.OnConnected(absl::Now(), &connections_manager_, &connection_)); + EXPECT_TRUE(session_.OnConnected(absl::Now(), &connection_)); IntroductionFrame frame; FileMetadata file1; FileMetadata file2; @@ -220,8 +218,7 @@ TEST_F(IncomingShareSessionTest, ProcessIntroductionFilesTooLarge) { } TEST_F(IncomingShareSessionTest, ProcessIntroductionEmptyText) { - EXPECT_TRUE( - session_.OnConnected(absl::Now(), &connections_manager_, &connection_)); + EXPECT_TRUE(session_.OnConnected(absl::Now(), &connection_)); IntroductionFrame frame; frame.mutable_text_metadata(); @@ -231,8 +228,7 @@ TEST_F(IncomingShareSessionTest, ProcessIntroductionEmptyText) { } TEST_F(IncomingShareSessionTest, ProcessIntroductionSuccess) { - EXPECT_TRUE( - session_.OnConnected(absl::Now(), &connections_manager_, &connection_)); + EXPECT_TRUE(session_.OnConnected(absl::Now(), &connection_)); FileMetadata filemeta1 = introduction_frame_.file_metadata(0); FileAttachment file1(filemeta1.id(), filemeta1.size(), filemeta1.name(), filemeta1.mime_type(), filemeta1.type(), @@ -281,8 +277,7 @@ TEST_F(IncomingShareSessionTest, ProcessIntroductionSuccess) { TEST_F(IncomingShareSessionTest, PayloadTransferUpdateCompleteWithWrongPayloadType) { - EXPECT_TRUE( - session_.OnConnected(absl::Now(), &connections_manager_, &connection_)); + EXPECT_TRUE(session_.OnConnected(absl::Now(), &connection_)); EXPECT_THAT(session_.ProcessIntroduction(introduction_frame_), Eq(std::nullopt)); int64_t payload_id1 = introduction_frame_.file_metadata(0).payload_id(); @@ -338,8 +333,7 @@ TEST_F(IncomingShareSessionTest, TEST_F(IncomingShareSessionTest, PayloadTransferUpdateCompleteWithMissingFilePayloads) { - EXPECT_TRUE( - session_.OnConnected(absl::Now(), &connections_manager_, &connection_)); + EXPECT_TRUE(session_.OnConnected(absl::Now(), &connection_)); EXPECT_THAT(session_.ProcessIntroduction(introduction_frame_), Eq(std::nullopt)); std::filesystem::path file1_path = "/usr/tmp/file1"; @@ -412,8 +406,7 @@ TEST_F(IncomingShareSessionTest, TEST_F(IncomingShareSessionTest, PayloadTransferUpdateCompleteWithMissingTextPayloads) { - EXPECT_TRUE( - session_.OnConnected(absl::Now(), &connections_manager_, &connection_)); + EXPECT_TRUE(session_.OnConnected(absl::Now(), &connection_)); EXPECT_THAT(session_.ProcessIntroduction(introduction_frame_), Eq(std::nullopt)); std::filesystem::path file1_path = "/usr/tmp/file1"; @@ -483,8 +476,7 @@ TEST_F(IncomingShareSessionTest, TEST_F(IncomingShareSessionTest, PayloadTransferUpdateCompleteWithMissingWifiPayloads) { - EXPECT_TRUE( - session_.OnConnected(absl::Now(), &connections_manager_, &connection_)); + EXPECT_TRUE(session_.OnConnected(absl::Now(), &connection_)); EXPECT_THAT(session_.ProcessIntroduction(introduction_frame_), Eq(std::nullopt)); std::filesystem::path file1_path = "/usr/tmp/file1"; @@ -552,8 +544,7 @@ TEST_F(IncomingShareSessionTest, } TEST_F(IncomingShareSessionTest, GetPayloadFilePaths) { - EXPECT_TRUE( - session_.OnConnected(absl::Now(), &connections_manager_, &connection_)); + EXPECT_TRUE(session_.OnConnected(absl::Now(), &connection_)); IntroductionFrame introduction_frame; FileMetadata file1; FileMetadata file2; @@ -592,8 +583,7 @@ TEST_F(IncomingShareSessionTest, GetPayloadFilePaths) { } TEST_F(IncomingShareSessionTest, PayloadTransferUpdateCompleteWithSuccess) { - EXPECT_TRUE( - session_.OnConnected(absl::Now(), &connections_manager_, &connection_)); + EXPECT_TRUE(session_.OnConnected(absl::Now(), &connection_)); EXPECT_THAT(session_.ProcessIntroduction(introduction_frame_), Eq(std::nullopt)); std::filesystem::path file1_path = "/usr/tmp/file1"; @@ -670,8 +660,7 @@ TEST_F(IncomingShareSessionTest, PayloadTransferUpdateCompleteWithSuccess) { } TEST_F(IncomingShareSessionTest, PayloadTransferUpdateCancelled) { - EXPECT_TRUE( - session_.OnConnected(absl::Now(), &connections_manager_, &connection_)); + EXPECT_TRUE(session_.OnConnected(absl::Now(), &connection_)); EXPECT_THAT(session_.ProcessIntroduction(introduction_frame_), Eq(std::nullopt)); std::filesystem::path file1_path = "/usr/tmp/file1"; @@ -726,8 +715,7 @@ TEST_F(IncomingShareSessionTest, PayloadTransferUpdateCancelled) { } TEST_F(IncomingShareSessionTest, PayloadTransferUpdateFailed) { - EXPECT_TRUE( - session_.OnConnected(absl::Now(), &connections_manager_, &connection_)); + EXPECT_TRUE(session_.OnConnected(absl::Now(), &connection_)); EXPECT_THAT(session_.ProcessIntroduction(introduction_frame_), Eq(std::nullopt)); std::filesystem::path file1_path = "/usr/tmp/file1"; @@ -775,8 +763,7 @@ TEST_F(IncomingShareSessionTest, PayloadTransferUpdateFailed) { } TEST_F(IncomingShareSessionTest, PayloadTransferUpdateInProgress) { - EXPECT_TRUE( - session_.OnConnected(absl::Now(), &connections_manager_, &connection_)); + EXPECT_TRUE(session_.OnConnected(absl::Now(), &connection_)); EXPECT_THAT(session_.ProcessIntroduction(introduction_frame_), Eq(std::nullopt)); std::filesystem::path file1_path = "/usr/tmp/file1"; @@ -827,7 +814,6 @@ TEST_F(IncomingShareSessionTest, PayloadTransferUpdateInProgress) { TEST_F(IncomingShareSessionTest, ReadyForTransferNotConnected) { session_.set_session_id(1234); - FakeClock clock; EXPECT_THAT( session_.ReadyForTransfer([]() {}, [](std::optional frame) {}), IsFalse()); @@ -835,8 +821,7 @@ TEST_F(IncomingShareSessionTest, ReadyForTransferNotConnected) { TEST_F(IncomingShareSessionTest, ReadyForTransferNotSelfShare) { session_.set_session_id(1234); - EXPECT_TRUE( - session_.OnConnected(absl::Now(), &connections_manager_, &connection_)); + EXPECT_TRUE(session_.OnConnected(absl::Now(), &connection_)); EXPECT_CALL( transfer_metadata_callback_, Call(_, HasStatus(TransferMetadata::Status::kAwaitingLocalConfirmation))); @@ -849,12 +834,12 @@ TEST_F(IncomingShareSessionTest, ReadyForTransferNotSelfShare) { TEST_F(IncomingShareSessionTest, ReadyForTransferSelfShare) { ShareTarget share_target; share_target.for_self_share = true; - IncomingShareSession session(task_runner_, analytics_recorder_, - std::string("XYCA"), share_target, + IncomingShareSession session(&clock_, task_runner_, &connections_manager_, + analytics_recorder_, std::string("XYCA"), + share_target, transfer_metadata_callback_.AsStdFunction()); session.set_session_id(1234); - EXPECT_TRUE( - session.OnConnected(absl::Now(), &connections_manager_, &connection_)); + EXPECT_TRUE(session.OnConnected(absl::Now(), &connection_)); EXPECT_CALL( transfer_metadata_callback_, Call(_, HasStatus(TransferMetadata::Status::kAwaitingLocalConfirmation))) @@ -867,8 +852,7 @@ TEST_F(IncomingShareSessionTest, ReadyForTransferSelfShare) { TEST_F(IncomingShareSessionTest, ReadyForTransferTimeout) { session_.set_session_id(1234); - EXPECT_TRUE( - session_.OnConnected(absl::Now(), &connections_manager_, &connection_)); + EXPECT_TRUE(session_.OnConnected(absl::Now(), &connection_)); EXPECT_CALL( transfer_metadata_callback_, Call(_, HasStatus(TransferMetadata::Status::kAwaitingLocalConfirmation))); @@ -886,8 +870,7 @@ TEST_F(IncomingShareSessionTest, ReadyForTransferTimeout) { TEST_F(IncomingShareSessionTest, ReadyForTransferTimeoutCancelled) { session_.set_session_id(1234); - EXPECT_TRUE( - session_.OnConnected(absl::Now(), &connections_manager_, &connection_)); + EXPECT_TRUE(session_.OnConnected(absl::Now(), &connection_)); EXPECT_CALL( transfer_metadata_callback_, Call(_, HasStatus(TransferMetadata::Status::kAwaitingLocalConfirmation))); @@ -914,27 +897,23 @@ TEST_F(IncomingShareSessionTest, ReadyForTransferTimeoutCancelled) { TEST_F(IncomingShareSessionTest, AcceptTransferNotConnected) { session_.set_session_id(1234); - FakeClock clock; - EXPECT_THAT(session_.AcceptTransfer(&clock, [](int64_t, TransferMetadata) {}), + EXPECT_THAT(session_.AcceptTransfer([](int64_t, TransferMetadata) {}), IsFalse()); } TEST_F(IncomingShareSessionTest, AcceptTransferNotReady) { session_.set_session_id(1234); - EXPECT_TRUE( - session_.OnConnected(absl::Now(), &connections_manager_, &connection_)); + EXPECT_TRUE(session_.OnConnected(absl::Now(), &connection_)); EXPECT_THAT(session_.ProcessIntroduction(introduction_frame_), Eq(std::nullopt)); - FakeClock clock; - EXPECT_THAT(session_.AcceptTransfer(&clock, [](int64_t, TransferMetadata) {}), + EXPECT_THAT(session_.AcceptTransfer([](int64_t, TransferMetadata) {}), IsFalse()); } TEST_F(IncomingShareSessionTest, AcceptTransferSuccess) { session_.set_session_id(1234); - EXPECT_TRUE( - session_.OnConnected(absl::Now(), &connections_manager_, &connection_)); + EXPECT_TRUE(session_.OnConnected(absl::Now(), &connection_)); EXPECT_THAT(session_.ProcessIntroduction(introduction_frame_), Eq(std::nullopt)); EXPECT_THAT( @@ -958,8 +937,7 @@ TEST_F(IncomingShareSessionTest, AcceptTransferSuccess) { Property(&SharingLog::receive_attachments_start, HasSessionId(1234))))))); - FakeClock clock; - EXPECT_THAT(session_.AcceptTransfer(&clock, [](int64_t, TransferMetadata) {}), + EXPECT_THAT(session_.AcceptTransfer([](int64_t, TransferMetadata) {}), IsTrue()); for (auto it : session_.attachment_payload_map()) { @@ -978,7 +956,7 @@ TEST_F(IncomingShareSessionTest, AcceptTransferSuccess) { } TEST_F(IncomingShareSessionTest, ProcessKeyVerificationResultSuccess) { - session_.OnConnected(absl::Now(), &connections_manager_, &connection_); + session_.OnConnected(absl::Now(), &connection_); session_.SetTokenForTests("1234"); bool introduction_received = false; @@ -1012,7 +990,7 @@ TEST_F(IncomingShareSessionTest, ProcessKeyVerificationResultSuccess) { } TEST_F(IncomingShareSessionTest, ProcessKeyVerificationResultFail) { - session_.OnConnected(absl::Now(), &connections_manager_, &connection_); + session_.OnConnected(absl::Now(), &connection_); session_.SetTokenForTests("1234"); bool introduction_received = false; @@ -1045,7 +1023,7 @@ TEST_F(IncomingShareSessionTest, ProcessKeyVerificationResultFail) { } TEST_F(IncomingShareSessionTest, ProcessKeyVerificationResultUnable) { - session_.OnConnected(absl::Now(), &connections_manager_, &connection_); + session_.OnConnected(absl::Now(), &connection_); session_.SetTokenForTests("1234"); bool introduction_received = false; @@ -1078,7 +1056,7 @@ TEST_F(IncomingShareSessionTest, ProcessKeyVerificationResultUnable) { } TEST_F(IncomingShareSessionTest, ProcessKeyVerificationResultUnknown) { - session_.OnConnected(absl::Now(), &connections_manager_, &connection_); + session_.OnConnected(absl::Now(), &connection_); session_.SetTokenForTests("1234"); bool introduction_received = false; @@ -1111,7 +1089,7 @@ TEST_F(IncomingShareSessionTest, ProcessKeyVerificationResultUnknown) { } TEST_F(IncomingShareSessionTest, TryUpgradeBandwidthNotNeeded) { - session_.OnConnected(absl::Now(), &connections_manager_, &connection_); + session_.OnConnected(absl::Now(), &connection_); EXPECT_THAT(session_.TryUpgradeBandwidth(), IsFalse()); EXPECT_THAT(connections_manager_.DidUpgradeBandwidth(kEndpointId), IsFalse()); @@ -1141,7 +1119,7 @@ TEST_F(IncomingShareSessionTest, TryUpgradeBandwidthNeeded) { } )pb", &introduction_frame)); - session_.OnConnected(absl::Now(), &connections_manager_, &connection_); + session_.OnConnected(absl::Now(), &connection_); EXPECT_THAT(session_.ProcessIntroduction(introduction_frame), Eq(std::nullopt)); @@ -1157,7 +1135,7 @@ TEST_F(IncomingShareSessionTest, SendFailureResponseNotConnected) { } TEST_F(IncomingShareSessionTest, SendFailureResponseConnected) { - session_.OnConnected(absl::Now(), &connections_manager_, &connection_); + session_.OnConnected(absl::Now(), &connection_); EXPECT_CALL(transfer_metadata_callback_, Call(_, HasStatus(TransferMetadata::Status::kNotEnoughSpace))); diff --git a/sharing/nearby_sharing_service_impl.cc b/sharing/nearby_sharing_service_impl.cc index 85a1d50c..e5223378 100644 --- a/sharing/nearby_sharing_service_impl.cc +++ b/sharing/nearby_sharing_service_impl.cc @@ -826,9 +826,8 @@ void NearbySharingServiceImpl::Accept( GetIncomingShareSession(share_target_id); if (incoming_session != nullptr) { // Incoming session. - bool accept_success = incoming_session->AcceptTransfer( - context_->GetClock(), - absl::bind_front( + bool accept_success = + incoming_session->AcceptTransfer(absl::bind_front( &NearbySharingServiceImpl::IncomingPayloadTransferUpdate, this)); std::move(status_codes_callback)( @@ -1069,8 +1068,7 @@ void NearbySharingServiceImpl::OnIncomingConnection( IncomingShareSession& session = CreateIncomingShareSession( placeholder_share_target, endpoint_id, /*certificate=*/std::nullopt); session.set_session_id(analytics_recorder_->GenerateNextId()); - session.OnConnected(context_->GetClock()->Now(), - nearby_connections_manager_.get(), connection); + session.OnConnected(context_->GetClock()->Now(), connection); connection->SetDisconnectionListener([this, placeholder_share_target_id]() { OnConnectionDisconnected(placeholder_share_target_id); }); @@ -2448,8 +2446,7 @@ void NearbySharingServiceImpl::OnOutgoingConnection( absl::Time connect_start_time, NearbyConnection* connection, OutgoingShareSession& session) { int64_t share_target_id = session.share_target().id; - if (!session.OnConnected(connect_start_time, - nearby_connections_manager_.get(), connection)) { + if (!session.OnConnected(connect_start_time, connection)) { session.Abort(session.disconnect_status()); return; } @@ -2476,7 +2473,7 @@ void NearbySharingServiceImpl::OnOutgoingConnection( return; } session.RunPairedKeyVerification( - context_->GetClock(), ToProtoOsType(device_info_.GetOsType()), + ToProtoOsType(device_info_.GetOsType()), { .visibility = settings_->GetVisibility(), .last_visibility = settings_->GetLastVisibility(), @@ -2819,8 +2816,7 @@ void NearbySharingServiceImpl::OnIncomingDecryptedCertificate( *share_target, endpoint_id, std::move(certificate)); // Copy session id from placeholder session to actual session. session.set_session_id(session_id); - session.OnConnected(context_->GetClock()->Now(), - nearby_connections_manager_.get(), connection); + session.OnConnected(context_->GetClock()->Now(), connection); // Need to rebind the disconnect listener to the new share target id. connection->SetDisconnectionListener( [this, share_target_id]() { OnConnectionDisconnected(share_target_id); }); @@ -2834,7 +2830,7 @@ void NearbySharingServiceImpl::OnIncomingDecryptedCertificate( return; } session.RunPairedKeyVerification( - context_->GetClock(), ToProtoOsType(device_info_.GetOsType()), + ToProtoOsType(device_info_.GetOsType()), { .visibility = settings_->GetVisibility(), .last_visibility = settings_->GetLastVisibility(), @@ -2973,7 +2969,6 @@ void NearbySharingServiceImpl::OnReceiveConnectionResponse( NearbyFlags::GetInstance().GetBoolFlag( config_package_nearby::nearby_sharing_feature:: kEnableTransferCancellationOptimization), - context_->GetClock(), [this, share_target_id]( std::optional frame) { OnFrameRead(share_target_id, std::move(frame)); @@ -3001,10 +2996,8 @@ void NearbySharingServiceImpl::OnStorageCheckCompleted( } // Don't need to wait for user to accept for Self share. NL_LOG(INFO) << __func__ << ": Auto-accepting self share."; - session.AcceptTransfer( - context_->GetClock(), - absl::bind_front(&NearbySharingServiceImpl::IncomingPayloadTransferUpdate, - this)); + session.AcceptTransfer(absl::bind_front( + &NearbySharingServiceImpl::IncomingPayloadTransferUpdate, this)); OnTransferStarted(/*is_incoming=*/true); } @@ -3260,7 +3253,8 @@ IncomingShareSession& NearbySharingServiceImpl::CreateIncomingShareSession( std::optional certificate) { NL_DCHECK(share_target.is_incoming); auto [it, inserted] = incoming_share_session_map_.try_emplace( - share_target.id, *service_thread_, *analytics_recorder_, + share_target.id, context_->GetClock(), *service_thread_, + nearby_connections_manager_.get(), *analytics_recorder_, std::string(endpoint_id), share_target, absl::bind_front(&NearbySharingServiceImpl::OnIncomingTransferUpdate, this)); @@ -3486,7 +3480,8 @@ void NearbySharingServiceImpl::CreateOutgoingShareSession( const ShareTarget& share_target, absl::string_view endpoint_id, std::optional certificate) { auto [it_out, inserted] = outgoing_share_session_map_.try_emplace( - share_target.id, *service_thread_, *analytics_recorder_, + share_target.id, context_->GetClock(), *service_thread_, + nearby_connections_manager_.get(), *analytics_recorder_, std::string(endpoint_id), share_target, absl::bind_front(&NearbySharingServiceImpl::OnOutgoingTransferUpdate, this)); diff --git a/sharing/nearby_sharing_service_impl_test.cc b/sharing/nearby_sharing_service_impl_test.cc index b720a605..765393e1 100644 --- a/sharing/nearby_sharing_service_impl_test.cc +++ b/sharing/nearby_sharing_service_impl_test.cc @@ -630,16 +630,14 @@ class NearbySharingServiceImplTest : public testing::Test { default_instance() .New(); paired_key_encryption_frame->set_signed_data( - is_incoming - ? std::string(GetIncomingConnectionSignedData().begin(), - GetIncomingConnectionSignedData().end()) - : std::string(GetOutgoingConnectionSignedData().begin(), - GetOutgoingConnectionSignedData().end())); + is_incoming ? std::string(GetIncomingConnectionSignedData().begin(), + GetIncomingConnectionSignedData().end()) + : std::string(GetOutgoingConnectionSignedData().begin(), + GetOutgoingConnectionSignedData().end())); paired_key_encryption_frame->set_secret_id_hash( std::string(GetPrivateCertificateHashAuthToken().begin(), GetPrivateCertificateHashAuthToken().end())); - v1_frame->set_allocated_paired_key_encryption( - paired_key_encryption_frame); + v1_frame->set_allocated_paired_key_encryption(paired_key_encryption_frame); std::vector encryption_bytes(frame.ByteSizeLong()); frame.SerializeToArray(encryption_bytes.data(), encryption_bytes.size()); @@ -661,7 +659,7 @@ class NearbySharingServiceImplTest : public testing::Test { } std::vector CreateTestEndpointInfo(uint8_t vendor_id = kVendorId) { - std::unique_ptr advertisement = Advertisement::NewInstance( + std::unique_ptr advertisement = Advertisement::NewInstance( GetNearbyShareTestEncryptedMetadataKey().salt(), GetNearbyShareTestEncryptedMetadataKey().encrypted_key(), kDeviceType, kDeviceName, vendor_id); @@ -1014,7 +1012,7 @@ class NearbySharingServiceImplTest : public testing::Test { } std::vector CreateInvalidTestEndpointInfo() { - std::unique_ptr advertisement = Advertisement::NewInstance( + std::unique_ptr advertisement = Advertisement::NewInstance( GetNearbyShareTestEncryptedMetadataKey().salt(), GetNearbyShareTestEncryptedMetadataKey().encrypted_key(), kDeviceType, std::nullopt, kVendorId); @@ -1146,8 +1144,9 @@ class NearbySharingServiceImplTest : public testing::Test { EXPECT_TRUE( success_notification.WaitForNotificationWithTimeout(kWaitTimeout)); FlushTesting(); - EXPECT_FALSE(fake_nearby_connections_manager_->connection_endpoint_info( - kEndpointId).has_value()); + EXPECT_FALSE( + fake_nearby_connections_manager_->connection_endpoint_info(kEndpointId) + .has_value()); EXPECT_FALSE(fake_nearby_connections_manager_->has_incoming_payloads()); // To avoid UAF in OnIncomingTransferUpdate(). @@ -1306,9 +1305,7 @@ class TestObserver : public NearbySharingService::Observer { void OnLanStatusChanged(AdapterState state) override { lan_state_ = state; } - void OnCredentialError() override { - credential_error_called_ = true; - } + void OnCredentialError() override { credential_error_called_ = true; } void OnShutdown() override { shutdown_called_ = true; @@ -2555,8 +2552,7 @@ TEST_F(NearbySharingServiceImplTest, IncomingConnectionOutOfStorage) { nearby::sharing::service::proto::FileMetadata* file_metadata = introduction_frame->add_file_metadata(); file_metadata->set_name("name"); - file_metadata->set_type( - nearby::sharing::service::proto::FileMetadata::AUDIO); + file_metadata->set_type(nearby::sharing::service::proto::FileMetadata::AUDIO); file_metadata->set_payload_id(1); file_metadata->set_size(kFreeDiskSpace + 1); file_metadata->set_mime_type("mime type"); @@ -2611,8 +2607,7 @@ TEST_F(NearbySharingServiceImplTest, IncomingConnectionFileSizeOverflow) { nearby::sharing::service::proto::FileMetadata* file_metadata = introduction_frame->add_file_metadata(); file_metadata->set_name("name_1"); - file_metadata->set_type( - nearby::sharing::service::proto::FileMetadata::AUDIO); + file_metadata->set_type(nearby::sharing::service::proto::FileMetadata::AUDIO); file_metadata->set_payload_id(1); file_metadata->set_size(std::numeric_limits::max()); file_metadata->set_mime_type("mime type"); @@ -3268,11 +3263,10 @@ TEST_F(NearbySharingServiceImplTest, SendTextFailedToConnect) { DiscoverShareTarget(transfer_callback, discovery_callback); absl::Notification notification; - ExpectTransferUpdates( - transfer_callback, target_id, - {TransferMetadata::Status::kConnecting, - TransferMetadata::Status::kFailed}, - [&]() { notification.Notify(); }); + ExpectTransferUpdates(transfer_callback, target_id, + {TransferMetadata::Status::kConnecting, + TransferMetadata::Status::kFailed}, + [&]() { notification.Notify(); }); EXPECT_CALL(*mock_app_info_, SetActiveFlag()); EXPECT_EQ(SendAttachments(target_id, CreateTextAttachments({kTextPayload})), @@ -3289,11 +3283,10 @@ TEST_F(NearbySharingServiceImplTest, SendTextFailedKeyVerification) { DiscoverShareTarget(transfer_callback, discovery_callback); absl::Notification notification; - ExpectTransferUpdates( - transfer_callback, target_id, - {TransferMetadata::Status::kConnecting, - TransferMetadata::Status::kDeviceAuthenticationFailed}, - [&]() { notification.Notify(); }); + ExpectTransferUpdates(transfer_callback, target_id, + {TransferMetadata::Status::kConnecting, + TransferMetadata::Status::kDeviceAuthenticationFailed}, + [&]() { notification.Notify(); }); SetUpKeyVerification(/*is_incoming=*/false, PairedKeyResultFrame::FAIL); fake_nearby_connections_manager_->SetRawAuthenticationToken(kEndpointId, @@ -5058,7 +5051,8 @@ TEST_F(NearbySharingServiceImplTest, RemoveIncomingPayloads) { ShareTarget share_target; share_target.is_incoming = true; IncomingShareSession session( - *sharing_service_task_runner_, analytics_recorder, "endpoint_id", + fake_context_.fake_clock(), *sharing_service_task_runner_, + fake_nearby_connections_manager_, analytics_recorder, "endpoint_id", share_target, [](const IncomingShareSession&, const TransferMetadata&) {}); service_->RemoveIncomingPayloads(session); diff --git a/sharing/outgoing_share_session.cc b/sharing/outgoing_share_session.cc index 09b74b72..8524f3d5 100644 --- a/sharing/outgoing_share_session.cc +++ b/sharing/outgoing_share_session.cc @@ -55,17 +55,17 @@ using ::nearby::sharing::service::proto::IntroductionFrame; using ::nearby::sharing::service::proto::V1Frame; OutgoingShareSession::OutgoingShareSession( - TaskRunner& service_thread, + Clock* clock, TaskRunner& service_thread, + NearbyConnectionsManager* connections_manager, analytics::AnalyticsRecorder& analytics_recorder, std::string endpoint_id, const ShareTarget& share_target, std::function transfer_update_callback) - : ShareSession(service_thread, analytics_recorder, std::move(endpoint_id), - share_target), + : ShareSession(clock, service_thread, connections_manager, + analytics_recorder, std::move(endpoint_id), share_target), transfer_update_callback_(std::move(transfer_update_callback)) {} -OutgoingShareSession::OutgoingShareSession(OutgoingShareSession&&) = - default; +OutgoingShareSession::OutgoingShareSession(OutgoingShareSession&&) = default; OutgoingShareSession::~OutgoingShareSession() = default; @@ -104,9 +104,7 @@ void OutgoingShareSession::OnConnectionDisconnected() { } } - -std::vector OutgoingShareSession::GetFilePaths() - const { +std::vector OutgoingShareSession::GetFilePaths() const { std::vector file_paths; file_paths.reserve(attachment_container().GetFileAttachments().size()); for (const FileAttachment& file_attachment : @@ -273,7 +271,7 @@ bool OutgoingShareSession::AcceptTransfer( } void OutgoingShareSession::SendPayloads( - bool enable_transfer_cancellation_optimization, Clock* clock, + bool enable_transfer_cancellation_optimization, std::function< void(std::optional frame)> frame_read_callback, @@ -293,38 +291,36 @@ void OutgoingShareSession::SendPayloads( NL_VLOG(1) << __func__ << ": The connection was accepted. Payloads are now being sent."; if (enable_transfer_cancellation_optimization) { - InitSendPayload(clock, std::move(update_callback)); + InitSendPayload(std::move(update_callback)); SendNextPayload(); } else { - SendAllPayloads(clock, std::move(update_callback)); + SendAllPayloads(std::move(update_callback)); } } void OutgoingShareSession::SendAllPayloads( - Clock* clock, std::function update_callback) { set_payload_tracker(std::make_unique( - clock, share_target().id, attachment_container(), + &clock(), share_target().id, attachment_container(), attachment_payload_map(), std::move(update_callback))); for (auto& payload : ExtractTextPayloads()) { - connections_manager()->Send( + connections_manager().Send( endpoint_id(), std::make_unique(payload), payload_tracker()); } for (auto& payload : ExtractFilePayloads()) { - connections_manager()->Send( + connections_manager().Send( endpoint_id(), std::make_unique(payload), payload_tracker()); } for (auto& payload : ExtractWifiCredentialsPayloads()) { - connections_manager()->Send( + connections_manager().Send( endpoint_id(), std::make_unique(payload), payload_tracker()); } } void OutgoingShareSession::InitSendPayload( - Clock* clock, std::function update_callback) { set_payload_tracker(std::make_unique( - clock, share_target().id, attachment_container(), + &clock(), share_target().id, attachment_container(), attachment_payload_map(), std::move(update_callback))); } @@ -332,7 +328,7 @@ void OutgoingShareSession::SendNextPayload() { std::optional payload = ExtractNextPayload(); if (payload.has_value()) { NL_LOG(INFO) << __func__ << ": Send payload " << payload->id; - connections_manager()->Send( + connections_manager().Send( endpoint_id(), std::make_unique(*payload), payload_tracker()); } else { NL_LOG(WARNING) << __func__ << ": There is no paylaods to send."; diff --git a/sharing/outgoing_share_session.h b/sharing/outgoing_share_session.h index 9b3d10a8..5402e893 100644 --- a/sharing/outgoing_share_session.h +++ b/sharing/outgoing_share_session.h @@ -30,6 +30,7 @@ #include "sharing/analytics/analytics_recorder.h" #include "sharing/certificates/nearby_share_decrypted_public_certificate.h" #include "sharing/nearby_connection.h" +#include "sharing/nearby_connections_manager.h" #include "sharing/nearby_connections_types.h" #include "sharing/nearby_file_handler.h" #include "sharing/paired_key_verification_runner.h" @@ -45,7 +46,8 @@ namespace nearby::sharing { class OutgoingShareSession : public ShareSession { public: OutgoingShareSession( - TaskRunner& service_thread, + Clock* clock, TaskRunner& service_thread, + NearbyConnectionsManager* connections_manager, analytics::AnalyticsRecorder& analytics_recorder, std::string endpoint_id, const ShareTarget& share_target, std::function @@ -117,7 +119,7 @@ class OutgoingShareSession : public ShareSession { // `update_callback`. // Any other frames received will be passed to `frame_read_callback`. void SendPayloads( - bool enable_transfer_cancellation_optimization, Clock* clock, + bool enable_transfer_cancellation_optimization, std::function< void(std::optional frame)> frame_read_callback, @@ -146,13 +148,11 @@ class OutgoingShareSession : public ShareSession { // Create a payload status listener to send status change to // `update_callback`. Send all payloads to NearbyConnectionManager. void SendAllPayloads( - Clock* clock, std::function update_callback); // Create a payload status listener to send status change to // `update_callback`. void InitSendPayload( - Clock* clock, std::function update_callback); std::vector ExtractTextPayloads(); @@ -162,7 +162,6 @@ class OutgoingShareSession : public ShareSession { bool FillIntroductionFrame( nearby::sharing::service::proto::IntroductionFrame* introduction) const; - std::optional obfuscated_gaia_id_; // All payloads are in the same order as the attachments in the share target. std::vector text_payloads_; diff --git a/sharing/outgoing_share_session_test.cc b/sharing/outgoing_share_session_test.cc index 9698f2e9..fd25c289 100644 --- a/sharing/outgoing_share_session_test.cc +++ b/sharing/outgoing_share_session_test.cc @@ -83,8 +83,8 @@ constexpr absl::string_view kEndpointId = "ABCD"; class OutgoingShareSessionTest : public ::testing::Test { public: OutgoingShareSessionTest() - : session_(fake_task_runner_, analytics_recorder_, - std::string(kEndpointId), share_target_, + : session_(&fake_clock_, fake_task_runner_, &connections_manager_, + analytics_recorder_, std::string(kEndpointId), share_target_, transfer_metadata_callback_.AsStdFunction()), text1_(nearby::sharing::service::proto::TextMetadata::URL, "A bit of text body", "Some text title", "text/html"), @@ -123,8 +123,9 @@ class OutgoingShareSessionTest : public ::testing::Test { TEST_F(OutgoingShareSessionTest, GetFilePaths) { OutgoingShareSession session( - fake_task_runner_, analytics_recorder_, std::string(kEndpointId), - share_target_, [](OutgoingShareSession&, const TransferMetadata&) {}); + &fake_clock_, fake_task_runner_, &connections_manager_, + analytics_recorder_, std::string(kEndpointId), share_target_, + [](OutgoingShareSession&, const TransferMetadata&) {}); AttachmentContainer container(std::vector{}, std::vector{file1_, file2_}, std::vector{}); @@ -139,8 +140,9 @@ TEST_F(OutgoingShareSessionTest, GetFilePaths) { TEST_F(OutgoingShareSessionTest, CreateTextPayloadsWithNoTextAttachments) { OutgoingShareSession session( - fake_task_runner_, analytics_recorder_, std::string(kEndpointId), - share_target_, [](OutgoingShareSession&, const TransferMetadata&) {}); + &fake_clock_, fake_task_runner_, &connections_manager_, + analytics_recorder_, std::string(kEndpointId), share_target_, + [](OutgoingShareSession&, const TransferMetadata&) {}); session.CreateTextPayloads(); const std::vector& payloads = session.text_payloads(); @@ -171,8 +173,9 @@ TEST_F(OutgoingShareSessionTest, CreateTextPayloads) { TEST_F(OutgoingShareSessionTest, CreateFilePayloadsWithNoFileAttachments) { OutgoingShareSession session( - fake_task_runner_, analytics_recorder_, std::string(kEndpointId), - share_target_, [](OutgoingShareSession&, const TransferMetadata&) {}); + &fake_clock_, fake_task_runner_, &connections_manager_, + analytics_recorder_, std::string(kEndpointId), share_target_, + [](OutgoingShareSession&, const TransferMetadata&) {}); EXPECT_THAT( session.CreateFilePayloads(std::vector()), @@ -219,8 +222,9 @@ TEST_F(OutgoingShareSessionTest, CreateFilePayloads) { TEST_F(OutgoingShareSessionTest, CreateWifiPayloadsWithNoWifiAttachments) { OutgoingShareSession session( - fake_task_runner_, analytics_recorder_, std::string(kEndpointId), - share_target_, [](OutgoingShareSession&, const TransferMetadata&) {}); + &fake_clock_, fake_task_runner_, &connections_manager_, + analytics_recorder_, std::string(kEndpointId), share_target_, + [](OutgoingShareSession&, const TransferMetadata&) {}); session.CreateWifiCredentialsPayloads(); const std::vector& payloads = session.file_payloads(); @@ -254,7 +258,7 @@ TEST_F(OutgoingShareSessionTest, SendIntroductionWithoutPayloads) { TEST_F(OutgoingShareSessionTest, SendIntroductionSuccess) { session_.set_session_id(1234); FakeNearbyConnection connection; - session_.OnConnected(absl::Now(), &connections_manager_, &connection); + session_.OnConnected(absl::Now(), &connection); std::vector file_infos; file_infos.push_back({ .size = 12355L, @@ -326,7 +330,7 @@ TEST_F(OutgoingShareSessionTest, SendIntroductionTimeout) { session_.SetAttachmentContainer(std::move(container)); session_.set_session_id(1234); FakeNearbyConnection connection; - session_.OnConnected(absl::Now(), &connections_manager_, &connection); + session_.OnConnected(absl::Now(), &connection); session_.CreateTextPayloads(); EXPECT_CALL( mock_event_logger_, @@ -351,7 +355,7 @@ TEST_F(OutgoingShareSessionTest, SendIntroductionTimeoutCancelled) { session_.SetAttachmentContainer(std::move(container)); session_.set_session_id(1234); FakeNearbyConnection connection; - session_.OnConnected(absl::Now(), &connections_manager_, &connection); + session_.OnConnected(absl::Now(), &connection); session_.CreateTextPayloads(); EXPECT_CALL( mock_event_logger_, @@ -388,7 +392,7 @@ TEST_F(OutgoingShareSessionTest, AcceptTransferNotConnected) { TEST_F(OutgoingShareSessionTest, AcceptTransferNotReady) { session_.set_session_id(1234); FakeNearbyConnection connection; - session_.OnConnected(absl::Now(), &connections_manager_, &connection); + session_.OnConnected(absl::Now(), &connection); EXPECT_THAT( session_.AcceptTransfer([](std::optional) {}), @@ -400,7 +404,7 @@ TEST_F(OutgoingShareSessionTest, AcceptTransferSuccess) { session_.SetAttachmentContainer(std::move(container)); session_.set_session_id(1234); FakeNearbyConnection connection; - session_.OnConnected(absl::Now(), &connections_manager_, &connection); + session_.OnConnected(absl::Now(), &connection); session_.CreateTextPayloads(); EXPECT_CALL(mock_event_logger_, Log(Matcher( @@ -489,7 +493,7 @@ TEST_F(OutgoingShareSessionTest, HandleConnectionResponseAcceptResponse) { ConnectionResponseFrame response; response.set_status(ConnectionResponseFrame::ACCEPT); FakeNearbyConnection connection; - session_.OnConnected(absl::Now(), &connections_manager_, &connection); + session_.OnConnected(absl::Now(), &connection); EXPECT_CALL(transfer_metadata_callback_, Call(_, HasStatus(TransferMetadata::Status::kInProgress))); @@ -548,11 +552,11 @@ TEST_F(OutgoingShareSessionTest, SendPayloadsDisableCancellationOptimization) { Property(&SharingLog::send_attachments_start, HasSessionId(1234))))))); FakeNearbyConnection connection; - session_.OnConnected(absl::Now(), &connections_manager_, &connection); + session_.OnConnected(absl::Now(), &connection); session_.SendPayloads( /*enable_transfer_cancellation_optimization=*/ - false, &fake_clock_, [](std::optional frame) {}, + false, [](std::optional frame) {}, transfer_metadata_callback.AsStdFunction()); auto payload_listener = session_.payload_tracker().lock(); @@ -590,11 +594,11 @@ TEST_F(OutgoingShareSessionTest, SendPayloadsEnableCancellationOptimization) { Property(&SharingLog::send_attachments_start, HasSessionId(1234))))))); FakeNearbyConnection connection; - session_.OnConnected(absl::Now(), &connections_manager_, &connection); + session_.OnConnected(absl::Now(), &connection); session_.SendPayloads( /*enable_transfer_cancellation_optimization=*/ - true, &fake_clock_, [](std::optional frame) {}, + true, [](std::optional frame) {}, transfer_metadata_callback.AsStdFunction()); auto payload_listener = session_.payload_tracker().lock(); @@ -633,11 +637,11 @@ TEST_F(OutgoingShareSessionTest, SendNextPayload) { Property(&SharingLog::send_attachments_start, HasSessionId(1234))))))); FakeNearbyConnection connection; - session_.OnConnected(absl::Now(), &connections_manager_, &connection); + session_.OnConnected(absl::Now(), &connection); session_.SendPayloads( /*enable_transfer_cancellation_optimization=*/ - true, &fake_clock_, [](std::optional frame) {}, + true, [](std::optional frame) {}, transfer_metadata_callback.AsStdFunction()); EXPECT_CALL(send_payload_callback, Call(_, _)) @@ -670,7 +674,7 @@ TEST_F(OutgoingShareSessionTest, SendNextPayload) { TEST_F(OutgoingShareSessionTest, ProcessKeyVerificationResultFail) { FakeNearbyConnection connection; - session_.OnConnected(absl::Now(), &connections_manager_, &connection); + session_.OnConnected(absl::Now(), &connection); session_.SetTokenForTests("1234"); EXPECT_THAT( @@ -685,7 +689,7 @@ TEST_F(OutgoingShareSessionTest, ProcessKeyVerificationResultFail) { TEST_F(OutgoingShareSessionTest, ProcessKeyVerificationResultSuccess) { FakeNearbyConnection connection; - session_.OnConnected(absl::Now(), &connections_manager_, &connection); + session_.OnConnected(absl::Now(), &connection); session_.SetTokenForTests("1234"); EXPECT_THAT( @@ -700,7 +704,7 @@ TEST_F(OutgoingShareSessionTest, ProcessKeyVerificationResultSuccess) { TEST_F(OutgoingShareSessionTest, DelayCompleteMetadataReceiverDisconnect) { FakeNearbyConnection connection; - session_.OnConnected(absl::Now(), &connections_manager_, &connection); + session_.OnConnected(absl::Now(), &connection); TransferMetadata complete_metadata = TransferMetadataBuilder() .set_status(TransferMetadata::Status::kComplete) @@ -717,7 +721,7 @@ TEST_F(OutgoingShareSessionTest, DelayCompleteMetadataReceiverDisconnect) { TEST_F(OutgoingShareSessionTest, DelayCompleteMetadataDisconnectTimeout) { FakeNearbyConnection connection; - session_.OnConnected(absl::Now(), &connections_manager_, &connection); + session_.OnConnected(absl::Now(), &connection); TransferMetadata complete_metadata = TransferMetadataBuilder() .set_status(TransferMetadata::Status::kComplete) @@ -762,7 +766,7 @@ TEST_F(OutgoingShareSessionTest, UpdateSessionForDedupWithoutCertificate) { TEST_F(OutgoingShareSessionTest, UpdateSessionForDedupConnectedIsNoOp) { auto share_target_org = session_.share_target(); FakeNearbyConnection connection; - session_.OnConnected(absl::Now(), &connections_manager_, &connection); + session_.OnConnected(absl::Now(), &connection); ShareTarget share_target2{ "test_update_name", ::nearby::network::Url(), ShareTargetType::kPhone, /* is_incoming */ true, "test_update_full_name", diff --git a/sharing/share_session.cc b/sharing/share_session.cc index a0675c7c..2c0fb34a 100644 --- a/sharing/share_session.cc +++ b/sharing/share_session.cc @@ -67,11 +67,14 @@ std::string TokenToFourDigitString(const std::vector& bytes) { } // namespace -ShareSession::ShareSession(TaskRunner& service_thread, +ShareSession::ShareSession(Clock* clock, TaskRunner& service_thread, + NearbyConnectionsManager* connections_manager, analytics::AnalyticsRecorder& analytics_recorder, std::string endpoint_id, const ShareTarget& share_target) - : service_thread_(service_thread), + : clock_(*clock), + service_thread_(service_thread), + connections_manager_(*connections_manager), analytics_recorder_(analytics_recorder), endpoint_id_(std::move(endpoint_id)), self_share_(share_target.for_self_share), @@ -115,10 +118,7 @@ void ShareSession::set_disconnect_status( } bool ShareSession::OnConnected(absl::Time connect_start_time, - NearbyConnectionsManager* connections_manager, NearbyConnection* connection) { - NL_DCHECK(connections_manager) << "Connections manager must not be null"; - connections_manager_ = connections_manager; if (!OnNewConnection(connection)) { return false; } @@ -155,7 +155,7 @@ void ShareSession::Abort(TransferMetadata::Status status) { } void ShareSession::RunPairedKeyVerification( - Clock* clock, OSType os_type, + OSType os_type, const PairedKeyVerificationRunner::VisibilityHistory& visibility_history, NearbyShareCertificateManager* certificate_manager, const std::vector& token, @@ -165,8 +165,8 @@ void ShareSession::RunPairedKeyVerification( token_ = TokenToFourDigitString(token); key_verification_runner_ = std::make_shared( - clock, os_type, IsIncoming(), visibility_history, token, - connection_, certificate_, certificate_manager, frames_reader_.get(), + &clock_, os_type, IsIncoming(), visibility_history, token, connection_, + certificate_, certificate_manager, frames_reader_.get(), kReadFramesTimeout); key_verification_runner_->Run(std::move(callback)); } @@ -186,11 +186,8 @@ void ShareSession::SetAttachmentPayloadId(int64_t attachment_id, } void ShareSession::CancelPayloads() { - if (connections_manager_ == nullptr) { - return; - } for (const auto& [attachment_id, payload_id] : attachment_payload_map_) { - connections_manager_->Cancel(payload_id); + connections_manager_.Cancel(payload_id); } } diff --git a/sharing/share_session.h b/sharing/share_session.h index 70ab1b22..1a9d9b79 100644 --- a/sharing/share_session.h +++ b/sharing/share_session.h @@ -48,7 +48,8 @@ namespace nearby::sharing { // This class is thread-compatible. class ShareSession { public: - ShareSession(TaskRunner& service_thread, + ShareSession(Clock* clock, TaskRunner& service_thread, + NearbyConnectionsManager* connections_manager, analytics::AnalyticsRecorder& analytics_recorder, std::string endpoint_id, const ShareTarget& share_target); ShareSession(ShareSession&&); @@ -105,7 +106,6 @@ class ShareSession { // Notifies the ShareTargetInfo that the connection has been established. // Returns true if the connection was successfully established. bool OnConnected(absl::Time connect_start_time, - NearbyConnectionsManager* connections_manager, NearbyConnection* connection); // Send TransferMetadataUpdate with the final status. @@ -113,7 +113,7 @@ class ShareSession { void Abort(TransferMetadata::Status status); void RunPairedKeyVerification( - Clock* clock, location::nearby::proto::sharing::OSType os_type, + location::nearby::proto::sharing::OSType os_type, const PairedKeyVerificationRunner::VisibilityHistory& visibility_history, NearbyShareCertificateManager* certificate_manager, const std::vector& token, @@ -151,6 +151,8 @@ class ShareSession { virtual bool OnNewConnection(NearbyConnection* connection) = 0; virtual void OnConnectionDisconnected() {} + Clock& clock() const { return clock_; } + analytics::AnalyticsRecorder& analytics_recorder() { return analytics_recorder_; }; @@ -172,7 +174,7 @@ class ShareSession { PairedKeyVerificationRunner::PairedKeyVerificationResult result, location::nearby::proto::sharing::OSType share_target_os_type); - NearbyConnectionsManager* connections_manager() { + NearbyConnectionsManager& connections_manager() { return connections_manager_; } void set_endpoint_id(absl::string_view endpoint_id) { @@ -184,11 +186,12 @@ class ShareSession { } private: + Clock& clock_; TaskRunner& service_thread_; + NearbyConnectionsManager& connections_manager_; analytics::AnalyticsRecorder& analytics_recorder_; std::string endpoint_id_; std::optional certificate_; - NearbyConnectionsManager* connections_manager_ = nullptr; NearbyConnection* connection_ = nullptr; // If not empty, this is the 4 digit token used to verify the connection. // If token is empty, it means self-share and verification is not needed. diff --git a/sharing/share_session_test.cc b/sharing/share_session_test.cc index 996b53f2..51c4f01b 100644 --- a/sharing/share_session_test.cc +++ b/sharing/share_session_test.cc @@ -55,8 +55,8 @@ constexpr absl::string_view kEndpointId = "12345"; class TestShareSession : public ShareSession { public: TestShareSession(std::string endpoint_id, const ShareTarget& share_target) - : ShareSession(fake_task_runner_, analytics_recorder_, - std::move(endpoint_id), share_target), + : ShareSession(&fake_clock_, fake_task_runner_, &connections_manager_, + analytics_recorder_, std::move(endpoint_id), share_target), is_incoming_(share_target.is_incoming) {} bool IsIncoming() const override { return is_incoming_; } @@ -76,6 +76,10 @@ class TestShareSession : public ShareSession { share_target_os_type); } + FakeNearbyConnectionsManager& connections_manager() { + return connections_manager_; + } + MOCK_METHOD(void, InvokeTransferUpdateCallback, (const TransferMetadata& metadata), (override)); MOCK_METHOD(void, OnConnectionDisconnected, (), (override)); @@ -89,6 +93,7 @@ class TestShareSession : public ShareSession { private: FakeClock fake_clock_; FakeTaskRunner fake_task_runner_{&fake_clock_, 1}; + FakeNearbyConnectionsManager connections_manager_; nearby::analytics::MockEventLogger mock_event_logger_; analytics::AnalyticsRecorder analytics_recorder_{/*vendor_id=*/0, &mock_event_logger_}; @@ -139,32 +144,27 @@ TEST(ShareSessionTest, SetDisconnectStatus) { } TEST(ShareSessionTest, OnConnectedFails) { - FakeNearbyConnectionsManager connections_manager; ShareTarget share_target; TestShareSession session(std::string(kEndpointId), share_target); session.SetOnNewConnectionResult(false); - EXPECT_FALSE(session.OnConnected(absl::Now(), &connections_manager, + EXPECT_FALSE(session.OnConnected(absl::Now(), /*connection=*/nullptr)); } TEST(ShareSessionTest, OnConnectedSucceeds) { - FakeNearbyConnectionsManager connections_manager; ShareTarget share_target; TestShareSession session(std::string(kEndpointId), share_target); FakeNearbyConnection connection; session.SetOnNewConnectionResult(true); absl::Time connect_start_time = absl::Now(); - EXPECT_TRUE(session.OnConnected(connect_start_time, &connections_manager, - &connection)); + EXPECT_TRUE(session.OnConnected(connect_start_time, &connection)); EXPECT_EQ(session.connection_start_time(), connect_start_time); EXPECT_EQ(session.connection(), &connection); } TEST(ShareSessionTest, IncomingRunPairedKeyVerificationSuccess) { - FakeNearbyConnectionsManager connections_manager; - FakeClock fake_clock; FakeNearbyShareCertificateManager certificate_manager; FakeNearbyConnection connection; std::vector token = {0, 1, 2, 3, 4, 5}; @@ -173,13 +173,12 @@ TEST(ShareSessionTest, IncomingRunPairedKeyVerificationSuccess) { TestShareSession session(std::string(kEndpointId), share_target); session.SetOnNewConnectionResult(true); absl::Time connect_start_time = absl::Now(); - EXPECT_TRUE(session.OnConnected(connect_start_time, &connections_manager, - &connection)); + EXPECT_TRUE(session.OnConnected(connect_start_time, &connection)); absl::Notification notification; PairedKeyVerificationRunner::PairedKeyVerificationResult verification_result; session.RunPairedKeyVerification( - &fake_clock, OSType::WINDOWS, + OSType::WINDOWS, { .visibility = proto::DeviceVisibility::DEVICE_VISIBILITY_EVERYONE, .last_visibility = @@ -245,28 +244,24 @@ TEST(ShareSessionTest, OnDisconnect) { } TEST(ShareSessionTest, CancelPayloads) { - FakeNearbyConnectionsManager connections_manager; FakeNearbyConnection connection; ShareTarget share_target; TestShareSession session(std::string(kEndpointId), share_target); - EXPECT_TRUE( - session.OnConnected(absl::Now(), &connections_manager, &connection)); + EXPECT_TRUE(session.OnConnected(absl::Now(), &connection)); session.SetAttachmentPayloadId(1, 2); session.SetAttachmentPayloadId(3, 4); session.CancelPayloads(); - EXPECT_TRUE(connections_manager.WasPayloadCanceled(2)); - EXPECT_TRUE(connections_manager.WasPayloadCanceled(4)); + EXPECT_TRUE(session.connections_manager().WasPayloadCanceled(2)); + EXPECT_TRUE(session.connections_manager().WasPayloadCanceled(4)); } TEST(ShareSessionTest, WriteResponseFrame) { - FakeNearbyConnectionsManager connections_manager; ShareTarget share_target; TestShareSession session(std::string(kEndpointId), share_target); FakeNearbyConnection connection; - EXPECT_TRUE( - session.OnConnected(absl::Now(), &connections_manager, &connection)); + EXPECT_TRUE(session.OnConnected(absl::Now(), &connection)); session.WriteResponseFrame(ConnectionResponseFrame::REJECT); @@ -280,12 +275,10 @@ TEST(ShareSessionTest, WriteResponseFrame) { } TEST(ShareSessionTest, WriteCancelFrame) { - FakeNearbyConnectionsManager connections_manager; ShareTarget share_target; TestShareSession session(std::string(kEndpointId), share_target); FakeNearbyConnection connection; - EXPECT_TRUE( - session.OnConnected(absl::Now(), &connections_manager, &connection)); + EXPECT_TRUE(session.OnConnected(absl::Now(), &connection)); session.WriteCancelFrame(); @@ -297,12 +290,10 @@ TEST(ShareSessionTest, WriteCancelFrame) { } TEST(ShareSessionTest, HandleKeyVerificationResultFail) { - FakeNearbyConnectionsManager connections_manager; ShareTarget share_target; TestShareSession session(std::string(kEndpointId), share_target); FakeNearbyConnection connection; - EXPECT_TRUE( - session.OnConnected(absl::Now(), &connections_manager, &connection)); + EXPECT_TRUE(session.OnConnected(absl::Now(), &connection)); session.SetTokenForTests("9876"); EXPECT_FALSE(session.HandleKeyVerificationResult( @@ -313,13 +304,11 @@ TEST(ShareSessionTest, HandleKeyVerificationResultFail) { } TEST(ShareSessionTest, HandleKeyVerificationResultSelfShareSuccess) { - FakeNearbyConnectionsManager connections_manager; ShareTarget share_target; share_target.for_self_share = true; TestShareSession session(std::string(kEndpointId), share_target); FakeNearbyConnection connection; - EXPECT_TRUE( - session.OnConnected(absl::Now(), &connections_manager, &connection)); + EXPECT_TRUE(session.OnConnected(absl::Now(), &connection)); session.SetTokenForTests("9876"); EXPECT_TRUE(session.HandleKeyVerificationResult( @@ -331,12 +320,10 @@ TEST(ShareSessionTest, HandleKeyVerificationResultSelfShareSuccess) { } TEST(ShareSessionTest, HandleKeyVerificationResultNotSelfShareSuccess) { - FakeNearbyConnectionsManager connections_manager; ShareTarget share_target; TestShareSession session(std::string(kEndpointId), share_target); FakeNearbyConnection connection; - EXPECT_TRUE( - session.OnConnected(absl::Now(), &connections_manager, &connection)); + EXPECT_TRUE(session.OnConnected(absl::Now(), &connection)); session.SetTokenForTests("9876"); EXPECT_TRUE(session.HandleKeyVerificationResult( @@ -349,13 +336,11 @@ TEST(ShareSessionTest, HandleKeyVerificationResultNotSelfShareSuccess) { } TEST(ShareSessionTest, HandleKeyVerificationResultSelfShareUnable) { - FakeNearbyConnectionsManager connections_manager; ShareTarget share_target; share_target.for_self_share = true; TestShareSession session(std::string(kEndpointId), share_target); FakeNearbyConnection connection; - EXPECT_TRUE( - session.OnConnected(absl::Now(), &connections_manager, &connection)); + EXPECT_TRUE(session.OnConnected(absl::Now(), &connection)); session.SetTokenForTests("9876"); EXPECT_TRUE(session.HandleKeyVerificationResult( @@ -367,12 +352,10 @@ TEST(ShareSessionTest, HandleKeyVerificationResultSelfShareUnable) { } TEST(ShareSessionTest, HandleKeyVerificationResultNotSelfShareUnable) { - FakeNearbyConnectionsManager connections_manager; ShareTarget share_target; TestShareSession session(std::string(kEndpointId), share_target); FakeNearbyConnection connection; - EXPECT_TRUE( - session.OnConnected(absl::Now(), &connections_manager, &connection)); + EXPECT_TRUE(session.OnConnected(absl::Now(), &connection)); session.SetTokenForTests("9876"); EXPECT_TRUE(session.HandleKeyVerificationResult( @@ -384,12 +367,10 @@ TEST(ShareSessionTest, HandleKeyVerificationResultNotSelfShareUnable) { } TEST(ShareSessionTest, HandleKeyVerificationResultUnknown) { - FakeNearbyConnectionsManager connections_manager; ShareTarget share_target; TestShareSession session(std::string(kEndpointId), share_target); FakeNearbyConnection connection; - EXPECT_TRUE( - session.OnConnected(absl::Now(), &connections_manager, &connection)); + EXPECT_TRUE(session.OnConnected(absl::Now(), &connection)); session.SetTokenForTests("9876"); EXPECT_FALSE(session.HandleKeyVerificationResult( @@ -410,15 +391,13 @@ TEST(ShareSessionTest, AbortNotConnected) { } TEST(ShareSessionTest, AbortConnected) { - FakeNearbyConnectionsManager connections_manager; ShareTarget share_target; TestShareSession session(std::string(kEndpointId), share_target); FakeNearbyConnection connection; bool disconnected = false; connection.SetDisconnectionListener( [&disconnected]() { disconnected = true; }); - EXPECT_TRUE( - session.OnConnected(absl::Now(), &connections_manager, &connection)); + EXPECT_TRUE(session.OnConnected(absl::Now(), &connection)); EXPECT_CALL(session, InvokeTransferUpdateCallback(AllOf( HasStatus(TransferMetadata::Status::kNotEnoughSpace), IsFinalStatus()))); @@ -429,15 +408,13 @@ TEST(ShareSessionTest, AbortConnected) { } TEST(ShareSessionTest, Disconnect) { - FakeNearbyConnectionsManager connections_manager; ShareTarget share_target; TestShareSession session(std::string(kEndpointId), share_target); FakeNearbyConnection connection; bool disconnected = false; connection.SetDisconnectionListener( [&disconnected]() { disconnected = true; }); - EXPECT_TRUE( - session.OnConnected(absl::Now(), &connections_manager, &connection)); + EXPECT_TRUE(session.OnConnected(absl::Now(), &connection)); session.Disconnect();