Update device name in chooser when device changes name (desktops)

chrome/browser/ui/bluetooth/bluetooth_chooser_controller.cc

Index: chrome/browser/ui/bluetooth/bluetooth_chooser_controller.cc
diff --git a/chrome/browser/ui/bluetooth/bluetooth_chooser_controller.cc b/chrome/browser/ui/bluetooth/bluetooth_chooser_controller.cc
index 49dccb9597a74dc264ad4d833939de0df9a7e93d..ebcec2341512d27f9ab888a0da75cd919a891d97 100644
--- a/chrome/browser/ui/bluetooth/bluetooth_chooser_controller.cc
+++ b/chrome/browser/ui/bluetooth/bluetooth_chooser_controller.cc
@@ -4,6 +4,8 @@
 
 #include "chrome/browser/ui/bluetooth/bluetooth_chooser_controller.h"
 
+#include <algorithm>
+
 #include "base/logging.h"
 #include "base/strings/utf_string_conversions.h"
 #include "chrome/browser/net/referrer.h"
@@ -170,16 +172,36 @@ void BluetoothChooserController::AddOrUpdateDevice(
     bool is_gatt_connected,
     bool is_paired,
     const int8_t* rssi) {
-  auto result = device_id_to_name_map_.insert({device_id, device_name});
-  if (!result.second) {
+  auto name_it = device_id_to_name_map_.find(device_id);
+  if (name_it != device_id_to_name_map_.end()) {
     // TODO(ortuno): Update device's information.

[msw, 2016/08/12 22:19:33]

q: can you remove this TODO now that we're updating the device name?
(ditto for the TODO in the header)

[juncai, 2016/08/15 18:11:19]

Done.

-    // https://crbug.com/634366 Update name
+    if (should_update_name) {
+      base::string16 previous_device_name = name_it->second;
+      name_it->second = device_name;
+
+      const auto& it = device_name_map_.find(previous_device_name);
+      DCHECK(it != device_name_map_.end());
+      DCHECK_GT(it->second, 0);
+
+      if (--(it->second) == 0)
+        device_name_map_.erase(it);
+
+      ++device_name_map_[device_name];
+    }
+
+    size_t index = std::distance(
+        device_ids_.begin(),
+        std::find(device_ids_.begin(), device_ids_.end(), device_id));
+    DCHECK_NE(device_ids_.size(), index);
     // http://crbug.com/543466 Update connection and paired status
     // http://crbug.com/629689 Update RSSI.
+    if (view())
+      view()->OnOptionUpdated(index);

[msw, 2016/08/12 22:19:33]

Do we want to report that the option was updated if the name wasn't changed? Why
would we ever supply false for |should_update_name|?

[juncai, 2016/08/15 18:11:19]

Regarding the first question, if the |device_name| is the same as the
|previous_device_name|, it still needs to call view()->OnOptionUpdated(index);
since its connection or paired status or RSSI may be changed.

Regarding the second question, the |device_name| gets value from:
https://cs.chromium.org/chromium/src/device/bluetooth/bluetooth_device.h?q=bl...

And sometimes when a Bluetooth device stops advertising, it will return a string
like "Unknown device" or "Unsupported device". So in this case,
https://cs.chromium.org/chromium/src/device/bluetooth/bluetooth_device.h?q=bl...
will return that the name is not valid, so it will pass false to
|should_update_name|, and it will not update the device's name.

[msw, 2016/08/15 19:04:58]

Okay, good to know; thanks.
Ah, okay, please add a comment to that effect at the function decl.

[juncai, 2016/08/15 23:53:33]

Done.

     return;
   }
 
   device_ids_.push_back(device_id);
+  device_id_to_name_map_.insert({device_id, device_name});
   ++device_name_map_[device_name];
   if (view())
     view()->OnOptionAdded(device_ids_.size() - 1);