Implement WebBluetooth descriptor.writeValue()

content/browser/bluetooth/web_bluetooth_service_impl.cc

Index: content/browser/bluetooth/web_bluetooth_service_impl.cc
diff --git a/content/browser/bluetooth/web_bluetooth_service_impl.cc b/content/browser/bluetooth/web_bluetooth_service_impl.cc
index 5adce0eeaf41874eec81f3bf15c7b6341768c44f..3f3bfb35179d1c754c283ec46422fd1c37b572f9 100644
--- a/content/browser/bluetooth/web_bluetooth_service_impl.cc
+++ b/content/browser/bluetooth/web_bluetooth_service_impl.cc
@@ -775,6 +775,45 @@ void WebBluetoothServiceImpl::RemoteDescriptorReadValue(
                  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,
@@ -988,11 +1027,27 @@ 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 =