[hid] Handle devices with empty product name strings

HID peripherals may provide a product name string. When a device
has no product name string or the string is empty, generate a
placeholder string based on the vendor and product IDs. This
allows the device to be identified in permissions contexts.

BUG=1126182

Change-Id: I756f2a5d3bd8519fd4aa7855afb5b2ac8b3f6114
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2487724
Commit-Queue: Matt Reynolds <mattreynolds@chromium.org>
Reviewed-by: Reilly Grant <reillyg@chromium.org>
Cr-Commit-Position: refs/heads/master@{#824075}

chrome/browser/hid/hid_chooser_context.cc

@@ -6,13 +6,16 @@
 
 #include <utility>
 
+#include "base/strings/stringprintf.h"
 #include "base/strings/utf_string_conversions.h"
 #include "base/values.h"
 #include "chrome/browser/content_settings/host_content_settings_map_factory.h"
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/usb/usb_blocklist.h"
+#include "chrome/grit/generated_resources.h"
 #include "components/content_settings/core/common/content_settings_types.h"
 #include "content/public/browser/device_service.h"
+#include "ui/base/l10n/l10n_util.h"
 
 namespace {
 
@@ -23,12 +26,14 @@ constexpr char kHidProductIdKey[] = "product-id";
 constexpr char kHidSerialNumberKey[] = "serial-number";
 
 bool CanStorePersistentEntry(const device::mojom::HidDeviceInfo& device) {
-  return !device.serial_number.empty();
+  return !device.serial_number.empty() && !device.product_name.empty();
 }
 
 base::Value DeviceInfoToValue(const device::mojom::HidDeviceInfo& device) {
   base::Value value(base::Value::Type::DICTIONARY);
-  value.SetStringKey(kHidDeviceNameKey, device.product_name);
+  value.SetStringKey(
+      kHidDeviceNameKey,
+      base::UTF16ToUTF8(HidChooserContext::DisplayNameFromDeviceInfo(device)));
   value.SetIntKey(kHidVendorIdKey, device.vendor_id);
   value.SetIntKey(kHidProductIdKey, device.product_id);
   if (CanStorePersistentEntry(device)) {
@@ -70,6 +75,22 @@ HidChooserContext::~HidChooserContext() {
   DCHECK(!permission_observer_list_.might_have_observers());
 }
 
+// static
+base::string16 HidChooserContext::DisplayNameFromDeviceInfo(
+    const device::mojom::HidDeviceInfo& device) {
+  auto vendor_id_string =
+      base::ASCIIToUTF16(base::StringPrintf("0x%04x", device.vendor_id));
+  auto product_id_string =
+      base::ASCIIToUTF16(base::StringPrintf("0x%04x", device.product_id));
+  if (device.product_name.empty()) {
+    return l10n_util::GetStringFUTF16(IDS_HID_CHOOSER_ITEM_WITHOUT_NAME,
+                                      vendor_id_string, product_id_string);
+  }
+  return l10n_util::GetStringFUTF16(IDS_HID_CHOOSER_ITEM_WITH_NAME,
+                                    base::UTF8ToUTF16(device.product_name),
+                                    vendor_id_string, product_id_string);
+}
+
 base::string16 HidChooserContext::GetObjectDisplayName(
     const base::Value& object) {
   const std::string* name = object.FindStringKey(kHidDeviceNameKey);

chrome/browser/hid/hid_chooser_context.h

@@ -33,11 +33,6 @@ class Value;
 class HidChooserContext : public permissions::ChooserContextBase,
                           public device::mojom::HidManagerClient {
  public:
-  explicit HidChooserContext(Profile* profile);
-  HidChooserContext(const HidChooserContext&) = delete;
-  HidChooserContext& operator=(const HidChooserContext&) = delete;
-  ~HidChooserContext() override;
-
   // This observer can be used to be notified when HID devices are connected or
   // disconnected.
   class DeviceObserver : public base::CheckedObserver {
@@ -51,6 +46,15 @@ class HidChooserContext : public permissions::ChooserContextBase,
     virtual void OnHidChooserContextShutdown() = 0;
   };
 
+  explicit HidChooserContext(Profile* profile);
+  HidChooserContext(const HidChooserContext&) = delete;
+  HidChooserContext& operator=(const HidChooserContext&) = delete;
+  ~HidChooserContext() override;
+
+  // Returns a human-readable string identifier for |device|.
+  static base::string16 DisplayNameFromDeviceInfo(
+      const device::mojom::HidDeviceInfo& device);
+
   // permissions::ChooserContextBase implementation:
   bool IsValidObject(const base::Value& object) override;
   // In addition these methods from ChooserContextBase are overridden in order

chrome/browser/ui/hid/hid_chooser_controller.cc

@@ -57,8 +57,6 @@ HidChooserController::HidChooserController(
 
   chooser_context_->GetHidManager()->GetDevices(base::BindOnce(
       &HidChooserController::OnGotDevices, weak_factory_.GetWeakPtr()));
-
-  // TODO(mattreynolds): Register to receive device added and removed events.
 }
 
 HidChooserController::~HidChooserController() {
@@ -86,16 +84,7 @@ base::string16 HidChooserController::GetOption(size_t index) const {
   DCHECK_LT(index, items_.size());
   DCHECK(base::Contains(device_map_, items_[index]));
   const auto& device = *device_map_.find(items_[index])->second.front();
-  if (device.product_name.empty()) {
-    return l10n_util::GetStringFUTF16(
-        IDS_HID_CHOOSER_ITEM_WITHOUT_NAME,
-        base::ASCIIToUTF16(base::StringPrintf("0x%04x", device.vendor_id)),
-        base::ASCIIToUTF16(base::StringPrintf("0x%04x", device.product_id)));
-  }
-  return l10n_util::GetStringFUTF16(
-      IDS_HID_CHOOSER_ITEM_WITH_NAME, base::UTF8ToUTF16(device.product_name),
-      base::ASCIIToUTF16(base::StringPrintf("0x%04x", device.vendor_id)),
-      base::ASCIIToUTF16(base::StringPrintf("0x%04x", device.product_id)));
+  return HidChooserContext::DisplayNameFromDeviceInfo(device);
 }
 
 bool HidChooserController::IsPaired(size_t index) const {