bluetooth: Provide more device attributes in requestDevice reply.

bluetooth: Provide more device attributes in requestDevice reply.

BluetoothDispatcherHost is extended to supply all the
attributes of the BluetoothDevice web API.

Also, add device::BluetoothDevice::VENDOR_ID_MAX_VALUE for
use in the IPC message.

Depends on https://codereview.chromium.org/882223002/ to
define new attributes on WebBluetoothDevice, and will not
be visible to script until
https://codereview.chromium.org/876623003/

BUG=438305
Committed: https://crrev.com/3478c0790481841a0440e050bf5d1f870a33e0f1
Cr-Commit-Position: refs/heads/master@{#313999}

[scheib, 2015-01-28 21:11:00]

Patchset #1 (id:1) has been deleted

[scheib, 2015-01-28 21:15:06]

scheib@chromium.org changed reviewers:
+ mek@chromium.org

[Marijn Kruisselbrink, 2015-01-28 21:45:56]

https://codereview.chromium.org/885723002/diff/20001/content/browser/bluetoot...
File content/browser/bluetooth/bluetooth_dispatcher_host.cc (right):

https://codereview.chromium.org/885723002/diff/20001/content/browser/bluetoot...
content/browser/bluetooth/bluetooth_dispatcher_host.cc:15: using
device::BluetoothDevice;
you should probably get rid of this using statement. Now with both
device::BluetoothDevice and content::BluetoothDevice you have to be explicit
everywhere anyway.

https://codereview.chromium.org/885723002/diff/20001/content/common/bluetooth...
File content/common/bluetooth/bluetooth_device.cc (right):

https://codereview.chromium.org/885723002/diff/20001/content/common/bluetooth...
content/common/bluetooth/bluetooth_device.cc:13: name(base::EmptyString16()),
From the EmptyString16 documentation: "DO NOT USE THESE AS A GENERAL-PURPOSE
SUBSTITUTE FOR DEFAULT CONSTRUCTORS"

https://codereview.chromium.org/885723002/diff/20001/content/common/bluetooth...
content/common/bluetooth/bluetooth_device.cc:54: std::vector<std::string>
uuids(uuid_list.size());
I think this ends up creating a vector that is twice the size of uuid_list, with
first uuid_list.size() empty strings, followed by the actual strings

https://codereview.chromium.org/885723002/diff/20001/content/common/bluetooth...
File content/common/bluetooth/bluetooth_device.h (right):

https://codereview.chromium.org/885723002/diff/20001/content/common/bluetooth...
content/common/bluetooth/bluetooth_device.h:19: struct CONTENT_EXPORT
BluetoothDevice {
I was wondering if you could just use blink::WebBluetoothDevice directly (but I
think I've convinced myself that wouldn't be worth it).
The problems would be the WebVector<WebString>, and probably even the WebString
instances themseves.

ViewHostMsg_CreateWindow has a similar struct and gets around this (with its
blink::WebWindowFeatures struct) by passing the WebWindowFeatures struct minus
its WebVector field, and separately passing the vector as a
std::vector<base::string16>, and then merging it back together in the browser
process (see
https://code.google.com/p/chromium/codesearch#chromium/src/content/common/vie...
and
https://code.google.com/p/chromium/codesearch#chromium/src/content/browser/re...)

https://codereview.chromium.org/885723002/diff/20001/content/common/bluetooth...
content/common/bluetooth/bluetooth_device.h:22: const base::string16& name_in,
why name_in and not just name?

[scheib, 2015-01-28 22:23:55]

+armansito as bluetooth expert and device/bluetooth OWNERS

https://codereview.chromium.org/885723002/diff/20001/content/browser/bluetoot...
File content/browser/bluetooth/bluetooth_dispatcher_host.cc (right):

https://codereview.chromium.org/885723002/diff/20001/content/browser/bluetoot...
content/browser/bluetooth/bluetooth_dispatcher_host.cc:15: using
device::BluetoothDevice;
On 2015/01/28 21:45:56, Marijn Kruisselbrink wrote:
> you should probably get rid of this using statement. Now with both
> device::BluetoothDevice and content::BluetoothDevice you have to be explicit
> everywhere anyway.

Done.

https://codereview.chromium.org/885723002/diff/20001/content/common/bluetooth...
File content/common/bluetooth/bluetooth_device.cc (right):

https://codereview.chromium.org/885723002/diff/20001/content/common/bluetooth...
content/common/bluetooth/bluetooth_device.cc:13: name(base::EmptyString16()),
On 2015/01/28 21:45:56, Marijn Kruisselbrink wrote:
> From the EmptyString16 documentation: "DO NOT USE THESE AS A GENERAL-PURPOSE
> SUBSTITUTE FOR DEFAULT CONSTRUCTORS"

Done.

https://codereview.chromium.org/885723002/diff/20001/content/common/bluetooth...
content/common/bluetooth/bluetooth_device.cc:54: std::vector<std::string>
uuids(uuid_list.size());
On 2015/01/28 21:45:56, Marijn Kruisselbrink wrote:
> I think this ends up creating a vector that is twice the size of uuid_list,
with
> first uuid_list.size() empty strings, followed by the actual strings

Done. Thanks, was a bug.

https://codereview.chromium.org/885723002/diff/20001/content/common/bluetooth...
File content/common/bluetooth/bluetooth_device.h (right):

https://codereview.chromium.org/885723002/diff/20001/content/common/bluetooth...
content/common/bluetooth/bluetooth_device.h:19: struct CONTENT_EXPORT
BluetoothDevice {
On 2015/01/28 21:45:56, Marijn Kruisselbrink wrote:
> I was wondering if you could just use blink::WebBluetoothDevice directly (but
I
> think I've convinced myself that wouldn't be worth it).
> The problems would be the WebVector<WebString>, and probably even the
WebString
> instances themseves.
> 
> ViewHostMsg_CreateWindow has a similar struct and gets around this (with its
> blink::WebWindowFeatures struct) by passing the WebWindowFeatures struct minus
> its WebVector field, and separately passing the vector as a
> std::vector<base::string16>, and then merging it back together in the browser
> process (see
>
https://code.google.com/p/chromium/codesearch#chromium/src/content/common/vie...
> and
>
https://code.google.com/p/chromium/codesearch#chromium/src/content/browser/re...)
> 

Yes, the strings and vectors are a major issue, also now DEPS are set to prevent
this type of sharing. I had a brief chat with jam@ about that and it seems the
duplication is favored.

https://codereview.chromium.org/885723002/diff/20001/content/common/bluetooth...
content/common/bluetooth/bluetooth_device.h:22: const base::string16& name_in,
On 2015/01/28 21:45:56, Marijn Kruisselbrink wrote:
> why name_in and not just name?

Done.

[scheib, 2015-01-28 22:27:04]

scheib@chromium.org changed reviewers:
+ armansito@chromium.org

[scheib, 2015-01-28 22:27:04]

+armansito as bluetooth expert and device/bluetooth OWNERS

[Marijn Kruisselbrink, 2015-01-28 22:32:44]

lgtm

[armansito, 2015-01-29 04:27:33]

https://codereview.chromium.org/885723002/diff/40001/content/common/bluetooth...
File content/common/bluetooth/bluetooth_device.h (right):

https://codereview.chromium.org/885723002/diff/40001/content/common/bluetooth...
content/common/bluetooth/bluetooth_device.h:1: // Copyright 2014 The Chromium
Authors. All rights reserved.
nit: s/2014/2015/

https://codereview.chromium.org/885723002/diff/40001/content/common/bluetooth...
content/common/bluetooth/bluetooth_device.h:45: std::vector<std::string> uuids; 
// 128bit UUIDs with dashes. 36 chars.
You could just store these as device::BluetoothUUID as well.

[scheib, 2015-01-29 16:23:03]

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

[scheib, 2015-01-29 16:23:04]

+tsepez IPC messages OWNERS review.

https://codereview.chromium.org/885723002/diff/40001/content/common/bluetooth...
File content/common/bluetooth/bluetooth_device.h (right):

https://codereview.chromium.org/885723002/diff/40001/content/common/bluetooth...
content/common/bluetooth/bluetooth_device.h:1: // Copyright 2014 The Chromium
Authors. All rights reserved.
On 2015/01/29 04:27:33, armansito wrote:
> nit: s/2014/2015/

Done.

https://codereview.chromium.org/885723002/diff/40001/content/common/bluetooth...
content/common/bluetooth/bluetooth_device.h:45: std::vector<std::string> uuids; 
// 128bit UUIDs with dashes. 36 chars.
On 2015/01/29 04:27:33, armansito wrote:
> You could just store these as device::BluetoothUUID as well.

I can't send device::BluetoothUUID all the way to the blink bindings, so I
convert them here where all the rest of the src/device interfacing is done. By
the time messages get to bluetooth_dispatcher.cc in the renderer process there
are no references to src/device.

[Tom Sepez, 2015-01-29 17:27:20]

Messages LGTM.

[armansito, 2015-01-29 17:37:46]

lgtm

[scheib, 2015-01-30 00:13:13]

The CQ bit was checked by scheib@chromium.org

[commit-bot: I haz the power, 2015-01-30 00:14:06]

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

[scheib, 2015-01-30 00:18:06]

The CQ bit was checked by scheib@chromium.org

[scheib, 2015-01-30 00:18:33]

Patchset #3 (id:60001) has been deleted

[commit-bot: I haz the power, 2015-01-30 00:19:15]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/885723002/80001

[commit-bot: I haz the power, 2015-01-30 00:28:40]

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

[commit-bot: I haz the power, 2015-01-30 00:28:41]

Try jobs failed on following builders:
  android_chromium_gn_compile_rel on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/android_chromiu...)

[scheib, 2015-01-30 00:28:47]

The CQ bit was unchecked by scheib@chromium.org

[scheib, 2015-01-30 16:37:23]

Patchset #4 (id:100001) has been deleted

[scheib, 2015-01-30 19:17:28]

Patchset #8 (id:200001) has been deleted

[scheib, 2015-01-30 19:17:34]

Patchset #7 (id:180001) has been deleted

[scheib, 2015-01-30 19:17:40]

Patchset #6 (id:160001) has been deleted

[scheib, 2015-01-30 19:17:46]

Patchset #5 (id:140001) has been deleted

[scheib, 2015-01-30 19:17:52]

Patchset #4 (id:120001) has been deleted

[scheib, 2015-01-30 19:20:34]

The CQ bit was checked by scheib@chromium.org

[commit-bot: I haz the power, 2015-01-30 19:21:31]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/885723002/220001

[commit-bot: I haz the power, 2015-01-30 20:29:43]

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

[commit-bot: I haz the power, 2015-01-30 20:29:44]

Try jobs failed on following builders:
  linux_chromium_chromeos_rel_ng on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[scheib, 2015-01-30 21:19:57]

The CQ bit was checked by scheib@chromium.org

[commit-bot: I haz the power, 2015-01-30 21:21:18]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/885723002/220001

[commit-bot: I haz the power, 2015-01-30 21:48:51]

Committed patchset #4 (id:220001)

[commit-bot: I haz the power, 2015-01-30 21:49:37]

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