From 1f6ee5849d3d73a94330c19ba3c6f19ed601a0a1 Mon Sep 17 00:00:00 2001 From: Francis Tsui Date: Fri, 20 Dec 2024 12:59:11 -0800 Subject: [PATCH] Deprecate enable_transfer_cancellation_optimization flag. PiperOrigin-RevId: 708404548 --- .../generated/nearby_sharing_feature_flags.h | 8 +-- sharing/nearby_sharing_service_impl.cc | 17 ++--- sharing/nearby_sharing_service_impl_test.cc | 4 -- sharing/outgoing_share_session.cc | 41 +---------- sharing/outgoing_share_session.h | 13 ---- sharing/outgoing_share_session_test.cc | 71 +------------------ 6 files changed, 11 insertions(+), 143 deletions(-) diff --git a/sharing/flags/generated/nearby_sharing_feature_flags.h b/sharing/flags/generated/nearby_sharing_feature_flags.h index 426056d0..64aed3b5 100755 --- a/sharing/flags/generated/nearby_sharing_feature_flags.h +++ b/sharing/flags/generated/nearby_sharing_feature_flags.h @@ -53,9 +53,6 @@ constexpr auto kEnableSelfShareUi = // Enable/disable sending desktop events constexpr auto kEnableSendingDesktopEvents = flags::Flag(kConfigPackage, "45459748", false); -// Enable/disable optimization for transfer cancellation. -constexpr auto kEnableTransferCancellationOptimization = - flags::Flag(kConfigPackage, "45429881", false); // Disable/enable the WebRTC medium in Nearby Sharing SDK. constexpr auto kEnableWebrtcMedium = flags::Flag(kConfigPackage, "45411620", false); @@ -88,8 +85,8 @@ constexpr auto kDiscoveryCacheLostExpiryMs = // When true, honor 3P client_id & client_secret in the gRPC request constexpr auto kHonor3PClientIdAndSecret = flags::Flag(kConfigPackage, "45665616", false); -// When UnregisterShareTarget, the time in milliseconds a cached entry can be in -// LOST state. +// The amount of time in milliseconds a share target stays in discovery cache in +// receive disabled state after a transfer. constexpr auto kUnregisterTargetDiscoveryCacheLostExpiryMs = flags::Flag(kConfigPackage, "45663103", 10000); // Enable/disable QR Code UI @@ -120,7 +117,6 @@ inline absl::btree_map&> GetBoolFlags() { {45411589, kEnableRetryResumeTransfer}, {45418908, kEnableSelfShareUi}, {45459748, kEnableSendingDesktopEvents}, - {45429881, kEnableTransferCancellationOptimization}, {45411620, kEnableWebrtcMedium}, {45411353, kSenderSkipsConfirmation}, {45409033, kShowAutoUpdateSetting}, diff --git a/sharing/nearby_sharing_service_impl.cc b/sharing/nearby_sharing_service_impl.cc index 6ef2d10e..9fb44791 100644 --- a/sharing/nearby_sharing_service_impl.cc +++ b/sharing/nearby_sharing_service_impl.cc @@ -2639,15 +2639,11 @@ void NearbySharingServiceImpl::OnOutgoingTransferUpdate( } // 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()) { - session.SendNextPayload(); - } + 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()) { + session.SendNextPayload(); } if (has_foreground_send_surface && metadata.is_final_status()) { @@ -2848,9 +2844,6 @@ void NearbySharingServiceImpl::OnReceiveConnectionResponse( return; } session->SendPayloads( - NearbyFlags::GetInstance().GetBoolFlag( - config_package_nearby::nearby_sharing_feature:: - kEnableTransferCancellationOptimization), [this, share_target_id]( std::optional frame) { OnFrameRead(share_target_id, std::move(frame)); diff --git a/sharing/nearby_sharing_service_impl_test.cc b/sharing/nearby_sharing_service_impl_test.cc index c0203e7c..56527ec4 100644 --- a/sharing/nearby_sharing_service_impl_test.cc +++ b/sharing/nearby_sharing_service_impl_test.cc @@ -3672,10 +3672,6 @@ TEST_F(NearbySharingServiceImplTest, SendFilesSuccess) { } TEST_F(NearbySharingServiceImplTest, SendWifiCredentialsSuccess) { - NearbyFlags::GetInstance().OverrideBoolFlagValue( - config_package_nearby::nearby_sharing_feature:: - kEnableTransferCancellationOptimization, - true); MockTransferUpdateCallback transfer_callback; MockShareTargetDiscoveredCallback discovery_callback; int64_t target_id = diff --git a/sharing/outgoing_share_session.cc b/sharing/outgoing_share_session.cc index 77ea8b5e..401b24b3 100644 --- a/sharing/outgoing_share_session.cc +++ b/sharing/outgoing_share_session.cc @@ -324,7 +324,6 @@ bool OutgoingShareSession::AcceptTransfer( } void OutgoingShareSession::SendPayloads( - bool enable_transfer_cancellation_optimization, std::function< void(std::optional frame)> frame_read_callback, @@ -341,34 +340,8 @@ void OutgoingShareSession::SendPayloads( /*transfer_position=*/1, /*concurrent_connections=*/1); VLOG(1) << "The connection was accepted. Payloads are now being sent."; - if (enable_transfer_cancellation_optimization) { - InitSendPayload(std::move(payload_transder_update_callback)); - SendNextPayload(); - } else { - SendAllPayloads(std::move(payload_transder_update_callback)); - } -} - -void OutgoingShareSession::SendAllPayloads( - std::function payload_transder_update_callback) { - InitializePayloadTracker(std::move(payload_transder_update_callback)); - for (auto& payload : ExtractTextPayloads()) { - connections_manager().Send( - endpoint_id(), std::make_unique(payload), payload_tracker()); - } - for (auto& payload : ExtractFilePayloads()) { - connections_manager().Send( - endpoint_id(), std::make_unique(payload), payload_tracker()); - } - for (auto& payload : ExtractWifiCredentialsPayloads()) { - connections_manager().Send( - endpoint_id(), std::make_unique(payload), payload_tracker()); - } -} - -void OutgoingShareSession::InitSendPayload( - std::function payload_transder_update_callback) { InitializePayloadTracker(std::move(payload_transder_update_callback)); + SendNextPayload(); } void OutgoingShareSession::SendNextPayload() { @@ -472,18 +445,6 @@ OutgoingShareSession::HandleConnectionResponse( return TransferMetadata::Status::kFailed; } -std::vector OutgoingShareSession::ExtractTextPayloads() { - return std::move(text_payloads_); -} - -std::vector OutgoingShareSession::ExtractFilePayloads() { - return std::move(file_payloads_); -} - -std::vector OutgoingShareSession::ExtractWifiCredentialsPayloads() { - return std::move(wifi_credentials_payloads_); -} - std::optional OutgoingShareSession::ExtractNextPayload() { if (!text_payloads_.empty()) { Payload payload = text_payloads_.back(); diff --git a/sharing/outgoing_share_session.h b/sharing/outgoing_share_session.h index b8d58feb..6d5157e9 100644 --- a/sharing/outgoing_share_session.h +++ b/sharing/outgoing_share_session.h @@ -119,7 +119,6 @@ class OutgoingShareSession : public ShareSession { // `payload_transder_update_callback`. // Any other frames received will be passed to `frame_read_callback`. void SendPayloads( - bool enable_transfer_cancellation_optimization, std::function< void(std::optional frame)> frame_read_callback, @@ -166,18 +165,6 @@ class OutgoingShareSession : public ShareSession { // Calculates transport type based on attachment size. TransportType GetTransportType(bool disable_wifi_hotspot) const; - // Create a payload status listener to send status change to - // `payload_transder_update_callback`. - // Send all payloads to NearbyConnectionManager. - void SendAllPayloads(std::function payload_transder_update_callback); - - // Create a payload status listener to send status change to - // `payload_transder_update_callback`. - void InitSendPayload(std::function payload_transder_update_callback); - - std::vector ExtractTextPayloads(); - std::vector ExtractFilePayloads(); - std::vector ExtractWifiCredentialsPayloads(); std::optional ExtractNextPayload(); bool FillIntroductionFrame( nearby::sharing::service::proto::IntroductionFrame* introduction) const; diff --git a/sharing/outgoing_share_session_test.cc b/sharing/outgoing_share_session_test.cc index b787e31b..07069331 100644 --- a/sharing/outgoing_share_session_test.cc +++ b/sharing/outgoing_share_session_test.cc @@ -676,68 +676,7 @@ TEST_F(OutgoingShareSessionTest, HandleConnectionResponseAcceptResponse) { ASSERT_THAT(status.has_value(), IsFalse()); } -TEST_F(OutgoingShareSessionTest, SendPayloadsDisableCancellationOptimization) { - InitSendAttachments(CreateDefaultAttachmentContainer()); - session_.set_session_id(1234); - std::vector file_infos; - file_infos.push_back({ - .size = 12355L, - .file_path = file1_.file_path().value(), - }); - session_.CreateFilePayloads(file_infos); - session_.CreateTextPayloads(); - session_.CreateWifiCredentialsPayloads(); - MockFunction payload_transder_update_callback; - StrictMock, - std::weak_ptr)>> - send_payload_callback; - connections_manager_.set_send_payload_callback( - send_payload_callback.AsStdFunction()); - EXPECT_CALL(send_payload_callback, Call(_, _)) - .WillOnce(Invoke( - [this]( - std::unique_ptr payload, - std::weak_ptr) { - payload->id = session_.attachment_payload_map().at(file1_.id()); - })) - .WillOnce(Invoke( - [this]( - std::unique_ptr payload, - std::weak_ptr) { - payload->id = session_.attachment_payload_map().at(text1_.id()); - })) - .WillOnce(Invoke( - [this]( - std::unique_ptr payload, - std::weak_ptr) { - payload->id = session_.attachment_payload_map().at(text2_.id()); - })) - .WillOnce(Invoke( - [this]( - std::unique_ptr payload, - std::weak_ptr) { - payload->id = session_.attachment_payload_map().at(wifi1_.id()); - })); - EXPECT_CALL(mock_event_logger_, - Log(Matcher( - AllOf((HasCategory(EventCategory::SENDING_EVENT), - HasEventType(EventType::SEND_ATTACHMENTS_START), - Property(&SharingLog::send_attachments_start, - HasSessionId(1234))))))); - NearbyConnectionImpl connection(device_info_); - ConnectionSuccess(&connection); - - session_.SendPayloads( - /*enable_transfer_cancellation_optimization=*/ - false, [](std::optional frame) {}, - payload_transder_update_callback.AsStdFunction()); - - auto payload_listener = session_.payload_tracker().lock(); - EXPECT_THAT(payload_listener, IsTrue()); -} - -TEST_F(OutgoingShareSessionTest, SendPayloadsEnableCancellationOptimization) { +TEST_F(OutgoingShareSessionTest, SendPayloads) { InitSendAttachments(CreateDefaultAttachmentContainer()); session_.set_session_id(1234); std::vector file_infos; @@ -771,9 +710,7 @@ TEST_F(OutgoingShareSessionTest, SendPayloadsEnableCancellationOptimization) { NearbyConnectionImpl connection(device_info_); ConnectionSuccess(&connection); - session_.SendPayloads( - /*enable_transfer_cancellation_optimization=*/ - true, [](std::optional frame) {}, + session_.SendPayloads([](std::optional frame) {}, payload_transder_update_callback.AsStdFunction()); auto payload_listener = session_.payload_tracker().lock(); @@ -815,9 +752,7 @@ TEST_F(OutgoingShareSessionTest, SendNextPayload) { NearbyConnectionImpl connection(device_info_); ConnectionSuccess(&connection); - session_.SendPayloads( - /*enable_transfer_cancellation_optimization=*/ - true, [](std::optional frame) {}, + session_.SendPayloads([](std::optional frame) {}, payload_transder_update_callback.AsStdFunction()); EXPECT_CALL(send_payload_callback, Call(_, _))