Use new wrapper types for web_bluetooth.mojom

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 <string>
 #include <vector>
 
 #include "base/logging.h"
 #include "base/optional.h"
 #include "base/stl_util.h"
 #include "base/strings/string_util.h"
 #include "content/browser/bluetooth/bluetooth_blacklist.h"
 #include "content/common/bluetooth/web_bluetooth_device_id.h"
 
 using device::BluetoothUUID;
 
 namespace content {
 
+namespace {
+
+std::ostream& operator<<(std::ostream& out,
+                         const WebBluetoothDeviceId& device_id) {
+  return out << device_id.str();
+}
+
+}  // namespace
+
 BluetoothAllowedDevicesMap::BluetoothAllowedDevicesMap() {}
 BluetoothAllowedDevicesMap::~BluetoothAllowedDevicesMap() {}
 
 const WebBluetoothDeviceId& BluetoothAllowedDevicesMap::AddDevice(
     const url::Origin& origin,
     const std::string& device_address,
     const blink::mojom::WebBluetoothRequestDeviceOptionsPtr& options) {
   VLOG(1) << "Adding a device to Map of Allowed Devices.";
 
   // "Unique" Origins generate the same key in maps, therefore are not
@@ skipping 129 matching lines @@
     device_id = WebBluetoothDeviceId::Create();
   }
   return device_id;
 }
 
 void BluetoothAllowedDevicesMap::AddUnionOfServicesTo(
     const blink::mojom::WebBluetoothRequestDeviceOptionsPtr& options,
     std::unordered_set<BluetoothUUID, device::BluetoothUUIDHash>*
         unionOfServices) {
   for (const auto& filter : options->filters) {
-    for (const base::Optional<BluetoothUUID>& uuid : filter->services) {
-      unionOfServices->insert(uuid.value());
+    if (filter->services) {

[ortuno, 2016/11/21 04:42:31]

All this nesting makes the code less readable. I would change it to:

for (const auto& filter : options->filters) {
  if (!filter->services) {
    continue;
  }
  for (const BluetoothUUID& uuid : filter->services.value()) {
    unionOfServices->insert(uuid);
  }
}

Also is there any test in bluetooth_allowed_devices_map_unittest.cc that tests
this?

[juncai, 2016/11/21 21:27:05]

Added test for this.

Done.

+      for (const BluetoothUUID& uuid : filter->services.value()) {
+        unionOfServices->insert(uuid);
+      }
     }
   }
-  for (const base::Optional<BluetoothUUID>& uuid : options->optional_services) {
-    unionOfServices->insert(uuid.value());
+  for (const BluetoothUUID& uuid : options->optional_services) {
+    unionOfServices->insert(uuid);
   }
 }
 
 }  // namespace content