diff --git a/sharing/flags/generated/nearby_sharing_feature_flags.h b/sharing/flags/generated/nearby_sharing_feature_flags.h index 50182240..be8ea523 100755 --- a/sharing/flags/generated/nearby_sharing_feature_flags.h +++ b/sharing/flags/generated/nearby_sharing_feature_flags.h @@ -76,6 +76,8 @@ constexpr auto kUseGrpcClient = // When true, dedup discovered endpoints. constexpr auto kApplyEndpointsDedup = flags::Flag(kConfigPackage, "45656298", false); +constexpr auto kDedupInUnregisterShareTarget = + flags::Flag(kConfigPackage, "45664277", false); // When true, delete the file payload which received unexpectedly. constexpr auto kDeleteUnexpectedReceivedFileFix = flags::Flag(kConfigPackage, "45657036", false); @@ -97,6 +99,10 @@ constexpr auto kEnableConflictBanner = // Enable a persistent BETA label. constexpr auto kEnableMacosBetaLabel = flags::Flag(kConfigPackage, "45662570", true); +// When UnregisterShareTarget, the time in milliseconds a cached entry can be in +// LOST state. +constexpr auto kUnregisterTargetDiscoveryCacheLostExpiryMs = + flags::Flag(kConfigPackage, "45663103", 10000); inline absl::btree_map&> GetBoolFlags() { return { @@ -113,6 +119,7 @@ inline absl::btree_map&> GetBoolFlags() { {45409033, kShowAutoUpdateSetting}, {45630055, kUseGrpcClient}, {45656298, kApplyEndpointsDedup}, + {45664277, kDedupInUnregisterShareTarget}, {45657036, kDeleteUnexpectedReceivedFileFix}, {45417647, kEnableQrCodeUi}, {45410558, kShowAdminModeWarning}, @@ -126,6 +133,7 @@ inline absl::btree_map&> GetInt64Flags() { {45632386, kDelayEndpointLossMs}, {45401358, kLoggingLevel}, {45658774, kDiscoveryCacheLostExpiryMs}, + {45663103, kUnregisterTargetDiscoveryCacheLostExpiryMs}, }; } diff --git a/sharing/nearby_sharing_service_impl.cc b/sharing/nearby_sharing_service_impl.cc index 344b8d68..b0f5964e 100644 --- a/sharing/nearby_sharing_service_impl.cc +++ b/sharing/nearby_sharing_service_impl.cc @@ -39,6 +39,7 @@ #include "absl/random/random.h" #include "absl/status/statusor.h" #include "absl/strings/escaping.h" +#include "absl/strings/str_cat.h" #include "absl/strings/str_format.h" #include "absl/strings/string_view.h" #include "absl/time/time.h" @@ -1699,7 +1700,10 @@ void NearbySharingServiceImpl::HandleEndpointLost( if (NearbyFlags::GetInstance().GetBoolFlag( config_package_nearby::nearby_sharing_feature:: kApplyEndpointsDedup)) { - MoveToDiscoveryCache(endpoint_id); + MoveToDiscoveryCache(endpoint_id, + NearbyFlags::GetInstance().GetInt64Flag( + config_package_nearby::nearby_sharing_feature:: + kDiscoveryCacheLostExpiryMs)); } else { RemoveOutgoingShareTargetAndReportLost(endpoint_id); } @@ -2149,7 +2153,6 @@ void NearbySharingServiceImpl::StartScanning() { InvalidateReceiveSurfaceState(); ClearOutgoingShareSessionMap(); - discovery_cache_.clear(); discovered_advertisements_to_retry_map_.clear(); discovered_advertisements_retried_set_.clear(); @@ -2192,8 +2195,6 @@ NearbySharingService::StatusCodes NearbySharingServiceImpl::StopScanning() { discovered_advertisements_to_retry_map_.clear(); discovered_advertisements_retried_set_.clear(); - TriggerDiscoveryCacheExpiryTimers(); - // Note: We don't know if we stopped scanning in preparation to send a file, // or we stopped because the user left the page. We'll invalidate after a // short delay. @@ -3274,7 +3275,7 @@ bool NearbySharingServiceImpl::FindDuplicateInOutgoingShareTargets( std::optional NearbySharingServiceImpl::RemoveOutgoingShareTargetWithEndpointId( absl::string_view endpoint_id) { - VLOG(1) << "Outgoing connection to " << endpoint_id + VLOG(1) << __func__ << ":Outgoing connection to " << endpoint_id << " disconnected, cancel disconnection timer"; disconnection_timeout_alarms_.erase(endpoint_id); auto it = outgoing_share_target_map_.find(endpoint_id); @@ -3316,7 +3317,7 @@ void NearbySharingServiceImpl::TriggerDiscoveryCacheExpiryTimers() { } void NearbySharingServiceImpl::MoveToDiscoveryCache( - absl::string_view endpoint_id) { + absl::string_view endpoint_id, uint64_t expiry_ms) { std::optional share_target_opt = RemoveOutgoingShareTargetWithEndpointId(endpoint_id); if (!share_target_opt.has_value()) { @@ -3325,20 +3326,18 @@ void NearbySharingServiceImpl::MoveToDiscoveryCache( DiscoveryCacheEntry cache_entry; cache_entry.share_target = std::move(share_target_opt.value()); cache_entry.expiry_timer = std::make_unique( - *service_thread_, "discovery_cache_timeout", - absl::Milliseconds(NearbyFlags::GetInstance().GetInt64Flag( - config_package_nearby::nearby_sharing_feature:: - kDiscoveryCacheLostExpiryMs)), - [this, endpoint_id = std::string(endpoint_id)]() { + *service_thread_, absl::StrCat("discovery_cache_timeout_", expiry_ms), + absl::Milliseconds(expiry_ms), + [this, expiry_ms, endpoint_id = std::string(endpoint_id)]() { auto it = discovery_cache_.find(endpoint_id); if (it == discovery_cache_.end()) { LOG(WARNING) << "Trying to remove endpoint_id: " << endpoint_id - << " from discovery cache, but cannot find it"; + << " from discovery_cache, but cannot find it"; return; } LOG(INFO) << ": Removing (endpoint_id=" << endpoint_id << ", share_target.id=" << it->second.share_target.id - << ") from discovery cache"; + << ") from discovery_cache after " << expiry_ms << "ms"; ShareTarget share_target = std::move(discovery_cache_.extract(it).mapped().share_target); @@ -3349,7 +3348,8 @@ void NearbySharingServiceImpl::MoveToDiscoveryCache( entry.second.OnShareTargetLost(share_target); } - VLOG(1) << __func__ + VLOG(1) << "discovery_cache entry: " << endpoint_id << " timeout after " + << expiry_ms << "ms" << ": [Dedupped] Reported OnShareTargetLost to all surfaces " "for share_target: " << share_target.ToString(); @@ -3432,7 +3432,7 @@ void NearbySharingServiceImpl::ClearOutgoingShareSessionMap() { } void NearbySharingServiceImpl::UnregisterShareTarget(int64_t share_target_id) { - LOG(INFO) << __func__ << ": Unregistering share target " << share_target_id; + LOG(INFO) << __func__ << ": Unregister share target " << share_target_id; // For metrics. all_cancelled_share_target_ids_.erase(share_target_id); @@ -3456,10 +3456,25 @@ void NearbySharingServiceImpl::UnregisterShareTarget(int64_t share_target_id) { // Find the endpoint id that matches the given share target. auto it = outgoing_share_session_map_.find(share_target_id); if (it != outgoing_share_session_map_.end()) { - RemoveOutgoingShareTargetAndReportLost(it->second.endpoint_id()); + if (NearbyFlags::GetInstance().GetBoolFlag( + config_package_nearby::nearby_sharing_feature:: + kApplyEndpointsDedup) && + NearbyFlags::GetInstance().GetBoolFlag( + config_package_nearby::nearby_sharing_feature:: + kDedupInUnregisterShareTarget)) { + LOG(INFO) << __func__ << ": [Dedupped] Move the endpoint " + << it->second.endpoint_id() << " to discovery_cache."; + MoveToDiscoveryCache( + it->second.endpoint_id(), + NearbyFlags::GetInstance().GetInt64Flag( + config_package_nearby::nearby_sharing_feature:: + kUnregisterTargetDiscoveryCacheLostExpiryMs)); + } else { + RemoveOutgoingShareTargetAndReportLost(it->second.endpoint_id()); + } } else { - // Be careful not to clear out the share session map if a new session was - // started during the cancellation delay. + // Be careful not to clear out the share session map if a new session + // was started during the cancellation delay. if (!is_scanning_ && !is_transferring_) { LOG(INFO) << "Cannot find session for target " << share_target_id << " clearing all outgoing sessions."; @@ -3468,7 +3483,8 @@ void NearbySharingServiceImpl::UnregisterShareTarget(int64_t share_target_id) { TriggerDiscoveryCacheExpiryTimers(); } - VLOG(1) << __func__ << ": Unregister share target: " << share_target_id; + VLOG(1) << __func__ + << ": Unregister outgoing share target: " << share_target_id; } } @@ -3515,8 +3531,9 @@ void NearbySharingServiceImpl::OnStartDiscoveryResult(Status status) { VLOG(1) << __func__ << ": StartDiscovery over Nearby Connections was successful."; - // Periodically download certificates if there are discovered, contact-based - // advertisements that cannot decrypt any currently stored certificates. + // Periodically download certificates if there are discovered, + // contact-based advertisements that cannot decrypt any currently stored + // certificates. ScheduleCertificateDownloadDuringDiscovery(/*attempt_count=*/0); } else { LOG(ERROR) << __func__ diff --git a/sharing/nearby_sharing_service_impl.h b/sharing/nearby_sharing_service_impl.h index 36b8757b..339ba991 100644 --- a/sharing/nearby_sharing_service_impl.h +++ b/sharing/nearby_sharing_service_impl.h @@ -377,10 +377,12 @@ class NearbySharingServiceImpl const ShareTarget& share_target, absl::string_view endpoint_id, std::optional certificate); - void MoveToDiscoveryCache(absl::string_view endpoint_id); + // Move the endpoint to the discovery cache with the given expiry time. + void MoveToDiscoveryCache(absl::string_view endpoint_id, uint64_t expiry_ms); // Immediately expire all timers in the discovery cache. (i.e. report // ShareTargetLost) void TriggerDiscoveryCacheExpiryTimers(); + // Update the entry in outgoing_share_session_map_ with the new share target // and OnShareTargetUpdated is called. void DeduplicateInOutgoingShareTarget(