Extract SharingUtils from SharingService with device renaming functions

To move the callback logic to SharingFCMSender, we would also need to
move the device naming part to SharingUtils since we use GetDeviceName
while sending the message using FCM.

Bug: 1018274
Change-Id: I5f67d439a22015105b6202e9bba9f204034a492d
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1887616
Commit-Queue: Himanshu Jaju <himanshujaju@chromium.org>
Reviewed-by: Richard Knoll <knollr@chromium.org>
Cr-Commit-Position: refs/heads/master@{#710744}

chrome/browser/BUILD.gn

@@ -1580,6 +1580,8 @@ jumbo_static_library("browser") {
     "sharing/sharing_service_proxy_android.h",
     "sharing/sharing_sync_preference.cc",
     "sharing/sharing_sync_preference.h",
+    "sharing/sharing_utils.cc",
+    "sharing/sharing_utils.h",
     "sharing/vapid_key_manager.cc",
     "sharing/vapid_key_manager.h",
     "shell_integration.cc",

chrome/browser/sharing/sharing_service.cc

@@ -12,7 +12,6 @@
 #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,9 +26,9 @@
 #include "chrome/browser/sharing/sharing_message_handler.h"
 #include "chrome/browser/sharing/sharing_metrics.h"
 #include "chrome/browser/sharing/sharing_sync_preference.h"
+#include "chrome/browser/sharing/sharing_utils.h"
 #include "chrome/browser/sharing/sms/sms_fetch_request_handler.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"
@@ -37,7 +36,6 @@
 #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"
 
 #if defined(OS_ANDROID)
 #include "chrome/browser/sharing/shared_clipboard/shared_clipboard_message_handler_android.h"
@@ -47,79 +45,6 @@
 #endif  // defined(OS_ANDROID)
 
 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);
-}
-
-struct DeviceNames {
-  std::string full_name;
-  std::string short_name;
-};
-
-// Returns full and short names for |device|.
-DeviceNames GetDeviceNames(const syncer::DeviceInfo* device) {
-  DCHECK(device);
-  DeviceNames device_names;
-
-  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()) {
-    device_names.full_name = device_names.short_name = device->client_name();
-    return device_names;
-  }
-
-  sync_pb::SyncEnums::DeviceType type = device->device_type();
-
-  // For chromeOS, return manufacturer + model.
-  if (type == sync_pb::SyncEnums::TYPE_CROS) {
-    device_names.short_name = device_names.full_name =
-        base::StrCat({hardware_info.manufacturer, " ", hardware_info.model});
-    return device_names;
-  }
-
-  if (hardware_info.manufacturer == "Apple Inc.") {
-    // Internal names of Apple devices are formatted as MacbookPro2,3 or
-    // iPhone2,1 or Ipad4,1.
-    device_names.short_name = hardware_info.model.substr(
-        0, hardware_info.model.find_first_of("0123456789,"));
-    device_names.full_name = hardware_info.model;
-    return device_names;
-  }
-
-  device_names.short_name =
-      base::StrCat({hardware_info.manufacturer, " ", GetDeviceType(type)});
-  device_names.full_name =
-      base::StrCat({device_names.short_name, " ", hardware_info.model});
-  return device_names;
-}
-
 // Clones device with new device name.
 std::unique_ptr<syncer::DeviceInfo> CloneDevice(
     const syncer::DeviceInfo* device,
@@ -245,8 +170,9 @@ std::unique_ptr<syncer::DeviceInfo> SharingService::GetDeviceByGuid(
 
   std::unique_ptr<syncer::DeviceInfo> device_info =
       device_info_tracker_->GetDeviceInfo(guid);
-  return CloneDevice(device_info.get(),
-                     GetDeviceNames(device_info.get()).full_name);
+  device_info->set_client_name(
+      GetSharingDeviceNames(device_info.get()).full_name);
+  return device_info;
 }
 
 SharingService::SharingDeviceList SharingService::GetDeviceCandidates(
@@ -333,7 +259,7 @@ void SharingService::SendMessageToDevice(
   }
 
   std::unique_ptr<syncer::DeviceInfo> sender_device_info = CloneDevice(
-      local_device_info, GetDeviceNames(local_device_info).full_name);
+      local_device_info, GetSharingDeviceNames(local_device_info).full_name);
   sender_device_info->set_sharing_info(
       sync_prefs_->GetLocalSharingInfo(local_device_info));
 
@@ -608,19 +534,19 @@ SharingService::SharingDeviceList SharingService::RenameAndDeduplicateDevices(
                      device2->last_updated_timestamp();
             });
 
-  std::unordered_map<std::string, DeviceNames> device_candidate_names;
+  std::unordered_map<std::string, SharingDeviceNames> device_candidate_names;
   std::unordered_set<std::string> full_device_names;
   std::unordered_map<std::string, int> short_device_name_counter;
 
   // To prevent adding candidates with same full name as local device.
   full_device_names.insert(
-      GetDeviceNames(local_device_info_provider_->GetLocalDeviceInfo())
+      GetSharingDeviceNames(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());
+    SharingDeviceNames device_names = GetSharingDeviceNames(device.get());
 
     // Only insert the first occurrence of each device name.
     auto inserted = full_device_names.insert(device_names.full_name);
@@ -633,17 +559,18 @@ SharingService::SharingDeviceList SharingService::RenameAndDeduplicateDevices(
 
   // Rename filtered devices.
   SharingDeviceList device_candidates;
-  for (const auto& device : devices) {
+  for (auto& device : devices) {
     auto it = device_candidate_names.find(device->guid());
     if (it == device_candidate_names.end())
       continue;
 
-    const DeviceNames& device_names = it->second;
+    const SharingDeviceNames& device_names = it->second;
     bool is_short_name_unique =
         short_device_name_counter[device_names.short_name] == 1;
-    device_candidates.push_back(CloneDevice(
-        device.get(), is_short_name_unique ? device_names.short_name
-                                           : device_names.full_name));
+
+    device->set_client_name(is_short_name_unique ? device_names.short_name
+                                                 : device_names.full_name);
+    device_candidates.push_back(std::move(device));
   }
 
   return device_candidates;

chrome/browser/sharing/sharing_utils.cc

+// Copyright 2019 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "chrome/browser/sharing/sharing_utils.h"
+
+#include "base/strings/strcat.h"
+#include "chrome/grit/generated_resources.h"
+#include "components/sync_device_info/device_info.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);
+}
+
+}  // namespace
+
+SharingDeviceNames GetSharingDeviceNames(const syncer::DeviceInfo* device) {
+  DCHECK(device);
+  SharingDeviceNames device_names;
+
+  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()) {
+    device_names.full_name = device_names.short_name = device->client_name();
+    return device_names;
+  }
+
+  sync_pb::SyncEnums::DeviceType type = device->device_type();
+
+  // For chromeOS, return manufacturer + model.
+  if (type == sync_pb::SyncEnums::TYPE_CROS) {
+    device_names.short_name = device_names.full_name =
+        base::StrCat({hardware_info.manufacturer, " ", hardware_info.model});
+    return device_names;
+  }
+
+  if (hardware_info.manufacturer == "Apple Inc.") {
+    // Internal names of Apple devices are formatted as MacbookPro2,3 or
+    // iPhone2,1 or Ipad4,1.
+    device_names.short_name = hardware_info.model.substr(
+        0, hardware_info.model.find_first_of("0123456789,"));
+    device_names.full_name = hardware_info.model;
+    return device_names;
+  }
+
+  device_names.short_name =
+      base::StrCat({hardware_info.manufacturer, " ", GetDeviceType(type)});
+  device_names.full_name =
+      base::StrCat({device_names.short_name, " ", hardware_info.model});
+  return device_names;
+}

chrome/browser/sharing/sharing_utils.h

+// Copyright 2019 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#ifndef CHROME_BROWSER_SHARING_SHARING_UTILS_H_
+#define CHROME_BROWSER_SHARING_SHARING_UTILS_H_
+
+#include <string>
+
+namespace syncer {
+class DeviceInfo;
+}  // namespace syncer
+
+struct SharingDeviceNames {
+  std::string full_name;
+  std::string short_name;
+};
+
+// Returns full and short names for |device|.
+SharingDeviceNames GetSharingDeviceNames(const syncer::DeviceInfo* device);
+
+#endif  // CHROME_BROWSER_SHARING_SHARING_UTILS_H_

chrome/browser/sharing/sharing_utils_unittest.cc

+// Copyright 2019 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "chrome/browser/sharing/sharing_utils.h"
+
+#include "components/sync_device_info/device_info.h"
+#include "testing/gtest/include/gtest/gtest.h"
+
+namespace {
+static std::unique_ptr<syncer::DeviceInfo> CreateFakeDeviceInfo(
+    const std::string& id,
+    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", device_type, "device_id",
+      hardware_info,
+      /*last_updated_timestamp=*/base::Time::Now(),
+      /*send_tab_to_self_receiving_enabled=*/false,
+      syncer::DeviceInfo::SharingInfo(
+          "fcm_token", "P256dh", "auth_secret",
+          std::set<sync_pb::SharingSpecificFields::EnabledFeatures>{
+              sync_pb::SharingSpecificFields::CLICK_TO_CALL}));
+}
+}  // namespace
+
+TEST(SharingUtilsTest, GetSharingDeviceNames_AppleDevices) {
+  std::unique_ptr<syncer::DeviceInfo> device = CreateFakeDeviceInfo(
+      "guid", "name", sync_pb::SyncEnums_DeviceType_TYPE_MAC,
+      {"Apple Inc.", "MacbookPro1,1", ""});
+  SharingDeviceNames names = GetSharingDeviceNames(device.get());
+
+  EXPECT_EQ("MacbookPro1,1", names.full_name);
+  EXPECT_EQ("MacbookPro", names.short_name);
+}
+
+TEST(SharingUtilsTest, GetSharingDeviceNames_ChromeOSDevices) {
+  std::unique_ptr<syncer::DeviceInfo> device = CreateFakeDeviceInfo(
+      "guid", "name", sync_pb::SyncEnums_DeviceType_TYPE_CROS,
+      {"google", "Chromebook", ""});
+  SharingDeviceNames names = GetSharingDeviceNames(device.get());
+
+  EXPECT_EQ("google Chromebook", names.full_name);
+  EXPECT_EQ("google Chromebook", names.short_name);
+}
+
+TEST(SharingUtilsTest, GetSharingDeviceNames_AndroidPhones) {
+  std::unique_ptr<syncer::DeviceInfo> device = CreateFakeDeviceInfo(
+      "guid", "name", sync_pb::SyncEnums_DeviceType_TYPE_PHONE,
+      {"google", "Pixel 2", ""});
+  SharingDeviceNames names = GetSharingDeviceNames(device.get());
+
+  EXPECT_EQ("google Phone Pixel 2", names.full_name);
+  EXPECT_EQ("google Phone", names.short_name);
+}
+
+TEST(SharingUtilsTest, GetSharingDeviceNames_AndroidTablets) {
+  std::unique_ptr<syncer::DeviceInfo> device = CreateFakeDeviceInfo(
+      "guid", "name", sync_pb::SyncEnums_DeviceType_TYPE_TABLET,
+      {"google", "Pixel Slate", ""});
+  SharingDeviceNames names = GetSharingDeviceNames(device.get());
+
+  EXPECT_EQ("google Tablet Pixel Slate", names.full_name);
+  EXPECT_EQ("google Tablet", names.short_name);
+}
+
+TEST(SharingUtilsTest, GetSharingDeviceNames_Desktops) {
+  std::unique_ptr<syncer::DeviceInfo> device = CreateFakeDeviceInfo(
+      "guid", "name", sync_pb::SyncEnums_DeviceType_TYPE_WIN,
+      {"Dell", "BX123", ""});
+  SharingDeviceNames names = GetSharingDeviceNames(device.get());
+
+  EXPECT_EQ("Dell Computer BX123", names.full_name);
+  EXPECT_EQ("Dell Computer", names.short_name);
+}

chrome/test/BUILD.gn

@@ -3229,6 +3229,7 @@ test("unit_tests") {
     "../browser/sharing/sharing_fcm_sender_unittest.cc",
     "../browser/sharing/sharing_service_unittest.cc",
     "../browser/sharing/sharing_sync_preference_unittest.cc",
+    "../browser/sharing/sharing_utils_unittest.cc",
     "../browser/sharing/vapid_key_manager_unittest.cc",
     "../browser/shell_integration_win_unittest.cc",
     "../browser/signin/account_consistency_mode_manager_unittest.cc",