Rename devices used in Sharing.

After introduction of transport only sync mode, we need to make device
names consistent across both sync modes in Sharing. This has introduced
dynamic naming in Sharing based on rules.

Screenshot - https://drive.google.com/open?id=12wFSp0bSmQJL3-S5Gw_2IgqTVvMNH_Ls

Bug: 1013565
Change-Id: I84d4ef3172523c9d81794b099c6e08178525dc15
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1856321
Commit-Queue: Himanshu Jaju <himanshujaju@chromium.org>
Reviewed-by: Alex Chau <alexchau@chromium.org>
Cr-Commit-Position: refs/heads/master@{#705896}

chrome/app/generated_resources.grd

@@ -6652,6 +6652,19 @@ the Bookmarks menu.">
         Download
       </message>
 
+    <!-- Different device types using in Sharing. -->
+      <message name="IDS_BROWSER_SHARING_DEVICE_TYPE_COMPUTER" desc = "The label for a device of type computer.">
+        Computer
+      </message>
+      <message name="IDS_BROWSER_SHARING_DEVICE_TYPE_DEVICE" desc = "The label for a generic device.">
+        Device
+      </message>
+      <message name="IDS_BROWSER_SHARING_DEVICE_TYPE_PHONE" desc = "The label for a device of type phone.">
+        Phone
+      </message>
+      <message name="IDS_BROWSER_SHARING_DEVICE_TYPE_TABLET" desc = "The label for a device of type tablet.">
+        Tablet
+      </message>
 
       <!-- Sharing features. -->
       <if expr="not use_titlecase">

chrome/app/generated_resources_grd/IDS_BROWSER_SHARING_DEVICE_TYPE_COMPUTER.png.sha1

+e2f58aa03825c6decf2d008aabb2da8d33a4af78
\ No newline at end of file

chrome/app/generated_resources_grd/IDS_BROWSER_SHARING_DEVICE_TYPE_DEVICE.png.sha1

+e2f58aa03825c6decf2d008aabb2da8d33a4af78
\ No newline at end of file

chrome/app/generated_resources_grd/IDS_BROWSER_SHARING_DEVICE_TYPE_PHONE.png.sha1

+e2f58aa03825c6decf2d008aabb2da8d33a4af78
\ No newline at end of file

chrome/app/generated_resources_grd/IDS_BROWSER_SHARING_DEVICE_TYPE_TABLET.png.sha1

+e2f58aa03825c6decf2d008aabb2da8d33a4af78
\ No newline at end of file

chrome/browser/sharing/sharing_browsertest.cc

@@ -79,15 +79,19 @@ void SharingBrowserTest::SetUpDevices(
     std::unique_ptr<syncer::DeviceInfo> fake_device =
         std::make_unique<syncer::DeviceInfo>(
             device->guid(),
-            base::StrCat(
-                {"testing_device_", base::NumberToString(device_id++)}),
+            base::StrCat({"testing_device_", base::NumberToString(device_id)}),
             device->chrome_version(), device->sync_user_agent(),
             device->device_type(), device->signin_scoped_device_id(),
-            device->hardware_info(), device->last_updated_timestamp(),
+            base::SysInfo::HardwareInfo{
+                "Google",
+                base::StrCat({"model", base::NumberToString(device_id)}),
+                "serial_number"},
+            device->last_updated_timestamp(),
             device->send_tab_to_self_receiving_enabled(),
             device->sharing_info());
     fake_device_info_tracker_.Add(fake_device.get());
     device_infos_.push_back(std::move(fake_device));
+    device_id++;
   }
 }
 

chrome/browser/sharing/sharing_service.cc

@@ -12,6 +12,7 @@
 #include "base/bind.h"
 #include "base/feature_list.h"
 #include "base/guid.h"
+#include "base/strings/strcat.h"
 #include "base/strings/utf_string_conversions.h"
 #include "base/task/post_task.h"
 #include "base/time/time.h"
@@ -27,12 +28,108 @@
 #include "chrome/browser/sharing/sharing_metrics.h"
 #include "chrome/browser/sharing/sharing_sync_preference.h"
 #include "chrome/browser/sharing/vapid_key_manager.h"
+#include "chrome/grit/generated_resources.h"
 #include "components/gcm_driver/crypto/gcm_encryption_provider.h"
 #include "components/gcm_driver/gcm_driver.h"
 #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 "content/public/browser/browser_task_traits.h"
+#include "ui/base/l10n/l10n_util.h"
+
+namespace {
+// Util function to return a string denoting the type of device.
+std::string GetDeviceType(sync_pb::SyncEnums::DeviceType type) {
+  int device_type_message_id = -1;
+
+  switch (type) {
+    case sync_pb::SyncEnums::TYPE_LINUX:
+    case sync_pb::SyncEnums::TYPE_WIN:
+    case sync_pb::SyncEnums::TYPE_CROS:
+    case sync_pb::SyncEnums::TYPE_MAC:
+      device_type_message_id = IDS_BROWSER_SHARING_DEVICE_TYPE_COMPUTER;
+      break;
+
+    case sync_pb::SyncEnums::TYPE_UNSET:
+    case sync_pb::SyncEnums::TYPE_OTHER:
+      device_type_message_id = IDS_BROWSER_SHARING_DEVICE_TYPE_DEVICE;
+      break;
+
+    case sync_pb::SyncEnums::TYPE_PHONE:
+      device_type_message_id = IDS_BROWSER_SHARING_DEVICE_TYPE_PHONE;
+      break;
+
+    case sync_pb::SyncEnums::TYPE_TABLET:
+      device_type_message_id = IDS_BROWSER_SHARING_DEVICE_TYPE_TABLET;
+      break;
+  }
+
+  return l10n_util::GetStringUTF8(device_type_message_id);
+}
+
+// Util function to get name for a single device. |is_full_name| determines
+// whether we should add the model name for the device.
+std::string GetDeviceName(const syncer::DeviceInfo* device, bool is_full_name) {
+  DCHECK(device);
+
+  base::SysInfo::HardwareInfo hardware_info = device->hardware_info();
+
+  // The model might be empty if other device is still on M78 or lower with sync
+  // turned on.
+  if (hardware_info.model.empty())
+    return device->client_name();
+
+  sync_pb::SyncEnums::DeviceType type = device->device_type();
+
+  // For chromeOS, return manufacturer + model.
+  if (type == sync_pb::SyncEnums::TYPE_CROS) {
+    return base::StrCat({device->hardware_info().manufacturer, " ",
+                         device->hardware_info().model});
+  }
+
+  if (hardware_info.manufacturer == "Apple Inc.") {
+    if (is_full_name)
+      return hardware_info.model;
+
+    // Returns a more readable hardware model. Internal names of Apple devices
+    // are formatted as MacbookPro2,3 or iPhone2,1 or Ipad4,1.
+    return hardware_info.model.substr(
+        0, hardware_info.model.find_first_of("0123456789,"));
+  }
+
+  std::string device_name =
+      base::StrCat({hardware_info.manufacturer, " ", GetDeviceType(type)});
+  if (is_full_name)
+    base::StrAppend(&device_name, {" ", hardware_info.model});
+
+  return device_name;
+}
+
+// Util function to rename devices for Sharing.
+std::vector<std::unique_ptr<syncer::DeviceInfo>> RenameDevices(
+    const std::vector<std::unique_ptr<syncer::DeviceInfo>>& devices) {
+  std::map<std::string, int> similar_devices_counter;
+  for (const auto& device : devices)
+    similar_devices_counter[GetDeviceName(device.get(),
+                                          /*is_full_name=*/false)]++;
+
+  std::vector<std::unique_ptr<syncer::DeviceInfo>> renamed_devices;
+  for (const auto& device : devices) {
+    bool is_full_name_required = similar_devices_counter[GetDeviceName(
+                                     device.get(), /*is_full_name=*/false)] > 1;
+    std::string device_name =
+        GetDeviceName(device.get(), is_full_name_required);
+
+    renamed_devices.push_back(std::make_unique<syncer::DeviceInfo>(
+        device->guid(), device_name, device->chrome_version(),
+        device->sync_user_agent(), device->device_type(),
+        device->signin_scoped_device_id(), device->hardware_info(),
+        device->last_updated_timestamp(),
+        device->send_tab_to_self_receiving_enabled(), device->sharing_info()));
+  }
+  return renamed_devices;
+}
+}  // namespace
 
 SharingService::SharingService(
     std::unique_ptr<SharingSyncPreference> sync_prefs,
@@ -128,6 +225,8 @@ std::unique_ptr<syncer::DeviceInfo> SharingService::GetDeviceByGuid(
   return device_info_tracker_->GetDeviceInfo(guid);
 }
 
+// TODO(crbug.com/1014107) - Simplify this function. The function does a number
+// of things and some calls are duplicated at the moment.
 std::vector<std::unique_ptr<syncer::DeviceInfo>>
 SharingService::GetDeviceCandidates(
     sync_pb::SharingSpecificFields::EnabledFeatures required_feature) const {
@@ -149,7 +248,11 @@ SharingService::GetDeviceCandidates(
                      device2->last_updated_timestamp();
             });
 
-  std::unordered_set<std::string> device_names;
+  std::unordered_set<std::string> full_device_names;
+  // To prevent adding candidates with same name as local device.
+  full_device_names.insert(
+      GetDeviceName(local_device_info, /*is_full_name=*/true));
+
   for (auto& device : all_devices) {
     // If the current device is considered expired for our purposes, stop here
     // since the next devices in the vector are at least as expired than this
@@ -157,9 +260,6 @@ SharingService::GetDeviceCandidates(
     if (device->last_updated_timestamp() < min_updated_time)
       break;
 
-    if (local_device_info->client_name() == device->client_name())
-      continue;
-
     base::Optional<syncer::DeviceInfo::SharingInfo> sharing_info =
         sync_prefs_->GetSharingInfo(device.get());
     if (!sharing_info ||
@@ -168,7 +268,8 @@ SharingService::GetDeviceCandidates(
     }
 
     // Only insert the first occurrence of each device name.
-    auto inserted = device_names.insert(device->client_name());
+    auto inserted = full_device_names.insert(
+        GetDeviceName(device.get(), /*is_full_name=*/true));
     if (inserted.second)
       device_candidates.push_back(std::move(device));
   }
@@ -176,6 +277,8 @@ SharingService::GetDeviceCandidates(
   // TODO(knollr): Remove devices from |sync_prefs_| that are in
   // |synced_devices| but not in |all_devices|?
 
+  device_candidates = RenameDevices(device_candidates);
+
   return device_candidates;
 }
 

chrome/browser/sharing/sharing_service_unittest.cc

@@ -203,11 +203,14 @@ class SharingServiceTest : public testing::Test {
  protected:
   static std::unique_ptr<syncer::DeviceInfo> CreateFakeDeviceInfo(
       const std::string& id,
-      const std::string& name) {
+      const std::string& name,
+      sync_pb::SyncEnums_DeviceType device_type =
+          sync_pb::SyncEnums_DeviceType_TYPE_LINUX,
+      base::SysInfo::HardwareInfo hardware_info =
+          base::SysInfo::HardwareInfo()) {
     return std::make_unique<syncer::DeviceInfo>(
-        id, name, "chrome_version", "user_agent",
-        sync_pb::SyncEnums_DeviceType_TYPE_LINUX, "device_id",
-        base::SysInfo::HardwareInfo(),
+        id, name, "chrome_version", "user_agent", device_type, "device_id",
+        hardware_info,
         /*last_updated_timestamp=*/base::Time::Now(),
         /*send_tab_to_self_receiving_enabled=*/false,
         syncer::DeviceInfo::SharingInfo(
@@ -342,9 +345,11 @@ TEST_F(SharingServiceTest, GetDeviceCandidates_MissingRequirements) {
 
 TEST_F(SharingServiceTest, GetDeviceCandidates_DuplicateDeviceNames) {
   // Add first device.
+  base::SysInfo::HardwareInfo info{"Google", "Pixel C", "serialno"};
+
   std::string id1 = base::GenerateGUID();
-  std::unique_ptr<syncer::DeviceInfo> device_info_1 =
-      CreateFakeDeviceInfo(id1, kDeviceName);
+  std::unique_ptr<syncer::DeviceInfo> device_info_1 = CreateFakeDeviceInfo(
+      id1, "Device1", sync_pb::SyncEnums_DeviceType_TYPE_TABLET, info);
   fake_device_info_sync_service.GetDeviceInfoTracker()->Add(
       device_info_1.get());
 
@@ -353,15 +358,19 @@ TEST_F(SharingServiceTest, GetDeviceCandidates_DuplicateDeviceNames) {
 
   // Add second device.
   std::string id2 = base::GenerateGUID();
-  std::unique_ptr<syncer::DeviceInfo> device_info_2 =
-      CreateFakeDeviceInfo(id2, kDeviceName);
+  std::unique_ptr<syncer::DeviceInfo> device_info_2 = CreateFakeDeviceInfo(
+      id2, "Device2", sync_pb::SyncEnums_DeviceType_TYPE_TABLET, info);
   fake_device_info_sync_service.GetDeviceInfoTracker()->Add(
       device_info_2.get());
 
-  // Add third device which is same as local device ("name").
+  // Add third device which is same as local device except guid.
   std::string id3 = base::GenerateGUID();
-  std::unique_ptr<syncer::DeviceInfo> device_info_3 =
-      CreateFakeDeviceInfo(id3, "name");
+  const syncer::DeviceInfo* local_device_info =
+      fake_device_info_sync_service.GetLocalDeviceInfoProvider()
+          ->GetLocalDeviceInfo();
+  std::unique_ptr<syncer::DeviceInfo> device_info_3 = CreateFakeDeviceInfo(
+      id3, local_device_info->client_name(), local_device_info->device_type(),
+      local_device_info->hardware_info());
   fake_device_info_sync_service.GetDeviceInfoTracker()->Add(
       device_info_3.get());
 
@@ -744,3 +753,133 @@ TEST_F(SharingServiceTest, DeviceCandidatesReadyAfterAddObserver) {
 
   ASSERT_TRUE(device_candidates_initialized_);
 }
+
+TEST_F(SharingServiceTest, DeviceCandidatesNames_Computers) {
+  std::unique_ptr<syncer::DeviceInfo> computer1 = CreateFakeDeviceInfo(
+      base::GenerateGUID(), "Fake device 1",
+      sync_pb::SyncEnums_DeviceType_TYPE_WIN, {"Dell", "PC3999", "sno one"});
+
+  fake_device_info_sync_service.GetDeviceInfoTracker()->Add(computer1.get());
+  std::vector<std::unique_ptr<syncer::DeviceInfo>> candidates =
+      GetSharingService()->GetDeviceCandidates(
+          sync_pb::SharingSpecificFields::CLICK_TO_CALL);
+
+  ASSERT_EQ(1u, candidates.size());
+  EXPECT_EQ("Dell Computer", candidates[0]->client_name());
+
+  task_environment_.FastForwardBy(base::TimeDelta::FromSeconds(10));
+
+  std::unique_ptr<syncer::DeviceInfo> computer2 = CreateFakeDeviceInfo(
+      base::GenerateGUID(), "Fake device 2",
+      sync_pb::SyncEnums_DeviceType_TYPE_LINUX, {"Dell", "PC3998", "sno two"});
+
+  fake_device_info_sync_service.GetDeviceInfoTracker()->Add(computer2.get());
+  candidates = GetSharingService()->GetDeviceCandidates(
+      sync_pb::SharingSpecificFields::CLICK_TO_CALL);
+
+  ASSERT_EQ(2u, candidates.size());
+  EXPECT_EQ("Dell Computer PC3998", candidates[0]->client_name());
+  EXPECT_EQ("Dell Computer PC3999", candidates[1]->client_name());
+}
+
+TEST_F(SharingServiceTest, DeviceCandidatesNames_AppleDevices) {
+  std::unique_ptr<syncer::DeviceInfo> computer1 =
+      CreateFakeDeviceInfo(base::GenerateGUID(), "Fake device 1",
+                           sync_pb::SyncEnums_DeviceType_TYPE_TABLET,
+                           {"Apple Inc.", "iPad2,2", "sno one"});
+  fake_device_info_sync_service.GetDeviceInfoTracker()->Add(computer1.get());
+
+  task_environment_.FastForwardBy(base::TimeDelta::FromSeconds(10));
+
+  std::unique_ptr<syncer::DeviceInfo> computer2 =
+      CreateFakeDeviceInfo(base::GenerateGUID(), "Fake device 2",
+                           sync_pb::SyncEnums_DeviceType_TYPE_PHONE,
+                           {"Apple Inc.", "iPhone1,1", "sno two"});
+  fake_device_info_sync_service.GetDeviceInfoTracker()->Add(computer2.get());
+
+  task_environment_.FastForwardBy(base::TimeDelta::FromSeconds(10));
+
+  std::unique_ptr<syncer::DeviceInfo> computer3 =
+      CreateFakeDeviceInfo(base::GenerateGUID(), "Fake device 3",
+                           sync_pb::SyncEnums_DeviceType_TYPE_MAC,
+                           {"Apple Inc.", "MacbookPro1,1", "sno three"});
+  fake_device_info_sync_service.GetDeviceInfoTracker()->Add(computer3.get());
+
+  std::vector<std::unique_ptr<syncer::DeviceInfo>> candidates =
+      GetSharingService()->GetDeviceCandidates(
+          sync_pb::SharingSpecificFields::CLICK_TO_CALL);
+
+  ASSERT_EQ(3u, candidates.size());
+  EXPECT_EQ("MacbookPro", candidates[0]->client_name());
+  EXPECT_EQ("iPhone", candidates[1]->client_name());
+  EXPECT_EQ("iPad", candidates[2]->client_name());
+}
+
+TEST_F(SharingServiceTest, DeviceCandidatesNames_AndroidDevices) {
+  std::unique_ptr<syncer::DeviceInfo> computer1 =
+      CreateFakeDeviceInfo(base::GenerateGUID(), "Fake device 1",
+                           sync_pb::SyncEnums_DeviceType_TYPE_TABLET,
+                           {"Google", "Pixel Slate", "sno one"});
+  fake_device_info_sync_service.GetDeviceInfoTracker()->Add(computer1.get());
+
+  std::vector<std::unique_ptr<syncer::DeviceInfo>> candidates =
+      GetSharingService()->GetDeviceCandidates(
+          sync_pb::SharingSpecificFields::CLICK_TO_CALL);
+
+  ASSERT_EQ(1u, candidates.size());
+  EXPECT_EQ("Google Tablet", candidates[0]->client_name());
+
+  task_environment_.FastForwardBy(base::TimeDelta::FromSeconds(10));
+
+  std::unique_ptr<syncer::DeviceInfo> computer2 =
+      CreateFakeDeviceInfo(base::GenerateGUID(), "Fake device 2",
+                           sync_pb::SyncEnums_DeviceType_TYPE_PHONE,
+                           {"Google", "Pixel 3", "sno two"});
+  fake_device_info_sync_service.GetDeviceInfoTracker()->Add(computer2.get());
+
+  candidates = GetSharingService()->GetDeviceCandidates(
+      sync_pb::SharingSpecificFields::CLICK_TO_CALL);
+
+  ASSERT_EQ(2u, candidates.size());
+  EXPECT_EQ("Google Phone", candidates[0]->client_name());
+  EXPECT_EQ("Google Tablet", candidates[1]->client_name());
+
+  task_environment_.FastForwardBy(base::TimeDelta::FromSeconds(10));
+
+  std::unique_ptr<syncer::DeviceInfo> computer3 =
+      CreateFakeDeviceInfo(base::GenerateGUID(), "Fake device 3",
+                           sync_pb::SyncEnums_DeviceType_TYPE_PHONE,
+                           {"Google", "Pixel 2", "sno three"});
+  fake_device_info_sync_service.GetDeviceInfoTracker()->Add(computer3.get());
+
+  candidates = GetSharingService()->GetDeviceCandidates(
+      sync_pb::SharingSpecificFields::CLICK_TO_CALL);
+
+  ASSERT_EQ(3u, candidates.size());
+  EXPECT_EQ("Google Phone Pixel 2", candidates[0]->client_name());
+  EXPECT_EQ("Google Phone Pixel 3", candidates[1]->client_name());
+  EXPECT_EQ("Google Tablet", candidates[2]->client_name());
+}
+
+TEST_F(SharingServiceTest, DeviceCandidatesNames_Chromebooks) {
+  std::unique_ptr<syncer::DeviceInfo> computer1 =
+      CreateFakeDeviceInfo(base::GenerateGUID(), "Fake device 1",
+                           sync_pb::SyncEnums_DeviceType_TYPE_CROS,
+                           {"Dell", "Chromebook", "sno one"});
+  fake_device_info_sync_service.GetDeviceInfoTracker()->Add(computer1.get());
+
+  task_environment_.FastForwardBy(base::TimeDelta::FromSeconds(10));
+
+  std::unique_ptr<syncer::DeviceInfo> computer2 = CreateFakeDeviceInfo(
+      base::GenerateGUID(), "Fake device 2",
+      sync_pb::SyncEnums_DeviceType_TYPE_CROS, {"HP", "Chromebook", "sno one"});
+  fake_device_info_sync_service.GetDeviceInfoTracker()->Add(computer2.get());
+
+  std::vector<std::unique_ptr<syncer::DeviceInfo>> candidates =
+      GetSharingService()->GetDeviceCandidates(
+          sync_pb::SharingSpecificFields::CLICK_TO_CALL);
+
+  ASSERT_EQ(2u, candidates.size());
+  EXPECT_EQ("HP Chromebook", candidates[0]->client_name());
+  EXPECT_EQ("Dell Chromebook", candidates[1]->client_name());
+}