Process join binding time and cert_ids in BindingResponse message.

PiperOrigin-RevId: 951113941

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",

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",

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<absl::Time> 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<CertificateDownloadContext>(
-      nearby_identity_client_, std::move(device_id), join_time_,
+      nearby_identity_client_, std::move(device_id), join_time,
       [this, &download_succeeded, &notification](
           absl::StatusOr<std::vector<PublicCertificate>> 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: "

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<void(bool)> 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<TaskRunner> 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<absl::Time> join_time_;
+  std::optional<absl::Time> 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

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();
 }

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);

sharing/outgoing_share_session.cc

@@ -649,7 +649,7 @@ OutgoingShareSession::ProcessPayloadTransferUpdates() {
 void OutgoingShareSession::StartPeerBinding(
     std::string binding_id, BindingRequest::Type binding_type,
     absl::Span<const std::string> cert_ids,
-    absl::AnyInvocable<void(BindingResponse::Status)> callback) {
+    absl::AnyInvocable<void(const BindingResponse&)> 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<V1Frame> 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);
 }

sharing/outgoing_share_session.h

@@ -174,7 +174,7 @@ class OutgoingShareSession : public ShareSession {
       nearby::sharing::service::proto::BindingRequest::Type binding_type,
       absl::Span<const std::string> cert_ids,
       absl::AnyInvocable<
-          void(nearby::sharing::service::proto::BindingResponse::Status)>
+          void(const nearby::sharing::service::proto::BindingResponse&)>
           callback);
 
  protected:

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