From 493e21d4bac43c729168d6daf22f5539240f3e15 Mon Sep 17 00:00:00 2001 From: Francis Tsui Date: Mon, 20 Jul 2026 16:23:14 -0700 Subject: [PATCH] Process join binding time and cert_ids in BindingResponse message. PiperOrigin-RevId: 951113941 --- sharing/BUILD | 1 + sharing/certificates/BUILD | 2 ++ .../nearby_share_certificate_manager_impl.cc | 22 +++++++++----- .../nearby_share_certificate_manager_impl.h | 10 +++++-- sharing/nearby_sharing_service_impl.cc | 30 +++++++++++++++---- sharing/nearby_sharing_service_impl.h | 3 +- sharing/outgoing_share_session.cc | 14 ++++----- sharing/outgoing_share_session.h | 2 +- sharing/outgoing_share_session_test.cc | 30 +++++++++++-------- 9 files changed, 77 insertions(+), 37 deletions(-) diff --git a/sharing/BUILD b/sharing/BUILD index dea41594..8c7986c7 100644 --- a/sharing/BUILD +++ b/sharing/BUILD @@ -426,6 +426,7 @@ cc_library( "//sharing/proto:share_cc_proto", "//sharing/proto:wire_format_cc_proto", "//sharing/scheduling", + "//third_party/gloop/util/time:protoutil", "@com_google_absl//absl/base:core_headers", "@com_google_absl//absl/base:nullability", "@com_google_absl//absl/container:flat_hash_map", diff --git a/sharing/certificates/BUILD b/sharing/certificates/BUILD index 28338d86..f2205e16 100644 --- a/sharing/certificates/BUILD +++ b/sharing/certificates/BUILD @@ -66,6 +66,8 @@ cc_library( "//third_party/gloop/util/time:protoutil", "//util/hash:highway_fingerprint", "@com_google_absl//absl/algorithm", + "@com_google_absl//absl/algorithm:container", + "@com_google_absl//absl/base:core_headers", "@com_google_absl//absl/base:nullability", "@com_google_absl//absl/container:btree", "@com_google_absl//absl/container:flat_hash_map", diff --git a/sharing/certificates/nearby_share_certificate_manager_impl.cc b/sharing/certificates/nearby_share_certificate_manager_impl.cc index a6ca1f9f..db2afa89 100644 --- a/sharing/certificates/nearby_share_certificate_manager_impl.cc +++ b/sharing/certificates/nearby_share_certificate_manager_impl.cc @@ -34,6 +34,7 @@ #include "location/nearby/sharing/lib/account/account_manager.h" #include "location/nearby/sharing/lib/rpc/sharing_rpc_client.h" #include "absl/algorithm/algorithm.h" +#include "absl/algorithm/container.h" #include "absl/base/nullability.h" #include "absl/container/flat_hash_map.h" #include "absl/memory/memory.h" @@ -41,6 +42,7 @@ #include "absl/strings/escaping.h" #include "absl/strings/str_cat.h" #include "absl/strings/string_view.h" +#include "absl/synchronization/mutex.h" #include "absl/synchronization/notification.h" #include "absl/time/time.h" #include "absl/types/span.h" @@ -476,14 +478,19 @@ bool NearbyShareCertificateManagerImpl::DownloadPublicCertificatesInExecutor() { } // Clear join_time if it is expired. - if (join_time_.has_value() && - context_->GetClock()->Now() > join_time_discard_time_) { - join_time_.reset(); + std::optional join_time; + { + absl::MutexLock lock(join_time_mutex_); + if (join_time_.has_value() && + context_->GetClock()->Now() > join_time_discard_time_) { + join_time_.reset(); + } + join_time = join_time_; } bool download_succeeded = false; absl::Notification notification; auto context = std::make_unique( - nearby_identity_client_, std::move(device_id), join_time_, + nearby_identity_client_, std::move(device_id), join_time, [this, &download_succeeded, ¬ification]( absl::StatusOr> certificates_status) { if (!certificates_status.ok()) { @@ -760,6 +767,7 @@ void NearbyShareCertificateManagerImpl::SetVendorId(int32_t vendor_id) { void NearbyShareCertificateManagerImpl::SetJoinBindingTime( absl::Time join_binding_time, absl::Duration life_time) { + absl::MutexLock lock(join_time_mutex_); join_time_ = join_binding_time; join_time_discard_time_ = context_->GetClock()->Now() + life_time; } @@ -988,9 +996,9 @@ bool NearbyShareCertificateManagerImpl::UpdateAccountInfoInExecutor() { get_account_info_succeeded = true; const auto& capabilities = response->account_info().capabilities(); bool has_titanium_capability = - (std::find(capabilities.begin(), capabilities.end(), - google::nearby::identity::v1::AccountInfo:: - CAPABILITY_TITANIUM) != capabilities.end()); + (absl::c_find(capabilities, + google::nearby::identity::v1::AccountInfo:: + CAPABILITY_TITANIUM) != capabilities.end()); preference_manager_.SetBoolean(PrefNames::kAdvancedProtectionEnabled, has_titanium_capability); LOG(INFO) << "GetAccountInfo succeeded, advanced protection enabled: " diff --git a/sharing/certificates/nearby_share_certificate_manager_impl.h b/sharing/certificates/nearby_share_certificate_manager_impl.h index 3f1bf793..ad1d7fcf 100644 --- a/sharing/certificates/nearby_share_certificate_manager_impl.h +++ b/sharing/certificates/nearby_share_certificate_manager_impl.h @@ -26,9 +26,11 @@ #include "location/nearby/sharing/lib/account/account_manager.h" #include "location/nearby/sharing/lib/rpc/sharing_rpc_client.h" #include "absl/base/nullability.h" +#include "absl/base/thread_annotations.h" #include "absl/functional/any_invocable.h" #include "absl/status/statusor.h" #include "absl/strings/string_view.h" +#include "absl/synchronization/mutex.h" #include "absl/time/time.h" #include "internal/base/file_path.h" #include "internal/platform/task_runner.h" @@ -94,7 +96,8 @@ class NearbyShareCertificateManagerImpl void ClearPublicCertificates(std::function callback) override; void SetVendorId(int32_t vendor_id) override; void SetJoinBindingTime(absl::Time join_binding_time, - absl::Duration life_time) override; + absl::Duration life_time) + ABSL_LOCKS_EXCLUDED(join_time_mutex_) override; std::string Dump() const override; void AddBindingToPublicCertificate( absl::string_view certificate_id, absl::string_view binding_id) override; @@ -227,11 +230,12 @@ class NearbyShareCertificateManagerImpl account_info_update_scheduler_; std::unique_ptr executor_; + absl::Mutex join_time_mutex_; // Set to the transaction timestamp of the last successful pairing if // available. This is returned from the phone in the BindingResponse message. - std::optional join_time_; + std::optional join_time_ ABSL_GUARDED_BY(join_time_mutex_); // The time when the join_time_ will be discarded. - absl::Time join_time_discard_time_; + absl::Time join_time_discard_time_ ABSL_GUARDED_BY(join_time_mutex_); }; } // namespace nearby::sharing diff --git a/sharing/nearby_sharing_service_impl.cc b/sharing/nearby_sharing_service_impl.cc index 4cbede27..9d63cea3 100644 --- a/sharing/nearby_sharing_service_impl.cc +++ b/sharing/nearby_sharing_service_impl.cc @@ -47,6 +47,7 @@ #include "absl/synchronization/mutex.h" #include "absl/time/time.h" #include "absl/types/span.h" +#include "third_party/gloop/util/time/protoutil.h" #include "internal/base/file_path.h" #include "internal/flags/nearby_flags.h" #include "internal/network/url.h" @@ -141,6 +142,11 @@ constexpr absl::Duration kProcessNetworkChangeTimerDelay = absl::Seconds(1); // fully when the system is stable. constexpr absl::Duration kResumeDelay = absl::Milliseconds(500); +// The duration to use join binding time when downloading public certificates. +// The default BE database query staleness is 30s. We extend this t0 40s to +// ensure that we have some overlap. +constexpr absl::Duration kJoinBindingTimeLifeTime = absl::Seconds(40); + // The maximum number of certificate downloads that can be performed during a // discovery session. // Assuming a 2min discovery session and 10s download interval. @@ -2794,10 +2800,9 @@ void NearbySharingServiceImpl::OnInitiateSyncBindingResponse( << ": Sync binding rpc succeeded: id=" << binding_id; session->StartPeerBinding( binding_id, BindingRequest::FILESYNC, GetCertIdsForSyncBinding(), - [this, share_target_id, - binding_id](BindingResponse::Status status) { - OnPeerSyncBindingComplete(share_target_id, binding_id, status); - }); + absl::bind_front( + &NearbySharingServiceImpl::OnPeerSyncBindingComplete, this, + share_target_id, binding_id)); } else { LOG(INFO) << __func__ << ": Sync binding rpc failed."; session->Abort(TransferMetadata::Status::kFailed); @@ -2807,7 +2812,7 @@ void NearbySharingServiceImpl::OnInitiateSyncBindingResponse( void NearbySharingServiceImpl::OnPeerSyncBindingComplete( int64_t share_target_id, absl::string_view binding_id, - BindingResponse::Status status) { + const BindingResponse& binding_response) { OutgoingShareSession* session = outgoing_targets_manager_.GetOutgoingShareSession(share_target_id); if (!session || !session->IsConnected()) { @@ -2815,7 +2820,7 @@ void NearbySharingServiceImpl::OnPeerSyncBindingComplete( << share_target_id; return; } - if (status != BindingResponse::SUCCESS) { + if (binding_response.status() != BindingResponse::SUCCESS) { LOG(INFO) << __func__ << ": Sync binding response failed."; session->Abort(TransferMetadata::Status::kFailed); return; @@ -2841,7 +2846,20 @@ void NearbySharingServiceImpl::OnPeerSyncBindingComplete( .set_binding_id(binding_id) .set_status(TransferMetadata::Status::kComplete) .build()); + // Update binding id in peer certificates so we can identify the sync peer + // immediately without waiting for cert sync from Backend. + for (const auto& cert_id : binding_response.cert_ids()) { + certificate_manager_->AddBindingToPublicCertificate(cert_id, binding_id); + } + if (binding_response.has_join_binding_time()) { + auto join_binding_time = + util_time::DecodeGoogleApiProto(binding_response.join_binding_time()); + if (join_binding_time.ok()) { + certificate_manager_->SetJoinBindingTime(join_binding_time.value(), + kJoinBindingTimeLifeTime); + } + } // Download public certificates again to update the newly added sync binding. certificate_manager_->DownloadPublicCertificates(); } diff --git a/sharing/nearby_sharing_service_impl.h b/sharing/nearby_sharing_service_impl.h index 7223f632..5a045971 100644 --- a/sharing/nearby_sharing_service_impl.h +++ b/sharing/nearby_sharing_service_impl.h @@ -37,6 +37,7 @@ #include "absl/functional/any_invocable.h" #include "absl/status/statusor.h" #include "absl/strings/string_view.h" +#include "absl/synchronization/mutex.h" #include "absl/time/time.h" #include "absl/types/span.h" #include "internal/platform/clock.h" @@ -416,7 +417,7 @@ class NearbySharingServiceImpl // Called when Bindings response frame is received from the peer. void OnPeerSyncBindingComplete( int64_t share_target_id, absl::string_view binding_id, - service::proto::BindingResponse::Status status); + const service::proto::BindingResponse& binding_response); // Notify all registered send surfaces of share target state changes. void NotifyShareTargetDiscovered(const ShareTarget& share_target); diff --git a/sharing/outgoing_share_session.cc b/sharing/outgoing_share_session.cc index eb613ab9..d1fa96dc 100644 --- a/sharing/outgoing_share_session.cc +++ b/sharing/outgoing_share_session.cc @@ -649,7 +649,7 @@ OutgoingShareSession::ProcessPayloadTransferUpdates() { void OutgoingShareSession::StartPeerBinding( std::string binding_id, BindingRequest::Type binding_type, absl::Span cert_ids, - absl::AnyInvocable callback) { + absl::AnyInvocable callback) { Frame frame; frame.set_version(Frame::V1); V1Frame* v1_frame = frame.mutable_v1(); @@ -671,19 +671,19 @@ void OutgoingShareSession::StartPeerBinding( nearby::sharing::service::proto::V1Frame::BINDINGS, [callback = std::move(callback)]( bool is_timeout, std::optional frame) mutable { + BindingResponse failure_response; + failure_response.set_status(BindingResponse::FAILURE); if (!frame.has_value()) { - std::move(callback)(BindingResponse::FAILURE); + std::move(callback)(failure_response); return; } if (!frame->has_bindings() || - !frame->bindings().has_binding_response() || - frame->bindings().binding_response().status() != - BindingResponse::SUCCESS) { - std::move(callback)(BindingResponse::FAILURE); + !frame->bindings().has_binding_response()) { + std::move(callback)(failure_response); return; } // Peer binding flow completed successfully. - std::move(callback)(BindingResponse::SUCCESS); + std::move(callback)(frame->bindings().binding_response()); }, kReadResponseFrameTimeout); } diff --git a/sharing/outgoing_share_session.h b/sharing/outgoing_share_session.h index 5ff89dce..1731d04d 100644 --- a/sharing/outgoing_share_session.h +++ b/sharing/outgoing_share_session.h @@ -174,7 +174,7 @@ class OutgoingShareSession : public ShareSession { nearby::sharing::service::proto::BindingRequest::Type binding_type, absl::Span cert_ids, absl::AnyInvocable< - void(nearby::sharing::service::proto::BindingResponse::Status)> + void(const nearby::sharing::service::proto::BindingResponse&)> callback); protected: diff --git a/sharing/outgoing_share_session_test.cc b/sharing/outgoing_share_session_test.cc index 43cf62f0..f13e04b9 100644 --- a/sharing/outgoing_share_session_test.cc +++ b/sharing/outgoing_share_session_test.cc @@ -74,6 +74,7 @@ using ::nearby::sharing::service::proto::V1Frame; using ::nearby::sharing::service::proto::WifiCredentials; using ::testing::_; using ::testing::AllOf; +using ::testing::ElementsAre; using ::testing::Eq; using ::protobuf_matchers::EqualsProto; using ::testing::InSequence; @@ -947,11 +948,11 @@ TEST_F(OutgoingShareSessionTest, StartPeerBindingSuccess) { AllOf(HasStatus(TransferMetadata::Status::kAwaitingRemoteAcceptance), HasUsage(ShareSessionUsage::kPairing)))); - BindingResponse::Status binding_response_status = BindingResponse::FAILURE; + BindingResponse binding_response; session_.StartPeerBinding( "test_binding_id", BindingRequest::FILESYNC, {"cert_id_1", "cert_id_2"}, - [&binding_response_status](BindingResponse::Status status) { - binding_response_status = status; + [&binding_response](const BindingResponse& response) { + binding_response = response; }); Frame frame; @@ -969,6 +970,8 @@ TEST_F(OutgoingShareSessionTest, StartPeerBindingSuccess) { bindings { binding_response { status: SUCCESS + cert_ids: "cert_id_3" + cert_ids: "cert_id_4" } } } @@ -980,7 +983,9 @@ TEST_F(OutgoingShareSessionTest, StartPeerBindingSuccess) { IsTrue()); connection.WriteMessage(std::move(data)); - EXPECT_THAT(binding_response_status, Eq(BindingResponse::SUCCESS)); + EXPECT_THAT(binding_response.status(), Eq(BindingResponse::SUCCESS)); + EXPECT_THAT(binding_response.cert_ids(), + ElementsAre("cert_id_3", "cert_id_4")); } TEST_F(OutgoingShareSessionTest, StartPeerBindingTimeout) { @@ -1015,11 +1020,11 @@ TEST_F(OutgoingShareSessionTest, StartPeerBindingTimeout) { AllOf(HasStatus(TransferMetadata::Status::kAwaitingRemoteAcceptance), HasUsage(ShareSessionUsage::kPairing)))); - BindingResponse::Status binding_response_status = BindingResponse::FAILURE; + BindingResponse binding_response; session_.StartPeerBinding( "test_binding_id", BindingRequest::FILESYNC, {}, - [&binding_response_status](BindingResponse::Status status) { - binding_response_status = status; + [&binding_response](const BindingResponse& response) { + binding_response = response; }); Frame frame; @@ -1031,7 +1036,7 @@ TEST_F(OutgoingShareSessionTest, StartPeerBindingTimeout) { fake_clock_.FastForward(absl::Seconds(60)); fake_task_runner_.SyncWithTimeout(absl::Milliseconds(100)); - EXPECT_THAT(binding_response_status, Eq(BindingResponse::FAILURE)); + EXPECT_THAT(binding_response.status(), Eq(BindingResponse::FAILURE)); } TEST_F(OutgoingShareSessionTest, StartPeerBindingFailure) { @@ -1066,11 +1071,11 @@ TEST_F(OutgoingShareSessionTest, StartPeerBindingFailure) { AllOf(HasStatus(TransferMetadata::Status::kAwaitingRemoteAcceptance), HasUsage(ShareSessionUsage::kPairing)))); - BindingResponse::Status binding_response_status = BindingResponse::FAILURE; + BindingResponse binding_response; session_.StartPeerBinding( "test_binding_id", BindingRequest::FILESYNC, {}, - [&binding_response_status](BindingResponse::Status status) { - binding_response_status = status; + [&binding_response](const BindingResponse& response) { + binding_response = response; }); Frame frame; @@ -1099,7 +1104,8 @@ TEST_F(OutgoingShareSessionTest, StartPeerBindingFailure) { IsTrue()); connection.WriteMessage(std::move(data)); - EXPECT_THAT(binding_response_status, Eq(BindingResponse::FAILURE)); + EXPECT_THAT(binding_response.status(), Eq(BindingResponse::FAILURE)); + EXPECT_THAT(binding_response.cert_ids(), IsEmpty()); } } // namespace