bluetooth: Move read value to mojo

bluetooth: Move read value to mojo

readValue now uses WebBluetoothService. Also renable the characteristicvaluechanged event.

BUG=508771
Committed: https://crrev.com/12e91f07ed210722934545562246c9ffb55cf91c
Cr-Commit-Position: refs/heads/master@{#387736}

[ortuno, 2016-04-13 22:48:29]

Description was changed from

==========
bluetooth: (WIP) Move read value to mojo

BUG=
==========

to

==========
bluetooth: Move read value to mojo

readValue now uses WebBluetoothService. Also renable the
characteristicvaluechanged event.

BUG=508771
==========

[ortuno, 2016-04-13 22:48:45]

ortuno@chromium.org changed reviewers:
+ jyasskin@chromium.org

[ortuno, 2016-04-13 22:48:46]

jyasskin: PTAL

[Jeffrey Yasskin, 2016-04-15 20:59:35]

lgtm

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

https://codereview.chromium.org/1865613002/diff/40001/content/browser/bluetoo...
content/browser/bluetooth/web_bluetooth_service_impl.cc:301:
mojo::Array<uint8_t>::From(value));
Do the by-value and move thing again here. (The caller might not take advantage
of it now, but we can push the optimization through in the future.)

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

https://codereview.chromium.org/1865613002/diff/40001/content/browser/bluetoo...
content/browser/bluetooth/web_bluetooth_service_impl.h:100: // Callbacks for
BluetoothGattCharacteristic::WriteRemoteCharacteristic.
Add a blank line between groups of callbacks.

https://codereview.chromium.org/1865613002/diff/40001/content/renderer/blueto...
File content/renderer/bluetooth/web_bluetooth_impl.cc (right):

https://codereview.chromium.org/1865613002/diff/40001/content/renderer/blueto...
content/renderer/bluetooth/web_bluetooth_impl.cc:133:
base::ThreadTaskRunnerHandle::Get()->PostTask(
Actually, don't post this task:
https://github.com/WebBluetoothCG/web-bluetooth/pull/209 reversed the order of
the event and promise resolution.

[Jeffrey Yasskin, 2016-04-15 21:01:55]

https://codereview.chromium.org/1865613002/diff/40001/content/renderer/blueto...
File content/renderer/bluetooth/web_bluetooth_impl.cc (right):

https://codereview.chromium.org/1865613002/diff/40001/content/renderer/blueto...
content/renderer/bluetooth/web_bluetooth_impl.cc:133:
base::ThreadTaskRunnerHandle::Get()->PostTask(
On 2016/04/15 20:59:35, Jeffrey Yasskin wrote:
> Actually, don't post this task:
> https://github.com/WebBluetoothCG/web-bluetooth/pull/209 reversed the order of
> the event and promise resolution.

Or you can hold off on changing this and the tests. I filed
https://crbug.com/604028 to track it.

[ortuno, 2016-04-15 21:34:07]

Thanks!

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

https://codereview.chromium.org/1865613002/diff/40001/content/browser/bluetoo...
content/browser/bluetooth/web_bluetooth_service_impl.cc:301:
mojo::Array<uint8_t>::From(value));
On 2016/04/15 at 20:59:35, Jeffrey Yasskin wrote:
> Do the by-value and move thing again here. (The caller might not take
advantage of it now, but we can push the optimization through in the future.)

I tried but OnReadValueSuccess needs to be a ValueCallback[1] and that takes a
reference. But maybe I'm missing something?

[1]
https://code.google.com/p/chromium/codesearch#chromium/src/device/bluetooth/b...

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

https://codereview.chromium.org/1865613002/diff/40001/content/browser/bluetoo...
content/browser/bluetooth/web_bluetooth_service_impl.h:100: // Callbacks for
BluetoothGattCharacteristic::WriteRemoteCharacteristic.
On 2016/04/15 at 20:59:35, Jeffrey Yasskin wrote:
> Add a blank line between groups of callbacks.

Done.

https://codereview.chromium.org/1865613002/diff/40001/content/renderer/blueto...
File content/renderer/bluetooth/web_bluetooth_impl.cc (right):

https://codereview.chromium.org/1865613002/diff/40001/content/renderer/blueto...
content/renderer/bluetooth/web_bluetooth_impl.cc:133:
base::ThreadTaskRunnerHandle::Get()->PostTask(
On 2016/04/15 at 21:01:55, Jeffrey Yasskin wrote:
> On 2016/04/15 20:59:35, Jeffrey Yasskin wrote:
> > Actually, don't post this task:
> > https://github.com/WebBluetoothCG/web-bluetooth/pull/209 reversed the order
of
> > the event and promise resolution.
> 
> Or you can hold off on changing this and the tests. I filed
https://crbug.com/604028 to track it.

Interesting. So we don't need to post a task on the browser side either! I'll
hold off.

[ortuno, 2016-04-15 21:35:07]

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

[commit-bot: I haz the power, 2016-04-15 21:35:42]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1865613002/60001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1865613002/60001

[Jeffrey Yasskin, 2016-04-15 21:38:52]

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

https://codereview.chromium.org/1865613002/diff/40001/content/browser/bluetoo...
content/browser/bluetooth/web_bluetooth_service_impl.cc:301:
mojo::Array<uint8_t>::From(value));
On 2016/04/15 21:34:07, ortuno wrote:
> On 2016/04/15 at 20:59:35, Jeffrey Yasskin wrote:
> > Do the by-value and move thing again here. (The caller might not take
> advantage of it now, but we can push the optimization through in the future.)
> 
> I tried but OnReadValueSuccess needs to be a ValueCallback[1] and that takes a
> reference. But maybe I'm missing something?
> 
> [1]
>
https://code.google.com/p/chromium/codesearch#chromium/src/device/bluetooth/b...
> 

That's a good reason. We should eventually fix that, but no need to try in this
CL.

[ortuno, 2016-04-15 22:19:02]

ortuno@chromium.org changed reviewers:
+ tsepez@chromium.org

[ortuno, 2016-04-15 22:19:03]

tsepez: PTAL at content/common/bluetooth_messages.h and
bluetooth/web_bluetooth.mojom

[Tom Sepez, 2016-04-15 22:35:07]

Messages / Mojo LGTM.

[commit-bot: I haz the power, 2016-04-15 22:50:24]

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

[commit-bot: I haz the power, 2016-04-15 22:50:25]

Dry run: This issue passed the CQ dry run.

[ortuno, 2016-04-15 22:50:58]

The CQ bit was checked by ortuno@chromium.org

[ortuno, 2016-04-15 22:50:58]

The patchset sent to the CQ was uploaded after l-g-t-m from
jyasskin@chromium.org
Link to the patchset: https://codereview.chromium.org/1865613002/#ps60001
(title: "Address jyasskin's comments")

[commit-bot: I haz the power, 2016-04-15 22:51:25]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1865613002/60001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1865613002/60001

[commit-bot: I haz the power, 2016-04-15 22:57:55]

Committed patchset #4 (id:60001)

[commit-bot: I haz the power, 2016-04-15 22:59:03]

Description was changed from

==========
bluetooth: Move read value to mojo

readValue now uses WebBluetoothService. Also renable the
characteristicvaluechanged event.

BUG=508771
==========

to

==========
bluetooth: Move read value to mojo

readValue now uses WebBluetoothService. Also renable the
characteristicvaluechanged event.

BUG=508771

Committed: https://crrev.com/12e91f07ed210722934545562246c9ffb55cf91c
Cr-Commit-Position: refs/heads/master@{#387736}
==========

[commit-bot: I haz the power, 2016-04-15 22:59:04]

Patchset 4 (id:??) landed as
https://crrev.com/12e91f07ed210722934545562246c9ffb55cf91c
Cr-Commit-Position: refs/heads/master@{#387736}