bluetooth: Browser-side implementation of readValue

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 a2c26ec8f833135eee8edafe1cae370e840a3b33..b3c543b2db0f450c8af37212763ac4c3a0003d68 100644
--- a/content/browser/bluetooth/bluetooth_dispatcher_host.cc
+++ b/content/browser/bluetooth/bluetooth_dispatcher_host.cc
@@ -58,6 +58,7 @@ bool BluetoothDispatcherHost::OnMessageReceived(const IPC::Message& message) {
   IPC_MESSAGE_HANDLER(BluetoothHostMsg_ConnectGATT, OnConnectGATT)
   IPC_MESSAGE_HANDLER(BluetoothHostMsg_GetPrimaryService, OnGetPrimaryService)
   IPC_MESSAGE_HANDLER(BluetoothHostMsg_GetCharacteristic, OnGetCharacteristic)
+  IPC_MESSAGE_HANDLER(BluetoothHostMsg_ReadValue, OnReadValue)
   IPC_MESSAGE_UNHANDLED(handled = false)
   IPC_END_MESSAGE_MAP()
   return handled;
@@ -194,10 +195,21 @@ void BluetoothDispatcherHost::OnGetCharacteristic(
   for (BluetoothGattCharacteristic* characteristic :
        service->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 the characteristic existed already check that service_instance_id is
+      // the same.
+      if (!insert_result.second)

[scheib, 2015/06/11 20:20:50]

Reduce comment size,
if (!insert_result.second)  // If a value is already in map DCHECK it's valid.

[ortuno, 2015/06/11 21:17:09]

Done.

[scheib, 2015/06/11 21:51:57]

You can do it on one line, 
"""
if (!insert_result.second)  // If a value is already in map DCHECK it's valid.
"""

[ortuno, 2015/06/11 22:04:38]

That's longer that character limit.

+        DCHECK(insert_result.first->second == service_instance_id);
+
       // TODO(ortuno): Use generated instance ID instead.
       // https://crbug.com/495379
       Send(new BluetoothMsg_GetCharacteristicSuccess(
-          thread_id, request_id, characteristic->GetIdentifier()));
+          thread_id, request_id, characteristic_instance_id));
       return;
     }
   }
@@ -205,6 +217,67 @@ void BluetoothDispatcherHost::OnGetCharacteristic(
                                                BluetoothError::NOT_FOUND));
 }
 
+void BluetoothDispatcherHost::OnReadValue(
+    int thread_id,
+    int request_id,
+    const std::string& characteristic_instance_id) {
+  DCHECK_CURRENTLY_ON(BrowserThread::UI);
+
+  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);
+  // A service_instance_id not in the map implies a hostile renderer

[scheib, 2015/06/11 20:20:50]

Isn't the first map check sufficient for bad renderer detection? If we found the
characteristic_iter, then the renderer is OK. If we then can't follow it through
service_to_device that's a data consistency error in this class.

What to do about it:
Probably DCHECK and handle it as a NOT_FOUND for now, with TODO to use
InvalidStateError https://github.com/WebBluetoothCG/web-bluetooth/issues/130

[ortuno, 2015/06/11 21:17:09]

Done.

+  // 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;
+  }
+
+  device::BluetoothDevice* device =
+      adapter_->GetDevice(device_iter->second /* device_instance_id */);
+  if (device == nullptr) {
+    Send(new BluetoothMsg_ReadCharacteristicValueError(
+        thread_id, request_id, BluetoothError::NETWORK_ERROR));
+    return;
+  }
+
+  BluetoothGattService* service = device->GetGattService(service_instance_id);
+  if (service == nullptr) {
+    // TODO(ortuno): Change to InvalidStateError once implemented:
+    // http://crbug.com/499014
+    Send(new BluetoothMsg_ReadCharacteristicValueError(
+        thread_id, request_id, BluetoothError::NETWORK_ERROR));
+    return;
+  }
+
+  BluetoothGattCharacteristic* characteristic =
+      service->GetCharacteristic(characteristic_instance_id);
+  if (characteristic == nullptr) {
+    Send(new BluetoothMsg_ReadCharacteristicValueError(
+        thread_id, request_id, BluetoothError::NETWORK_ERROR));
+    return;
+  }
+
+  characteristic->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::OnDiscoverySessionStarted(
     int thread_id,
     int request_id,
@@ -329,4 +402,21 @@ void BluetoothDispatcherHost::OnServicesDiscovered(
                                                BluetoothError::NOT_FOUND));
 }
 
+void BluetoothDispatcherHost::OnCharacteristicValueRead(
+    int thread_id,
+    int request_id,
+    const std::vector<uint8>& value) {
+  Send(new BluetoothMsg_ReadCharacteristicValueSuccess(thread_id, request_id,
+                                                       value));
+}
+
+void BluetoothDispatcherHost::OnCharacteristicReadValueError(
+    int thread_id,
+    int request_id,
+    device::BluetoothGattService::GattErrorCode) {
+  // TODO(ortuno): Send a different Error based on the GattErrorCode.

[scheib, 2015/06/11 20:20:49]

Issue number for this.

[ortuno, 2015/06/11 21:17:09]

Done.

+  Send(new BluetoothMsg_ReadCharacteristicValueError(
+      thread_id, request_id, BluetoothError::NETWORK_ERROR));
+}
+
 }  // namespace content