diff --git a/sharing/BUILD b/sharing/BUILD index 24a00337..7513e441 100644 --- a/sharing/BUILD +++ b/sharing/BUILD @@ -85,7 +85,6 @@ cc_library( "//sharing:__subpackages__", ], deps = [ - ":attachments", ":connection_types", "//internal/network:url", "//sharing/common:enum", @@ -581,7 +580,6 @@ cc_test( name = "share_target_test", srcs = ["share_target_test.cc"], deps = [ - ":attachments", ":types", "//internal/network:url", "//sharing/common:enum", diff --git a/sharing/analytics/analytics_recorder.cc b/sharing/analytics/analytics_recorder.cc index a7aa2d79..82cceb26 100644 --- a/sharing/analytics/analytics_recorder.cc +++ b/sharing/analytics/analytics_recorder.cc @@ -268,8 +268,9 @@ void AnalyticsRecorder::NewEstablishConnection( int64_t session_id, ::location::nearby::proto::sharing::EstablishConnectionStatus connection_status, - ShareTarget share_target, int transfer_position, int concurrent_connections, - int64_t duration_millis, std::optional referrer_package) { + const ShareTarget& share_target, int transfer_position, + int concurrent_connections, int64_t duration_millis, + std::optional referrer_package) { std::unique_ptr sharing_log = CreateSharingLog( EventCategory::SENDING_EVENT, EventType::ESTABLISH_CONNECTION); @@ -429,7 +430,7 @@ void AnalyticsRecorder::NewDescribeAttachments( } void AnalyticsRecorder::NewDiscoverShareTarget( - ShareTarget share_target, int64_t session_id, + const ShareTarget& share_target, int64_t session_id, int64_t latency_since_scanning_start_millis, int64_t flow_id, std::optional referrer_package, int64_t latency_since_send_surface_registered_millis) { @@ -593,7 +594,7 @@ void AnalyticsRecorder::NewDismissFastInitialization() { } void AnalyticsRecorder::NewReceiveIntroduction( - int64_t session_id, ShareTarget share_target, + int64_t session_id, const ShareTarget& share_target, std::optional referrer_package, ::location::nearby::proto::sharing::OSType share_target_os_type) { std::unique_ptr sharing_log = CreateSharingLog( @@ -696,7 +697,7 @@ void AnalyticsRecorder::NewScanForShareTargetsStart( } void AnalyticsRecorder::NewSendAttachmentsEnd( - int64_t session_id, int64_t sent_bytes, ShareTarget share_target, + int64_t session_id, int64_t sent_bytes, const ShareTarget& share_target, ::location::nearby::proto::sharing::AttachmentTransmissionStatus status, int transfer_position, int concurrent_connections, int64_t duration_millis, std::optional referrer_package, @@ -759,7 +760,7 @@ void AnalyticsRecorder::NewSendFastInitialization() { void AnalyticsRecorder::NewSendStart(int64_t session_id, int transfer_position, int concurrent_connections, - ShareTarget share_target) { + const ShareTarget& share_target) { std::unique_ptr sharing_log = CreateSharingLog(EventCategory::SENDING_EVENT, EventType::SEND_START); @@ -793,7 +794,7 @@ void AnalyticsRecorder::NewSendIntroduction( } void AnalyticsRecorder::NewSendIntroduction( - int64_t session_id, ShareTarget share_target, int transfer_position, + int64_t session_id, const ShareTarget& share_target, int transfer_position, int concurrent_connections, ::location::nearby::proto::sharing::OSType share_target_os_type) { std::unique_ptr sharing_log = CreateSharingLog( diff --git a/sharing/analytics/analytics_recorder.h b/sharing/analytics/analytics_recorder.h index aab84bd2..4bcd357c 100644 --- a/sharing/analytics/analytics_recorder.h +++ b/sharing/analytics/analytics_recorder.h @@ -44,7 +44,7 @@ class AnalyticsRecorder { int64_t session_id, location::nearby::proto::sharing::EstablishConnectionStatus connection_status, - ShareTarget share_target, int transfer_position, + const ShareTarget& share_target, int transfer_position, int concurrent_connections, int64_t duration_millis, std::optional referrer_package); @@ -74,7 +74,7 @@ class AnalyticsRecorder { void NewDescribeAttachments(const AttachmentContainer& attachments); void NewDiscoverShareTarget( - ShareTarget share_target, int64_t session_id, + const ShareTarget& share_target, int64_t session_id, int64_t latency_since_scanning_start_millis, int64_t flow_id, std::optional referrer_package, int64_t latency_since_send_surface_registered_millis); @@ -105,7 +105,7 @@ class AnalyticsRecorder { void NewDismissFastInitialization(); void NewReceiveIntroduction( - int64_t session_id, ShareTarget share_target, + int64_t session_id, const ShareTarget& share_target, std::optional referrer_package, location::nearby::proto::sharing::OSType share_target_os_type); @@ -126,7 +126,7 @@ class AnalyticsRecorder { std::optional referrer_package); void NewSendAttachmentsEnd( - int64_t session_id, int64_t sent_bytes, ShareTarget share_target, + int64_t session_id, int64_t sent_bytes, const ShareTarget& share_target, location::nearby::proto::sharing::AttachmentTransmissionStatus status, int transfer_position, int concurrent_connections, int64_t duration_millis, std::optional referrer_package, @@ -142,7 +142,8 @@ class AnalyticsRecorder { void NewSendFastInitialization(); void NewSendStart(int64_t session_id, int transfer_position, - int concurrent_connections, ShareTarget share_target); + int concurrent_connections, + const ShareTarget& share_target); void NewSendIntroduction( ShareTargetType target_type, int64_t session_id, @@ -150,8 +151,8 @@ class AnalyticsRecorder { location::nearby::proto::sharing::OSType share_target_os_type); void NewSendIntroduction( - int64_t session_id, ShareTarget share_target, int transfer_position, - int concurrent_connections, + int64_t session_id, const ShareTarget& share_target, + int transfer_position, int concurrent_connections, location::nearby::proto::sharing::OSType share_target_os_type); void NewSetVisibility(nearby::sharing::proto::DeviceVisibility src_visibility, diff --git a/sharing/attachment_container.cc b/sharing/attachment_container.cc index 94e49f25..74e9ff23 100644 --- a/sharing/attachment_container.cc +++ b/sharing/attachment_container.cc @@ -66,4 +66,20 @@ void AttachmentContainer::ClearAttachments() { } } +std::vector AttachmentContainer::GetAttachmentIds() const { + std::vector attachment_ids; + + attachment_ids.reserve(GetAttachmentCount()); + for (const auto& file : file_attachments_) + attachment_ids.push_back(file.id()); + + for (const auto& text : text_attachments_) + attachment_ids.push_back(text.id()); + + for (const auto& wifi_credentials : wifi_credentials_attachments_) + attachment_ids.push_back(wifi_credentials.id()); + + return attachment_ids; +} + } // namespace nearby::sharing diff --git a/sharing/attachment_container.h b/sharing/attachment_container.h index c1d92c15..7a0d2896 100644 --- a/sharing/attachment_container.h +++ b/sharing/attachment_container.h @@ -93,6 +93,9 @@ class AttachmentContainer { // place. void ClearAttachments(); + // Returns the list of attachment IDs of attachments in this container. + std::vector GetAttachmentIds() const; + private: std::vector text_attachments_; std::vector file_attachments_; diff --git a/sharing/attachment_container_test.cc b/sharing/attachment_container_test.cc index 48f4343d..e3c7a147 100644 --- a/sharing/attachment_container_test.cc +++ b/sharing/attachment_container_test.cc @@ -14,7 +14,9 @@ #include "sharing/attachment_container.h" +#include #include // NOLINT +#include #include #include "gmock/gmock.h" @@ -58,8 +60,10 @@ bool operator==(const WifiCredentialsAttachment& lhs, namespace { using testing::Eq; +using testing::IsEmpty; using testing::IsFalse; using testing::IsTrue; +using testing::SizeIs; using testing::UnorderedElementsAre; class AttachmentContainerTest : public ::testing::Test { @@ -76,7 +80,7 @@ class AttachmentContainerTest : public ::testing::Test { "text/plain", /*batch_id=*/456547, nearby::sharing::Attachment::SourceType::kContextMenu), - file1_(/*id=*/436346, /*size=*/100000, "someFileName", "image/jpeg", + file1_(/*id=*/436346L, /*size=*/100000, "someFileName", "image/jpeg", nearby::sharing::service::proto::FileMetadata::IMAGE, "/usr/local/tmp", /*batch_id=*/66657L, nearby::sharing::Attachment::SourceType::kSelectFilesButton), @@ -186,5 +190,38 @@ TEST_F(AttachmentContainerTest, HasAttachments) { EXPECT_THAT(container.HasAttachments(), IsTrue()); } +TEST_F(AttachmentContainerTest, ClearAttachments) { + AttachmentContainer container(std::vector{text1_, text2_}, + std::vector{file1_}, + std::vector{wifi1_}); + + container.ClearAttachments(); + + ASSERT_THAT(container.GetTextAttachments(), SizeIs(2)); + EXPECT_THAT(container.GetTextAttachments()[0].text_body(), IsEmpty()); + EXPECT_THAT(container.GetTextAttachments()[1].text_body(), IsEmpty()); + ASSERT_THAT(container.GetFileAttachments(), SizeIs(1)); + EXPECT_THAT(container.GetFileAttachments()[0].file_path(), Eq(std::nullopt)); + ASSERT_THAT(container.GetWifiCredentialsAttachments(), SizeIs(1)); + EXPECT_THAT(container.GetWifiCredentialsAttachments()[0].password(), + IsEmpty()); + EXPECT_THAT(container.GetWifiCredentialsAttachments()[0].is_hidden(), + IsFalse()); +} + +TEST_F(AttachmentContainerTest, GetAttachmentIds) { + AttachmentContainer container(std::vector{text1_, text2_}, + std::vector{file1_}, + std::vector{wifi1_}); + + std::vector attachment_ids = container.GetAttachmentIds(); + + EXPECT_THAT(attachment_ids, SizeIs(4)); + EXPECT_THAT(attachment_ids, + UnorderedElementsAre(text1_.id(), text2_.id(), file1_.id(), + wifi1_.id())); +} + } // namespace + } // namespace nearby::sharing diff --git a/sharing/incoming_share_target_info.cc b/sharing/incoming_share_target_info.cc index e07018fb..eeec98fd 100644 --- a/sharing/incoming_share_target_info.cc +++ b/sharing/incoming_share_target_info.cc @@ -27,10 +27,10 @@ namespace sharing { IncomingShareTargetInfo::IncomingShareTargetInfo( std::string endpoint_id, const ShareTarget& share_target, - std::function + std::function transfer_update_callback) - : ShareTargetInfo(std::move(endpoint_id), share_target, - std::move(transfer_update_callback)) {} + : ShareTargetInfo(std::move(endpoint_id), share_target), + transfer_update_callback_(std::move(transfer_update_callback)) {} IncomingShareTargetInfo::IncomingShareTargetInfo(IncomingShareTargetInfo&&) = default; @@ -40,5 +40,10 @@ IncomingShareTargetInfo& IncomingShareTargetInfo::operator=( IncomingShareTargetInfo::~IncomingShareTargetInfo() = default; +void IncomingShareTargetInfo::InvokeTransferUpdateCallback( + const TransferMetadata& metadata) { + transfer_update_callback_(*this, metadata); +} + } // namespace sharing } // namespace nearby diff --git a/sharing/incoming_share_target_info.h b/sharing/incoming_share_target_info.h index 10ededb2..3f87d4d3 100644 --- a/sharing/incoming_share_target_info.h +++ b/sharing/incoming_share_target_info.h @@ -26,15 +26,23 @@ namespace sharing { class IncomingShareTargetInfo : public ShareTargetInfo { public: - IncomingShareTargetInfo( - std::string endpoint_id, const ShareTarget& share_target, - std::function - transfer_update_callback); + IncomingShareTargetInfo(std::string endpoint_id, + const ShareTarget& share_target, + std::function + transfer_update_callback); IncomingShareTargetInfo(IncomingShareTargetInfo&&); IncomingShareTargetInfo& operator=(IncomingShareTargetInfo&&); ~IncomingShareTargetInfo() override; bool IsIncoming() const override { return true; } + + protected: + void InvokeTransferUpdateCallback(const TransferMetadata& metadata) override; + + private: + std::function + transfer_update_callback_; }; } // namespace sharing diff --git a/sharing/nearby_sharing_service_impl.cc b/sharing/nearby_sharing_service_impl.cc index 4f0946fd..df1e2f61 100644 --- a/sharing/nearby_sharing_service_impl.cc +++ b/sharing/nearby_sharing_service_impl.cc @@ -28,6 +28,7 @@ #include #include #include +#include #include #include @@ -439,15 +440,14 @@ void NearbySharingServiceImpl::RegisterSendSurface( last_outgoing_metadata_.has_value()) { // When a new share sheet is registered, we want to immediately show // the in-progress bar. + auto& [share_target, attachment_container, transfer_metadata] = + *last_outgoing_metadata_; // TODO(b/341740930): Make sure we absolutely do not deliver updates // to blocked targets, and block any interaction with them if the // request comes from a surface with the blocked vendor ID. - wrapped_callback.OnShareTargetDiscovered( - last_outgoing_metadata_->first); + wrapped_callback.OnShareTargetDiscovered(share_target); transfer_callback->OnTransferUpdate( - last_outgoing_metadata_->first, - last_outgoing_metadata_->first.attachment_container, - last_outgoing_metadata_->second); + share_target, attachment_container, transfer_metadata); } // Sync down data from Nearby server when the sending flow starts, @@ -578,10 +578,10 @@ void NearbySharingServiceImpl::RegisterReceiveSurface( // it catch up with most recent transfer metadata immediately. if (state == ReceiveSurfaceState::kForeground && last_incoming_metadata_) { + auto& [share_target, attachment_container, transfer_metadata] = + *last_incoming_metadata_; transfer_callback->OnTransferUpdate( - last_incoming_metadata_->first, - last_incoming_metadata_->first.attachment_container, - last_incoming_metadata_->second); + share_target, attachment_container, transfer_metadata); } GetReceiveCallbacksMapFromState(state).insert( @@ -730,20 +730,18 @@ void NearbySharingServiceImpl::SendAttachments( return; } - ShareTarget share_target = info->share_target(); - share_target.attachment_container = std::move(*attachment_container); + info->SetAttachmentContainer(std::move(*attachment_container)); app_info_->SetActiveFlag(); // Set session ID. info->set_session_id(analytics_recorder_->GenerateNextId()); - info->set_share_target(share_target); // Log analytics event of sending start. analytics_recorder_->NewSendStart( info->session_id(), - /*transfer_position=*/GetConnectedShareTargetPos(share_target), + /*transfer_position=*/GetConnectedShareTargetPos(), /*concurrent_connections=*/GetConnectedShareTargetCount(), - share_target); + info->share_target()); send_attachments_timestamp_ = context_->GetClock()->Now(); OnTransferStarted(/*is_incoming=*/false); @@ -757,16 +755,10 @@ void NearbySharingServiceImpl::SendAttachments( .set_status(TransferMetadata::Status::kConnecting) .build()); - CreatePayloads(*info, - [this, endpoint_info = std::move(*endpoint_info)]( - ShareTarget share_target, bool success) { - // Log analytics event of describing attachments. - analytics_recorder_->NewDescribeAttachments( - share_target.attachment_container); - - OnCreatePayloads(std::move(endpoint_info), - share_target, success); - }); + CreatePayloads(*info, [this, endpoint_info = std::move(*endpoint_info)]( + int64_t share_target_id, bool success) { + OnCreatePayloads(std::move(endpoint_info), share_target_id, success); + }); std::move(status_codes_callback)(StatusCodes::kOk); }); @@ -798,11 +790,13 @@ void NearbySharingServiceImpl::Accept( } bool is_incoming = info->IsIncoming(); - std::optional> metadata = - is_incoming ? last_incoming_metadata_ : last_outgoing_metadata_; + std::optional< + std::tuple> + metadata = + is_incoming ? last_incoming_metadata_ : last_outgoing_metadata_; if (!ReadyToAccept(info->self_share(), metadata.has_value() - ? metadata->second.status() + ? std::get<2>(*metadata).status() : TransferMetadata::Status::kUnknown)) { NL_LOG(WARNING) << __func__ << ": out of order API call."; status_codes_callback(StatusCodes::kOutOfOrderApiCall); @@ -810,14 +804,13 @@ void NearbySharingServiceImpl::Accept( } is_waiting_to_record_accept_to_transfer_start_metric_ = is_incoming; - ShareTarget share_target = info->share_target(); if (is_incoming) { incoming_share_accepted_timestamp_ = context_->GetClock()->Now(); - ReceivePayloads(share_target, std::move(status_codes_callback)); + ReceivePayloads(*info, std::move(status_codes_callback)); return; } - std::move(status_codes_callback)(SendPayloads(share_target)); + std::move(status_codes_callback)(SendPayloads(*info)); }); } @@ -911,8 +904,8 @@ void NearbySharingServiceImpl::DoCancel( // cancellation signals. Also, note that there might not be any ongoing // payload transfer, for example, if a connection has not been established // yet. - ShareTarget cached_share_target = info->share_target(); - for (int64_t attachment_id : cached_share_target.GetAttachmentIds()) { + for (int64_t attachment_id : + info->attachment_container().GetAttachmentIds()) { std::optional payload_id = GetAttachmentPayloadId(attachment_id); if (payload_id) { nearby_connections_manager_->Cancel(*payload_id); @@ -1096,7 +1089,7 @@ NearbySharingServiceImpl::InternalUnregisterSendSurface( } if (!foreground_send_surface_map_.empty() && last_outgoing_metadata_ && - last_outgoing_metadata_->second.is_final_status()) { + std::get<2>(*last_outgoing_metadata_).is_final_status()) { // We already saw the final status in the foreground // Nullify it so the next time the user opens sharing, it starts the UI from // the beginning @@ -1115,11 +1108,11 @@ NearbySharingServiceImpl::InternalUnregisterSendSurface( // Displays the most recent payload status processed by foreground surfaces on // background surfaces. if (foreground_send_surface_map_.empty() && last_outgoing_metadata_) { + auto& [share_target, attachment_container, transfer_metadata] = + *last_outgoing_metadata_; for (auto& background_transfer_callback : background_send_surface_map_) { background_transfer_callback.first->OnTransferUpdate( - last_outgoing_metadata_->first, - last_outgoing_metadata_->first.attachment_container, - last_outgoing_metadata_->second); + share_target, attachment_container, transfer_metadata); } } @@ -1150,7 +1143,7 @@ NearbySharingServiceImpl::InternalUnregisterReceiveSurface( } if (!foreground_receive_callbacks_map_.empty() && last_incoming_metadata_ && - last_incoming_metadata_->second.is_final_status()) { + std::get<2>(*last_incoming_metadata_).is_final_status()) { // We already saw the final status in the foreground. // Nullify it so the next time the user opens sharing, it starts the UI from // the beginning @@ -1166,11 +1159,11 @@ NearbySharingServiceImpl::InternalUnregisterReceiveSurface( // Displays the most recent payload status processed by foreground surfaces on // background surface. if (foreground_receive_callbacks_map_.empty() && last_incoming_metadata_) { + auto& [share_target, attachment_container, transfer_metadata] = + *last_incoming_metadata_; for (auto& background_callback : background_receive_callbacks_map_) { background_callback.first->OnTransferUpdate( - last_incoming_metadata_->first, - last_incoming_metadata_->first.attachment_container, - last_incoming_metadata_->second); + share_target, attachment_container, transfer_metadata); } } @@ -2515,14 +2508,15 @@ void NearbySharingServiceImpl::OnTransferStarted(bool is_incoming) { } void NearbySharingServiceImpl::ReceivePayloads( - ShareTarget share_target, + ShareTargetInfo& share_target_info, std::function status_codes_callback) { mutual_acceptance_timeout_alarm_->Stop(); std::filesystem::path download_path = std::filesystem::u8path(settings_->GetCustomSavePath()); - const AttachmentContainer& container = share_target.attachment_container; + const AttachmentContainer& container = + share_target_info.attachment_container(); // Register payload path for all valid file payloads. for (const auto& file : container.GetFileAttachments()) { std::optional payload_id = GetAttachmentPayloadId(file.id()); @@ -2539,60 +2533,59 @@ void NearbySharingServiceImpl::ReceivePayloads( file.file_name().cend()); attachment_info_map_[file.id()].file_path = std::move(file_path); } - OnPayloadPathsRegistered(share_target, std::move(status_codes_callback)); + OnPayloadPathsRegistered(share_target_info, std::move(status_codes_callback)); } NearbySharingService::StatusCodes NearbySharingServiceImpl::SendPayloads( - const ShareTarget& share_target) { + ShareTargetInfo& info) { NL_VLOG(1) << __func__ << ": Preparing to send payloads to " - << share_target.id; - ShareTargetInfo* info = GetShareTargetInfo(share_target.id); - if (!info || !info->connection()) { + << info.share_target().id; + if (!info.connection()) { NL_LOG(WARNING) << __func__ << ": Failed to send payload due to missing connection."; return StatusCodes::kOutOfOrderApiCall; } - ShareTarget cached_share_target = info->share_target(); // Log analytics event of sending attachment start. analytics_recorder_->NewSendAttachmentsStart( - info->session_id(), cached_share_target.attachment_container, - /*transfer_position=*/GetConnectedShareTargetPos(share_target), + info.session_id(), info.attachment_container(), + /*transfer_position=*/GetConnectedShareTargetPos(), /*concurrent_connections=*/GetConnectedShareTargetCount()); - info->UpdateTransferMetadata( + info.UpdateTransferMetadata( TransferMetadataBuilder() - .set_token(info->token()) + .set_token(info.token()) .set_status(TransferMetadata::Status::kAwaitingRemoteAcceptance) .build()); - ReceiveConnectionResponse(cached_share_target); + ReceiveConnectionResponse(info); return StatusCodes::kOk; } void NearbySharingServiceImpl::OnPayloadPathsRegistered( - const ShareTarget& share_target, + ShareTargetInfo& info, std::function status_codes_callback) { - ShareTargetInfo* info = GetShareTargetInfo(share_target.id); - if (!info || !info->connection()) { + if (!info.connection()) { NL_LOG(WARNING) << __func__ << ": Accept invoked for unknown share target"; std::move(status_codes_callback)(StatusCodes::kOutOfOrderApiCall); return; } - NearbyConnection* connection = info->connection(); + NearbyConnection* connection = info.connection(); // Log analytics event of starting to receive payloads. analytics_recorder_->NewReceiveAttachmentsStart( - receiving_session_id_, share_target.attachment_container); + receiving_session_id_, info.attachment_container()); - info->set_payload_tracker(std::make_shared( - context_, share_target.id, share_target.attachment_container, + int64_t share_target_id = info.share_target().id; + info.set_payload_tracker(std::make_shared( + context_, share_target_id, info.attachment_container(), attachment_info_map_, absl::bind_front(&NearbySharingServiceImpl::OnPayloadTransferUpdate, this))); // Register status listener for all payloads. - for (int64_t attachment_id : share_target.GetAttachmentIds()) { + for (int64_t attachment_id : + info.attachment_container().GetAttachmentIds()) { std::optional payload_id = GetAttachmentPayloadId(attachment_id); if (!payload_id) { NL_LOG(WARNING) << __func__ @@ -2605,10 +2598,10 @@ void NearbySharingServiceImpl::OnPayloadPathsRegistered( << *payload_id; nearby_connections_manager_->RegisterPayloadStatusListener( - *payload_id, info->payload_tracker()); + *payload_id, info.payload_tracker()); NL_VLOG(1) << __func__ << ": Accepted incoming files from share target - " - << share_target.id; + << share_target_id; } WriteResponseFrame( @@ -2616,14 +2609,14 @@ void NearbySharingServiceImpl::OnPayloadPathsRegistered( nearby::sharing::service::proto::ConnectionResponseFrame::ACCEPT); NL_VLOG(1) << __func__ << ": Successfully wrote response frame"; - info->UpdateTransferMetadata( + info.UpdateTransferMetadata( TransferMetadataBuilder() .set_status(TransferMetadata::Status::kAwaitingRemoteAcceptance) - .set_token(info->token()) + .set_token(info.token()) .build()); - std::string endpoint_id = info->endpoint_id(); - if (share_target.attachment_container.GetTotalAttachmentsSize() >= + std::string endpoint_id = info.endpoint_id(); + if (info.attachment_container().GetTotalAttachmentsSize() >= kAttachmentsSizeThresholdOverHighQualityMedium) { // Upgrade bandwidth regardless of advertising visibility because either // the system or the user has verified the sender's identity; the @@ -2637,85 +2630,82 @@ void NearbySharingServiceImpl::OnPayloadPathsRegistered( } void NearbySharingServiceImpl::OnOutgoingConnection( - const ShareTarget& share_target, absl::Time connect_start_time, - NearbyConnection* connection) { - int64_t share_target_id = share_target.id; - OutgoingShareTargetInfo* info = GetOutgoingShareTargetInfo(share_target_id); - bool success = info && connection; - - if (!success) { + absl::Time connect_start_time, NearbyConnection* connection, + OutgoingShareTargetInfo& info) { + if (!connection) { NL_LOG(WARNING) << __func__ << ": Failed to initiate connection to share target " - << share_target_id; + << info.share_target().id; TransferMetadata::Status transfer_status = TransferMetadata::Status::kFailedToInitiateOutgoingConnection; - if (info != nullptr && - info->connection_layer_status() == Status::kTimeout) { + if (info.connection_layer_status() == Status::kTimeout) { transfer_status = TransferMetadata::Status::kTimedOut; - info->set_connection_layer_status(Status::kUnknown); + info.set_connection_layer_status(Status::kUnknown); } - AbortAndCloseConnectionIfNecessary(transfer_status, share_target_id); + AbortAndCloseConnectionIfNecessary(transfer_status, info.share_target().id); return; } - info->set_connection(connection); - info->set_disconnect_status( + info.set_connection(connection); + info.set_disconnect_status( TransferMetadata::Status::kUnexpectedDisconnection); connection->SetDisconnectionListener( - [this, share_target_id]() { OnConnectionDisconnected(share_target_id); }); + [this, share_target_id = info.share_target().id]() { + OnConnectionDisconnected(share_target_id); + }); // Log analytics event of establishing connection. analytics_recorder_->NewEstablishConnection( - info->session_id(), EstablishConnectionStatus::CONNECTION_STATUS_SUCCESS, - share_target, - /*transfer_position=*/GetConnectedShareTargetPos(share_target), + info.session_id(), EstablishConnectionStatus::CONNECTION_STATUS_SUCCESS, + info.share_target(), + /*transfer_position=*/GetConnectedShareTargetPos(), /*concurrent_connections=*/GetConnectedShareTargetCount(), - info->connection_start_time().has_value() + info.connection_start_time().has_value() ? absl::ToInt64Milliseconds((context_->GetClock()->Now() - - *(info->connection_start_time()))) + *(info.connection_start_time()))) : 0, std::nullopt); std::optional four_digit_token = TokenToFourDigitString( nearby_connections_manager_->GetRawAuthenticationToken( - info->endpoint_id())); + info.endpoint_id())); RunPairedKeyVerification( - share_target_id, info->endpoint_id(), - [this, share_target, four_digit_token = std::move(four_digit_token)]( + info.share_target().id, info.endpoint_id(), + [this, share_target_id = info.share_target().id, + four_digit_token = std::move(four_digit_token)]( PairedKeyVerificationRunner::PairedKeyVerificationResult result, OSType remote_os_type) { - OnOutgoingConnectionKeyVerificationDone(share_target, four_digit_token, - result, remote_os_type); + OnOutgoingConnectionKeyVerificationDone( + share_target_id, four_digit_token, result, remote_os_type); }); } void NearbySharingServiceImpl::SendIntroduction( - const ShareTarget& share_target, + OutgoingShareTargetInfo& info, std::optional four_digit_token) { // We successfully connected! Now lets build up Payloads for all the files we // want to send them. We won't send any just yet, but we'll send the Payload // IDs in our introduction frame so that they know what to expect if they // accept. NL_VLOG(1) << __func__ << ": Preparing to send introduction to " - << share_target.id; + << info.share_target().id; - ShareTargetInfo* info = GetShareTargetInfo(share_target.id); - if (!info || !info->connection()) { + if (!info.connection()) { NL_LOG(WARNING) << __func__ << ": No NearbyConnection tied to " - << share_target.id; + << info.share_target().id; return; } // Log analytics event of sending introduction. analytics_recorder_->NewSendIntroduction( - info->session_id(), share_target, - /*transfer_position=*/GetConnectedShareTargetPos(share_target), + info.session_id(), info.share_target(), + /*transfer_position=*/GetConnectedShareTargetPos(), /*concurrent_connections=*/GetConnectedShareTargetCount(), - info->os_type()); + info.os_type()); - NearbyConnection* connection = info->connection(); + NearbyConnection* connection = info.connection(); if (foreground_send_surface_map_.empty() && background_send_surface_map_.empty()) { @@ -2728,9 +2718,10 @@ void NearbySharingServiceImpl::SendIntroduction( auto introduction = std::make_unique(); introduction->set_start_transfer(true); - NL_VLOG(1) << __func__ << ": Sending attachments to " << share_target.id; + NL_VLOG(1) << __func__ << ": Sending attachments to " + << info.share_target().id; - const AttachmentContainer& container = share_target.attachment_container; + const AttachmentContainer& container = info.attachment_container(); // Write introduction of file payloads. for (const auto& file : container.GetFileAttachments()) { std::optional payload_id = GetAttachmentPayloadId(file.id()); @@ -2780,7 +2771,6 @@ void NearbySharingServiceImpl::SendIntroduction( wifi_credentials.security_type()); wifi_credentials_metadata->set_payload_id(*payload_id); } - info->set_share_target(share_target); if (introduction->file_metadata_size() == 0 && introduction->text_metadata_size() == 0 && @@ -2788,7 +2778,7 @@ void NearbySharingServiceImpl::SendIntroduction( NL_LOG(WARNING) << __func__ << ": No payloads tied to transfer, disconnecting."; AbortAndCloseConnectionIfNecessary( - TransferMetadata::Status::kMissingPayloads, share_target.id); + TransferMetadata::Status::kMissingPayloads, info.share_target().id); return; } @@ -2810,11 +2800,11 @@ void NearbySharingServiceImpl::SendIntroduction( mutual_acceptance_timeout_alarm_->Stop(); mutual_acceptance_timeout_alarm_->Start( absl::ToInt64Milliseconds(kReadResponseFrameTimeout), 0, - [this, share_target]() { - OnOutgoingMutualAcceptanceTimeout(share_target.id); + [this, share_target_id = info.share_target().id]() { + OnOutgoingMutualAcceptanceTimeout(share_target_id); }); - info->UpdateTransferMetadata( + info.UpdateTransferMetadata( TransferMetadataBuilder() .set_status(TransferMetadata::Status::kAwaitingLocalConfirmation) .set_token(four_digit_token) @@ -2823,21 +2813,21 @@ void NearbySharingServiceImpl::SendIntroduction( void NearbySharingServiceImpl::CreatePayloads( OutgoingShareTargetInfo& info, - std::function callback) { - ShareTarget share_target = info.share_target(); + std::function callback) { + int64_t share_target_id = info.share_target().id; if (!info.file_payloads().empty() || !info.text_payloads().empty() || !info.wifi_credentials_payloads().empty()) { // We may have already created the payloads in the case of retry, so we can // skip this step. - std::move(callback)(std::move(share_target), /*success=*/false); + std::move(callback)(share_target_id, /*success=*/false); return; } - const AttachmentContainer& container = share_target.attachment_container; + const AttachmentContainer& container = info.attachment_container(); info.set_text_payloads(CreateTextPayloads(container.GetTextAttachments())); info.set_wifi_credentials_payloads( CreateWifiCredentialsPayloads(container.GetWifiCredentialsAttachments())); if (container.GetFileAttachments().empty()) { - std::move(callback)(std::move(share_target), /*success=*/true); + std::move(callback)(share_target_id, /*success=*/true); return; } @@ -2849,23 +2839,22 @@ void NearbySharingServiceImpl::CreatePayloads( file_handler_.OpenFiles( std::move(file_paths), - [this, share_target = std::move(share_target), - callback = std::move(callback)]( + [this, share_target_id, callback = std::move(callback)]( std::vector file_infos) { RunOnNearbySharingServiceThread( - "open_files", [this, share_target = std::move(share_target), - callback = std::move(callback), - file_infos = std::move(file_infos)]() { - OnOpenFiles(std::move(share_target), std::move(callback), + "open_files", + [this, share_target_id, callback = std::move(callback), + file_infos = std::move(file_infos)]() { + OnOpenFiles(share_target_id, std::move(callback), std::move(file_infos)); }); }); } void NearbySharingServiceImpl::OnCreatePayloads( - std::vector endpoint_info, ShareTarget share_target, + std::vector endpoint_info, int64_t share_target_id, bool success) { - OutgoingShareTargetInfo* info = GetOutgoingShareTargetInfo(share_target.id); + OutgoingShareTargetInfo* info = GetOutgoingShareTargetInfo(share_target_id); bool has_payloads = info && (!info->text_payloads().empty() || !info->file_payloads().empty() || !info->wifi_credentials_payloads().empty()); @@ -2881,9 +2870,11 @@ void NearbySharingServiceImpl::OnCreatePayloads( } return; } + // Log analytics event of describing attachments. + analytics_recorder_->NewDescribeAttachments(info->attachment_container()); std::optional> bluetooth_mac_address = - GetBluetoothMacAddressForShareTarget(share_target.id); + GetBluetoothMacAddressForShareTarget(share_target_id); // For metrics. all_cancelled_share_target_ids_.clear(); @@ -2893,16 +2884,25 @@ void NearbySharingServiceImpl::OnCreatePayloads( nearby_connections_manager_->Connect( std::move(endpoint_info), info->endpoint_id(), std::move(bluetooth_mac_address), settings_->GetDataUsage(), - GetTransportType(share_target.attachment_container), - [this, share_target, info](NearbyConnection* connection, Status status) { + GetTransportType(info->attachment_container()), + [this, share_target_id](NearbyConnection* connection, Status status) { + OutgoingShareTargetInfo* info = + GetOutgoingShareTargetInfo(share_target_id); + if (info == nullptr) { + NL_LOG(WARNING) << __func__ + << "Nearby connection connected, but share target " + << share_target_id << " already disconnected."; + return; + } // Log analytics event of new connection. info->set_connection_layer_status(status); if (connection == nullptr) { analytics_recorder_->NewEstablishConnection( info->session_id(), EstablishConnectionStatus::CONNECTION_STATUS_FAILURE, - share_target, - /*transfer_position=*/GetConnectedShareTargetPos(share_target), + info->share_target(), + /*transfer_position=*/ + GetConnectedShareTargetPos(), /*concurrent_connections=*/GetConnectedShareTargetCount(), info->connection_start_time().has_value() ? absl::ToInt64Milliseconds(context_->GetClock()->Now() - @@ -2911,18 +2911,17 @@ void NearbySharingServiceImpl::OnCreatePayloads( std::nullopt); } - OnOutgoingConnection(share_target, context_->GetClock()->Now(), - connection); + OnOutgoingConnection(context_->GetClock()->Now(), connection, *info); }); } void NearbySharingServiceImpl::OnOpenFiles( - ShareTarget share_target, std::function callback, + int64_t share_target_id, std::function callback, std::vector files) { - OutgoingShareTargetInfo* info = GetOutgoingShareTargetInfo(share_target.id); - AttachmentContainer& container = share_target.attachment_container; + OutgoingShareTargetInfo* info = GetOutgoingShareTargetInfo(share_target_id); + AttachmentContainer& container = info->mutable_attachment_container(); if (!info || files.size() != container.GetFileAttachments().size()) { - std::move(callback)(std::move(share_target), /*success=*/false); + std::move(callback)(share_target_id, /*success=*/false); return; } @@ -2941,7 +2940,7 @@ void NearbySharingServiceImpl::OnOpenFiles( } info->set_file_payloads(std::move(payloads)); - std::move(callback)(std::move(share_target), /*success=*/true); + std::move(callback)(share_target_id, /*success=*/true); } std::vector NearbySharingServiceImpl::CreateTextPayloads( @@ -3118,20 +3117,23 @@ void NearbySharingServiceImpl::OnIncomingAdvertisementDecoded( } void NearbySharingServiceImpl::OnIncomingTransferUpdate( - const ShareTarget& share_target, const TransferMetadata& metadata) { + const IncomingShareTargetInfo& share_target_info, + const TransferMetadata& metadata) { // kInProgress status is logged extensively elsewhere so avoid the spam. if (metadata.status() != TransferMetadata::Status::kInProgress) { NL_VLOG(1) << __func__ << ": Nearby Share service: " << "Incoming transfer update for share target with ID " - << share_target.id << ": " + << share_target_info.share_target().id << ": " << TransferMetadata::StatusToString(metadata.status()); } if (metadata.status() != TransferMetadata::Status::kCancelled && metadata.status() != TransferMetadata::Status::kRejected) { last_incoming_metadata_ = - std::make_pair(share_target, TransferMetadataBuilder::Clone(metadata) - .set_is_original(false) - .build()); + std::make_tuple(share_target_info.share_target(), + share_target_info.attachment_container(), + TransferMetadataBuilder::Clone(metadata) + .set_is_original(false) + .build()); } else { last_incoming_metadata_ = std::nullopt; } @@ -3139,7 +3141,7 @@ void NearbySharingServiceImpl::OnIncomingTransferUpdate( if (metadata.is_final_status()) { // Log analytics event of receiving attachment end. int64_t received_bytes = - share_target.attachment_container.GetTotalAttachmentsSize() * + share_target_info.attachment_container().GetTotalAttachmentsSize() * metadata.progress() / 100; AttachmentTransmissionStatus transmission_status = ConvertToTransmissionStatus(metadata.status()); @@ -3152,7 +3154,7 @@ void NearbySharingServiceImpl::OnIncomingTransferUpdate( if (metadata.status() != TransferMetadata::Status::kComplete) { // For any type of failure, lets make sure any pending files get cleaned // up. - RemoveIncomingPayloads(share_target); + RemoveIncomingPayloads(share_target_info); } else { if (!nearby_connections_manager_->GetAndClearUnknownFilePathsToDelete() .empty()) { @@ -3170,48 +3172,46 @@ void NearbySharingServiceImpl::OnIncomingTransferUpdate( } for (auto& callback : callbacks) { - callback.first->OnTransferUpdate( - share_target, share_target.attachment_container, metadata); + callback.first->OnTransferUpdate(share_target_info.share_target(), + share_target_info.attachment_container(), + metadata); } } void NearbySharingServiceImpl::OnOutgoingTransferUpdate( - const ShareTarget& share_target, const TransferMetadata& metadata) { + OutgoingShareTargetInfo& share_target_info, + const TransferMetadata& metadata) { // kInProgress status is logged extensively elsewhere so avoid the spam. if (metadata.status() != TransferMetadata::Status::kInProgress) { NL_VLOG(1) << __func__ << ": Nearby Share service: " << "Outgoing transfer update for share target with ID " - << share_target.id << ": " + << share_target_info.share_target().id << ": " << TransferMetadata::StatusToString(metadata.status()); } - OutgoingShareTargetInfo* info = GetOutgoingShareTargetInfo(share_target.id); if (metadata.is_final_status()) { // Log analytics event of sending attachment end. int64_t sent_bytes = - share_target.attachment_container.GetTotalAttachmentsSize() * + share_target_info.attachment_container().GetTotalAttachmentsSize() * metadata.progress() / 100; AttachmentTransmissionStatus transmission_status = ConvertToTransmissionStatus(metadata.status()); - if (info == nullptr) { - // The situation may happen when user cancel connection during - // establishing connection. - NL_LOG(INFO) << "No share target info is created for share_target:" - << share_target.device_name; - } else { - analytics_recorder_->NewSendAttachmentsEnd( - info->session_id(), sent_bytes, share_target, transmission_status, - /*transfer_position=*/GetConnectedShareTargetPos(share_target), - /*concurrent_connections=*/GetConnectedShareTargetCount(), - /*duration_millis=*/info->connection_start_time().has_value() - ? absl::ToInt64Milliseconds(context_->GetClock()->Now() - - *(info->connection_start_time())) - : 0, - /*referrer_package=*/std::nullopt, - ConvertToConnectionLayerStatus(info->connection_layer_status()), - info->os_type()); - } + analytics_recorder_->NewSendAttachmentsEnd( + share_target_info.session_id(), sent_bytes, + share_target_info.share_target(), transmission_status, + /*transfer_position=*/GetConnectedShareTargetPos(), + /*concurrent_connections=*/GetConnectedShareTargetCount(), + /*duration_millis=*/ + share_target_info.connection_start_time().has_value() + ? absl::ToInt64Milliseconds( + context_->GetClock()->Now() - + *(share_target_info.connection_start_time())) + : 0, + /*referrer_package=*/std::nullopt, + ConvertToConnectionLayerStatus( + share_target_info.connection_layer_status()), + share_target_info.os_type()); is_connecting_ = false; OnTransferComplete(); } else if (metadata.status() == TransferMetadata::Status::kMediaDownloading || @@ -3222,40 +3222,37 @@ void NearbySharingServiceImpl::OnOutgoingTransferUpdate( } bool has_foreground_send_surface = !foreground_send_surface_map_.empty(); - if (info) { - ShareTarget cached_share_target = info->share_target(); - // only call transfer update when having share target info. - if (has_foreground_send_surface) { - for (auto& entry : foreground_send_surface_map_) { - entry.first->OnTransferUpdate(cached_share_target, - cached_share_target.attachment_container, - metadata); - } - } else { - for (auto& entry : background_send_surface_map_) { - entry.first->OnTransferUpdate(cached_share_target, - cached_share_target.attachment_container, - metadata); - } + // only call transfer update when having share target info. + if (has_foreground_send_surface) { + for (auto& entry : foreground_send_surface_map_) { + entry.first->OnTransferUpdate(share_target_info.share_target(), + share_target_info.attachment_container(), + metadata); } + } else { + for (auto& entry : background_send_surface_map_) { + entry.first->OnTransferUpdate(share_target_info.share_target(), + share_target_info.attachment_container(), + metadata); + } + } - // check whether need to send next payload. - if (NearbyFlags::GetInstance().GetBoolFlag( - config_package_nearby::nearby_sharing_feature:: - kEnableTransferCancellationOptimization)) { - if (metadata.in_progress_attachment_transferred_bytes().has_value() && - metadata.in_progress_attachment_total_bytes().has_value() && - *metadata.in_progress_attachment_transferred_bytes() == - *metadata.in_progress_attachment_total_bytes()) { - std::optional payload = info->ExtractNextPayload(); - if (payload.has_value()) { - NL_LOG(INFO) << __func__ << ": Send payload " << payload->id; - nearby_connections_manager_->Send(info->endpoint_id(), - std::make_unique(*payload), - info->payload_tracker()); - } else { - NL_LOG(WARNING) << __func__ << ": There is no paylaods to send."; - } + // check whether need to send next payload. + if (NearbyFlags::GetInstance().GetBoolFlag( + config_package_nearby::nearby_sharing_feature:: + kEnableTransferCancellationOptimization)) { + if (metadata.in_progress_attachment_transferred_bytes().has_value() && + metadata.in_progress_attachment_total_bytes().has_value() && + *metadata.in_progress_attachment_transferred_bytes() == + *metadata.in_progress_attachment_total_bytes()) { + std::optional payload = share_target_info.ExtractNextPayload(); + if (payload.has_value()) { + NL_LOG(INFO) << __func__ << ": Send payload " << payload->id; + nearby_connections_manager_->Send(share_target_info.endpoint_id(), + std::make_unique(*payload), + share_target_info.payload_tracker()); + } else { + NL_LOG(WARNING) << __func__ << ": There is no paylaods to send."; } } } @@ -3264,9 +3261,11 @@ void NearbySharingServiceImpl::OnOutgoingTransferUpdate( last_outgoing_metadata_ = std::nullopt; } else { last_outgoing_metadata_ = - std::make_pair(share_target, TransferMetadataBuilder::Clone(metadata) - .set_is_original(false) - .build()); + std::make_tuple(share_target_info.share_target(), + share_target_info.attachment_container(), + TransferMetadataBuilder::Clone(metadata) + .set_is_original(false) + .build()); } } @@ -3313,7 +3312,8 @@ void NearbySharingServiceImpl::OnIncomingDecryptedCertificate( NL_VLOG(1) << __func__ << ": Received incoming connection from " << share_target_id; - ShareTargetInfo* share_target_info = GetShareTargetInfo(share_target_id); + IncomingShareTargetInfo* share_target_info = + GetIncomingShareTargetInfo(share_target_id); NL_DCHECK(share_target_info); share_target_info->set_connection(connection); share_target_info->set_disconnect_status( @@ -3326,15 +3326,14 @@ void NearbySharingServiceImpl::OnIncomingDecryptedCertificate( nearby_connections_manager_->GetRawAuthenticationToken(endpoint_id)); RunPairedKeyVerification( - share_target->id, endpoint_id, - [this, share_target = *share_target, - four_digit_token = std::move(four_digit_token)]( + share_target_id, endpoint_id, + [this, share_target_id, four_digit_token = std::move(four_digit_token)]( PairedKeyVerificationRunner::PairedKeyVerificationResult verification_result, OSType remote_os_type) { - OnIncomingConnectionKeyVerificationDone(share_target, four_digit_token, - verification_result, - remote_os_type); + OnIncomingConnectionKeyVerificationDone( + share_target_id, four_digit_token, verification_result, + remote_os_type); }); } @@ -3374,10 +3373,10 @@ void NearbySharingServiceImpl::RunPairedKeyVerification( } void NearbySharingServiceImpl::OnIncomingConnectionKeyVerificationDone( - ShareTarget share_target, std::optional four_digit_token, + int64_t share_target_id, std::optional four_digit_token, PairedKeyVerificationRunner::PairedKeyVerificationResult result, OSType share_target_os_type) { - ShareTargetInfo* info = GetShareTargetInfo(share_target.id); + IncomingShareTargetInfo* info = GetIncomingShareTargetInfo(share_target_id); if (!info || !info->connection()) { NL_VLOG(1) << __func__ << ": Invalid connection or endpoint id"; return; @@ -3388,45 +3387,45 @@ void NearbySharingServiceImpl::OnIncomingConnectionKeyVerificationDone( switch (result) { case PairedKeyVerificationRunner::PairedKeyVerificationResult::kFail: NL_VLOG(1) << __func__ << ": Paired key handshake failed for target " - << share_target.id << ". Disconnecting."; + << share_target_id << ". Disconnecting."; AbortAndCloseConnectionIfNecessary( TransferMetadata::Status::kPairedKeyVerificationFailed, - share_target.id); + share_target_id); return; case PairedKeyVerificationRunner::PairedKeyVerificationResult::kSuccess: NL_VLOG(1) << __func__ << ": Paired key handshake succeeded for target - " - << share_target.id; - ReceiveIntroduction(share_target, /*four_digit_token=*/std::nullopt); + << share_target_id; + ReceiveIntroduction(*info, /*four_digit_token=*/std::nullopt); break; case PairedKeyVerificationRunner::PairedKeyVerificationResult::kUnable: NL_VLOG(1) << __func__ << ": Unable to verify paired key encryption when " "receiving connection from target - " - << share_target.id; + << share_target_id; if (four_digit_token) info->set_token(*four_digit_token); - ReceiveIntroduction(share_target, std::move(four_digit_token)); + ReceiveIntroduction(*info, std::move(four_digit_token)); break; case PairedKeyVerificationRunner::PairedKeyVerificationResult::kUnknown: NL_VLOG(1) << __func__ << ": Unknown PairedKeyVerificationResult for target " - << share_target.id << ". Disconnecting."; + << share_target_id << ". Disconnecting."; AbortAndCloseConnectionIfNecessary( TransferMetadata::Status::kPairedKeyVerificationFailed, - share_target.id); + share_target_id); break; } } void NearbySharingServiceImpl::OnOutgoingConnectionKeyVerificationDone( - const ShareTarget& share_target, + int64_t share_target_id, std::optional four_digit_token, PairedKeyVerificationRunner::PairedKeyVerificationResult result, OSType share_target_os_type) { - ShareTargetInfo* info = GetShareTargetInfo(share_target.id); + OutgoingShareTargetInfo* info = GetOutgoingShareTargetInfo(share_target_id); if (!info || !info->connection()) { return; } @@ -3436,24 +3435,24 @@ void NearbySharingServiceImpl::OnOutgoingConnectionKeyVerificationDone( switch (result) { case PairedKeyVerificationRunner::PairedKeyVerificationResult::kFail: NL_VLOG(1) << __func__ << ": Paired key handshake failed for target " - << share_target.id << ". Disconnecting."; + << share_target_id << ". Disconnecting."; AbortAndCloseConnectionIfNecessary( TransferMetadata::Status::kPairedKeyVerificationFailed, - share_target.id); + share_target_id); return; case PairedKeyVerificationRunner::PairedKeyVerificationResult::kSuccess: NL_VLOG(1) << __func__ << ": Paired key handshake succeeded for target - " - << share_target.id; - SendIntroduction(share_target, /*four_digit_token=*/std::nullopt); - SendPayloads(share_target); + << share_target_id; + SendIntroduction(*info, /*four_digit_token=*/std::nullopt); + SendPayloads(*info); return; case PairedKeyVerificationRunner::PairedKeyVerificationResult::kUnable: NL_VLOG(1) << __func__ << ": Unable to verify paired key encryption when " "initiating connection to target - " - << share_target.id; + << share_target_id; if (four_digit_token) { info->set_token(*four_digit_token); @@ -3465,47 +3464,47 @@ void NearbySharingServiceImpl::OnOutgoingConnectionKeyVerificationDone( NL_VLOG(1) << __func__ << ": Sender-side verification is disabled. Skipping " "token comparison with " - << share_target.id; - SendIntroduction(share_target, /*four_digit_token=*/std::nullopt); - SendPayloads(share_target); + << share_target_id; + SendIntroduction(*info, /*four_digit_token=*/std::nullopt); + SendPayloads(*info); } else { - SendIntroduction(share_target, std::move(four_digit_token)); + SendIntroduction(*info, std::move(four_digit_token)); } return; case PairedKeyVerificationRunner::PairedKeyVerificationResult::kUnknown: NL_VLOG(1) << __func__ << ": Unknown PairedKeyVerificationResult for target " - << share_target.id << ". Disconnecting."; + << share_target_id << ". Disconnecting."; AbortAndCloseConnectionIfNecessary( TransferMetadata::Status::kPairedKeyVerificationFailed, - share_target.id); + share_target_id); break; } } void NearbySharingServiceImpl::ReceiveIntroduction( - ShareTarget share_target, std::optional four_digit_token) { + const IncomingShareTargetInfo& info, + std::optional four_digit_token) { NL_LOG(INFO) << __func__ << ": Receiving introduction from " - << share_target.id; - ShareTargetInfo* info = GetShareTargetInfo(share_target.id); - NL_DCHECK(info && info->connection()); + << info.share_target().id; + NL_DCHECK(info.connection()); - info->frames_reader()->ReadFrame( + info.frames_reader()->ReadFrame( nearby::sharing::service::proto::V1Frame::INTRODUCTION, - [this, share_target = std::move(share_target), + [this, share_target_id = info.share_target().id, four_digit_token = std::move(four_digit_token)]( std::optional frame) { - OnReceivedIntroduction(std::move(share_target), + OnReceivedIntroduction(share_target_id, std::move(four_digit_token), std::move(frame)); }, kReadFramesTimeout); } void NearbySharingServiceImpl::OnReceivedIntroduction( - ShareTarget share_target, std::optional four_digit_token, + int64_t share_target_id, std::optional four_digit_token, std::optional frame) { - ShareTargetInfo* info = GetShareTargetInfo(share_target.id); + IncomingShareTargetInfo* info = GetIncomingShareTargetInfo(share_target_id); if (!info || !info->connection()) { NL_LOG(WARNING) << __func__ @@ -3515,7 +3514,7 @@ void NearbySharingServiceImpl::OnReceivedIntroduction( if (!frame.has_value()) { AbortAndCloseConnectionIfNecessary( - TransferMetadata::Status::kInvalidIntroductionFrame, share_target.id); + TransferMetadata::Status::kInvalidIntroductionFrame, share_target_id); NL_LOG(WARNING) << __func__ << ": Invalid introduction frame"; return; } @@ -3527,10 +3526,10 @@ void NearbySharingServiceImpl::OnReceivedIntroduction( nearby::sharing::service::proto::IntroductionFrame introduction_frame = std::move(frame->introduction()); - AttachmentContainer& container = share_target.attachment_container; + AttachmentContainer& container = info->mutable_attachment_container(); for (const auto& file : introduction_frame.file_metadata()) { if (file.size() <= 0) { - Fail(share_target.id, + Fail(share_target_id, TransferMetadata::Status::kUnsupportedAttachmentType); NL_LOG(WARNING) << __func__ @@ -3550,7 +3549,7 @@ void NearbySharingServiceImpl::OnReceivedIntroduction( container.AddFileAttachment(std::move(attachment)); if (std::numeric_limits::max() - file.size() < file_size_sum) { - Fail(share_target.id, TransferMetadata::Status::kNotEnoughSpace); + Fail(share_target_id, TransferMetadata::Status::kNotEnoughSpace); NL_LOG(WARNING) << __func__ << ": Ignoring introduction, total file size overflowed " "64 bit integer."; @@ -3561,7 +3560,7 @@ void NearbySharingServiceImpl::OnReceivedIntroduction( for (const auto& text : introduction_frame.text_metadata()) { if (text.size() <= 0) { - Fail(share_target.id, + Fail(share_target_id, TransferMetadata::Status::kUnsupportedAttachmentType); NL_LOG(WARNING) << __func__ @@ -3597,23 +3596,22 @@ void NearbySharingServiceImpl::OnReceivedIntroduction( NL_LOG(WARNING) << __func__ << ": No attachment is found for this share target. It can " "be result of unrecognizable attachment type"; - Fail(share_target.id, TransferMetadata::Status::kUnsupportedAttachmentType); + Fail(share_target_id, TransferMetadata::Status::kUnsupportedAttachmentType); NL_VLOG(1) << __func__ << ": We don't support the attachments sent by the sender. " "We have informed " - << share_target.id; + << share_target_id; return; } - info->set_share_target(share_target); // Log analytics event of receiving introduction. analytics_recorder_->NewReceiveIntroduction( - receiving_session_id_, share_target, /*referrer_package=*/std::nullopt, - info->os_type()); + receiving_session_id_, info->share_target(), + /*referrer_package=*/std::nullopt, info->os_type()); if (file_size_sum == 0) { - OnStorageCheckCompleted(std::move(share_target), + OnStorageCheckCompleted(share_target_id, std::move(four_digit_token), /*is_out_of_storage=*/false); return; @@ -3642,30 +3640,29 @@ void NearbySharingServiceImpl::OnReceivedIntroduction( bool is_out_of_storage = IsOutOfStorage(device_info_, download_path, file_size_sum); - OnStorageCheckCompleted(std::move(share_target), std::move(four_digit_token), + OnStorageCheckCompleted(share_target_id, std::move(four_digit_token), is_out_of_storage); } void NearbySharingServiceImpl::ReceiveConnectionResponse( - ShareTarget share_target) { + ShareTargetInfo& info) { NL_VLOG(1) << __func__ << ": Receiving response frame from " - << share_target.id; - ShareTargetInfo* info = GetShareTargetInfo(share_target.id); - NL_DCHECK(info && info->connection()); + << info.share_target().id; + NL_DCHECK(info.connection()); - info->frames_reader()->ReadFrame( + info.frames_reader()->ReadFrame( nearby::sharing::service::proto::V1Frame::RESPONSE, - [this, share_target = std::move(share_target)]( + [this, share_target_id = info.share_target().id]( std::optional frame) { - OnReceiveConnectionResponse(share_target, std::move(frame)); + OnReceiveConnectionResponse(share_target_id, std::move(frame)); }, kReadResponseFrameTimeout); } void NearbySharingServiceImpl::OnReceiveConnectionResponse( - ShareTarget share_target, + int64_t share_target_id, std::optional frame) { - OutgoingShareTargetInfo* info = GetOutgoingShareTargetInfo(share_target.id); + OutgoingShareTargetInfo* info = GetOutgoingShareTargetInfo(share_target_id); if (!info || !info->connection()) { NL_LOG(WARNING) << __func__ << ": Ignore received connection response, due to no " @@ -3679,7 +3676,7 @@ void NearbySharingServiceImpl::OnReceiveConnectionResponse( << ": Failed to read a response from the remote device. Disconnecting."; AbortAndCloseConnectionIfNecessary( TransferMetadata::Status::kFailedToReadOutgoingConnectionResponse, - share_target.id); + share_target_id); return; } @@ -3696,9 +3693,9 @@ void NearbySharingServiceImpl::OnReceiveConnectionResponse( WriteProgressUpdateFrame(*info->connection(), true, std::nullopt); info->frames_reader()->ReadFrame( - [this, share_target]( + [this, share_target_id]( std::optional frame) { - OnFrameRead(share_target, std::move(frame)); + OnFrameRead(share_target_id, std::move(frame)); }); info->UpdateTransferMetadata( @@ -3707,7 +3704,7 @@ void NearbySharingServiceImpl::OnReceiveConnectionResponse( .build()); info->set_payload_tracker(std::make_unique( - context_, share_target.id, share_target.attachment_container, + context_, share_target_id, info->attachment_container(), attachment_info_map_, absl::bind_front(&NearbySharingServiceImpl::OnPayloadTransferUpdate, this))); @@ -3744,7 +3741,7 @@ void NearbySharingServiceImpl::OnReceiveConnectionResponse( } case nearby::sharing::service::proto::ConnectionResponseFrame::REJECT: AbortAndCloseConnectionIfNecessary(TransferMetadata::Status::kRejected, - share_target.id); + share_target_id); NL_VLOG(1) << __func__ << ": The connection was rejected. The connection has been closed."; @@ -3752,7 +3749,7 @@ void NearbySharingServiceImpl::OnReceiveConnectionResponse( case nearby::sharing::service::proto::ConnectionResponseFrame:: NOT_ENOUGH_SPACE: AbortAndCloseConnectionIfNecessary( - TransferMetadata::Status::kNotEnoughSpace, share_target.id); + TransferMetadata::Status::kNotEnoughSpace, share_target_id); NL_VLOG(1) << __func__ << ": The connection was rejected because the remote device " "does not have enough space for our attachments. The " @@ -3762,7 +3759,7 @@ void NearbySharingServiceImpl::OnReceiveConnectionResponse( UNSUPPORTED_ATTACHMENT_TYPE: AbortAndCloseConnectionIfNecessary( TransferMetadata::Status::kUnsupportedAttachmentType, - share_target.id); + share_target_id); NL_VLOG(1) << __func__ << ": The connection was rejected because the remote device " "does not support the attachments we were sending. The " @@ -3770,14 +3767,14 @@ void NearbySharingServiceImpl::OnReceiveConnectionResponse( break; case nearby::sharing::service::proto::ConnectionResponseFrame::TIMED_OUT: AbortAndCloseConnectionIfNecessary(TransferMetadata::Status::kTimedOut, - share_target.id); + share_target_id); NL_VLOG(1) << __func__ << ": The connection was rejected because the remote device " "timed out. The connection has been closed."; break; default: AbortAndCloseConnectionIfNecessary(TransferMetadata::Status::kFailed, - share_target.id); + share_target_id); NL_VLOG(1) << __func__ << ": The connection failed. The connection has been closed."; break; @@ -3785,9 +3782,8 @@ void NearbySharingServiceImpl::OnReceiveConnectionResponse( } void NearbySharingServiceImpl::OnStorageCheckCompleted( - ShareTarget share_target, std::optional four_digit_token, + int64_t share_target_id, std::optional four_digit_token, bool is_out_of_storage) { - int64_t share_target_id = share_target.id; if (is_out_of_storage) { Fail(share_target_id, TransferMetadata::Status::kNotEnoughSpace); NL_LOG(WARNING) << __func__ @@ -3855,14 +3851,14 @@ void NearbySharingServiceImpl::OnStorageCheckCompleted( } frames_reader->ReadFrame( - [this, share_target = std::move(share_target)]( + [this, share_target_id]( std::optional frame) { - OnFrameRead(std::move(share_target), std::move(frame)); + OnFrameRead(share_target_id, std::move(frame)); }); } void NearbySharingServiceImpl::OnFrameRead( - ShareTarget share_target, + int64_t share_target_id, std::optional frame) { if (!frame.has_value()) { // This is the case when the connection has been closed since we wait @@ -3872,11 +3868,11 @@ void NearbySharingServiceImpl::OnFrameRead( switch (frame->type()) { case nearby::sharing::service::proto::V1Frame::CANCEL: - RunOnAnyThread("cancel_transfer", [this, share_target]() { + RunOnAnyThread("cancel_transfer", [this, share_target_id]() { NL_LOG(INFO) << __func__ << ": Read the cancel frame, closing connection"; DoCancel( - share_target.id, [](StatusCodes status_codes) {}, + share_target_id, [](StatusCodes status_codes) {}, /*is_initiator_of_cancellation=*/false); }); break; @@ -3886,7 +3882,7 @@ void NearbySharingServiceImpl::OnFrameRead( break; case nearby::sharing::service::proto::V1Frame::PROGRESS_UPDATE: - HandleProgressUpdateFrame(share_target, frame->progress_update()); + HandleProgressUpdateFrame(share_target_id, frame->progress_update()); break; default: @@ -3894,7 +3890,7 @@ void NearbySharingServiceImpl::OnFrameRead( break; } - ShareTargetInfo* info = GetShareTargetInfo(share_target.id); + ShareTargetInfo* info = GetShareTargetInfo(share_target_id); if (!info || !info->frames_reader()) { NL_LOG(WARNING) << __func__ << ": Stopped reading further frames, due to no connection " @@ -3903,22 +3899,22 @@ void NearbySharingServiceImpl::OnFrameRead( } info->frames_reader()->ReadFrame( - [this, share_target = std::move(share_target)]( + [this, share_target_id]( std::optional frame) { - OnFrameRead(share_target, std::move(frame)); + OnFrameRead(share_target_id, std::move(frame)); }); } void NearbySharingServiceImpl::HandleProgressUpdateFrame( - const ShareTarget& share_target, + int64_t share_target_id, const nearby::sharing::service::proto::ProgressUpdateFrame& progress_update_frame) { if (progress_update_frame.has_start_transfer() && progress_update_frame.start_transfer()) { - ShareTargetInfo* info = GetShareTargetInfo(share_target.id); + ShareTargetInfo* info = GetShareTargetInfo(share_target_id); if (info != nullptr && - share_target.attachment_container.GetTotalAttachmentsSize() >= + info->attachment_container().GetTotalAttachmentsSize() >= kAttachmentsSizeThresholdOverHighQualityMedium) { NL_LOG(INFO) << __func__ @@ -3930,9 +3926,9 @@ void NearbySharingServiceImpl::HandleProgressUpdateFrame( } if (progress_update_frame.has_progress()) { - NL_LOG(WARNING) << __func__ << ": Current progress for ShareTarget " - << share_target.id << " is " - << progress_update_frame.progress(); + NL_LOG(INFO) << __func__ << ": Current progress for ShareTarget " + << share_target_id << " is " + << progress_update_frame.progress(); } } @@ -4051,19 +4047,18 @@ void NearbySharingServiceImpl::OnPayloadTransferUpdate( is_waiting_to_record_accept_to_transfer_start_metric_ = false; } - 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); + UpdateFilePath(info->mutable_attachment_container()); } if (metadata.status() == TransferMetadata::Status::kComplete) { - if (!OnIncomingPayloadsComplete(share_target)) { + if (!OnIncomingPayloadsComplete(share_target_id)) { payload_incomplete = true; - share_target.attachment_container.ClearAttachments(); + info->mutable_attachment_container().ClearAttachments(); } if (IsBackgroundScanningFeatureEnabled()) { @@ -4078,10 +4073,9 @@ void NearbySharingServiceImpl::OnPayloadTransferUpdate( } 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); + UpdateFilePath(info->mutable_attachment_container()); } } - info->set_share_target(share_target); } // Make sure to call this before calling Disconnect, or we risk losing some @@ -4109,10 +4103,7 @@ void NearbySharingServiceImpl::OnPayloadTransferUpdate( } bool NearbySharingServiceImpl::OnIncomingPayloadsComplete( - ShareTarget& share_target) { - NL_DCHECK(share_target.is_incoming); - - int64_t share_target_id = share_target.id; + int64_t share_target_id) { ShareTargetInfo* info = GetShareTargetInfo(share_target_id); if (!info || !info->connection()) { NL_VLOG(1) << __func__ << ": Connection not found for target - " @@ -4122,10 +4113,10 @@ bool NearbySharingServiceImpl::OnIncomingPayloadsComplete( } if (!update_file_paths_in_progress_) { - UpdateFilePath(share_target); + UpdateFilePath(info->mutable_attachment_container()); } - AttachmentContainer& container = share_target.attachment_container; + AttachmentContainer& container = info->mutable_attachment_container(); for (int i = 0; i < container.GetTextAttachments().size(); ++i) { TextAttachment& text = container.GetMutableTextAttachment(i); AttachmentInfo& attachment_info = attachment_info_map_[text.id()]; @@ -4206,11 +4197,11 @@ bool NearbySharingServiceImpl::OnIncomingPayloadsComplete( return true; } -void NearbySharingServiceImpl::UpdateFilePath(ShareTarget& share_target) { +void NearbySharingServiceImpl::UpdateFilePath( + AttachmentContainer& attachment_container) { for (int i = 0; - i < share_target.attachment_container.GetFileAttachments().size(); ++i) { - FileAttachment& file = - share_target.attachment_container.GetMutableFileAttachment(i); + i < attachment_container.GetFileAttachments().size(); ++i) { + FileAttachment& file = attachment_container.GetMutableFileAttachment(i); // Skip file if it already has file_path set. if (file.file_path().has_value()) { continue; @@ -4239,11 +4230,7 @@ void NearbySharingServiceImpl::UpdateFilePath(ShareTarget& share_target) { } void NearbySharingServiceImpl::RemoveIncomingPayloads( - ShareTarget share_target) { - if (!share_target.is_incoming) { - return; - } - + const IncomingShareTargetInfo& share_target_info) { NL_LOG(INFO) << __func__ << ": Cleaning up payloads due to transfer failure"; nearby_connections_manager_->ClearIncomingPayloads(); std::vector files_for_deletion; @@ -4258,7 +4245,8 @@ void NearbySharingServiceImpl::RemoveIncomingPayloads( files_for_deletion.push_back(*it); } } - const AttachmentContainer& container = share_target.attachment_container; + const AttachmentContainer& container = + share_target_info.attachment_container(); for (const auto& file : container.GetFileAttachments()) { if (!file.file_path().has_value()) continue; auto file_path = *file.file_path(); @@ -4451,7 +4439,7 @@ void NearbySharingServiceImpl::UnregisterShareTarget(int64_t share_target_id) { (incoming_share_target_info_map_.erase(share_target_id) > 0); if (is_incoming) { if (last_incoming_metadata_ && - last_incoming_metadata_->first.id == share_target_id) { + std::get<0>(*last_incoming_metadata_).id == share_target_id) { last_incoming_metadata_.reset(); } @@ -4459,7 +4447,7 @@ void NearbySharingServiceImpl::UnregisterShareTarget(int64_t share_target_id) { nearby_connections_manager_->ClearIncomingPayloads(); } else { if (last_outgoing_metadata_ && - last_outgoing_metadata_->first.id == share_target_id) { + std::get<0>(*last_outgoing_metadata_).id == share_target_id) { last_outgoing_metadata_.reset(); } // Find the endpoint id that matches the given share target. @@ -4767,8 +4755,7 @@ void NearbySharingServiceImpl::RunOnAnyThread(absl::string_view task_name, }); } -int NearbySharingServiceImpl::GetConnectedShareTargetPos( - const ShareTarget& target) { +int NearbySharingServiceImpl::GetConnectedShareTargetPos() { // Returns 1 before group sharing is enabled. return 1; } diff --git a/sharing/nearby_sharing_service_impl.h b/sharing/nearby_sharing_service_impl.h index 137b3cc5..1684d17b 100644 --- a/sharing/nearby_sharing_service_impl.h +++ b/sharing/nearby_sharing_service_impl.h @@ -24,7 +24,7 @@ #include #include #include -#include +#include #include #include "absl/base/attributes.h" @@ -323,25 +323,25 @@ class NearbySharingServiceImpl void OnTransferStarted(bool is_incoming); void ReceivePayloads( - ShareTarget share_target, + ShareTargetInfo& share_target_info, std::function status_codes_callback); - StatusCodes SendPayloads(const ShareTarget& share_target); + StatusCodes SendPayloads(ShareTargetInfo& info); void OnPayloadPathsRegistered( - const ShareTarget& share_target, + ShareTargetInfo& info, std::function status_codes_callback); - void OnOutgoingConnection(const ShareTarget& share_target, - absl::Time connect_start_time, - NearbyConnection* connection); - void SendIntroduction(const ShareTarget& share_target, + void OnOutgoingConnection(absl::Time connect_start_time, + NearbyConnection* connection, + OutgoingShareTargetInfo& info); + void SendIntroduction(OutgoingShareTargetInfo& info, std::optional four_digit_token); void CreatePayloads(OutgoingShareTargetInfo& info, - std::function callback); + std::function callback); void OnCreatePayloads(std::vector endpoint_info, - ShareTarget share_target, bool success); - void OnOpenFiles(ShareTarget share_target, - std::function callback, + int64_t share_target_id, bool success); + void OnOpenFiles(int64_t share_target_id, + std::function callback, std::vector files); std::vector CreateTextPayloads( const std::vector& attachments); @@ -360,10 +360,12 @@ class NearbySharingServiceImpl void OnIncomingAdvertisementDecoded( absl::string_view endpoint_id, int64_t placeholder_share_target_id, std::unique_ptr advertisement); - void OnIncomingTransferUpdate(const ShareTarget& share_target, - const TransferMetadata& metadata); - void OnOutgoingTransferUpdate(const ShareTarget& share_target, - const TransferMetadata& metadata); + void OnIncomingTransferUpdate( + const IncomingShareTargetInfo& share_target_info, + const TransferMetadata& metadata); + void OnOutgoingTransferUpdate( + OutgoingShareTargetInfo& share_target_info, + const TransferMetadata& metadata); void CloseConnection(int64_t share_target_id); void OnIncomingDecryptedCertificate( absl::string_view endpoint_id, const Advertisement& advertisement, @@ -376,31 +378,31 @@ class NearbySharingServiceImpl ::location::nearby::proto::sharing::OSType)> callback); void OnIncomingConnectionKeyVerificationDone( - ShareTarget share_target, std::optional four_digit_token, + int64_t share_target_id, std::optional four_digit_token, PairedKeyVerificationRunner::PairedKeyVerificationResult result, ::location::nearby::proto::sharing::OSType share_target_os_type); void OnOutgoingConnectionKeyVerificationDone( - const ShareTarget& share_target, + int64_t share_target_id, std::optional four_digit_token, PairedKeyVerificationRunner::PairedKeyVerificationResult result, ::location::nearby::proto::sharing::OSType share_target_os_type); - void ReceiveIntroduction(ShareTarget share_target, + void ReceiveIntroduction(const IncomingShareTargetInfo& info, std::optional four_digit_token); void OnReceivedIntroduction( - ShareTarget share_target, std::optional four_digit_token, + int64_t share_target_id, std::optional four_digit_token, std::optional frame); - void ReceiveConnectionResponse(ShareTarget share_target); + void ReceiveConnectionResponse(ShareTargetInfo& info); void OnReceiveConnectionResponse( - ShareTarget share_target, + int64_t share_target_id, std::optional frame); - void OnStorageCheckCompleted(ShareTarget share_target, + void OnStorageCheckCompleted(int64_t share_target_id, std::optional four_digit_token, bool is_out_of_storage); void OnFrameRead( - ShareTarget share_target, + int64_t share_target_id, std::optional frame); void HandleProgressUpdateFrame( - const ShareTarget& share_target, + int64_t share_target_id, const nearby::sharing::service::proto::ProgressUpdateFrame& progress_update_frame); @@ -419,8 +421,8 @@ class NearbySharingServiceImpl void OnPayloadTransferUpdate(int64_t share_target_id, TransferMetadata metadata); - bool OnIncomingPayloadsComplete(ShareTarget& share_target); - void RemoveIncomingPayloads(ShareTarget share_target); + bool OnIncomingPayloadsComplete(int64_t share_target_id); + void RemoveIncomingPayloads(const IncomingShareTargetInfo& share_target_info); void Disconnect(int64_t share_target_id, TransferMetadata metadata); void OnDisconnectingConnectionTimeout(absl::string_view endpoint_id); @@ -485,7 +487,7 @@ class NearbySharingServiceImpl absl::AnyInvocable task); // Returns a 1-based position.It is used by group share feature. - int GetConnectedShareTargetPos(const ShareTarget& target); + int GetConnectedShareTargetPos(); // Returns the share target count. It is used by group share feature. int GetConnectedShareTargetCount(); @@ -497,7 +499,7 @@ class NearbySharingServiceImpl TransportType GetTransportType(const AttachmentContainer& container) const; // Update file path for the file attachment. - void UpdateFilePath(ShareTarget& share_target); + void UpdateFilePath(AttachmentContainer& container); // Returns true if Shutdown() has been called. bool IsShuttingDown(); @@ -554,10 +556,10 @@ class NearbySharingServiceImpl // Registers the most recent TransferMetadata and ShareTarget used for // transitioning notifications between foreground surfaces and background // surfaces. Empty if no metadata is available. - std::optional> + std::optional> last_incoming_metadata_; // The most recent outgoing TransferMetadata and ShareTarget. - std::optional> + std::optional> last_outgoing_metadata_; // A map of ShareTarget id to IncomingShareTargetInfo. This lets us know which // Nearby Connections endpoint and public certificate are related to the diff --git a/sharing/nearby_sharing_service_impl_test.cc b/sharing/nearby_sharing_service_impl_test.cc index 3654c158..2ef39d8e 100644 --- a/sharing/nearby_sharing_service_impl_test.cc +++ b/sharing/nearby_sharing_service_impl_test.cc @@ -64,6 +64,7 @@ #include "sharing/fast_initiation/nearby_fast_initiation_impl.h" #include "sharing/file_attachment.h" #include "sharing/flags/generated/nearby_sharing_feature_flags.h" +#include "sharing/incoming_share_target_info.h" #include "sharing/internal/api/mock_app_info.h" #include "sharing/internal/api/mock_sharing_platform.h" #include "sharing/internal/api/preference_manager.h" @@ -1715,7 +1716,6 @@ TEST_F(NearbySharingServiceImplTest, .WillOnce([&](ShareTarget share_target) { EXPECT_FALSE(share_target.is_incoming); EXPECT_TRUE(share_target.is_known); - EXPECT_FALSE(share_target.attachment_container.HasAttachments()); EXPECT_EQ(share_target.device_name, kDeviceName); EXPECT_EQ(share_target.type, kDeviceType); EXPECT_TRUE(share_target.device_id); @@ -1769,7 +1769,6 @@ TEST_F(NearbySharingServiceImplTest, RegisterSendSurfaceEmptyCertificate) { .WillOnce([](ShareTarget share_target) { EXPECT_FALSE(share_target.is_incoming); EXPECT_FALSE(share_target.is_known); - EXPECT_FALSE(share_target.attachment_container.HasAttachments()); EXPECT_EQ(share_target.device_name, kDeviceName); EXPECT_FALSE(share_target.image_url); EXPECT_EQ(share_target.type, kDeviceType); @@ -4490,17 +4489,16 @@ TEST_F(NearbySharingServiceImplTest, RetryDiscoveredEndpointsDownloadLimit) { TEST_F(NearbySharingServiceImplTest, OpenSharedTarget) { ShareTarget share_target; - share_target.attachment_container.AddTextAttachment( + auto container = std::make_unique(); + container->AddTextAttachment( TextAttachment(TextMetadata::TEXT, "body", "title", "mime")); NearbySharingService::StatusCodes result; absl::Notification notification; - service_->Open( - share_target, - std::make_unique(share_target.attachment_container), - [&](NearbySharingService::StatusCodes status_code) { - result = status_code; - notification.Notify(); - }); + service_->Open(share_target, std::move(container), + [&](NearbySharingService::StatusCodes status_code) { + result = status_code; + notification.Notify(); + }); ASSERT_TRUE(notification.WaitForNotificationWithTimeout(kWaitTimeout)); EXPECT_EQ(result, NearbySharingService::StatusCodes::kOk); @@ -5040,7 +5038,10 @@ TEST_F(NearbySharingServiceImplTest, RemoveIncomingPayloads) { UnorderedElementsAre("test1.txt", "test2.txt")); ShareTarget share_target; share_target.is_incoming = true; - service_->RemoveIncomingPayloads(share_target); + IncomingShareTargetInfo share_target_info( + "endpoint_id", share_target, + [](const IncomingShareTargetInfo&, const TransferMetadata&) {}); + service_->RemoveIncomingPayloads(share_target_info); EXPECT_EQ( fake_nearby_connections_manager_->GetUnknownFilePathsToDeleteForTesting() .size(), diff --git a/sharing/outgoing_share_target_info.cc b/sharing/outgoing_share_target_info.cc index a0a33584..3463d65f 100644 --- a/sharing/outgoing_share_target_info.cc +++ b/sharing/outgoing_share_target_info.cc @@ -30,10 +30,10 @@ namespace sharing { OutgoingShareTargetInfo::OutgoingShareTargetInfo( std::string endpoint_id, const ShareTarget& share_target, - std::function + std::function transfer_update_callback) - : ShareTargetInfo(std::move(endpoint_id), share_target, - std::move(transfer_update_callback)) {} + : ShareTargetInfo(std::move(endpoint_id), share_target), + transfer_update_callback_(std::move(transfer_update_callback)) {} OutgoingShareTargetInfo::OutgoingShareTargetInfo(OutgoingShareTargetInfo&&) = default; @@ -43,6 +43,11 @@ OutgoingShareTargetInfo& OutgoingShareTargetInfo::operator=( OutgoingShareTargetInfo::~OutgoingShareTargetInfo() = default; +void OutgoingShareTargetInfo::InvokeTransferUpdateCallback( + const TransferMetadata& metadata) { + transfer_update_callback_(*this, metadata); +} + std::vector OutgoingShareTargetInfo::ExtractTextPayloads() { return std::move(text_payloads_); } diff --git a/sharing/outgoing_share_target_info.h b/sharing/outgoing_share_target_info.h index 3690f43d..2357faa3 100644 --- a/sharing/outgoing_share_target_info.h +++ b/sharing/outgoing_share_target_info.h @@ -32,10 +32,11 @@ namespace sharing { // A description of the outgoing connection to a remote device. class OutgoingShareTargetInfo : public ShareTargetInfo { public: - OutgoingShareTargetInfo( - std::string endpoint_id, const ShareTarget& share_target, - std::function - transfer_update_callback); + OutgoingShareTargetInfo(std::string endpoint_id, + const ShareTarget& share_target, + std::function + transfer_update_callback); OutgoingShareTargetInfo(OutgoingShareTargetInfo&&); OutgoingShareTargetInfo& operator=(OutgoingShareTargetInfo&&); ~OutgoingShareTargetInfo() override; @@ -81,12 +82,17 @@ class OutgoingShareTargetInfo : public ShareTargetInfo { std::vector ExtractWifiCredentialsPayloads(); std::optional ExtractNextPayload(); + protected: + void InvokeTransferUpdateCallback(const TransferMetadata& metadata) override; + private: std::optional obfuscated_gaia_id_; std::vector text_payloads_; std::vector file_payloads_; std::vector wifi_credentials_payloads_; Status connection_layer_status_; + std::function + transfer_update_callback_; }; } // namespace sharing diff --git a/sharing/share_target.cc b/sharing/share_target.cc index 0612580d..e0f7be5f 100644 --- a/sharing/share_target.cc +++ b/sharing/share_target.cc @@ -16,7 +16,6 @@ #include #include -#include #include #include #include @@ -26,11 +25,7 @@ #include "absl/strings/str_format.h" #include "absl/strings/str_join.h" #include "internal/network/url.h" -#include "sharing/attachment.h" #include "sharing/common/nearby_share_enums.h" -#include "sharing/file_attachment.h" -#include "sharing/text_attachment.h" -#include "sharing/wifi_credentials_attachment.h" namespace nearby { namespace sharing { @@ -45,17 +40,11 @@ ShareTarget::ShareTarget() { id = ++kLastGeneratedId; } ShareTarget::ShareTarget( std::string device_name, Url image_url, ShareTargetType type, - std::vector text_attachments, - std::vector file_attachments, - std::vector wifi_credentials_attachments, bool is_incoming, std::optional full_name, bool is_known, std::optional device_id, bool for_self_share) : device_name(std::move(device_name)), image_url(std::move(image_url)), type(type), - attachment_container(std::move(text_attachments), - std::move(file_attachments), - std::move(wifi_credentials_attachments)), is_incoming(is_incoming), full_name(std::move(full_name)), is_known(is_known), @@ -74,27 +63,6 @@ ShareTarget& ShareTarget::operator=(ShareTarget&&) = default; ShareTarget::~ShareTarget() = default; -std::vector ShareTarget::GetAttachmentIds() const { - std::vector attachment_ids; - - attachment_ids.reserve(attachment_container.GetAttachmentCount()); - for (const auto& file : attachment_container.GetFileAttachments()) - attachment_ids.push_back(file.id()); - - for (const auto& text : attachment_container.GetTextAttachments()) - attachment_ids.push_back(text.id()); - - for (const auto& wifi_credentials : - attachment_container.GetWifiCredentialsAttachments()) - attachment_ids.push_back(wifi_credentials.id()); - - return attachment_ids; -} - -int64_t ShareTarget::GetTotalAttachmentsSize() const { - return attachment_container.GetTotalAttachmentsSize(); -} - std::string ShareTarget::ToString() const { std::vector fmt; @@ -109,15 +77,6 @@ std::string ShareTarget::ToString() const { if (device_id) { fmt.push_back(absl::StrFormat("device_id: %s", *device_id)); } - fmt.push_back( - absl::StrFormat("file_attachments_size: %d", - attachment_container.GetFileAttachments().size())); - fmt.push_back( - absl::StrFormat("text_attachments_size: %d", - attachment_container.GetTextAttachments().size())); - fmt.push_back(absl::StrFormat( - "wifi_credentials_attachments_size: %d", - attachment_container.GetWifiCredentialsAttachments().size())); fmt.push_back(absl::StrFormat("is_known: %d", is_known)); fmt.push_back(absl::StrFormat("is_incoming: %d", is_incoming)); fmt.push_back(absl::StrFormat("for_self_share: %d", for_self_share)); diff --git a/sharing/share_target.h b/sharing/share_target.h index c81475f9..cca6b76e 100644 --- a/sharing/share_target.h +++ b/sharing/share_target.h @@ -18,14 +18,9 @@ #include #include #include -#include #include "internal/network/url.h" -#include "sharing/attachment_container.h" #include "sharing/common/nearby_share_enums.h" -#include "sharing/file_attachment.h" -#include "sharing/text_attachment.h" -#include "sharing/wifi_credentials_attachment.h" namespace nearby { namespace sharing { @@ -36,9 +31,7 @@ struct ShareTarget { ShareTarget(); ShareTarget( std::string device_name, ::nearby::network::Url image_url, - ShareTargetType type, std::vector text_attachments, - std::vector file_attachments, - std::vector wifi_credentials_attachments, + ShareTargetType type, bool is_incoming, std::optional full_name, bool is_known, std::optional device_id, bool for_self_share); ShareTarget(const ShareTarget&); @@ -47,8 +40,6 @@ struct ShareTarget { ShareTarget& operator=(ShareTarget&&); ~ShareTarget(); - std::vector GetAttachmentIds() const; - int64_t GetTotalAttachmentsSize() const; std::string ToString() const; int64_t id; @@ -56,7 +47,6 @@ struct ShareTarget { // Uri that points to an image of the ShareTarget, if one exists. std::optional<::nearby::network::Url> image_url; ShareTargetType type = ShareTargetType::kUnknown; - AttachmentContainer attachment_container; bool is_incoming = false; std::optional full_name; // True if the local device has the PublicCertificate this target is diff --git a/sharing/share_target_info.cc b/sharing/share_target_info.cc index 7276640b..6d9eeee9 100644 --- a/sharing/share_target_info.cc +++ b/sharing/share_target_info.cc @@ -14,7 +14,6 @@ #include "sharing/share_target_info.h" -#include #include #include @@ -27,13 +26,10 @@ namespace nearby { namespace sharing { ShareTargetInfo::ShareTargetInfo( - std::string endpoint_id, const ShareTarget& share_target, - std::function - transfer_update_callback) + std::string endpoint_id, const ShareTarget& share_target) : endpoint_id_(std::move(endpoint_id)), self_share_(share_target.for_self_share), - share_target_(share_target), - transfer_update_callback_(std::move(transfer_update_callback)) {} + share_target_(share_target) {} ShareTargetInfo::ShareTargetInfo(ShareTargetInfo&&) = default; @@ -41,28 +37,20 @@ ShareTargetInfo& ShareTargetInfo::operator=(ShareTargetInfo&&) = default; ShareTargetInfo::~ShareTargetInfo() = default; -void ShareTargetInfo::set_share_target(const ShareTarget& share_target) { - NL_DCHECK(share_target.id == share_target_.id); - NL_DCHECK(share_target.for_self_share == share_target_.for_self_share); - share_target_ = share_target; -} - void ShareTargetInfo::UpdateTransferMetadata( const TransferMetadata& transfer_metadata) { - if (transfer_update_callback_) { - if (got_final_status_) { - // If we already got a final status, we can ignore any subsequent final - // statuses caused by race conditions. - NL_VLOG(1) - << __func__ << ": Transfer update decorator swallowed " - << "status update because a final status was already received: " - << share_target_.id << ": " - << TransferMetadata::StatusToString(transfer_metadata.status()); - return; - } - got_final_status_ = transfer_metadata.is_final_status(); - transfer_update_callback_(share_target_, transfer_metadata); + if (got_final_status_) { + // If we already got a final status, we can ignore any subsequent final + // statuses caused by race conditions. + NL_VLOG(1) + << __func__ << ": Transfer update decorator swallowed " + << "status update because a final status was already received: " + << share_target_.id << ": " + << TransferMetadata::StatusToString(transfer_metadata.status()); + return; } + got_final_status_ = transfer_metadata.is_final_status(); + InvokeTransferUpdateCallback(transfer_metadata); } void ShareTargetInfo::set_disconnect_status( diff --git a/sharing/share_target_info.h b/sharing/share_target_info.h index 22624ecc..84ce4529 100644 --- a/sharing/share_target_info.h +++ b/sharing/share_target_info.h @@ -16,7 +16,6 @@ #define THIRD_PARTY_NEARBY_SHARING_SHARE_TARGET_INFO_H_ #include -#include #include #include #include @@ -24,6 +23,7 @@ #include "absl/time/time.h" #include "proto/sharing_enums.pb.h" +#include "sharing/attachment_container.h" #include "sharing/certificates/nearby_share_decrypted_public_certificate.h" #include "sharing/incoming_frames_reader.h" #include "sharing/nearby_connection.h" @@ -40,9 +40,7 @@ namespace sharing { class ShareTargetInfo { public: ShareTargetInfo( - std::string endpoint_id, const ShareTarget& share_target, - std::function - transfer_update_callback); + std::string endpoint_id, const ShareTarget& share_target); ShareTargetInfo(ShareTargetInfo&&); ShareTargetInfo& operator=(ShareTargetInfo&&); virtual ~ShareTargetInfo(); @@ -87,7 +85,7 @@ class ShareTargetInfo { } std::weak_ptr - payload_tracker() { + payload_tracker() const { return payload_tracker_->GetWeakPtr(); } @@ -95,11 +93,11 @@ class ShareTargetInfo { payload_tracker_ = std::move(payload_tracker); } - int64_t session_id() { return session_id_; } + int64_t session_id() const { return session_id_; } void set_session_id(int64_t session_id) { session_id_ = session_id; } - std::optional connection_start_time() { + std::optional connection_start_time() const { return connection_start_time_; } @@ -108,7 +106,9 @@ class ShareTargetInfo { connection_start_time_ = connection_start_time; } - ::location::nearby::proto::sharing::OSType os_type() { return os_type_; } + ::location::nearby::proto::sharing::OSType os_type() const { + return os_type_; + } void set_os_type(::location::nearby::proto::sharing::OSType os_type) { os_type_ = os_type; @@ -116,9 +116,7 @@ class ShareTargetInfo { bool self_share() const { return self_share_; } - void set_share_target(const ShareTarget& share_target); - - ShareTarget share_target() const { return share_target_; } + const ShareTarget& share_target() const { return share_target_; } // Sets the status to send in the TransferMetadataUpdate on connection // disconnect. If |status| is kUnknown, then no TransferMetadataUpdate will be @@ -130,6 +128,20 @@ class ShareTargetInfo { } void OnDisconnect(); + void SetAttachmentContainer(AttachmentContainer container) { + attachment_container_ = std::move(container); + } + const AttachmentContainer& attachment_container() const { + return attachment_container_; + } + + AttachmentContainer& mutable_attachment_container() { + return attachment_container_; + } + + protected: + virtual void InvokeTransferUpdateCallback( + const TransferMetadata& metadata) = 0; private: std::string endpoint_id_; @@ -146,12 +158,11 @@ class ShareTargetInfo { bool self_share_ = false; ShareTarget share_target_; bool got_final_status_ = false; - std::function - transfer_update_callback_; // The status sent in the TransferMetadataUpdate on connection disconnect. // If status is kUnknown, then no TransferMetadataUpdate will be sent. TransferMetadata::Status disconnect_status_ = TransferMetadata::Status::kUnknown; + AttachmentContainer attachment_container_; }; } // namespace sharing diff --git a/sharing/share_target_info_test.cc b/sharing/share_target_info_test.cc index a2883453..920f6a74 100644 --- a/sharing/share_target_info_test.cc +++ b/sharing/share_target_info_test.cc @@ -14,12 +14,10 @@ #include "sharing/share_target_info.h" -#include +#include #include #include -#include "gmock/gmock.h" -#include "protobuf-matchers/protocol-buffer-matchers.h" #include "gtest/gtest.h" #include "absl/strings/string_view.h" #include "sharing/share_target.h" @@ -28,38 +26,42 @@ namespace nearby::sharing { namespace { -using testing::_; -using testing::Eq; -using testing::Invoke; -using testing::IsTrue; -using testing::MockFunction; constexpr absl::string_view kEndpointId = "12345"; +// A test class which makes ShareTargetInfo testable since the class is +// abstract. class TestShareTargetInfo : public ShareTargetInfo { public: TestShareTargetInfo( - std::string endpoint_id, const ShareTarget& share_target, - std::function - transfer_update_callback) - : ShareTargetInfo(std::move(endpoint_id), share_target, - std::move(transfer_update_callback)), + std::string endpoint_id, const ShareTarget& share_target) + : ShareTargetInfo(std::move(endpoint_id), share_target), is_incoming_(share_target.is_incoming) {} bool IsIncoming() const override { return is_incoming_; } + int TransferUpdateCount() { return transfer_update_count_; } + + std::optional LastTransferMetadata() { + return last_transfer_metadata_; + } + + protected: + void InvokeTransferUpdateCallback( + const TransferMetadata& metadata) override { + ++transfer_update_count_; + last_transfer_metadata_ = metadata; + } + private: const bool is_incoming_; + int transfer_update_count_ = 0; + std::optional last_transfer_metadata_; }; TEST(ShareTargetInfoTest, UpdateTransferMetadata) { - MockFunction - update_callback; ShareTarget share_target; - TestShareTargetInfo info(std::string(kEndpointId), share_target, - update_callback.AsStdFunction()); - - EXPECT_CALL(update_callback, Call(_, _)).Times(2); + TestShareTargetInfo info(std::string(kEndpointId), share_target); info.UpdateTransferMetadata( TransferMetadataBuilder() @@ -69,16 +71,13 @@ TEST(ShareTargetInfoTest, UpdateTransferMetadata) { TransferMetadataBuilder() .set_status(TransferMetadata::Status::kInProgress) .build()); + + EXPECT_EQ(info.TransferUpdateCount(), 2); } TEST(ShareTargetInfoTest, UpdateTransferMetadataAfterFinalStatus) { - MockFunction - update_callback; ShareTarget share_target; - TestShareTargetInfo info(std::string(kEndpointId), share_target, - update_callback.AsStdFunction()); - - EXPECT_CALL(update_callback, Call(_, _)); + TestShareTargetInfo info(std::string(kEndpointId), share_target); info.UpdateTransferMetadata( TransferMetadataBuilder() @@ -88,36 +87,31 @@ TEST(ShareTargetInfoTest, UpdateTransferMetadataAfterFinalStatus) { TransferMetadataBuilder() .set_status(TransferMetadata::Status::kInProgress) .build()); + + EXPECT_EQ(info.TransferUpdateCount(), 1); } TEST(ShareTargetInfoTest, SetDisconnectStatus) { - MockFunction - update_callback; ShareTarget share_target; - TestShareTargetInfo info(std::string(kEndpointId), share_target, - update_callback.AsStdFunction()); + TestShareTargetInfo info(std::string(kEndpointId), share_target); info.set_disconnect_status(TransferMetadata::Status::kCancelled); EXPECT_EQ(info.disconnect_status(), TransferMetadata::Status::kCancelled); } TEST(ShareTargetInfoTest, OnDisconnect) { - MockFunction - update_callback; ShareTarget share_target; - TestShareTargetInfo info(std::string(kEndpointId), share_target, - update_callback.AsStdFunction()); + TestShareTargetInfo info(std::string(kEndpointId), share_target); info.set_disconnect_status(TransferMetadata::Status::kCancelled); EXPECT_EQ(info.disconnect_status(), TransferMetadata::Status::kCancelled); - EXPECT_CALL(update_callback, Call(_, _)) - .WillOnce(Invoke([](const ShareTarget& share_target, - const TransferMetadata& transfer_metadata) { - EXPECT_THAT(transfer_metadata.status(), - Eq(TransferMetadata::Status::kCancelled)); - EXPECT_THAT(transfer_metadata.is_final_status(), IsTrue()); - })); info.OnDisconnect(); + + EXPECT_EQ(info.TransferUpdateCount(), 1); + ASSERT_TRUE(info.LastTransferMetadata().has_value()); + EXPECT_EQ(info.LastTransferMetadata()->status(), + TransferMetadata::Status::kCancelled); + EXPECT_TRUE(info.LastTransferMetadata()->is_final_status()); } } // namespace diff --git a/sharing/share_target_test.cc b/sharing/share_target_test.cc index e20e7950..c13bb19b 100644 --- a/sharing/share_target_test.cc +++ b/sharing/share_target_test.cc @@ -20,9 +20,6 @@ #include "gtest/gtest.h" #include "internal/network/url.h" #include "sharing/common/nearby_share_enums.h" -#include "sharing/file_attachment.h" -#include "sharing/text_attachment.h" -#include "sharing/wifi_credentials_attachment.h" namespace nearby { namespace sharing { @@ -38,9 +35,6 @@ std::vector GetTestData() { ShareTarget share_target2{"test_name", ::nearby::network::Url(), ShareTargetType::kPhone, - std::vector(), - std::vector(), - std::vector(), /* is_incoming */ true, "test_full_name", /* is_known */ false, @@ -53,16 +47,12 @@ std::vector GetTestData() { ShareTargetToStringTestData>* kShareTargetToStringTestData = new std::vector({ {share_target1, - "ShareTarget"}, {share_target2, "ShareTarget"}, + "is_known: 0, is_incoming: 1, for_self_share: 1, vendor_id: 0>"}, }); return *kShareTargetToStringTestData;