bluetooth: When adding a device a second time merge the list of services.

content/browser/bluetooth/bluetooth_allowed_devices_map.cc

 // Copyright 2015 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 "content/browser/bluetooth/bluetooth_allowed_devices_map.h"
 
 #include <vector>
 
 #include "base/base64.h"
 #include "base/logging.h"
@@ skipping 37 matching lines @@
   // "Unique" Origins generate the same key in maps. The set of "unique"
   // Origins that generate the same key does not intersect the set of
   // potentially trustworthy origins; since Bluetooth is only available for
   // potntially trustworthy origins we should never receive a request from a
   // "unique" Origin.
   // See url::Origin for what constitutes a "unique" Origin and the
   // Secure Contexts spec for what constitutes a Trusworthy Origin:
   // https://w3c.github.io/webappsec-secure-contexts/
   CHECK(!origin.unique());
 
-  if (ContainsKey(origin_to_device_address_to_id_map_[origin],
-                  device_address)) {
+  auto device_address_to_id_map = origin_to_device_address_to_id_map_[origin];
+  auto id_iter = device_address_to_id_map.find(device_address);
+  if (id_iter != device_address_to_id_map.end()) {
     VLOG(1) << "Device already in map of allowed devices.";
+    const auto& device_id = id_iter->second;
+
+    // Update set of services.
+    auto new_services = UnionOfServices(filters, optional_services);

[Jeffrey Yasskin, 2016/02/03 23:18:18]

We could optimize this by having UnionOfServices append to the existing set from
origin_to_device_id_to_services_map_. I think it might be more readable too.
Since this only happens on user interaction, and then only once, we should care
more about readability than performance.

Something like:

void BluetoothAllowedDevicesMap::AddUnionOfServicesTo(
    const std::vector<BluetoothScanFilter>& filters,
    const std::vector<BluetoothUUID>& optional_services,
    std::set<std::string>* unionOfServices) {
  for (const auto& filter : filters) {
    for (const BluetoothUUID& uuid : filter.services) {
      unionOfServices->insert(uuid.canonical_value());
    }
  }
  for (const BluetoothUUID& uuid : optional_services) {
    unionOfServices->insert(uuid.canonical_value());
  }
}

AddUnionOfServicesTo(filters, optional_services,
&origin_to_device_id_to_services_map_[origin][device_id]);

[ortuno, 2016/02/04 02:21:09]

Done. Also replaced the UnionOfServices function since it was too similar.

[Jeffrey Yasskin, 2016/02/04 04:03:40]

Yep, that's what I meant. Thanks!

+    auto& old_services =
+        origin_to_device_id_to_services_map_[origin][device_id];
+    old_services.insert(new_services.begin(), new_services.end());
+
     return origin_to_device_address_to_id_map_[origin][device_address];
   }
   const std::string device_id = GenerateDeviceId();
   VLOG(1) << "Id generated for device: " << device_id;
 
   origin_to_device_address_to_id_map_[origin][device_address] = device_id;
   origin_to_device_id_to_address_map_[origin][device_id] = device_address;
   origin_to_device_id_to_services_map_[origin][device_id] =
       UnionOfServices(filters, optional_services);
 
@@ skipping 93 matching lines @@
       unionOfServices.insert(uuid.canonical_value());
     }
   }
   for (const BluetoothUUID& uuid : optional_services) {
     unionOfServices.insert(uuid.canonical_value());
   }
   return unionOfServices;
 }
 
 }  // namespace content