bluetooth: Use BluetoothUUID instead of string when sending uuids

content/browser/bluetooth/web_bluetooth_service_impl.cc

Index: content/browser/bluetooth/web_bluetooth_service_impl.cc
diff --git a/content/browser/bluetooth/web_bluetooth_service_impl.cc b/content/browser/bluetooth/web_bluetooth_service_impl.cc
index f2fa91880cb70c87cd37ae5ebe7ee73796bf0017..65f046665a1453dc363178e50e7a6da1b6824688 100644
--- a/content/browser/bluetooth/web_bluetooth_service_impl.cc
+++ b/content/browser/bluetooth/web_bluetooth_service_impl.cc
@@ -27,6 +27,8 @@
 #include "content/public/browser/web_contents.h"
 #include "device/bluetooth/bluetooth_remote_gatt_characteristic.h"
 
+using device::BluetoothUUID;
+
 namespace content {
 
 namespace {
@@ -130,14 +132,15 @@ blink::mojom::WebBluetoothError TranslateGATTErrorAndRecord(
 // Replace when implemented. http://crbug.com/552022
 std::vector<device::BluetoothRemoteGattCharacteristic*>
 GetCharacteristicsByUUID(device::BluetoothRemoteGattService* service,
-                         const std::string& characteristic_uuid) {
+                         const BluetoothUUID& characteristic_uuid) {
   std::vector<device::BluetoothRemoteGattCharacteristic*> characteristics;
-  VLOG(1) << "Looking for characteristic: " << characteristic_uuid;
+  VLOG(1) << "Looking for characteristic: "
+          << characteristic_uuid.canonical_value();
   for (device::BluetoothRemoteGattCharacteristic* characteristic :
        service->GetCharacteristics()) {
     VLOG(1) << "Characteristic in cache: "
             << characteristic->GetUUID().canonical_value();
-    if (characteristic->GetUUID().canonical_value() == characteristic_uuid) {
+    if (characteristic->GetUUID() == characteristic_uuid) {
       characteristics.push_back(characteristic);
     }
   }
@@ -148,14 +151,13 @@ GetCharacteristicsByUUID(device::BluetoothRemoteGattService* service,
 // Replace when implemented. http://crbug.com/552022
 std::vector<device::BluetoothRemoteGattService*> GetPrimaryServicesByUUID(
     device::BluetoothDevice* device,
-    const std::string& service_uuid) {
+    const BluetoothUUID& service_uuid) {
   std::vector<device::BluetoothRemoteGattService*> services;
-  VLOG(1) << "Looking for service: " << service_uuid;
+  VLOG(1) << "Looking for service: " << service_uuid.canonical_value();
   for (device::BluetoothRemoteGattService* service :
        device->GetGattServices()) {
     VLOG(1) << "Service in cache: " << service->GetUUID().canonical_value();
-    if (service->GetUUID().canonical_value() == service_uuid &&
-        service->IsPrimary()) {
+    if (service->GetUUID() == service_uuid && service->IsPrimary()) {
       services.push_back(service);
     }
   }
@@ -364,14 +366,15 @@ void WebBluetoothServiceImpl::RemoteServerDisconnect(
 
 void WebBluetoothServiceImpl::RemoteServerGetPrimaryService(
     const mojo::String& device_id,
-    const mojo::String& service_uuid,
+    const std::unique_ptr<BluetoothUUID>& service_uuid,
     const RemoteServerGetPrimaryServiceCallback& callback) {
   DCHECK_CURRENTLY_ON(BrowserThread::UI);
+
   RecordWebBluetoothFunctionCall(UMAWebBluetoothFunction::GET_PRIMARY_SERVICE);
-  RecordGetPrimaryServiceService(device::BluetoothUUID(service_uuid));
+  RecordGetPrimaryServiceService(service_uuid);
 
   if (!allowed_devices_map_.IsOriginAllowedToAccessService(
-          GetOrigin(), device_id, service_uuid)) {
+          GetOrigin(), device_id, *service_uuid)) {
     callback.Run(blink::mojom::WebBluetoothError::NOT_ALLOWED_TO_ACCESS_SERVICE,
                  nullptr /* service */);
     return;
@@ -395,32 +398,35 @@ void WebBluetoothServiceImpl::RemoteServerGetPrimaryService(
   // We can't know if a service is present or not until GATT service discovery
   // is complete for the device.
   if (query_result.device->IsGattServicesDiscoveryComplete()) {
-    RemoteServerGetPrimaryServiceImpl(service_uuid, callback,
-                                      query_result.device);
+    RemoteServerGetPrimaryServiceImpl(
+        base::WrapUnique(new BluetoothUUID(*service_uuid)), callback,
+        query_result.device);
     return;
   }
 
   VLOG(1) << "Services not yet discovered.";
-  pending_primary_services_requests_[device_address].push_back(
-      base::Bind(&WebBluetoothServiceImpl::RemoteServerGetPrimaryServiceImpl,
-                 base::Unretained(this), service_uuid, callback));
+  pending_primary_services_requests_[device_address].push_back(base::Bind(
+      &WebBluetoothServiceImpl::RemoteServerGetPrimaryServiceImpl,
+      base::Unretained(this),
+      base::Passed(base::WrapUnique(new BluetoothUUID(*service_uuid))),
+      callback));
 }
 
 void WebBluetoothServiceImpl::RemoteServiceGetCharacteristics(
     const mojo::String& service_instance_id,
     blink::mojom::WebBluetoothGATTQueryQuantity quantity,
-    const mojo::String& characteristics_uuid,
+    const std::unique_ptr<BluetoothUUID>& characteristics_uuid,
     const RemoteServiceGetCharacteristicsCallback& callback) {
   DCHECK_CURRENTLY_ON(BrowserThread::UI);
+
   RecordWebBluetoothFunctionCall(
       quantity == blink::mojom::WebBluetoothGATTQueryQuantity::SINGLE
           ? UMAWebBluetoothFunction::SERVICE_GET_CHARACTERISTIC
           : UMAWebBluetoothFunction::SERVICE_GET_CHARACTERISTICS);
   RecordGetCharacteristicsCharacteristic(quantity, characteristics_uuid);
 
-  if (!characteristics_uuid.is_null() &&
-      BluetoothBlacklist::Get().IsExcluded(
-          device::BluetoothUUID(characteristics_uuid))) {
+  if (characteristics_uuid.get() &&

[Jeffrey Yasskin, 2016/05/28 04:38:06]

unique_ptr and base::Optional can be directly used in an && expression without
an explicit .get().

[ortuno, 2016/05/31 17:30:47]

I think that .get() is more explicit, but I've been told to use remove the
.get() a bunch of times. So I'll try to do that from now on.

[Jeffrey Yasskin, 2016/05/31 21:52:57]

I'm totally happy with things like "!= nullptr" or "!= base::nullopt", which
express the type of the thing getting compared, but .get() doesn't state the
type so it seems less useful.

+      BluetoothBlacklist::Get().IsExcluded(*characteristics_uuid)) {
     RecordGetCharacteristicsOutcome(quantity,
                                     UMAGetCharacteristicOutcome::BLACKLISTED);
     callback.Run(
@@ -444,10 +450,10 @@ void WebBluetoothServiceImpl::RemoteServiceGetCharacteristics(
   }
 
   std::vector<device::BluetoothRemoteGattCharacteristic*> characteristics =
-      characteristics_uuid.is_null()
-          ? query_result.service->GetCharacteristics()
-          : GetCharacteristicsByUUID(query_result.service,
-                                     characteristics_uuid);
+      characteristics_uuid.get()
+          ? GetCharacteristicsByUUID(query_result.service,
+                                     *characteristics_uuid)
+          : query_result.service->GetCharacteristics();
 
   mojo::Array<blink::mojom::WebBluetoothRemoteGATTCharacteristicPtr>
       response_characteristics;
@@ -483,13 +489,14 @@ void WebBluetoothServiceImpl::RemoteServiceGetCharacteristics(
                  std::move(response_characteristics));
     return;
   }
+
   RecordGetCharacteristicsOutcome(
-      quantity, characteristics_uuid.is_null()
-                    ? UMAGetCharacteristicOutcome::NO_CHARACTERISTICS
-                    : UMAGetCharacteristicOutcome::NOT_FOUND);
-  callback.Run(characteristics_uuid.is_null()
-                   ? blink::mojom::WebBluetoothError::NO_CHARACTERISTICS_FOUND
-                   : blink::mojom::WebBluetoothError::CHARACTERISTIC_NOT_FOUND,
+      quantity, characteristics_uuid.get()
+                    ? UMAGetCharacteristicOutcome::NOT_FOUND
+                    : UMAGetCharacteristicOutcome::NO_CHARACTERISTICS);
+  callback.Run(characteristics_uuid.get()
+                   ? blink::mojom::WebBluetoothError::CHARACTERISTIC_NOT_FOUND
+                   : blink::mojom::WebBluetoothError::NO_CHARACTERISTICS_FOUND,
                nullptr /* characteristics */);
   return;
 }
@@ -668,14 +675,14 @@ void WebBluetoothServiceImpl::RequestDeviceImpl(
 }
 
 void WebBluetoothServiceImpl::RemoteServerGetPrimaryServiceImpl(
-    const std::string& service_uuid,
+    std::unique_ptr<BluetoothUUID> service_uuid,
     const RemoteServerGetPrimaryServiceCallback& callback,
     device::BluetoothDevice* device) {
   DCHECK_CURRENTLY_ON(BrowserThread::UI);
   DCHECK(device->IsGattServicesDiscoveryComplete());
 
   std::vector<device::BluetoothRemoteGattService*> services =
-      GetPrimaryServicesByUUID(device, service_uuid);
+      GetPrimaryServicesByUUID(device, *service_uuid);
 
   if (services.empty()) {
     VLOG(1) << "Service not found in device.";
@@ -725,9 +732,9 @@ void WebBluetoothServiceImpl::OnGetDeviceSuccess(
   VLOG(1) << "UUIDs: ";
 
   mojo::Array<mojo::String> filtered_uuids;
-  for (const device::BluetoothUUID& uuid : device->GetUUIDs()) {
+  for (const BluetoothUUID& uuid : device->GetUUIDs()) {
     if (allowed_devices_map_.IsOriginAllowedToAccessService(
-            GetOrigin(), device_id_for_origin, uuid.canonical_value())) {
+            GetOrigin(), device_id_for_origin, uuid)) {
       VLOG(1) << "\t Allowed: " << uuid.canonical_value();
       filtered_uuids.push_back(uuid.canonical_value());
     } else {
@@ -886,8 +893,7 @@ CacheQueryResult WebBluetoothServiceImpl::QueryCacheForService(
   if (result.service == nullptr) {
     result.outcome = CacheQueryOutcome::NO_SERVICE;
   } else if (!allowed_devices_map_.IsOriginAllowedToAccessService(
-                 GetOrigin(), device_id,
-                 result.service->GetUUID().canonical_value())) {
+                 GetOrigin(), device_id, result.service->GetUUID())) {
     CrashRendererAndClosePipe(bad_message::BDH_SERVICE_NOT_ALLOWED_FOR_ORIGIN);
     return CacheQueryResult(CacheQueryOutcome::BAD_RENDERER);
   }