Implement WebBluetooth descriptor.writeValue()

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_id_to_characteristic_id_]
 // 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.
 
@@ skipping 757 matching lines @@
     return;
   }
 
   query_result.descriptor->ReadRemoteDescriptor(
       base::Bind(&WebBluetoothServiceImpl::OnDescriptorReadValueSuccess,
                  weak_ptr_factory_.GetWeakPtr(), callback),
       base::Bind(&WebBluetoothServiceImpl::OnDescriptorReadValueFailed,
                  weak_ptr_factory_.GetWeakPtr(), callback));
 }
 
+void WebBluetoothServiceImpl::RemoteDescriptorWriteValue(
+    const std::string& descriptor_instance_id,
+    const std::vector<uint8_t>& value,
+    const RemoteDescriptorWriteValueCallback& callback) {
+  DCHECK_CURRENTLY_ON(BrowserThread::UI);
+
+  // 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) {

[ortuno, 2017/01/28 00:22:51]

hmm we do this check on write value as well. I wonder if we should just typemap
values. No need to do anything in this patch, just thinking out loud.

[dougt, 2017/01/31 00:31:24]

Acknowledged.

[dcheng, 2017/01/31 01:27:01]

We need to check on both ends (as the renderer could be compromised).

There have been some thoughts about how to express this directly in the mojom
syntax, but nothing concrete yet.

+    CrashRendererAndClosePipe(bad_message::BDH_INVALID_WRITE_VALUE_LENGTH);
+    return;
+  }
+
+  const CacheQueryResult query_result =
+      QueryCacheForDescriptor(descriptor_instance_id);
+
+  if (query_result.outcome == CacheQueryOutcome::BAD_RENDERER) {
+    return;
+  }
+
+  if (query_result.outcome != CacheQueryOutcome::SUCCESS) {
+    callback.Run(query_result.GetWebResult());
+    return;
+  }
+
+  if (BluetoothBlocklist::Get().IsExcludedFromWrites(
+          query_result.descriptor->GetUUID())) {
+    callback.Run(blink::mojom::WebBluetoothResult::BLOCKLISTED_WRITE);
+    return;
+  }
+
+  query_result.descriptor->WriteRemoteDescriptor(
+      value, base::Bind(&WebBluetoothServiceImpl::OnDescriptorWriteValueSuccess,
+                        weak_ptr_factory_.GetWeakPtr(), callback),
+      base::Bind(&WebBluetoothServiceImpl::OnDescriptorWriteValueFailed,
+                 weak_ptr_factory_.GetWeakPtr(), callback));
+}
+
 void WebBluetoothServiceImpl::RequestDeviceImpl(
     blink::mojom::WebBluetoothRequestDeviceOptionsPtr options,
     const RequestDeviceCallback& callback,
     device::BluetoothAdapter* adapter) {
   // requestDevice() can only be called when processing a user-gesture and any
   // user gesture outside of a chooser should close the chooser. This does
   // not happen on all platforms so we don't DCHECK that the old one is closed.
   // We destroy the old chooser before constructing the new one to make sure
   // they can't conflict.
   device_chooser_controller_.reset();
@@ skipping 193 matching lines @@
     const std::vector<uint8_t>& value) {
   DCHECK_CURRENTLY_ON(BrowserThread::UI);
   RecordDescriptorReadValueOutcome(UMAGATTOperationOutcome::SUCCESS);
   callback.Run(blink::mojom::WebBluetoothResult::SUCCESS, value);
 }
 
 void WebBluetoothServiceImpl::OnDescriptorReadValueFailed(
     const RemoteDescriptorReadValueCallback& callback,
     device::BluetoothRemoteGattService::GattErrorCode error_code) {
   DCHECK_CURRENTLY_ON(BrowserThread::UI);
+  // TODO(667319) We are reporting failures to UMA but not reporting successes.

[ortuno, 2017/01/28 00:22:51]

I think this TODO is on the wrong function. We do report UMA for both cases in
readValue.

[dougt, 2017/01/31 00:31:24]

I have a CL ready to go and in your queue to fix reporting for
descriptor.writeValue().

[dcheng, 2017/01/31 01:27:01]

Nit: Should be a colon after ).

[dougt, 2017/01/31 18:07:16]

Done.

[ortuno, 2017/01/31 19:50:09]

The problem is not the lack of reporting but rather that this TODO is being
added to the ReadValue callback and not to the WriteValue callback which this
patch adds.

[dougt, 2017/01/31 20:59:16]

Done.

   callback.Run(TranslateGATTErrorAndRecord(error_code,
                                            UMAGATTOperation::DESCRIPTOR_READ),
                base::nullopt /* value */);
 }
 
+void WebBluetoothServiceImpl::OnDescriptorWriteValueSuccess(
+    const RemoteDescriptorWriteValueCallback& callback) {
+  DCHECK_CURRENTLY_ON(BrowserThread::UI);
+  callback.Run(blink::mojom::WebBluetoothResult::SUCCESS);
+}
+
+void WebBluetoothServiceImpl::OnDescriptorWriteValueFailed(
+    const RemoteDescriptorWriteValueCallback& callback,
+    device::BluetoothRemoteGattService::GattErrorCode error_code) {
+  DCHECK_CURRENTLY_ON(BrowserThread::UI);
+  // TODO(683477) reporting for .writeValue()
+  callback.Run(TranslateGATTErrorAndRecord(error_code,
+                                           UMAGATTOperation::DESCRIPTOR_WRITE));
+}
+
 CacheQueryResult WebBluetoothServiceImpl::QueryCacheForDevice(
     const WebBluetoothDeviceId& device_id) {
   const std::string& device_address =
       allowed_devices_map_.GetDeviceAddress(GetOrigin(), device_id);
   if (device_address.empty()) {
     CrashRendererAndClosePipe(bad_message::BDH_DEVICE_NOT_ALLOWED_FOR_ORIGIN);
     return CacheQueryResult(CacheQueryOutcome::BAD_RENDERER);
   }
 
   CacheQueryResult result;
@@ skipping 122 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();
   BluetoothAdapterFactoryWrapper::Get().ReleaseAdapter(this);
 }
 
 }  // namespace content