Fix local client showing up in device list

Issue - If a user has two instances of Chrome, one being M78-
and the other being M79+, we show the local device in list
of device candidates in M79 instance. This is because the deduplication
logic is based on the new device naming which is based on
hardware info which is available only on M79+ devices.

Fix - Add PersonalizableDeviceName to list of names used for deduplication.

Bug: 1018159
Change-Id: Ie34b0ad0c1b99a745b088211480b419674b45f63
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1879914Reviewed-by: Alex Chau <alexchau@chromium.org>
Reviewed-by: Richard Knoll <knollr@chromium.org>
Commit-Queue: Himanshu Jaju <himanshujaju@chromium.org>
Cr-Commit-Position: refs/heads/master@{#709523}

chrome/browser/sharing/sharing_service.cc

@@ -35,6 +35,7 @@
 #include "components/sync/driver/sync_service.h"
 #include "components/sync_device_info/device_info.h"
 #include "components/sync_device_info/local_device_info_provider.h"
+#include "components/sync_device_info/local_device_info_util.h"
 #include "content/public/browser/browser_task_traits.h"
 #include "ui/base/l10n/l10n_util.h"
 
@@ -130,6 +131,7 @@ std::unique_ptr<syncer::DeviceInfo> CloneDevice(
       device->last_updated_timestamp(),
       device->send_tab_to_self_receiving_enabled(), device->sharing_info());
 }
+
 }  // namespace
 
 SharingService::SharingService(
@@ -154,7 +156,7 @@ SharingService::SharingService(
       backoff_entry_(&kRetryBackoffPolicy),
       state_(State::DISABLED),
       is_observing_device_info_tracker_(false) {
-  // Remove old encryption info with empty authrozed_entity to avoid DCHECK.
+  // Remove old encryption info with empty authorized_entity to avoid DCHECK.
   // See http://crbug/987591
   if (gcm_driver) {
     gcm::GCMEncryptionProvider* encryption_provider =
@@ -222,6 +224,14 @@ SharingService::SharingService(
     // and only doing clean up via UnregisterDevice().
     UnregisterDevice();
   }
+
+  base::PostTaskAndReplyWithResult(
+      FROM_HERE,
+      {base::ThreadPool(), base::MayBlock(),
+       base::TaskShutdownBehavior::SKIP_ON_SHUTDOWN},
+      base::BindOnce(syncer::GetPersonalizableDeviceNameBlocking),
+      base::BindOnce(&SharingService::InitPersonalizableLocalDeviceName,
+                     weak_ptr_factory_.GetWeakPtr()));
 }
 
 SharingService::~SharingService() {
@@ -242,7 +252,8 @@ std::unique_ptr<syncer::DeviceInfo> SharingService::GetDeviceByGuid(
 
 SharingService::SharingDeviceList SharingService::GetDeviceCandidates(
     sync_pb::SharingSpecificFields::EnabledFeatures required_feature) const {
-  if (IsSyncDisabled() || !local_device_info_provider_->GetLocalDeviceInfo())
+  if (IsSyncDisabled() || !local_device_info_provider_->GetLocalDeviceInfo() ||
+      !personalizable_local_device_name_)
     return {};
 
   SharingDeviceList device_candidates =
@@ -640,6 +651,8 @@ SharingService::SharingDeviceList SharingService::RenameAndDeduplicateDevices(
   full_device_names.insert(
       GetDeviceNames(local_device_info_provider_->GetLocalDeviceInfo())
           .full_name);
+  // To prevent M78- instances of Chrome with same device model from showing up.
+  full_device_names.insert(*personalizable_local_device_name_);
 
   for (const auto& device : devices) {
     DeviceNames device_names = GetDeviceNames(device.get());
@@ -670,3 +683,9 @@ SharingService::SharingDeviceList SharingService::RenameAndDeduplicateDevices(
 
   return device_candidates;
 }
+
+void SharingService::InitPersonalizableLocalDeviceName(
+    std::string personalizable_local_device_name) {
+  personalizable_local_device_name_ =
+      std::move(personalizable_local_device_name);
+}

chrome/browser/sharing/sharing_service.h

@@ -183,6 +183,9 @@ class SharingService : public KeyedService,
   SharingDeviceList RenameAndDeduplicateDevices(
       SharingDeviceList devices) const;
 
+  void InitPersonalizableLocalDeviceName(
+      std::string personalizable_local_device_name);
+
   std::unique_ptr<SharingSyncPreference> sync_prefs_;
   std::unique_ptr<VapidKeyManager> vapid_key_manager_;
   std::unique_ptr<SharingDeviceRegistration> sharing_device_registration_;
@@ -198,6 +201,9 @@ class SharingService : public KeyedService,
   bool is_observing_device_info_tracker_;
   std::unique_ptr<syncer::LocalDeviceInfoProvider::Subscription>
       local_device_info_ready_subscription_;
+  // The personalized name is stored for deduplicating devices running older
+  // clients.
+  base::Optional<std::string> personalizable_local_device_name_;
 
   // List of callbacks for AddDeviceCandidatesInitializedObserver.
   std::vector<base::OnceClosure> device_candidates_initialized_callbacks_;

chrome/browser/sharing/sharing_service_unittest.cc

@@ -28,6 +28,7 @@
 #include "components/sync_device_info/device_info.h"
 #include "components/sync_device_info/fake_device_info_sync_service.h"
 #include "components/sync_device_info/local_device_info_provider.h"
+#include "components/sync_device_info/local_device_info_util.h"
 #include "components/sync_preferences/testing_pref_service_syncable.h"
 #include "content/public/test/browser_task_environment.h"
 #include "crypto/ec_private_key.h"
@@ -251,6 +252,7 @@ class SharingServiceTest : public testing::Test {
           &test_sync_service_,
           /* notification_display_service= */ nullptr);
     }
+    task_environment_.RunUntilIdle();
     return sharing_service_.get();
   }
 
@@ -964,3 +966,22 @@ TEST_F(SharingServiceTest, GetDeviceByGuid) {
       GetSharingService()->GetDeviceByGuid(guid);
   EXPECT_EQ("Dell Computer sno one", device_info->client_name());
 }
+
+// Tests transition from M78- to M79+ where same local device should not show up
+// by also checking for personalizable client name.
+TEST_F(SharingServiceTest,
+       DeduplicateLocalDeviceOnClientName_HardwareInfoNotAvailable) {
+  const std::string local_client_name =
+      syncer::GetPersonalizableDeviceNameBlocking();
+
+  std::string guid = base::GenerateGUID();
+  std::unique_ptr<syncer::DeviceInfo> computer = CreateFakeDeviceInfo(
+      guid, local_client_name, sync_pb::SyncEnums_DeviceType_TYPE_LINUX, {});
+  fake_device_info_sync_service.GetDeviceInfoTracker()->Add(computer.get());
+
+  std::vector<std::unique_ptr<syncer::DeviceInfo>> candidates =
+      GetSharingService()->GetDeviceCandidates(
+          sync_pb::SharingSpecificFields::CLICK_TO_CALL);
+
+  EXPECT_TRUE(candidates.empty());
+}