Move device ID generation to login success.

PiperOrigin-RevId: 742428028

sharing/certificates/nearby_share_certificate_manager_impl.cc

@@ -39,7 +39,6 @@
 #include "absl/synchronization/notification.h"
 #include "absl/time/time.h"
 #include "absl/types/span.h"
-#include "internal/flags/nearby_flags.h"
 #include "internal/platform/implementation/account_manager.h"
 #include "proto/identity/v1/resources.pb.h"
 #include "proto/identity/v1/rpcs.pb.h"
@@ -53,7 +52,6 @@
 #include "sharing/certificates/nearby_share_private_certificate.h"
 #include "sharing/common/nearby_share_prefs.h"
 #include "sharing/contacts/nearby_share_contact_manager.h"
-#include "sharing/flags/generated/nearby_sharing_feature_flags.h"
 #include "sharing/internal/api/bluetooth_adapter.h"
 #include "sharing/internal/api/preference_manager.h"
 #include "sharing/internal/api/public_certificate_database.h"
@@ -424,7 +422,9 @@ void NearbyShareCertificateManagerImpl::DownloadPublicCertificates() {
       return;
     }
 
-    if (!account_manager_.GetCurrentAccount().has_value()) {
+    std::string device_id = local_device_data_manager_->GetId();
+    if (!account_manager_.GetCurrentAccount().has_value() ||
+        device_id.empty()) {
       LOG(WARNING) << "Ignore certificates download, no logged in account.";
       download_public_certificates_scheduler_->HandleResult(/*success=*/true);
       return;
@@ -434,7 +434,7 @@ void NearbyShareCertificateManagerImpl::DownloadPublicCertificates() {
     // FetchNextPage() returns.
     auto context = std::make_unique<CertificateDownloadContext>(
         nearby_client_.get(), nearby_identity_client_.get(),
-        kDeviceIdPrefix + local_device_data_manager_->GetId(),
+        kDeviceIdPrefix + device_id,
         absl::bind_front(&NearbyShareCertificateManagerImpl::
                              OnPublicCertificatesDownloadFailure,
                          this),

sharing/local_device_data/BUILD

@@ -42,7 +42,6 @@ cc_library(
         "//sharing/proto:share_cc_proto",
         "//util/hash:highway_fingerprint",
         "@com_google_absl//absl/algorithm",
-        "@com_google_absl//absl/functional:any_invocable",
         "@com_google_absl//absl/memory",
         "@com_google_absl//absl/random",
         "@com_google_absl//absl/status:statusor",

sharing/local_device_data/nearby_share_local_device_data_manager_impl.cc

@@ -16,7 +16,6 @@
 
 #include <stddef.h>
 
-#include <array>
 #include <cstdint>
 #include <functional>
 #include <memory>
@@ -27,7 +26,6 @@
 
 #include "absl/algorithm/algorithm.h"
 #include "absl/memory/memory.h"
-#include "absl/random/random.h"
 #include "absl/status/statusor.h"
 #include "absl/strings/str_cat.h"
 #include "absl/strings/string_view.h"
@@ -62,19 +60,6 @@ using ::nearby::sharing::api::SharingRpcClientFactory;
 using ::nearby::sharing::proto::UpdateDeviceRequest;
 using ::nearby::sharing::proto::UpdateDeviceResponse;
 
-// Using the alphanumeric characters below, this provides 36^10 unique device
-// IDs. Note that the uniqueness requirement is not global; the IDs are only
-// used to differentiate between devices associated with a single GAIA account.
-// This ID length agrees with the GmsCore implementation.
-constexpr size_t kDeviceIdLength = 10;
-
-// Possible characters used in a randomly generated device ID. This agrees with
-// the GmsCore implementation.
-constexpr std::array<char, 36> kAlphaNumericChars = {
-    'A', 'B', 'C', 'D', 'E', 'F', 'G', 'H', 'I', 'J', 'K', 'L',
-    'M', 'N', 'O', 'P', 'Q', 'R', 'S', 'T', 'U', 'V', 'W', 'X',
-    'Y', 'Z', '0', '1', '2', '3', '4', '5', '6', '7', '8', '9'};
-
 constexpr absl::string_view kDeviceIdPrefix = "users/me/devices/";
 constexpr absl::string_view kContactsFieldMaskPath = "contacts";
 constexpr absl::string_view kCertificatesFieldMaskPath = "public_certificates";
@@ -142,18 +127,7 @@ NearbyShareLocalDeviceDataManagerImpl::
     ~NearbyShareLocalDeviceDataManagerImpl() = default;
 
 std::string NearbyShareLocalDeviceDataManagerImpl::GetId() {
-  std::string id =
-      preference_manager_.GetString(prefs::kNearbySharingDeviceIdName, "");
-  if (!id.empty()) return id;
-
-  absl::BitGen bitgen;
-  for (size_t i = 0; i < kDeviceIdLength; ++i)
-    id += kAlphaNumericChars[absl::Uniform(
-        bitgen, 0, static_cast<int>(kAlphaNumericChars.size()))];
-
-  preference_manager_.SetString(prefs::kNearbySharingDeviceIdName, id);
-
-  return id;
+  return preference_manager_.GetString(prefs::kNearbySharingDeviceIdName, "");
 }
 
 std::string NearbyShareLocalDeviceDataManagerImpl::GetDeviceName() const {
@@ -205,7 +179,9 @@ void NearbyShareLocalDeviceDataManagerImpl::UploadContacts(
           return;
         }
 
-        if (!account_manager_.GetCurrentAccount().has_value()) {
+        std::string device_id = GetId();
+        if (!account_manager_.GetCurrentAccount().has_value() ||
+            device_id.empty()) {
           LOG(WARNING) << __func__
                        << ": skip to upload contacts due "
                           "to no login account.";
@@ -215,7 +191,7 @@ void NearbyShareLocalDeviceDataManagerImpl::UploadContacts(
 
         UpdateDeviceRequest request;
         request.mutable_device()->set_name(
-            absl::StrCat(kDeviceIdPrefix, GetId()));
+            absl::StrCat(kDeviceIdPrefix, device_id));
         request.mutable_device()->mutable_contacts()->Add(contacts.begin(),
                                                           contacts.end());
         request.mutable_update_mask()->add_paths(
@@ -249,11 +225,18 @@ void NearbyShareLocalDeviceDataManagerImpl::PublishDevice(
       callback(/*success=*/false, /*contact_removed=*/false);
       return;
     }
+    std::string device_id = GetId();
+    if (device_id.empty()) {
+      LOG(WARNING) << __func__
+                   << ": [Call Identity API] failed, device id is empty.";
+      callback(/*success=*/false, /*contact_removed=*/false);
+      return;
+    }
     LOG(INFO) << __func__ << ": [Call Identity API]  Upload "
               << certificates.size() << " certificates.";
 
     PublishDeviceRequest request;
-    request.mutable_device()->set_name(absl::StrCat("devices/", GetId()));
+    request.mutable_device()->set_name(absl::StrCat("devices/", device_id));
     LOG(INFO) << __func__
               << ": [Call Identity API] PublishDeviceRequest with Device.name: "
               << request.device().name();
@@ -357,7 +340,9 @@ void NearbyShareLocalDeviceDataManagerImpl::UploadCertificates(
       return;
     }
 
-    if (!account_manager_.GetCurrentAccount().has_value()) {
+    std::string device_id = GetId();
+    if (!account_manager_.GetCurrentAccount().has_value() ||
+        device_id.empty()) {
       LOG(WARNING) << __func__
                    << ": skip to upload certificates due "
                       "to no login account.";
@@ -366,7 +351,7 @@ void NearbyShareLocalDeviceDataManagerImpl::UploadCertificates(
     }
     UpdateDeviceRequest request;
     request.mutable_device()->set_name(
-        absl::StrCat(kDeviceIdPrefix, GetId()));
+        absl::StrCat(kDeviceIdPrefix, device_id));
     request.mutable_device()->mutable_public_certificates()->Add(
         certificates.begin(), certificates.end());
     request.mutable_update_mask()->add_paths(

sharing/local_device_data/nearby_share_local_device_data_manager_impl.h

@@ -16,7 +16,6 @@
 #define THIRD_PARTY_NEARBY_SHARING_LOCAL_DEVICE_DATA_NEARBY_SHARE_LOCAL_DEVICE_DATA_MANAGER_IMPL_H_
 
 #include <memory>
-#include <optional>
 #include <string>
 #include <vector>
 
@@ -27,7 +26,6 @@
 #include "sharing/common/nearby_share_enums.h"
 #include "sharing/internal/api/preference_manager.h"
 #include "sharing/internal/api/sharing_rpc_client.h"
-#include "sharing/internal/impl/common/nearby_identity_grpc_client.h"
 #include "sharing/internal/public/context.h"
 #include "sharing/local_device_data/nearby_share_local_device_data_manager.h"
 #include "sharing/proto/rpc_resources.pb.h"

sharing/local_device_data/nearby_share_local_device_data_manager_impl_test.cc

@@ -16,7 +16,6 @@
 
 #include <stddef.h>
 
-#include <cctype>
 #include <memory>
 #include <optional>
 #include <string>
@@ -27,6 +26,7 @@
 #include "absl/status/status.h"
 #include "absl/status/statusor.h"
 #include "absl/strings/str_cat.h"
+#include "absl/strings/string_view.h"
 #include "absl/strings/substitute.h"
 #include "absl/time/time.h"
 #include "internal/platform/implementation/account_manager.h"
@@ -68,6 +68,7 @@ const char kFakeTooLongDeviceName[] = "this string is 33 bytes in UTF-8!";
 const char kFakeTooLongGivenName[] = "this is a 33-byte string in utf-8";
 constexpr char kTestAccountId[] = "test_account_id";
 constexpr char kTestProfileUserName[] = "test@google.com";
+constexpr absl::string_view kTestDeviceId = "1234567890";
 
 absl::StatusOr<UpdateDeviceResponse> CreateResponse(
     const std::optional<std::string>& full_name,
@@ -174,6 +175,10 @@ class NearbyShareLocalDeviceDataManagerImplTest
         [&returned_success](bool success) { returned_success = success; });
     Sync();
     EXPECT_TRUE(client()->list_public_certificates_requests().empty());
+    EXPECT_EQ(response.ok(), returned_success);
+    if (!response.ok()) {
+      return;
+    }
     std::vector<Contact> expected_fake_contacts = GetFakeContacts();
     for (size_t i = 0; i < expected_fake_contacts.size(); ++i) {
       EXPECT_EQ(expected_fake_contacts[i].SerializeAsString(),
@@ -185,8 +190,6 @@ class NearbyShareLocalDeviceDataManagerImplTest
                     .at(i)
                     .SerializeAsString());
     }
-
-    EXPECT_EQ(response.ok(), returned_success);
   }
 
   void UploadCertificates(
@@ -240,6 +243,10 @@ class NearbyShareLocalDeviceDataManagerImplTest
     return nearby_client_factory_.identity_instances().back();
   }
 
+  void SetDeviceId(absl::string_view id) {
+    preference_manager_.SetString(prefs::kNearbySharingDeviceIdName, id);
+  }
+
   void Sync() {
     EXPECT_TRUE(context_.last_sequenced_task_runner()->SyncWithTimeout(
         absl::Milliseconds(1000)));
@@ -260,12 +267,11 @@ class NearbyShareLocalDeviceDataManagerImplTest
 
 TEST_F(NearbyShareLocalDeviceDataManagerImplTest, DeviceId) {
   CreateManager();
-
+  SetDeviceId(kTestDeviceId);
   // A 10-character alphanumeric ID is automatically generated if one doesn't
   // already exist.
   std::string id = manager()->GetId();
-  EXPECT_EQ(id.size(), 10u);
-  for (const char c : id) EXPECT_TRUE(std::isalnum(c));
+  EXPECT_EQ(id, kTestDeviceId);
 
   // The ID is persisted.
   DestroyManager();
@@ -349,22 +355,26 @@ TEST_F(NearbyShareLocalDeviceDataManagerImplTest, SetDeviceName) {
 
 TEST_F(NearbyShareLocalDeviceDataManagerImplTest, UploadContacts_Success) {
   CreateManager();
+  SetDeviceId(kTestDeviceId);
   UploadContacts(CreateResponse(kFakeFullName, kFakeIconUrl, kFakeIconToken));
 }
 
 TEST_F(NearbyShareLocalDeviceDataManagerImplTest, UploadContacts_Failure) {
   CreateManager();
+  SetDeviceId(kTestDeviceId);
   UploadContacts(/*response=*/absl::InternalError(""));
 }
 
 TEST_F(NearbyShareLocalDeviceDataManagerImplTest, UploadCertificates_Success) {
   CreateManager();
+  SetDeviceId(kTestDeviceId);
   UploadCertificates(
       CreateResponse(kFakeFullName, kFakeIconUrl, kFakeIconToken));
 }
 
 TEST_F(NearbyShareLocalDeviceDataManagerImplTest, UploadCertificates_Failure) {
   CreateManager();
+  SetDeviceId(kTestDeviceId);
   UploadCertificates(/*response=*/absl::InternalError(""));
 }
 
@@ -412,6 +422,7 @@ std::vector<nearby::sharing::proto::PublicCertificate> GetTestCertificates() {
 TEST_F(NearbyShareLocalDeviceDataManagerImplTest,
        PublishDeviceInitialCall_ContactUpdateAdded) {
   CreateManager();
+  SetDeviceId(kTestDeviceId);
   bool returned_success;
   bool returned_make_another_call;
   PublishDeviceResponse response;
@@ -492,6 +503,7 @@ TEST_F(NearbyShareLocalDeviceDataManagerImplTest,
 TEST_F(NearbyShareLocalDeviceDataManagerImplTest,
        PublishDeviceInitialCall_ContactUpdateRemoved) {
   CreateManager();
+  SetDeviceId(kTestDeviceId);
   bool returned_success;
   bool returned_make_another_call;
   PublishDeviceResponse response;
@@ -518,6 +530,7 @@ TEST_F(NearbyShareLocalDeviceDataManagerImplTest,
 TEST_F(NearbyShareLocalDeviceDataManagerImplTest,
        PublishDeviceSecondCall_ContactUnchanged) {
   CreateManager();
+  SetDeviceId(kTestDeviceId);
   bool returned_success;
   bool returned_make_another_call;
   PublishDeviceResponse response;
@@ -544,6 +557,7 @@ TEST_F(NearbyShareLocalDeviceDataManagerImplTest,
 
 TEST_F(NearbyShareLocalDeviceDataManagerImplTest, PublishDevice_Failure) {
   CreateManager();
+  SetDeviceId(kTestDeviceId);
   bool returned_success;
   bool returned_make_another_call;
   identity_client()->SetPublishDeviceResponse(absl::InternalError(""));
@@ -560,7 +574,7 @@ TEST_F(NearbyShareLocalDeviceDataManagerImplTest, PublishDevice_Failure) {
 }
 
 TEST_F(NearbyShareLocalDeviceDataManagerImplTest,
-       PublishDevice_DeviceIdChangedAfterReset) {
+       PublishDevice_FailsWithEmptyDeviceId) {
   CreateManager();
   bool returned_success;
   PublishDeviceResponse response;
@@ -574,25 +588,8 @@ TEST_F(NearbyShareLocalDeviceDataManagerImplTest,
                            });
 
   Sync();
-  EXPECT_TRUE(returned_success);
-  ASSERT_EQ(identity_client()->publish_device_requests().size(), 1);
-  std::string device_id =
-      identity_client()->publish_device_requests().back().device().name();
-  // Reset device ID.
-  preference_manager_.SetString(prefs::kNearbySharingDeviceIdName,
-                                std::string());
-  manager()->PublishDevice(
-      GetTestCertificates(), /*is_second_call=*/true,
-      [&returned_success](bool success, bool make_another_call) {
-        returned_success = success;
-      });
-  Sync();
-  EXPECT_TRUE(returned_success);
-  ASSERT_EQ(identity_client()->publish_device_requests().size(), 2);
-  std::string new_device_id =
-      identity_client()->publish_device_requests().back().device().name();
-
-  EXPECT_NE(device_id, new_device_id);
+  EXPECT_FALSE(returned_success);
+  ASSERT_EQ(identity_client()->publish_device_requests().size(), 0);
 }
 
 }  // namespace

sharing/nearby_sharing_service_impl.cc

@@ -32,7 +32,6 @@
 #include <utility>
 #include <vector>
 
-#include "absl/algorithm/container.h"
 #include "absl/container/flat_hash_map.h"
 #include "absl/functional/any_invocable.h"
 #include "absl/functional/bind_front.h"
@@ -150,6 +149,19 @@ constexpr absl::string_view kConnectionListenerName = "nearby-share-service";
 constexpr absl::string_view kScreenStateListenerName = "nearby-share-service";
 constexpr absl::string_view kProfileRelativePath = "Google/Nearby/Sharing";
 
+// Using the alphanumeric characters below, this provides 36^10 unique device
+// IDs. Note that the uniqueness requirement is not global; the IDs are only
+// used to differentiate between devices associated with a single GAIA account.
+// This ID length agrees with the GmsCore implementation.
+constexpr size_t kDeviceIdLength = 10;
+
+// Possible characters used in a randomly generated device ID. This agrees with
+// the GmsCore implementation.
+constexpr std::array<char, 36> kAlphaNumericChars = {
+  'A', 'B', 'C', 'D', 'E', 'F', 'G', 'H', 'I', 'J', 'K', 'L',
+  'M', 'N', 'O', 'P', 'Q', 'R', 'S', 'T', 'U', 'V', 'W', 'X',
+  'Y', 'Z', '0', '1', '2', '3', '4', '5', '6', '7', '8', '9'};
+
 bool ShouldBlockSurfaceRegistration(BlockedVendorId registering_vendor_id,
                                     BlockedVendorId blocked_vendor_id) {
   return blocked_vendor_id != BlockedVendorId::kNone &&
@@ -188,6 +200,15 @@ OSType ToProtoOsType(::nearby::api::DeviceInfo::OsType os_type) {
   }
 }
 
+std::string GenerateDeviceId() {
+  std::string id;
+  absl::BitGen bitgen;
+  for (size_t i = 0; i < kDeviceIdLength; ++i)
+    id += kAlphaNumericChars[absl::Uniform(
+        bitgen, 0, static_cast<int>(kAlphaNumericChars.size()))];
+  return id;
+}
+
 }  // namespace
 
 NearbySharingServiceImpl::NearbySharingServiceImpl(
@@ -3590,6 +3611,10 @@ void NearbySharingServiceImpl::ResetAllSettings(bool logout) {
       LOG(INFO) << "Clear public certificates. result: " << result;
     });
   } else {
+    // on login generate a new device id
+    std::string device_id = GenerateDeviceId();
+    preference_manager_.SetString(prefs::kNearbySharingDeviceIdName, device_id);
+
     // should clear scheduled task to make it works immediately
     settings_->RemoveSettingsObserver(this);
     prefs::ResetSchedulers(preference_manager_);

sharing/nearby_sharing_service_impl_test.cc

@@ -17,6 +17,7 @@
 #include <stdint.h>
 #include <unistd.h>
 
+#include <cctype>
 #include <cstdio>
 #include <cstring>
 #include <filesystem>  // NOLINT(build/c++17)
@@ -4821,6 +4822,10 @@ TEST_F(NearbySharingServiceImplTest, LoginAndLogoutShouldResetSettings) {
   // Used to check whether the setting is cleared after login.
   service_->GetSettings()->SetIsAnalyticsEnabled(true);
 
+  std::string device_id =
+      preference_manager_.GetString(prefs::kNearbySharingDeviceIdName, "");
+  EXPECT_TRUE(device_id.empty());
+
   // Create account.
   AccountManager::Account account;
   account.id = kTestAccountId;
@@ -4830,11 +4835,17 @@ TEST_F(NearbySharingServiceImplTest, LoginAndLogoutShouldResetSettings) {
   std::unique_ptr<SigninAttempt> signin_attempt =
       service_->GetAccountManager()->Login("test_client_id",
                                            "test_client_secret");
+  account_manager().NotifyLogin(kTestAccountId);
   EXPECT_TRUE(sharing_service_task_runner_->SyncWithTimeout(kTaskWaitTimeout));
   EXPECT_TRUE(service_->GetSettings()->GetIsAnalyticsEnabled());
   ASSERT_TRUE(service_->GetAccountManager()->GetCurrentAccount().has_value());
   EXPECT_EQ(service_->GetAccountManager()->GetCurrentAccount()->id,
             kTestAccountId);
+  device_id =
+      preference_manager_.GetString(prefs::kNearbySharingDeviceIdName, "");
+  EXPECT_FALSE(device_id.empty());
+  EXPECT_EQ(device_id.size(), 10u);
+  for (const char c : device_id) EXPECT_TRUE(std::isalnum(c));
 
   // Logout user.
   absl::Notification logout_notification;
@@ -4848,6 +4859,9 @@ TEST_F(NearbySharingServiceImplTest, LoginAndLogoutShouldResetSettings) {
   EXPECT_TRUE(service_->GetSettings()->GetIsAnalyticsEnabled());
   EXPECT_FALSE(service_->GetAccountManager()->GetCurrentAccount().has_value());
   EXPECT_TRUE(sharing_service_task_runner_->SyncWithTimeout(kTaskWaitTimeout));
+  device_id =
+      preference_manager_.GetString(prefs::kNearbySharingDeviceIdName, "");
+  EXPECT_TRUE(device_id.empty());
 }
 
 TEST_F(NearbySharingServiceImplTest, LoginShouldSetContactsVisibility) {