Fix Sharing device naming

Uppercases every word in manufacturer names.
Also saves chromeos device model names as the family name(chromebook)
instead of model name(eve/sand).

Bug: 1016332
Change-Id: I8d088d1707156529abd1d5e254d7704c990662e3
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1879228Reviewed-by: Mikel Astiz <mastiz@chromium.org>
Reviewed-by: Michael van Ouwerkerk <mvanouwerkerk@chromium.org>
Commit-Queue: Himanshu Jaju <himanshujaju@chromium.org>
Cr-Commit-Position: refs/heads/master@{#710878}

chrome/browser/sharing/sharing_service_unittest.cc

@@ -430,7 +430,7 @@ TEST_F(SharingServiceTest, SendMessageToDeviceSuccess) {
   ASSERT_TRUE(
       sharing_message.ParseFromString(fake_gcm_driver_.message().payload));
   EXPECT_EQ("id", sharing_message.sender_guid());
-  EXPECT_EQ("manufacturer Computer model",
+  EXPECT_EQ("Manufacturer Computer model",
             sharing_message.sender_device_name());
 
   // Simulate ack message received by AckMessageHandler.

chrome/browser/sharing/sharing_utils.cc

@@ -39,6 +39,16 @@ std::string GetDeviceType(sync_pb::SyncEnums::DeviceType type) {
   return l10n_util::GetStringUTF8(device_type_message_id);
 }
 
+std::string CapitalizeWords(const std::string& sentence) {
+  std::string capitalized_sentence;
+  bool use_upper_case = true;
+  for (char ch : sentence) {
+    capitalized_sentence += (use_upper_case ? toupper(ch) : ch);
+    use_upper_case = !isalpha(ch);
+  }
+  return capitalized_sentence;
+}
+
 }  // namespace
 
 SharingDeviceNames GetSharingDeviceNames(const syncer::DeviceInfo* device) {
@@ -46,6 +56,7 @@ SharingDeviceNames GetSharingDeviceNames(const syncer::DeviceInfo* device) {
   SharingDeviceNames device_names;
 
   base::SysInfo::HardwareInfo hardware_info = device->hardware_info();
+  hardware_info.manufacturer = CapitalizeWords(hardware_info.manufacturer);
 
   // The model might be empty if other device is still on M78 or lower with sync
   // turned on.

chrome/browser/sharing/sharing_utils_unittest.cc

@@ -42,8 +42,8 @@ TEST(SharingUtilsTest, GetSharingDeviceNames_ChromeOSDevices) {
       {"google", "Chromebook", ""});
   SharingDeviceNames names = GetSharingDeviceNames(device.get());
 
-  EXPECT_EQ("google Chromebook", names.full_name);
-  EXPECT_EQ("google Chromebook", names.short_name);
+  EXPECT_EQ("Google Chromebook", names.full_name);
+  EXPECT_EQ("Google Chromebook", names.short_name);
 }
 
 TEST(SharingUtilsTest, GetSharingDeviceNames_AndroidPhones) {
@@ -52,8 +52,8 @@ TEST(SharingUtilsTest, GetSharingDeviceNames_AndroidPhones) {
       {"google", "Pixel 2", ""});
   SharingDeviceNames names = GetSharingDeviceNames(device.get());
 
-  EXPECT_EQ("google Phone Pixel 2", names.full_name);
-  EXPECT_EQ("google Phone", names.short_name);
+  EXPECT_EQ("Google Phone Pixel 2", names.full_name);
+  EXPECT_EQ("Google Phone", names.short_name);
 }
 
 TEST(SharingUtilsTest, GetSharingDeviceNames_AndroidTablets) {
@@ -62,8 +62,8 @@ TEST(SharingUtilsTest, GetSharingDeviceNames_AndroidTablets) {
       {"google", "Pixel Slate", ""});
   SharingDeviceNames names = GetSharingDeviceNames(device.get());
 
-  EXPECT_EQ("google Tablet Pixel Slate", names.full_name);
-  EXPECT_EQ("google Tablet", names.short_name);
+  EXPECT_EQ("Google Tablet Pixel Slate", names.full_name);
+  EXPECT_EQ("Google Tablet", names.short_name);
 }
 
 TEST(SharingUtilsTest, GetSharingDeviceNames_Desktops) {
@@ -75,3 +75,37 @@ TEST(SharingUtilsTest, GetSharingDeviceNames_Desktops) {
   EXPECT_EQ("Dell Computer BX123", names.full_name);
   EXPECT_EQ("Dell Computer", names.short_name);
 }
+
+TEST(SharingUtilsTest, CheckManufacturerNameCapitalization) {
+  std::unique_ptr<syncer::DeviceInfo> device = CreateFakeDeviceInfo(
+      "guid", "name", sync_pb::SyncEnums_DeviceType_TYPE_WIN,
+      {"foo bar", "model", ""});
+  SharingDeviceNames names = GetSharingDeviceNames(device.get());
+
+  EXPECT_EQ("Foo Bar Computer model", names.full_name);
+  EXPECT_EQ("Foo Bar Computer", names.short_name);
+
+  device = CreateFakeDeviceInfo("guid", "name",
+                                sync_pb::SyncEnums_DeviceType_TYPE_WIN,
+                                {"foo1bar", "model", ""});
+  names = GetSharingDeviceNames(device.get());
+
+  EXPECT_EQ("Foo1Bar Computer model", names.full_name);
+  EXPECT_EQ("Foo1Bar Computer", names.short_name);
+
+  device = CreateFakeDeviceInfo("guid", "name",
+                                sync_pb::SyncEnums_DeviceType_TYPE_WIN,
+                                {"foo_bar-FOO", "model", ""});
+  names = GetSharingDeviceNames(device.get());
+
+  EXPECT_EQ("Foo_Bar-FOO Computer model", names.full_name);
+  EXPECT_EQ("Foo_Bar-FOO Computer", names.short_name);
+
+  device = CreateFakeDeviceInfo("guid", "name",
+                                sync_pb::SyncEnums_DeviceType_TYPE_WIN,
+                                {"foo&bar foo", "model", ""});
+  names = GetSharingDeviceNames(device.get());
+
+  EXPECT_EQ("Foo&Bar Foo Computer model", names.full_name);
+  EXPECT_EQ("Foo&Bar Foo Computer", names.short_name);
+}

components/sync_device_info/device_info_sync_bridge.cc

@@ -443,6 +443,12 @@ void DeviceInfoSyncBridge::OnHardwareInfoRetrieved(
     base::SysInfo::HardwareInfo hardware_info) {
   local_hardware_info_ = std::move(hardware_info);
 
+#if defined(OS_CHROMEOS)
+  // For ChromeOS the returned model values are product code names like Eve. We
+  // want to use generic names like Chromebook.
+  local_hardware_info_.model = GetChromeOSDeviceNameFromType();
+#endif
+
   auto all_data = std::make_unique<ClientIdToSpecifics>();
   ClientIdToSpecifics* all_data_copy = all_data.get();
 

components/sync_device_info/device_info_sync_bridge.h

@@ -150,7 +150,10 @@ class DeviceInfoSyncBridge : public ModelTypeSyncBridge,
   std::string local_cache_guid_;
   std::string local_personalizable_device_name_;
   ClientIdToSpecifics all_data_;
+
+  // TODO(crbug.com/1019689): Replace hardware info with a custom data type.
   base::SysInfo::HardwareInfo local_hardware_info_;
+
   base::Optional<SyncMode> sync_mode_;
 
   // Registered observers, not owned.

components/sync_device_info/device_info_sync_bridge_unittest.cc

@@ -148,6 +148,11 @@ base::SysInfo::HardwareInfo GetLocalHardwareInfoBlocking() {
   base::SysInfo::GetHardwareInfo(base::BindLambdaForTesting(
       [&](base::SysInfo::HardwareInfo hardware_info) {
         info = std::move(hardware_info);
+#if defined(OS_CHROMEOS)
+        // For ChromeOS the returned model values are product code names like
+        // Eve. We want to use generic names like Chromebook.
+        info.model = GetChromeOSDeviceNameFromType();
+#endif
         run_loop.Quit();
       }));
   run_loop.Run();

components/sync_device_info/local_device_info_util.h

@@ -13,6 +13,10 @@ namespace syncer {
 
 sync_pb::SyncEnums::DeviceType GetLocalDeviceType();
 
+#if defined(OS_CHROMEOS)
+std::string GetChromeOSDeviceNameFromType();
+#endif
+
 // Returns the personalizable device name. This may contain
 // personally-identifiable information - e.g. Alex's MacbookPro.
 std::string GetPersonalizableDeviceNameBlocking();

components/sync_device_info/local_device_info_util_linux.cc

@@ -15,8 +15,8 @@
 
 namespace syncer {
 
-std::string GetPersonalizableDeviceNameInternal() {
 #if defined(OS_CHROMEOS)
+std::string GetChromeOSDeviceNameFromType() {
   switch (chromeos::GetDeviceType()) {
     case chromeos::DeviceType::kChromebase:
       return "Chromebase";
@@ -30,6 +30,12 @@ std::string GetPersonalizableDeviceNameInternal() {
       break;
   }
   return "Chromebook";
+}
+#endif
+
+std::string GetPersonalizableDeviceNameInternal() {
+#if defined(OS_CHROMEOS)
+  return GetChromeOSDeviceNameFromType();
 #else
   char hostname[HOST_NAME_MAX];
   if (gethostname(hostname, HOST_NAME_MAX) == 0)  // Success.