bluetooth: Use BluetoothUUID instead of string when sending uuids

content/browser/bluetooth/web_bluetooth_service_impl.cc

 // Copyright 2016 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.
 
 // ID Not In Map Note:
 // A service, characteristic, or descriptor ID not in the corresponding
 // WebBluetoothServiceImpl map [service_id_to_device_address_,
 // characteristic_id_to_service_id_, descriptor_to_characteristic_] implies a
 // hostile renderer because a renderer obtains the corresponding ID from this
 // class and it will be added to the map at that time.
 
 #include "content/browser/bluetooth/web_bluetooth_service_impl.h"
 
 #include <algorithm>
 
 #include "base/strings/utf_string_conversions.h"
 #include "base/threading/thread_task_runner_handle.h"
 #include "content/browser/bluetooth/bluetooth_adapter_factory_wrapper.h"
 #include "content/browser/bluetooth/bluetooth_blacklist.h"
 #include "content/browser/bluetooth/bluetooth_device_chooser_controller.h"
 #include "content/browser/bluetooth/bluetooth_metrics.h"
 #include "content/browser/bluetooth/frame_connected_bluetooth_devices.h"
 #include "content/browser/renderer_host/render_process_host_impl.h"
 #include "content/public/browser/browser_thread.h"
 #include "content/public/browser/navigation_handle.h"
 #include "content/public/browser/render_frame_host.h"
 #include "content/public/browser/web_contents.h"
 #include "device/bluetooth/bluetooth_remote_gatt_characteristic.h"
 
+using device::BluetoothUUID;
+
 namespace content {
 
 namespace {
 
 blink::mojom::WebBluetoothError TranslateConnectErrorAndRecord(
     device::BluetoothDevice::ConnectErrorCode error_code) {
   switch (error_code) {
     case device::BluetoothDevice::ERROR_UNKNOWN:
       RecordConnectGATTOutcome(UMAConnectGATTOutcome::UNKNOWN);
       return blink::mojom::WebBluetoothError::CONNECT_UNKNOWN_ERROR;
@@ skipping 83 matching lines @@
       return blink::mojom::WebBluetoothError::GATT_NOT_SUPPORTED;
   }
   NOTREACHED();
   return blink::mojom::WebBluetoothError::GATT_UNTRANSLATED_ERROR_CODE;
 }
 
 // TODO(ortuno): This should really be a BluetoothDevice method.
 // 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);
     }
   }
   return characteristics;
 }
 
 // TODO(ortuno): This should really be a BluetoothDevice method.
 // 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);
     }
   }
   return services;
 }
 
 }  // namespace
 
 WebBluetoothServiceImpl::WebBluetoothServiceImpl(
     RenderFrameHost* render_frame_host,
@@ skipping 188 matching lines @@
       UMAWebBluetoothFunction::REMOTE_GATT_SERVER_DISCONNECT);
 
   if (connected_devices_->IsConnectedToDeviceWithId(device_id)) {
     VLOG(1) << "Disconnecting device: " << device_id;
     connected_devices_->CloseConnectionToDeviceWithId(device_id);
   }
 }
 
 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;
   }
 
   const CacheQueryResult query_result = QueryCacheForDevice(device_id);
 
   if (query_result.outcome == CacheQueryOutcome::BAD_RENDERER) {
     binding_.Close();
     return;
   }
 
   if (query_result.outcome != CacheQueryOutcome::SUCCESS) {
     RecordGetPrimaryServiceOutcome(query_result.outcome);
     callback.Run(query_result.GetWebError(), nullptr /* service */);
     return;
   }
 
   const std::string& device_address = query_result.device->GetAddress();
 
   // 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(
         blink::mojom::WebBluetoothError::BLACKLISTED_CHARACTERISTIC_UUID,
         nullptr /* characteristics */);
     return;
   }
 
   const CacheQueryResult query_result =
       QueryCacheForService(service_instance_id);
 
   if (query_result.outcome == CacheQueryOutcome::BAD_RENDERER) {
     binding_.Close();
     return;
   }
 
   if (query_result.outcome != CacheQueryOutcome::SUCCESS) {
     RecordGetCharacteristicsOutcome(quantity, query_result.outcome);
     callback.Run(query_result.GetWebError(), nullptr /* characteristics */);
     return;
   }
 
   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;
   for (device::BluetoothRemoteGattCharacteristic* characteristic :
        characteristics) {
     if (BluetoothBlacklist::Get().IsExcluded(characteristic->GetUUID())) {
       continue;
     }
     std::string characteristic_instance_id = characteristic->GetIdentifier();
     auto insert_result = characteristic_id_to_service_id_.insert(
@@ skipping 15 matching lines @@
     }
   }
 
   if (!response_characteristics.empty()) {
     RecordGetCharacteristicsOutcome(quantity,
                                     UMAGetCharacteristicOutcome::SUCCESS);
     callback.Run(blink::mojom::WebBluetoothError::SUCCESS,
                  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;
 }
 
 void WebBluetoothServiceImpl::RemoteCharacteristicReadValue(
     const mojo::String& characteristic_instance_id,
     const RemoteCharacteristicReadValueCallback& callback) {
   DCHECK_CURRENTLY_ON(BrowserThread::UI);
   RecordWebBluetoothFunctionCall(
       UMAWebBluetoothFunction::CHARACTERISTIC_READ_VALUE);
@@ skipping 158 matching lines @@
 
   device_chooser_controller_->GetDevice(
       std::move(options),
       base::Bind(&WebBluetoothServiceImpl::OnGetDeviceSuccess,
                  weak_ptr_factory_.GetWeakPtr(), callback),
       base::Bind(&WebBluetoothServiceImpl::OnGetDeviceFailed,
                  weak_ptr_factory_.GetWeakPtr(), callback));
 }
 
 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.";
     RecordGetPrimaryServiceOutcome(UMAGetPrimaryServiceOutcome::NOT_FOUND);
     callback.Run(blink::mojom::WebBluetoothError::SERVICE_NOT_FOUND,
                  nullptr /* service */);
     return;
   }
 
   VLOG(1) << "Service found in device.";
@@ skipping 29 matching lines @@
     return;
   }
 
   const std::string device_id_for_origin =
       allowed_devices_map_.AddDevice(GetOrigin(), device_address, options);
 
   VLOG(1) << "Device: " << device->GetName();
   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 {
       VLOG(1) << "\t Not Allowed: " << uuid.canonical_value();
     }
   }
 
   blink::mojom::WebBluetoothDevicePtr device_ptr =
       blink::mojom::WebBluetoothDevice::New();
   device_ptr->id = device_id_for_origin;
@@ skipping 138 matching lines @@
 
   CacheQueryResult result = QueryCacheForDevice(device_id);
   if (result.outcome != CacheQueryOutcome::SUCCESS) {
     return result;
   }
 
   result.service = result.device->GetGattService(service_instance_id);
   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);
   }
   return result;
 }
 
 CacheQueryResult WebBluetoothServiceImpl::QueryCacheForCharacteristic(
     const std::string& characteristic_instance_id) {
   auto characteristic_iter =
       characteristic_id_to_service_id_.find(characteristic_instance_id);
@@ skipping 51 matching lines @@
   characteristic_id_to_service_id_.clear();
   service_id_to_device_address_.clear();
   connected_devices_.reset(
       new FrameConnectedBluetoothDevices(render_frame_host_));
   allowed_devices_map_ = BluetoothAllowedDevicesMap();
   device_chooser_controller_.reset();
   GetBluetoothAdapterFactoryWrapper()->ReleaseAdapter(this);
 }
 
 }  // namespace content