bluetooth: Refactor code that checks if device/service/characteristic is in cache

content/browser/bluetooth/bluetooth_dispatcher_host.cc

 // Copyright 2014 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.
 
 // NETWORK_ERROR Note:
 // When a device can't be found in the BluetoothAdapter, that generally
 // indicates that it's gone out of range. We reject with a NetworkError in that
 // case.
 // https://webbluetoothchrome.github.io/web-bluetooth/#dom-bluetoothdevice-connectgatt
 
@@ skipping 233 matching lines @@
   }
 
   const int thread_id;
   const int request_id;
   const std::vector<BluetoothScanFilter> filters;
   const std::vector<BluetoothUUID> optional_services;
   scoped_ptr<BluetoothChooser> chooser;
   scoped_ptr<device::BluetoothDiscoverySession> discovery_session;
 };
 
+blink::WebBluetoothError
+BluetoothDispatcherHost::CacheQueryOutcomeToWebBluetoothError(
+    BluetoothDispatcherHost::CacheQueryOutcome outcome) {
+  switch (outcome) {
+    case CacheQueryOutcome::SUCCESS:
+    case CacheQueryOutcome::BAD_RENDERER:
+      CHECK(false) << "Not errors";
+    case CacheQueryOutcome::NO_DEVICE:
+      return WebBluetoothError::DeviceNoLongerInRange;
+    case CacheQueryOutcome::NO_SERVICE:
+      return WebBluetoothError::ServiceNoLongerExists;
+    case CacheQueryOutcome::NO_CHARACTERISTIC:
+      return WebBluetoothError::CharacteristicNoLongerExists;
+  }
+  NOTIMPLEMENTED();
+  return WebBluetoothError::ENUM_MAX_VALUE;
+}
+
+// TODO(ortuno): There shouldn't be a need for each user of these functions
+// to histogram the outcome on their own. We should refactor bluetooth_metrics
+// to use RecordOperationOutcome(WebBluetooth::Operation, Outcome), that way
+// histogramming can be done inside these functions directly.
+std::pair<BluetoothDispatcherHost::CacheQueryOutcome, device::BluetoothDevice*>
+BluetoothDispatcherHost::QueryForDevice(const std::string& device_instance_id) {
+  device::BluetoothDevice* device = adapter_->GetDevice(device_instance_id);
+  CacheQueryOutcome device_outcome = CacheQueryOutcome::SUCCESS;
+  if (device == nullptr) {  // See "NETWORK_ERROR Note" above.
+    device_outcome = CacheQueryOutcome::NO_DEVICE;
+  }
+  return std::make_pair(device_outcome, device);
+}
+
+std::pair<BluetoothDispatcherHost::CacheQueryOutcome,
+          device::BluetoothGattService*>
+BluetoothDispatcherHost::QueryForService(
+    const std::string& service_instance_id) {
+  auto device_iter = service_to_device_.find(service_instance_id);
+  // A service_instance_id not in the map implies a hostile renderer
+  // because a renderer obtains the service id from this class and
+  // it will be added to the map at that time.
+  if (device_iter == service_to_device_.end()) {
+    // Kill the renderer
+    bad_message::ReceivedBadMessage(this, bad_message::BDH_INVALID_SERVICE_ID);
+    return std::make_pair(CacheQueryOutcome::BAD_RENDERER, nullptr);
+  }
+
+  // TODO(ortuno): Check if domain has access to device.
+  // https://crbug.com/493459
+  auto device_query_outcome = QueryForDevice(device_iter->second);
+  if (device_query_outcome.first != CacheQueryOutcome::SUCCESS) {
+    return std::make_pair(device_query_outcome.first, nullptr);
+  }
+
+  CacheQueryOutcome service_outcome = CacheQueryOutcome::SUCCESS;
+  // TODO(ortuno): Check if domain has access to service
+  // http://crbug.com/493460
+  device::BluetoothGattService* service =
+      device_query_outcome.second->GetGattService(service_instance_id);
+  if (!service) {
+    service_outcome = CacheQueryOutcome::NO_SERVICE;
+  }
+  return std::make_pair(service_outcome, service);
+}
+
+std::pair<BluetoothDispatcherHost::CacheQueryOutcome,
+          device::BluetoothGattCharacteristic*>
+BluetoothDispatcherHost::QueryForCharacteristic(
+    const std::string& characteristic_instance_id) {
+  auto characteristic_iter =
+      characteristic_to_service_.find(characteristic_instance_id);
+  // A characteristic_instance_id not in the map implies a hostile renderer
+  // because a renderer obtains the characteristic id from this class and
+  // it will be added to the map at that time.
+  if (characteristic_iter == characteristic_to_service_.end()) {
+    // Kill the renderer
+    bad_message::ReceivedBadMessage(this,
+                                    bad_message::BDH_INVALID_CHARACTERISTIC_ID);
+    return std::make_pair(CacheQueryOutcome::BAD_RENDERER, nullptr);
+  }
+
+  auto service_query_outcome = QueryForService(characteristic_iter->second);
+  if (service_query_outcome.first != CacheQueryOutcome::SUCCESS) {
+    return std::make_pair(service_query_outcome.first, nullptr);
+  }
+
+  CacheQueryOutcome characteristic_outcome = CacheQueryOutcome::SUCCESS;
+  BluetoothGattCharacteristic* characteristic =
+      service_query_outcome.second->GetCharacteristic(
+          characteristic_instance_id);
+  if (!characteristic) {
+    characteristic_outcome = CacheQueryOutcome::NO_CHARACTERISTIC;
+  }
+  return std::make_pair(characteristic_outcome, characteristic);
+}
+
 void BluetoothDispatcherHost::set_adapter(
     scoped_refptr<device::BluetoothAdapter> adapter) {
   DCHECK_CURRENTLY_ON(BrowserThread::UI);
   if (adapter_.get())
     adapter_->RemoveObserver(this);
   adapter_ = adapter;
   if (adapter_.get())
     adapter_->AddObserver(this);
 }
 
@@ skipping 177 matching lines @@
     int request_id,
     const std::string& device_instance_id) {
   DCHECK_CURRENTLY_ON(BrowserThread::UI);
   RecordWebBluetoothFunctionCall(UMAWebBluetoothFunction::CONNECT_GATT);
   const base::TimeTicks start_time = base::TimeTicks::Now();
 
   // TODO(ortuno): Right now it's pointless to check if the domain has access to
   // the device, because any domain can connect to any device. But once
   // permissions are implemented we should check that the domain has access to
   // the device. https://crbug.com/484745
-  device::BluetoothDevice* device = adapter_->GetDevice(device_instance_id);
-  if (device == nullptr) {  // See "NETWORK_ERROR Note" above.
-    RecordConnectGATTOutcome(UMAConnectGATTOutcome::NO_DEVICE);
+  auto device_query_result = QueryForDevice(device_instance_id);
+  if (device_query_result.first != CacheQueryOutcome::SUCCESS) {
+    if (device_query_result.first == CacheQueryOutcome::NO_DEVICE) {
+      RecordConnectGATTOutcome(UMAConnectGATTOutcome::NO_DEVICE);
+    }
     Send(new BluetoothMsg_ConnectGATTError(
-        thread_id, request_id, WebBluetoothError::DeviceNoLongerInRange));
+        thread_id, request_id,
+        CacheQueryOutcomeToWebBluetoothError(device_query_result.first)));
     return;
   }
-  device->CreateGattConnection(
+  device_query_result.second->CreateGattConnection(
       base::Bind(&BluetoothDispatcherHost::OnGATTConnectionCreated,
                  weak_ptr_factory_.GetWeakPtr(), thread_id, request_id,
                  device_instance_id, start_time),
       base::Bind(&BluetoothDispatcherHost::OnCreateGATTConnectionError,
                  weak_ptr_factory_.GetWeakPtr(), thread_id, request_id,
                  device_instance_id, start_time));
 }
 
 void BluetoothDispatcherHost::OnGetPrimaryService(
     int thread_id,
@@ skipping 20 matching lines @@
 
 void BluetoothDispatcherHost::OnGetCharacteristic(
     int thread_id,
     int request_id,
     const std::string& service_instance_id,
     const std::string& characteristic_uuid) {
   DCHECK_CURRENTLY_ON(BrowserThread::UI);
   RecordWebBluetoothFunctionCall(UMAWebBluetoothFunction::GET_CHARACTERISTIC);
   RecordGetCharacteristicCharacteristic(characteristic_uuid);
 
-  auto device_iter = service_to_device_.find(service_instance_id);
-  // A service_instance_id not in the map implies a hostile renderer
-  // because a renderer obtains the service id from this class and
-  // it will be added to the map at that time.
-  if (device_iter == service_to_device_.end()) {
-    // Kill the renderer
-    bad_message::ReceivedBadMessage(this, bad_message::BDH_INVALID_SERVICE_ID);
-    return;
-  }
+  auto service_query_outcome = QueryForService(service_instance_id);
 
-  // TODO(ortuno): Check if domain has access to device.
-  // https://crbug.com/493459
-  device::BluetoothDevice* device =
-      adapter_->GetDevice(device_iter->second /* device_instance_id */);
-
-  if (device == nullptr) {  // See "NETWORK_ERROR Note" above.
-    RecordGetCharacteristicOutcome(UMAGetCharacteristicOutcome::NO_DEVICE);
+  if (service_query_outcome.first != CacheQueryOutcome::SUCCESS) {
+    CacheQueryOutcome error_outcome = service_query_outcome.first;
+    if (error_outcome == CacheQueryOutcome::BAD_RENDERER) {
+      // QueryForService already took care of sending the message
+      return;
+    }
+    if (error_outcome == CacheQueryOutcome::NO_DEVICE) {
+      RecordGetCharacteristicOutcome(UMAGetCharacteristicOutcome::NO_DEVICE);
+    } else if (error_outcome == CacheQueryOutcome::NO_SERVICE) {
+      RecordGetCharacteristicOutcome(UMAGetCharacteristicOutcome::NO_SERVICE);
+    }
     Send(new BluetoothMsg_GetCharacteristicError(
-        thread_id, request_id, WebBluetoothError::DeviceNoLongerInRange));
-    return;
-  }
-
-  // TODO(ortuno): Check if domain has access to service
-  // http://crbug.com/493460
-  device::BluetoothGattService* service =
-      device->GetGattService(service_instance_id);
-  if (!service) {
-    RecordGetCharacteristicOutcome(UMAGetCharacteristicOutcome::NO_SERVICE);
-    Send(new BluetoothMsg_GetCharacteristicError(
-        thread_id, request_id, WebBluetoothError::ServiceNoLongerExists));
+        thread_id, request_id,
+        CacheQueryOutcomeToWebBluetoothError(error_outcome)));
     return;
   }
 
   for (BluetoothGattCharacteristic* characteristic :
-       service->GetCharacteristics()) {
+       service_query_outcome.second->GetCharacteristics()) {
     if (characteristic->GetUUID().canonical_value() == characteristic_uuid) {
       const std::string& characteristic_instance_id =
           characteristic->GetIdentifier();
 
       auto insert_result = characteristic_to_service_.insert(
           make_pair(characteristic_instance_id, service_instance_id));
 
       // If  value is already in map, DCHECK it's valid.
       if (!insert_result.second)
         DCHECK(insert_result.first->second == service_instance_id);
@@ skipping 12 matching lines @@
 }
 
 void BluetoothDispatcherHost::OnReadValue(
     int thread_id,
     int request_id,
     const std::string& characteristic_instance_id) {
   DCHECK_CURRENTLY_ON(BrowserThread::UI);
   RecordWebBluetoothFunctionCall(
       UMAWebBluetoothFunction::CHARACTERISTIC_READ_VALUE);
 
-  auto characteristic_iter =
-      characteristic_to_service_.find(characteristic_instance_id);
-  // A characteristic_instance_id not in the map implies a hostile renderer
-  // because a renderer obtains the characteristic id from this class and
-  // it will be added to the map at that time.
-  if (characteristic_iter == characteristic_to_service_.end()) {
-    // Kill the renderer
-    bad_message::ReceivedBadMessage(this,
-                                    bad_message::BDH_INVALID_CHARACTERISTIC_ID);
-    return;
-  }
-  const std::string& service_instance_id = characteristic_iter->second;
+  auto characteristic_query_outcome =
+      QueryForCharacteristic(characteristic_instance_id);
 
-  auto device_iter = service_to_device_.find(service_instance_id);
-
-  CHECK(device_iter != service_to_device_.end());
-
-  device::BluetoothDevice* device =
-      adapter_->GetDevice(device_iter->second /* device_instance_id */);
-  if (device == nullptr) {  // See "NETWORK_ERROR Note" above.
-    RecordCharacteristicReadValueOutcome(UMAGATTOperationOutcome::NO_DEVICE);
+  if (characteristic_query_outcome.first != CacheQueryOutcome::SUCCESS) {
+    CacheQueryOutcome error_outcome = characteristic_query_outcome.first;
+    if (error_outcome == CacheQueryOutcome::BAD_RENDERER) {
+      // The query handles the message.
+      return;
+    }
+    if (error_outcome == CacheQueryOutcome::NO_DEVICE) {
+      RecordCharacteristicReadValueOutcome(UMAGATTOperationOutcome::NO_DEVICE);
+    } else if (error_outcome == CacheQueryOutcome::NO_SERVICE) {
+      RecordCharacteristicReadValueOutcome(UMAGATTOperationOutcome::NO_SERVICE);
+    } else if (error_outcome == CacheQueryOutcome::NO_CHARACTERISTIC) {
+      RecordCharacteristicReadValueOutcome(
+          UMAGATTOperationOutcome::NO_CHARACTERISTIC);
+    }
     Send(new BluetoothMsg_ReadCharacteristicValueError(
-        thread_id, request_id, WebBluetoothError::DeviceNoLongerInRange));
+        thread_id, request_id,
+        CacheQueryOutcomeToWebBluetoothError(error_outcome)));
     return;
   }
 
-  BluetoothGattService* service = device->GetGattService(service_instance_id);
-  if (service == nullptr) {
-    RecordCharacteristicReadValueOutcome(UMAGATTOperationOutcome::NO_SERVICE);
-    Send(new BluetoothMsg_ReadCharacteristicValueError(
-        thread_id, request_id, WebBluetoothError::ServiceNoLongerExists));
-    return;
-  }
-
-  BluetoothGattCharacteristic* characteristic =
-      service->GetCharacteristic(characteristic_instance_id);
-  if (characteristic == nullptr) {
-    RecordCharacteristicReadValueOutcome(
-        UMAGATTOperationOutcome::NO_CHARACTERISTIC);
-    Send(new BluetoothMsg_ReadCharacteristicValueError(
-        thread_id, request_id,
-        WebBluetoothError::CharacteristicNoLongerExists));
-    return;
-  }
-
-  characteristic->ReadRemoteCharacteristic(
+  characteristic_query_outcome.second->ReadRemoteCharacteristic(
       base::Bind(&BluetoothDispatcherHost::OnCharacteristicValueRead,
                  weak_ptr_factory_.GetWeakPtr(), thread_id, request_id),
       base::Bind(&BluetoothDispatcherHost::OnCharacteristicReadValueError,
                  weak_ptr_factory_.GetWeakPtr(), thread_id, request_id));
 }
 
 void BluetoothDispatcherHost::OnWriteValue(
     int thread_id,
     int request_id,
     const std::string& characteristic_instance_id,
     const std::vector<uint8_t>& value) {
   DCHECK_CURRENTLY_ON(BrowserThread::UI);
   RecordWebBluetoothFunctionCall(
       UMAWebBluetoothFunction::CHARACTERISTIC_WRITE_VALUE);
 
   // Length check per step 3 of writeValue algorithm:
   // https://webbluetoothchrome.github.io/web-bluetooth/#dom-bluetoothgattcharacteristic-writevalue
   // We perform the length check on the renderer side. So if we
   // get a value with length > 512, we can assume it's a hostile
   // renderer and kill it.
   if (value.size() > 512) {
     bad_message::ReceivedBadMessage(
         this, bad_message::BDH_INVALID_WRITE_VALUE_LENGTH);
     return;
   }
 
-  auto characteristic_iter =
-      characteristic_to_service_.find(characteristic_instance_id);
-  // A characteristic_instance_id not in the map implies a hostile renderer
-  // because a renderer obtains the characteristic id from this class and
-  // it will be added to the map at that time.
-  if (characteristic_iter == characteristic_to_service_.end()) {
-    bad_message::ReceivedBadMessage(this,
-                                    bad_message::BDH_INVALID_CHARACTERISTIC_ID);
-    return;
-  }
-  const std::string& service_instance_id = characteristic_iter->second;
+  auto characteristic_query_outcome =
+      QueryForCharacteristic(characteristic_instance_id);
 
-  auto device_iter = service_to_device_.find(service_instance_id);
-
-  CHECK(device_iter != service_to_device_.end());
-
-  device::BluetoothDevice* device =
-      adapter_->GetDevice(device_iter->second /* device_instance_id */);
-  if (device == nullptr) {  // See "NETWORK_ERROR Note" above.
-    RecordCharacteristicWriteValueOutcome(UMAGATTOperationOutcome::NO_DEVICE);
+  if (characteristic_query_outcome.first != CacheQueryOutcome::SUCCESS) {
+    CacheQueryOutcome error_outcome = characteristic_query_outcome.first;
+    if (error_outcome == CacheQueryOutcome::BAD_RENDERER) {
+      // The query handles the message.
+      return;
+    }
+    if (error_outcome == CacheQueryOutcome::NO_DEVICE) {
+      RecordCharacteristicWriteValueOutcome(UMAGATTOperationOutcome::NO_DEVICE);
+    } else if (error_outcome == CacheQueryOutcome::NO_SERVICE) {
+      RecordCharacteristicWriteValueOutcome(
+          UMAGATTOperationOutcome::NO_SERVICE);
+    } else if (error_outcome == CacheQueryOutcome::NO_CHARACTERISTIC) {
+      RecordCharacteristicWriteValueOutcome(
+          UMAGATTOperationOutcome::NO_CHARACTERISTIC);
+    }
     Send(new BluetoothMsg_WriteCharacteristicValueError(
-        thread_id, request_id, WebBluetoothError::DeviceNoLongerInRange));
+        thread_id, request_id,
+        CacheQueryOutcomeToWebBluetoothError(error_outcome)));
     return;
   }
 
-  BluetoothGattService* service = device->GetGattService(service_instance_id);
-  if (service == nullptr) {
-    RecordCharacteristicWriteValueOutcome(UMAGATTOperationOutcome::NO_SERVICE);
-    Send(new BluetoothMsg_WriteCharacteristicValueError(
-        thread_id, request_id, WebBluetoothError::ServiceNoLongerExists));
-    return;
-  }
-
-  BluetoothGattCharacteristic* characteristic =
-      service->GetCharacteristic(characteristic_instance_id);
-  if (characteristic == nullptr) {
-    RecordCharacteristicWriteValueOutcome(
-        UMAGATTOperationOutcome::NO_CHARACTERISTIC);
-    Send(new BluetoothMsg_WriteCharacteristicValueError(
-        thread_id, request_id,
-        WebBluetoothError::CharacteristicNoLongerExists));
-    return;
-  }
-  characteristic->WriteRemoteCharacteristic(
+  characteristic_query_outcome.second->WriteRemoteCharacteristic(
       value, base::Bind(&BluetoothDispatcherHost::OnWriteValueSuccess,
                         weak_ptr_factory_.GetWeakPtr(), thread_id, request_id),
       base::Bind(&BluetoothDispatcherHost::OnWriteValueFailed,
                  weak_ptr_factory_.GetWeakPtr(), thread_id, request_id));
 }
 
 void BluetoothDispatcherHost::OnDiscoverySessionStarted(
     int chooser_id,
     scoped_ptr<device::BluetoothDiscoverySession> discovery_session) {
   DCHECK_CURRENTLY_ON(BrowserThread::UI);
@@ skipping 150 matching lines @@
                                          TranslateConnectError(error_code)));
 }
 
 void BluetoothDispatcherHost::OnServicesDiscovered(
     int thread_id,
     int request_id,
     const std::string& device_instance_id,
     const std::string& service_uuid) {
   DCHECK_CURRENTLY_ON(BrowserThread::UI);
 
-  device::BluetoothDevice* device = adapter_->GetDevice(device_instance_id);
-  if (device == nullptr) {  // See "NETWORK_ERROR Note" above.
-    RecordGetPrimaryServiceOutcome(UMAGetPrimaryServiceOutcome::NO_DEVICE);
+  auto device_query_result = QueryForDevice(device_instance_id);
+  if (device_query_result.first != CacheQueryOutcome::SUCCESS) {
+    if (device_query_result.first == CacheQueryOutcome::NO_DEVICE) {
+      RecordGetPrimaryServiceOutcome(UMAGetPrimaryServiceOutcome::NO_DEVICE);
+    }
     Send(new BluetoothMsg_GetPrimaryServiceError(
-        thread_id, request_id, WebBluetoothError::DeviceNoLongerInRange));
+        thread_id, request_id,
+        CacheQueryOutcomeToWebBluetoothError(device_query_result.first)));
     return;
   }
-  for (BluetoothGattService* service : device->GetGattServices()) {
+
+  for (BluetoothGattService* service :
+       device_query_result.second->GetGattServices()) {
     if (service->GetUUID().canonical_value() == service_uuid) {
       // TODO(ortuno): Use generated instance ID instead.
       // https://crbug.com/495379
       const std::string& service_identifier = service->GetIdentifier();
       auto insert_result = service_to_device_.insert(
           make_pair(service_identifier, device_instance_id));
 
       // If a value is already in map, DCHECK it's valid.
       if (!insert_result.second)
         DCHECK(insert_result.first->second == device_instance_id);
@@ skipping 50 matching lines @@
 
 void BluetoothDispatcherHost::ShowBluetoothPairingLink() {
   NOTIMPLEMENTED();
 }
 
 void BluetoothDispatcherHost::ShowBluetoothAdapterOffLink() {
   NOTIMPLEMENTED();
 }
 
 }  // namespace content