Implement WebBluetooth descriptor.writeValue()

Implement WebBluetooth descriptor.writeValue()

A GATT Descriptor provides further information about a characteristic's
value. Writing values to a GATT Descriptor can be used to control certain
behaviors of the given device.

For each characteristic there may be many descriptors as defined by
https://webbluetoothcg.github.io/web-bluetooth/#bluetoothremotegattdescriptor.

In Bug 660699, we added support for accessing descriptors from characteristics.

In Bug 668837, we added support for reads descriptors.

In this CL, we add support for writing the value of the descriptor.

BUG=668838
R=scheib

Review-Url: https://codereview.chromium.org/2654403002
Cr-Commit-Position: refs/heads/master@{#447525}
Committed: https://chromium.googlesource.com/chromium/src/+/be62e9d349e2570467d4d86326d9b8f31bedc0fc

[dougt, 2017-01-26 22:14:34]

The CQ bit was checked by dougt@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-01-26 22:15:11]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-01-26 22:19:14]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-01-26 22:19:15]

Dry run: Try jobs failed on following builders:
  ios-device on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-device/builds...)
  ios-device-xcode-clang on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-device-xcode-...)
  ios-simulator on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator/bui...)
  ios-simulator-xcode-clang on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator-xco...)
  mac_chromium_compile_dbg_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_comp...)

[dougt, 2017-01-27 21:37:58]

Patchset #1 (id:1) has been deleted

[dougt, 2017-01-27 21:38:15]

The CQ bit was checked by dougt@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-01-27 21:38:42]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[dougt, 2017-01-27 23:28:52]

dougt@chromium.org changed reviewers:
+ dcheng@chromium.org, ortuno@chromium.org

[dougt, 2017-01-27 23:28:53]

scheib, dcheng, ortuno

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

Not necessary for this patch but we should add the necessary strings to the
fuzzer.

https://codereview.chromium.org/2654403002/diff/20001/content/browser/bluetoo...
File content/browser/bluetooth/web_bluetooth_service_impl.cc (right):

https://codereview.chromium.org/2654403002/diff/20001/content/browser/bluetoo...
content/browser/bluetooth/web_bluetooth_service_impl.cc:787: if (value.size() >
512) {
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.

https://codereview.chromium.org/2654403002/diff/20001/content/browser/bluetoo...
content/browser/bluetooth/web_bluetooth_service_impl.cc:1030: // TODO(667319) We
are reporting failures to UMA but not reporting successes.
I think this TODO is on the wrong function. We do report UMA for both cases in
readValue.

https://codereview.chromium.org/2654403002/diff/20001/content/browser/bluetoo...
File content/browser/bluetooth/web_bluetooth_service_impl.h (right):

https://codereview.chromium.org/2654403002/diff/20001/content/browser/bluetoo...
content/browser/bluetooth/web_bluetooth_service_impl.h:139: 
nit: no new line.

https://codereview.chromium.org/2654403002/diff/20001/content/shell/browser/l...
File content/shell/browser/layout_test/layout_test_bluetooth_adapter_provider.cc
(right):

https://codereview.chromium.org/2654403002/diff/20001/content/shell/browser/l...
content/shell/browser/layout_test/layout_test_bluetooth_adapter_provider.cc:762:
ON_CALL(*user_description, WriteRemoteDescriptor(_, _, _))
Please update the docs in the header to reflect the new adapter properties.

https://codereview.chromium.org/2654403002/diff/20001/content/shell/browser/l...
content/shell/browser/layout_test/layout_test_bluetooth_adapter_provider.cc:771:
ON_CALL(*client_config, WriteRemoteDescriptor(_, _, _))
nit for consistency:

   // Crash if WriteRemoteDescriptor called. Not using GoogleMock's Expect
   // because this is used in layout tests that may not report a mock
   // expectation.

https://codereview.chromium.org/2654403002/diff/20001/third_party/WebKit/Layo...
File
third_party/WebKit/LayoutTests/bluetooth/descriptor/writeValue/value-too-long.html
(right):

https://codereview.chromium.org/2654403002/diff/20001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/bluetooth/descriptor/writeValue/value-too-long.html:22:
}, 'A regular write request to a writable descriptor should succeed.');
nit: Fix description and formatting.

https://codereview.chromium.org/2654403002/diff/20001/third_party/WebKit/Layo...
File
third_party/WebKit/LayoutTests/bluetooth/descriptor/writeValue/write-succeeds.html
(right):

https://codereview.chromium.org/2654403002/diff/20001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/bluetooth/descriptor/writeValue/write-succeeds.html:18:
descriptor.writeValue(new DataView(new ArrayBuffer(1 /* length */)))]));
This would fail in actual devices because there can only be one operation at a
time. Could you serialize this?

https://codereview.chromium.org/2654403002/diff/20001/third_party/WebKit/Layo...
File
third_party/WebKit/LayoutTests/bluetooth/script-tests/descriptor/service-is-removed.js
(right):

https://codereview.chromium.org/2654403002/diff/20001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/bluetooth/script-tests/descriptor/service-is-removed.js:5:
.then(() => requestDeviceWithKeyDown({
wow what happened with the formatting?

[commit-bot: I haz the power, 2017-01-28 00:30:03]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-01-28 00:30:04]

Dry run: This issue passed the CQ dry run.

[dougt, 2017-01-31 00:30:21]

The CQ bit was checked by dougt@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-01-31 00:30:44]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[dougt, 2017-01-31 00:31:25]

dcheng, ptal.

https://codereview.chromium.org/2654403002/diff/20001/content/browser/bluetoo...
File content/browser/bluetooth/web_bluetooth_service_impl.cc (right):

https://codereview.chromium.org/2654403002/diff/20001/content/browser/bluetoo...
content/browser/bluetooth/web_bluetooth_service_impl.cc:787: if (value.size() >
512) {
On 2017/01/28 00:22:51, ortuno wrote:
> 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.

Acknowledged.

https://codereview.chromium.org/2654403002/diff/20001/content/browser/bluetoo...
content/browser/bluetooth/web_bluetooth_service_impl.cc:1030: // TODO(667319) We
are reporting failures to UMA but not reporting successes.
On 2017/01/28 00:22:51, ortuno wrote:
> I think this TODO is on the wrong function. We do report UMA for both cases in
> readValue.

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

https://codereview.chromium.org/2654403002/diff/20001/content/browser/bluetoo...
File content/browser/bluetooth/web_bluetooth_service_impl.h (right):

https://codereview.chromium.org/2654403002/diff/20001/content/browser/bluetoo...
content/browser/bluetooth/web_bluetooth_service_impl.h:139: 
On 2017/01/28 00:22:51, ortuno wrote:
> nit: no new line.

Done.

https://codereview.chromium.org/2654403002/diff/20001/content/shell/browser/l...
File content/shell/browser/layout_test/layout_test_bluetooth_adapter_provider.cc
(right):

https://codereview.chromium.org/2654403002/diff/20001/content/shell/browser/l...
content/shell/browser/layout_test/layout_test_bluetooth_adapter_provider.cc:762:
ON_CALL(*user_description, WriteRemoteDescriptor(_, _, _))
On 2017/01/28 00:22:51, ortuno wrote:
> Please update the docs in the header to reflect the new adapter properties.

Done.

https://codereview.chromium.org/2654403002/diff/20001/content/shell/browser/l...
content/shell/browser/layout_test/layout_test_bluetooth_adapter_provider.cc:771:
ON_CALL(*client_config, WriteRemoteDescriptor(_, _, _))
On 2017/01/28 00:22:51, ortuno wrote:
> nit for consistency:
> 
>    // Crash if WriteRemoteDescriptor called. Not using GoogleMock's Expect
>    // because this is used in layout tests that may not report a mock
>    // expectation.

Done.

https://codereview.chromium.org/2654403002/diff/20001/third_party/WebKit/Layo...
File
third_party/WebKit/LayoutTests/bluetooth/descriptor/writeValue/value-too-long.html
(right):

https://codereview.chromium.org/2654403002/diff/20001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/bluetooth/descriptor/writeValue/value-too-long.html:22:
}, 'A regular write request to a writable descriptor should succeed.');
On 2017/01/28 00:22:51, ortuno wrote:
> nit: Fix description and formatting.

Done.

https://codereview.chromium.org/2654403002/diff/20001/third_party/WebKit/Layo...
File
third_party/WebKit/LayoutTests/bluetooth/descriptor/writeValue/write-succeeds.html
(right):

https://codereview.chromium.org/2654403002/diff/20001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/bluetooth/descriptor/writeValue/write-succeeds.html:18:
descriptor.writeValue(new DataView(new ArrayBuffer(1 /* length */)))]));
On 2017/01/28 00:22:51, ortuno wrote:
> This would fail in actual devices because there can only be one operation at a
> time. Could you serialize this?

Done.  Also addressed the test cases in characteristic/writeValue in a different
CL.

https://codereview.chromium.org/2654403002/diff/20001/third_party/WebKit/Layo...
File
third_party/WebKit/LayoutTests/bluetooth/script-tests/descriptor/service-is-removed.js
(right):

https://codereview.chromium.org/2654403002/diff/20001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/bluetooth/script-tests/descriptor/service-is-removed.js:5:
.then(() => requestDeviceWithKeyDown({
On 2017/01/28 00:22:51, ortuno wrote:
> wow what happened with the formatting?

git cl format, yo.

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

https://codereview.chromium.org/2654403002/diff/20001/content/browser/bluetoo...
File content/browser/bluetooth/web_bluetooth_service_impl.cc (right):

https://codereview.chromium.org/2654403002/diff/20001/content/browser/bluetoo...
content/browser/bluetooth/web_bluetooth_service_impl.cc:787: if (value.size() >
512) {
On 2017/01/28 00:22:51, ortuno wrote:
> 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.

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.

https://codereview.chromium.org/2654403002/diff/20001/content/browser/bluetoo...
content/browser/bluetooth/web_bluetooth_service_impl.cc:1030: // TODO(667319) We
are reporting failures to UMA but not reporting successes.
On 2017/01/28 00:22:51, ortuno wrote:
> I think this TODO is on the wrong function. We do report UMA for both cases in
> readValue.

Nit: Should be a colon after ).

https://codereview.chromium.org/2654403002/diff/20001/third_party/WebKit/Layo...
File
third_party/WebKit/LayoutTests/bluetooth/descriptor/readValue/gen-io-op-reconnect-during-error.html
(right):

https://codereview.chromium.org/2654403002/diff/20001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/bluetooth/descriptor/readValue/gen-io-op-reconnect-during-error.html:9:
let val = new Uint8Array([1]);
I guess these are all auto-generated, but it does feel weird to have an unused
|val| here. *shrug*

https://codereview.chromium.org/2654403002/diff/20001/third_party/WebKit/Sour...
File
third_party/WebKit/Source/modules/bluetooth/BluetoothRemoteGATTDescriptor.cpp
(right):

https://codereview.chromium.org/2654403002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/modules/bluetooth/BluetoothRemoteGATTDescriptor.cpp:140:
valueVector.append(value.bytes(), value.byteLength());
Is it possible to do this without copying?

[commit-bot: I haz the power, 2017-01-31 02:12:55]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-01-31 02:12:56]

Dry run: This issue passed the CQ dry run.

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

ortuno, dcheng, ptal

https://codereview.chromium.org/2654403002/diff/20001/content/browser/bluetoo...
File content/browser/bluetooth/web_bluetooth_service_impl.cc (right):

https://codereview.chromium.org/2654403002/diff/20001/content/browser/bluetoo...
content/browser/bluetooth/web_bluetooth_service_impl.cc:1030: // TODO(667319) We
are reporting failures to UMA but not reporting successes.
On 2017/01/31 01:27:01, dcheng wrote:
> On 2017/01/28 00:22:51, ortuno wrote:
> > I think this TODO is on the wrong function. We do report UMA for both cases
in
> > readValue.
> 
> Nit: Should be a colon after ).

Done.

https://codereview.chromium.org/2654403002/diff/20001/third_party/WebKit/Layo...
File
third_party/WebKit/LayoutTests/bluetooth/descriptor/readValue/gen-io-op-reconnect-during-error.html
(right):

https://codereview.chromium.org/2654403002/diff/20001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/bluetooth/descriptor/readValue/gen-io-op-reconnect-during-error.html:9:
let val = new Uint8Array([1]);
On 2017/01/31 01:27:01, dcheng wrote:
> I guess these are all auto-generated, but it does feel weird to have an unused
> |val| here. *shrug*

Much improvement is needed. I think we're waiting to move to web platform tests
to clean up things like this.

https://codereview.chromium.org/2654403002/diff/20001/third_party/WebKit/Sour...
File
third_party/WebKit/Source/modules/bluetooth/BluetoothRemoteGATTDescriptor.cpp
(right):

https://codereview.chromium.org/2654403002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/modules/bluetooth/BluetoothRemoteGATTDescriptor.cpp:140:
valueVector.append(value.bytes(), value.byteLength());
On 2017/01/31 01:27:01, dcheng wrote:
> Is it possible to do this without copying?

Two answers:

1) no. But I filed a bug:
https://bugs.chromium.org/p/chromium/issues/detail?id=687238

2) no. Per spec we're copying:
https://webbluetoothcg.github.io/web-bluetooth/#dom-bluetoothremotegattdescri...

We are discussing if this is an issue or not.

[dougt, 2017-01-31 18:48:58]

ortuno, dcheng, PTAL.

[dougt, 2017-01-31 18:52:01]

The CQ bit was checked by dougt@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-01-31 18:52:38]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

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

https://codereview.chromium.org/2654403002/diff/20001/content/browser/bluetoo...
File content/browser/bluetooth/web_bluetooth_service_impl.cc (right):

https://codereview.chromium.org/2654403002/diff/20001/content/browser/bluetoo...
content/browser/bluetooth/web_bluetooth_service_impl.cc:1030: // TODO(667319) We
are reporting failures to UMA but not reporting successes.
On 2017/01/31 at 18:07:16, dougt wrote:
> On 2017/01/31 01:27:01, dcheng wrote:
> > On 2017/01/28 00:22:51, ortuno wrote:
> > > I think this TODO is on the wrong function. We do report UMA for both
cases in
> > > readValue.
> > 
> > Nit: Should be a colon after ).
> 
> Done.

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.

https://codereview.chromium.org/2654403002/diff/60001/content/browser/bluetoo...
File content/browser/bluetooth/bluetooth_metrics.h (right):

https://codereview.chromium.org/2654403002/diff/60001/content/browser/bluetoo...
content/browser/bluetooth/bluetooth_metrics.h:39: DESCRIPTOR_WRITE_VALUE = 12,
Add this to histograms.xml

https://codereview.chromium.org/2654403002/diff/60001/content/shell/browser/l...
File content/shell/browser/layout_test/layout_test_bluetooth_adapter_provider.h
(right):

https://codereview.chromium.org/2654403002/diff/60001/content/shell/browser/l...
content/shell/browser/layout_test/layout_test_bluetooth_adapter_provider.h:346:
//                 Note: This descriptor can be written to and will always
Let's try to keep the formatting consistent:

//        - Mock Functions:
//           - Read: Calls success callback with
"gatt.characteristic_user_description".
//           - Write: Calls success callback.

https://codereview.chromium.org/2654403002/diff/60001/content/shell/browser/l...
content/shell/browser/layout_test/layout_test_bluetooth_adapter_provider.h:406:
//                 - Operations read / write nearly identical to the read and
Not sure why this comment is here?

https://codereview.chromium.org/2654403002/diff/60001/third_party/WebKit/Layo...
File
third_party/WebKit/LayoutTests/bluetooth/script-tests/descriptor/service-is-removed.js
(right):

https://codereview.chromium.org/2654403002/diff/60001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/bluetooth/script-tests/descriptor/service-is-removed.js:5:
.then(() => requestDeviceWithKeyDown({
Why is this the only file whose format changed? io-op-fails.js has the old
format.

[commit-bot: I haz the power, 2017-01-31 20:37:49]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-01-31 20:37:50]

Dry run: This issue passed the CQ dry run.

[dougt, 2017-01-31 20:59:05]

The CQ bit was checked by dougt@chromium.org to run a CQ dry run

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

https://codereview.chromium.org/2654403002/diff/20001/content/browser/bluetoo...
File content/browser/bluetooth/web_bluetooth_service_impl.cc (right):

https://codereview.chromium.org/2654403002/diff/20001/content/browser/bluetoo...
content/browser/bluetooth/web_bluetooth_service_impl.cc:1030: // TODO(667319) We
are reporting failures to UMA but not reporting successes.
On 2017/01/31 19:50:09, ortuno wrote:
> On 2017/01/31 at 18:07:16, dougt wrote:
> > On 2017/01/31 01:27:01, dcheng wrote:
> > > On 2017/01/28 00:22:51, ortuno wrote:
> > > > I think this TODO is on the wrong function. We do report UMA for both
> cases in
> > > > readValue.
> > > 
> > > Nit: Should be a colon after ).
> > 
> > Done.
> 
> 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.

Done.

https://codereview.chromium.org/2654403002/diff/60001/content/browser/bluetoo...
File content/browser/bluetooth/bluetooth_metrics.h (right):

https://codereview.chromium.org/2654403002/diff/60001/content/browser/bluetoo...
content/browser/bluetooth/bluetooth_metrics.h:39: DESCRIPTOR_WRITE_VALUE = 12,
This change is in https://codereview.chromium.org/2662433003 which has a lgtm. 
Do you want me to move that change to this CL?

https://codereview.chromium.org/2654403002/diff/60001/content/shell/browser/l...
File content/shell/browser/layout_test/layout_test_bluetooth_adapter_provider.h
(right):

https://codereview.chromium.org/2654403002/diff/60001/content/shell/browser/l...
content/shell/browser/layout_test/layout_test_bluetooth_adapter_provider.h:346:
//                 Note: This descriptor can be written to and will always
On 2017/01/31 19:50:09, ortuno wrote:
> Let's try to keep the formatting consistent:
> 
> //        - Mock Functions:
> //           - Read: Calls success callback with
> "gatt.characteristic_user_description".
> //           - Write: Calls success callback.

Done.

https://codereview.chromium.org/2654403002/diff/60001/content/shell/browser/l...
content/shell/browser/layout_test/layout_test_bluetooth_adapter_provider.h:406:
//                 - Operations read / write nearly identical to the read and
On 2017/01/31 19:50:09, ortuno wrote:
> Not sure why this comment is here?

I am documenting that under the Measurement Interval characteristic, there is a
user_descriptor descriptor.

https://codereview.chromium.org/2654403002/diff/60001/third_party/WebKit/Layo...
File
third_party/WebKit/LayoutTests/bluetooth/script-tests/descriptor/service-is-removed.js
(right):

https://codereview.chromium.org/2654403002/diff/60001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/bluetooth/script-tests/descriptor/service-is-removed.js:5:
.then(() => requestDeviceWithKeyDown({
On 2017/01/31 19:50:09, ortuno wrote:
> Why is this the only file whose format changed? io-op-fails.js has the old
> format.

io-op-fails.js has the new format (just look at all of that white space!).

[commit-bot: I haz the power, 2017-01-31 20:59:37]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-01-31 23:11:40]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-01-31 23:11:42]

Dry run: This issue passed the CQ dry run.

[ortuno, 2017-02-01 00:01:59]

https://codereview.chromium.org/2654403002/diff/60001/content/browser/bluetoo...
File content/browser/bluetooth/bluetooth_metrics.h (right):

https://codereview.chromium.org/2654403002/diff/60001/content/browser/bluetoo...
content/browser/bluetooth/bluetooth_metrics.h:39: DESCRIPTOR_WRITE_VALUE = 12,
On 2017/01/31 at 20:59:16, dougt wrote:
> This change is in https://codereview.chromium.org/2662433003 which has a lgtm.
 Do you want me to move that change to this CL?

Not really. This changes the WebBluetoothFunction enum in histograms.xml[1]
(which now that I see is missing all the descriptors enums.) The change in the
follow up is for the output of the descriptor.writeValue().

[1]
https://cs.chromium.org/chromium/src/tools/metrics/histograms/histograms.xml?...

https://codereview.chromium.org/2654403002/diff/60001/third_party/WebKit/Layo...
File
third_party/WebKit/LayoutTests/bluetooth/script-tests/descriptor/service-is-removed.js
(right):

https://codereview.chromium.org/2654403002/diff/60001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/bluetooth/script-tests/descriptor/service-is-removed.js:5:
.then(() => requestDeviceWithKeyDown({
On 2017/01/31 at 20:59:16, dougt wrote:
> On 2017/01/31 19:50:09, ortuno wrote:
> > Why is this the only file whose format changed? io-op-fails.js has the old
> > format.
> 
> io-op-fails.js has the new format (just look at all of that white space!).

I mean the old new format... I see a lot of whitespace on io-op-fails.js but not
much here. E.g. () => is on the same line as .then here but on io-op-fails.js
it's on a new line.

[dougt, 2017-02-01 00:04:54]

> I mean the old new format... I see a lot of whitespace on io-op-fails.js but
not
> much here. E.g. () => is on the same line as .then here but on io-op-fails.js
> it's on a new line.

/me shrugs.  git cl format doesn't change that file.

[ortuno, 2017-02-01 01:47:49]

lgtm but repeating bluetooth_metrics.h comment in case you didn't see:

[This is not currently address in the follow up patch]. This changes the
WebBluetoothFunction enum in histograms.xml[1] (which now that I see is missing
all the descriptors enums.) The change in the follow up is for the output of the
descriptor.writeValue().

[1]
https://cs.chromium.org/chromium/src/tools/metrics/histograms/histograms.xml?...

[dougt, 2017-02-01 02:36:13]

On 2017/02/01 01:47:49, ortuno wrote:
> lgtm but repeating bluetooth_metrics.h comment in case you didn't see:
> 
> [This is not currently address in the follow up patch]. This changes the
> WebBluetoothFunction enum in histograms.xml[1] (which now that I see is
missing
> all the descriptors enums.) The change in the follow up is for the output of
the
> descriptor.writeValue().
> 
> [1]
>
https://cs.chromium.org/chromium/src/tools/metrics/histograms/histograms.xml?...

follow up: https://codereview.chromium.org/2667053002

[dcheng, 2017-02-01 08:02:41]

LGTM. As mentioned, we can try to figure out how to address the issue of copying
in a followup (it seems like Mojo itself isn't well adapted to this atm)

[dougt, 2017-02-01 16:08:52]

The CQ bit was checked by dougt@chromium.org

[commit-bot: I haz the power, 2017-02-01 16:09:08]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-02-01 16:14:03]

CQ is committing da patch.

Bot data: {"patchset_id": 80001, "attempt_start_ts": 1485965332271520,
"parent_rev": "f2b188baeb2938427bf58b6ff6b61840f325055b", "commit_rev":
"be62e9d349e2570467d4d86326d9b8f31bedc0fc"}

[commit-bot: I haz the power, 2017-02-01 16:14:41]

Description was changed from

==========
Implement WebBluetooth descriptor.writeValue()

A GATT Descriptor provides further information about a characteristic's
value. Writing values to a GATT Descriptor can be used to control certain
behaviors of the given device.

For each characteristic there may be many descriptors as defined by
https://webbluetoothcg.github.io/web-bluetooth/#bluetoothremotegattdescriptor.

In Bug 660699, we added support for accessing descriptors from characteristics.

In Bug 668837, we added support for reads descriptors.

In this CL, we add support for writing the value of the descriptor.

BUG=668838
R=scheib
==========

to

==========
Implement WebBluetooth descriptor.writeValue()

A GATT Descriptor provides further information about a characteristic's
value. Writing values to a GATT Descriptor can be used to control certain
behaviors of the given device.

For each characteristic there may be many descriptors as defined by
https://webbluetoothcg.github.io/web-bluetooth/#bluetoothremotegattdescriptor.

In Bug 660699, we added support for accessing descriptors from characteristics.

In Bug 668837, we added support for reads descriptors.

In this CL, we add support for writing the value of the descriptor.

BUG=668838
R=scheib

Review-Url: https://codereview.chromium.org/2654403002
Cr-Commit-Position: refs/heads/master@{#447525}
Committed:
https://chromium.googlesource.com/chromium/src/+/be62e9d349e2570467d4d86326d9...
==========

[commit-bot: I haz the power, 2017-02-01 16:14:43]

Committed patchset #4 (id:80001) as
https://chromium.googlesource.com/chromium/src/+/be62e9d349e2570467d4d86326d9...