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

content/browser/bluetooth/bluetooth_dispatcher_host.cc

Index: content/browser/bluetooth/bluetooth_dispatcher_host.cc
diff --git a/content/browser/bluetooth/bluetooth_dispatcher_host.cc b/content/browser/bluetooth/bluetooth_dispatcher_host.cc
index c1a2395c4f2bb918ab826e0a4242535217dbd999..a2eca4d02a600fb7ae03493cffaf48916d253805 100644
--- a/content/browser/bluetooth/bluetooth_dispatcher_host.cc
+++ b/content/browser/bluetooth/bluetooth_dispatcher_host.cc
@@ -251,6 +251,101 @@ struct BluetoothDispatcherHost::RequestDeviceSession {
   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);
@@ -448,14 +543,17 @@ void BluetoothDispatcherHost::OnConnectGATT(
   // 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),
@@ -496,41 +594,27 @@ void BluetoothDispatcherHost::OnGetCharacteristic(
   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;
-  }
-
-  // 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);
-    Send(new BluetoothMsg_GetCharacteristicError(
-        thread_id, request_id, WebBluetoothError::DeviceNoLongerInRange));
-    return;
-  }
+  auto service_query_outcome = QueryForService(service_instance_id);
 
-  // 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);
+  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::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();
@@ -563,52 +647,30 @@ void BluetoothDispatcherHost::OnReadValue(
   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 device_iter = service_to_device_.find(service_instance_id);
-
-  CHECK(device_iter != service_to_device_.end());
+  auto characteristic_query_outcome =
+      QueryForCharacteristic(characteristic_instance_id);
 
-  device::BluetoothDevice* device =
-      adapter_->GetDevice(device_iter->second /* device_instance_id */);
-  if (device == nullptr) {  // See "NETWORK_ERROR Note" above.
-    RecordCharacteristicReadValueOutcome(UMAGATTOperationOutcome::NO_DEVICE);
-    Send(new BluetoothMsg_ReadCharacteristicValueError(
-        thread_id, request_id, WebBluetoothError::DeviceNoLongerInRange));
-    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);
+  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::CharacteristicNoLongerExists));
+        CacheQueryOutcomeToWebBluetoothError(error_outcome)));
     return;
   }
 
-  characteristic->ReadRemoteCharacteristic(
+  characteristic_query_outcome.second->ReadRemoteCharacteristic(
       base::Bind(&BluetoothDispatcherHost::OnCharacteristicValueRead,
                  weak_ptr_factory_.GetWeakPtr(), thread_id, request_id),
       base::Bind(&BluetoothDispatcherHost::OnCharacteristicReadValueError,
@@ -635,50 +697,31 @@ void BluetoothDispatcherHost::OnWriteValue(
     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 device_iter = service_to_device_.find(service_instance_id);
+  auto characteristic_query_outcome =
+      QueryForCharacteristic(characteristic_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);
-    Send(new BluetoothMsg_WriteCharacteristicValueError(
-        thread_id, request_id, WebBluetoothError::DeviceNoLongerInRange));
-    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);
+  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::CharacteristicNoLongerExists));
+        CacheQueryOutcomeToWebBluetoothError(error_outcome)));
     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,
@@ -849,14 +892,19 @@ void BluetoothDispatcherHost::OnServicesDiscovered(
     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