Replace "bluetooth" permission with manifest property.

Replace "bluetooth" permission with manifest property.

BUG=336824
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=250599

[rpaquay, 2014-01-23 19:12:35]

This CL should remove all traces of the "bluetooth" and "bluetoohDevices"
permissions. Instead, we now have a "bluetooth" manifest property.

[not at google - send to devlin, 2014-01-23 20:29:12]

yay!

https://codereview.chromium.org/145663004/diff/1/chrome/common/extensions/api...
File chrome/common/extensions/api/bluetooth/bluetooth_manifest_data.cc (right):

https://codereview.chromium.org/145663004/diff/1/chrome/common/extensions/api...
chrome/common/extensions/api/bluetooth/bluetooth_manifest_data.cc:35: return
false;
just

return data && data->permission()->CheckRequest(extension, request);

https://codereview.chromium.org/145663004/diff/1/chrome/common/extensions/api...
File chrome/common/extensions/api/bluetooth/bluetooth_manifest_handler.cc
(right):

https://codereview.chromium.org/145663004/diff/1/chrome/common/extensions/api...
chrome/common/extensions/api/bluetooth/bluetooth_manifest_handler.cc:19:
base::string16* error) {
indent

https://codereview.chromium.org/145663004/diff/1/chrome/common/extensions/api...
File chrome/common/extensions/api/bluetooth/bluetooth_manifest_permission.cc
(right):

https://codereview.chromium.org/145663004/diff/1/chrome/common/extensions/api...
chrome/common/extensions/api/bluetooth/bluetooth_manifest_permission.cc:23:
const char kErrorInvalidProfileUuid[] = "Invalid uuid '*'";
UUID

https://codereview.chromium.org/145663004/diff/1/chrome/common/extensions/api...
chrome/common/extensions/api/bluetooth/bluetooth_manifest_permission.cc:100: 
no blank line

https://codereview.chromium.org/145663004/diff/1/chrome/common/extensions/api...
chrome/common/extensions/api/bluetooth/bluetooth_manifest_permission.cc:134: }
We should run this by security. Showing two warnings here is a bit unnecessary
given how similar they are:

IDS_EXTENSION_PROMPT_WARNING_BLUETOOTH:
Access information about Bluetooth devices paired with your system.

IDS_EXTENSION_PROMPT_WARNING_BLUETOOTH_DEVICES:
Access Bluetooth devices paired with your system.

I think I understand the difference when reading it, but in most cases it seems
like permission spam.

Is there a more fine-grained permission scheme we can use? Like, "bluetooth"
should have a "uuid" field and a "scan": true field as well? Then the former
would be "Discover new Bluetooth devices". Then again... that's still much these
same.

Or... have 3 difference messages.
1. Scan for bluetooth devices
2. Scan and pair with bluetooth devices.
3. Pair with bluetooth devices.

If those are the differences.

Oh and another question - is it possible to actually be more accurate than
"access bluetooth devices"? We basically know what the devices are ahead of time
because they're whitelisted.

+meacer

https://codereview.chromium.org/145663004/diff/1/chrome/common/extensions/api...
chrome/common/extensions/api/bluetooth/bluetooth_manifest_permission.cc:144:
BluetoothManifestPermission::FromValue(*value, &error));
what about the error here? Not sure what is supposd to happen to it, maybe pass
to the caller?

https://codereview.chromium.org/145663004/diff/1/chrome/common/extensions/api...
chrome/common/extensions/api/bluetooth/bluetooth_manifest_permission.cc:156:
return scoped_ptr<base::Value>(bluetooth.ToValue().release()).Pass();
use bluetooth.ToValue().PassAs<base::Value>() here?

(btw you don't need the Pass() on the end there)

https://codereview.chromium.org/145663004/diff/1/chrome/common/extensions/api...
chrome/common/extensions/api/bluetooth/bluetooth_manifest_permission.cc:173:
std::set_difference(
Can you use
https://code.google.com/p/chromium/codesearch#chromium/src/base/stl_util.h&q=...
here?

https://codereview.chromium.org/145663004/diff/1/chrome/common/extensions/api...
chrome/common/extensions/api/bluetooth/bluetooth_manifest_permission.cc:188:
std::set_union(
(no equivalent for union but maybe there should be, same for all of these other
set operations)

https://codereview.chromium.org/145663004/diff/1/chrome/common/extensions/api...
File chrome/common/extensions/api/manifest_types.json (right):

https://codereview.chromium.org/145663004/diff/1/chrome/common/extensions/api...
chrome/common/extensions/api/manifest_types.json:212: "type": "array",
(FWIW it was an optional array that the JSON schema compiler didn't support and
required, partially, for there to be that custom bindings file... it's been long
fixed, obviously)

https://codereview.chromium.org/145663004/diff/1/chrome/test/data/extensions/...
File
chrome/test/data/extensions/api_test/bluetooth/discovery_callback/manifest.json
(right):

https://codereview.chromium.org/145663004/diff/1/chrome/test/data/extensions/...
chrome/test/data/extensions/api_test/bluetooth/discovery_callback/manifest.json:11:
"bluetooth": {}
ok this looks weird :) see suggestion earlier about that "scan" property. or
maybe it should be "discover". then make empty-object an error.

[meacer, 2014-01-23 22:25:14]

https://codereview.chromium.org/145663004/diff/1/chrome/common/extensions/api...
File chrome/common/extensions/api/bluetooth/bluetooth_manifest_permission.cc
(right):

https://codereview.chromium.org/145663004/diff/1/chrome/common/extensions/api...
chrome/common/extensions/api/bluetooth/bluetooth_manifest_permission.cc:134: }
Let's be consistent with the USB API. It has a generic warning ("Access USB
devices") and a device specific ("Access USB device <Device Name>") warning with
the device name. The usage in chrome-app-samples is that the generic warning is
requested at install time and the device warning is requested during runtime.

That said, I think install time warnings can be merged as Ben suggests (for both
Bluetooth and USB). If both bluetooth and bluetoothDevice permissions is
requested, can we show something like "Access USB Devices (<device name>)"?

[rpaquay, 2014-01-24 00:05:38]

https://codereview.chromium.org/145663004/diff/1/chrome/common/extensions/api...
File chrome/common/extensions/api/bluetooth/bluetooth_manifest_permission.cc
(right):

https://codereview.chromium.org/145663004/diff/1/chrome/common/extensions/api...
chrome/common/extensions/api/bluetooth/bluetooth_manifest_permission.cc:134: }
On 2014/01/23 22:25:14, Mustafa Emre Acer wrote:
> Let's be consistent with the USB API. It has a generic warning ("Access USB
> devices") and a device specific ("Access USB device <Device Name>") warning
with
> the device name. The usage in chrome-app-samples is that the generic warning
is
> requested at install time and the device warning is requested during runtime.
> 
> That said, I think install time warnings can be merged as Ben suggests (for
both
> Bluetooth and USB). If both bluetooth and bluetoothDevice permissions is
> requested, can we show something like "Access USB Devices (<device name>)"?

+keybuk: Scott, a couple of questions:
1) Do you think it would be reasonable to add a "device name" in the bluetooth
permission warning dialog? It is not clear to this can be done, as the uuid
prodived in the manifest file correspond to a profile the app registers.
2) Do you think it would make send to add additional permissions to bluetooth,
such as "scan for devices" (or something else). The reason we are asking is
because when we change "bluetooth" from a permission to a manifest entry, the
text in manifest.json looks weird by default:
instead of
  "permissions": ["bluetooth"]
we now have
  "bluetooth": {}
It would be nicer to have something like
    "bluetooth": {"scan": true}
and it would look like this when adding profiles uuid:
    "bluetooth": {"scan": true, "profiles": ["1234", "456"]}

[rpaquay, 2014-01-24 16:58:23]

ptal: addresses all code review feedback (except for the permission messages
question).

https://codereview.chromium.org/145663004/diff/1/chrome/common/extensions/api...
File chrome/common/extensions/api/bluetooth/bluetooth_manifest_data.cc (right):

https://codereview.chromium.org/145663004/diff/1/chrome/common/extensions/api...
chrome/common/extensions/api/bluetooth/bluetooth_manifest_data.cc:35: return
false;
On 2014/01/23 20:29:12, kalman wrote:
> just
> 
> return data && data->permission()->CheckRequest(extension, request);

Done.

https://codereview.chromium.org/145663004/diff/1/chrome/common/extensions/api...
File chrome/common/extensions/api/bluetooth/bluetooth_manifest_handler.cc
(right):

https://codereview.chromium.org/145663004/diff/1/chrome/common/extensions/api...
chrome/common/extensions/api/bluetooth/bluetooth_manifest_handler.cc:19:
base::string16* error) {
On 2014/01/23 20:29:12, kalman wrote:
> indent

Done.

https://codereview.chromium.org/145663004/diff/1/chrome/common/extensions/api...
File chrome/common/extensions/api/bluetooth/bluetooth_manifest_permission.cc
(right):

https://codereview.chromium.org/145663004/diff/1/chrome/common/extensions/api...
chrome/common/extensions/api/bluetooth/bluetooth_manifest_permission.cc:23:
const char kErrorInvalidProfileUuid[] = "Invalid uuid '*'";
On 2014/01/23 20:29:12, kalman wrote:
> UUID

Done.

https://codereview.chromium.org/145663004/diff/1/chrome/common/extensions/api...
chrome/common/extensions/api/bluetooth/bluetooth_manifest_permission.cc:100: 
On 2014/01/23 20:29:12, kalman wrote:
> no blank line

Done.

https://codereview.chromium.org/145663004/diff/1/chrome/common/extensions/api...
chrome/common/extensions/api/bluetooth/bluetooth_manifest_permission.cc:144:
BluetoothManifestPermission::FromValue(*value, &error));
On 2014/01/23 20:29:12, kalman wrote:
> what about the error here? Not sure what is supposed to happen to it, maybe
pass
> to the caller?
AFAIK, ultimately this method is called only when deserializing permissions form
the json format we use in extension_prefs, i.e. the persistent list of
permissions granted for a given extension (see
ExtensionPrefs::ReadPrefAsPermissionSet). So this is *not* related to giving
precise and meaningful error to the user when parsing manifest.json.

https://codereview.chromium.org/145663004/diff/1/chrome/common/extensions/api...
chrome/common/extensions/api/bluetooth/bluetooth_manifest_permission.cc:156:
return scoped_ptr<base::Value>(bluetooth.ToValue().release()).Pass();
On 2014/01/23 20:29:12, kalman wrote:
> use bluetooth.ToValue().PassAs<base::Value>() here?
> 
> (btw you don't need the Pass() on the end there)

Done.

https://codereview.chromium.org/145663004/diff/1/chrome/common/extensions/api...
chrome/common/extensions/api/bluetooth/bluetooth_manifest_permission.cc:173:
std::set_difference(
On 2014/01/23 20:29:12, kalman wrote:
> Can you use
>
https://code.google.com/p/chromium/codesearch#chromium/src/base/stl_util.h&q=...
> here?

Done.

https://codereview.chromium.org/145663004/diff/1/chrome/common/extensions/api...
chrome/common/extensions/api/bluetooth/bluetooth_manifest_permission.cc:188:
std::set_union(
On 2014/01/23 20:29:12, kalman wrote:
> (no equivalent for union but maybe there should be, same for all of these
other
> set operations)

Done.

https://codereview.chromium.org/145663004/diff/1/chrome/common/extensions/api...
File chrome/common/extensions/api/manifest_types.json (right):

https://codereview.chromium.org/145663004/diff/1/chrome/common/extensions/api...
chrome/common/extensions/api/manifest_types.json:212: "type": "array",
On 2014/01/23 20:29:12, kalman wrote:
> (FWIW it was an optional array that the JSON schema compiler didn't support
and
> required, partially, for there to be that custom bindings file... it's been
long
> fixed, obviously)
Ok, thanks for the info.

[keybuk, 2014-01-24 20:06:22]

On 2014/01/24 00:05:38, rpaquay wrote:
>
https://codereview.chromium.org/145663004/diff/1/chrome/common/extensions/api...
> File chrome/common/extensions/api/bluetooth/bluetooth_manifest_permission.cc
> (right):
> 
>
https://codereview.chromium.org/145663004/diff/1/chrome/common/extensions/api...
> chrome/common/extensions/api/bluetooth/bluetooth_manifest_permission.cc:134: }
> On 2014/01/23 22:25:14, Mustafa Emre Acer wrote:
> > Let's be consistent with the USB API. It has a generic warning ("Access USB
> > devices") and a device specific ("Access USB device <Device Name>") warning
> with
> > the device name. The usage in chrome-app-samples is that the generic warning
> is
> > requested at install time and the device warning is requested during
runtime.
> > 
> > That said, I think install time warnings can be merged as Ben suggests (for
> both
> > Bluetooth and USB). If both bluetooth and bluetoothDevice permissions is
> > requested, can we show something like "Access USB Devices (<device name>)"?
> 
> +keybuk: Scott, a couple of questions:
> 1) Do you think it would be reasonable to add a "device name" in the bluetooth
> permission warning dialog? It is not clear to this can be done, as the uuid
> prodived in the manifest file correspond to a profile the app registers.
> 2) Do you think it would make send to add additional permissions to bluetooth,
> such as "scan for devices" (or something else). The reason we are asking is
> because when we change "bluetooth" from a permission to a manifest entry, the
> text in manifest.json looks weird by default:
> instead of
>   "permissions": ["bluetooth"]
> we now have
>   "bluetooth": {}
> It would be nicer to have something like
>     "bluetooth": {"scan": true}
> and it would look like this when adding profiles uuid:
>     "bluetooth": {"scan": true, "profiles": ["1234", "456"]}

We don't do per-device permissions, since the user is already an active
participant in pairing a device already.

Bluetooth permissions are thus "per-profile".

We may want to have a permission like "scan" in the future, it's not clear-cut
right now though, if one app (even the aura shell) enables scanning, all apps
would receive the events and scanned devices - which makes the permission fairly
meaningless.

[keybuk, 2014-01-24 20:06:48]

by "per-profile" I also mean that they're install time permissions, not runtime

[not at google - send to devlin, 2014-01-24 21:41:16]

What do you mean that the user is "already an active participant in pairing a
device"? Do you mean they've already paired it using the OS? That doesn't
necessarily mean that we need to give access to it from an app, though (sorry if
I misunderstand).

We can filter scan results or whatever to only the apps that have requested it.

In any case - by "scan" I meant whatever the difference was between those two
permission strings, "Access information about Bluetooth devices paired with your
system" vs "Access Bluetooth devices paired with your system". I had presume
that meant "List bluetooth devices" vs "Access a specific bluetooth device".

[not at google - send to devlin, 2014-01-24 21:59:38]

https://codereview.chromium.org/145663004/diff/1/chrome/common/extensions/api...
File chrome/common/extensions/api/bluetooth/bluetooth_manifest_permission.cc
(right):

https://codereview.chromium.org/145663004/diff/1/chrome/common/extensions/api...
chrome/common/extensions/api/bluetooth/bluetooth_manifest_permission.cc:127:
IDS_EXTENSION_PROMPT_WARNING_BLUETOOTH)));
Ok if not for scanning what is this even for..? If it's just for the ability to
request the bluetooth devices that are already listed... it seems totally
redundant.

https://codereview.chromium.org/145663004/diff/1/chrome/common/extensions/api...
chrome/common/extensions/api/bluetooth/bluetooth_manifest_permission.cc:134: }
On 2014/01/24 00:05:39, rpaquay wrote:
> On 2014/01/23 22:25:14, Mustafa Emre Acer wrote:
> > Let's be consistent with the USB API. It has a generic warning ("Access USB
> > devices") and a device specific ("Access USB device <Device Name>") warning
> with
> > the device name. The usage in chrome-app-samples is that the generic warning
> is
> > requested at install time and the device warning is requested during
runtime.
> > 
> > That said, I think install time warnings can be merged as Ben suggests (for
> both
> > Bluetooth and USB). If both bluetooth and bluetoothDevice permissions is
> > requested, can we show something like "Access USB Devices (<device name>)"?
> 
> +keybuk: Scott, a couple of questions:
> 1) Do you think it would be reasonable to add a "device name" in the bluetooth
> permission warning dialog? It is not clear to this can be done, as the uuid
> prodived in the manifest file correspond to a profile the app registers.
> 2) Do you think it would make send to add additional permissions to bluetooth,
> such as "scan for devices" (or something else). The reason we are asking is
> because when we change "bluetooth" from a permission to a manifest entry, the
> text in manifest.json looks weird by default:
> instead of
>   "permissions": ["bluetooth"]
> we now have
>   "bluetooth": {}
> It would be nicer to have something like
>     "bluetooth": {"scan": true}
> and it would look like this when adding profiles uuid:
>     "bluetooth": {"scan": true, "profiles": ["1234", "456"]}
> 
> 
> 
> 

Mustafa, what you say makes sense (install vs runtime permissions), in which
case they should be treated separately, but that's not what the code looks like
here, it looks like they're all install time?

https://codereview.chromium.org/145663004/diff/1/chrome/common/extensions/api...
chrome/common/extensions/api/bluetooth/bluetooth_manifest_permission.cc:156:
return scoped_ptr<base::Value>(bluetooth.ToValue().release()).Pass();
On 2014/01/24 16:58:23, rpaquay wrote:
> On 2014/01/23 20:29:12, kalman wrote:
> > use bluetooth.ToValue().PassAs<base::Value>() here?
> > 
> > (btw you don't need the Pass() on the end there)
> 
> Done.

(Not confident in my C++ here but...) could you replace the std::copy with

bluetooth.profiles.reset(new std::vector<std::string>(
    profile_uuids_.begin(), profile_uuids_.end()));

?

[meacer, 2014-01-24 22:22:19]

https://codereview.chromium.org/145663004/diff/1/chrome/common/extensions/api...
File chrome/common/extensions/api/bluetooth/bluetooth_manifest_permission.cc
(right):

https://codereview.chromium.org/145663004/diff/1/chrome/common/extensions/api...
chrome/common/extensions/api/bluetooth/bluetooth_manifest_permission.cc:131:
PermissionMessage::kBluetoothDevices,
Corresponding permission in USB API is singular: kUsbDevice. You might want to
match that.

https://codereview.chromium.org/145663004/diff/1/chrome/common/extensions/api...
chrome/common/extensions/api/bluetooth/bluetooth_manifest_permission.cc:134: }
On 2014/01/24 21:59:38, kalman wrote:
> On 2014/01/24 00:05:39, rpaquay wrote:
> > On 2014/01/23 22:25:14, Mustafa Emre Acer wrote:
> > > Let's be consistent with the USB API. It has a generic warning ("Access
USB
> > > devices") and a device specific ("Access USB device <Device Name>")
warning
> > with
> > > the device name. The usage in chrome-app-samples is that the generic
warning
> > is
> > > requested at install time and the device warning is requested during
> runtime.
> > > 
> > > That said, I think install time warnings can be merged as Ben suggests
(for
> > both
> > > Bluetooth and USB). If both bluetooth and bluetoothDevice permissions is
> > > requested, can we show something like "Access USB Devices (<device
name>)"?
> > 
> > +keybuk: Scott, a couple of questions:
> > 1) Do you think it would be reasonable to add a "device name" in the
bluetooth
> > permission warning dialog? It is not clear to this can be done, as the uuid
> > prodived in the manifest file correspond to a profile the app registers.
> > 2) Do you think it would make send to add additional permissions to
bluetooth,
> > such as "scan for devices" (or something else). The reason we are asking is
> > because when we change "bluetooth" from a permission to a manifest entry,
the
> > text in manifest.json looks weird by default:
> > instead of
> >   "permissions": ["bluetooth"]
> > we now have
> >   "bluetooth": {}
> > It would be nicer to have something like
> >     "bluetooth": {"scan": true}
> > and it would look like this when adding profiles uuid:
> >     "bluetooth": {"scan": true, "profiles": ["1234", "456"]}
> > 
> > 
> > 
> > 
> 
> Mustafa, what you say makes sense (install vs runtime permissions), in which
> case they should be treated separately, but that's not what the code looks
like
> here, it looks like they're all install time?

The app can still choose to request bluetoothDevices at runtime though, right?
It's just not mandatory to do so. For USB, you had to request usbDevice during
runtime (via chrome.permissions.request), but apparently you can now request it
during install time too. So my suggestions is this:

bluetooth: Should be install time only.
bluetoothDevice: Install time or runtime, whichever the app chooses.

If both are requested at install time, they should be collapsed into a single
warning: "Access bluetooth devices including <device name>".
But now I'm not sure if it's possible to request bluetoothDevice at runtime,
since it won't be a permission but a top level key?

[not at google - send to devlin, 2014-01-24 22:30:02]

On 2014/01/24 22:22:19, Mustafa Emre Acer wrote:
>
https://codereview.chromium.org/145663004/diff/1/chrome/common/extensions/api...
> File chrome/common/extensions/api/bluetooth/bluetooth_manifest_permission.cc
> (right):
> 
>
https://codereview.chromium.org/145663004/diff/1/chrome/common/extensions/api...
> chrome/common/extensions/api/bluetooth/bluetooth_manifest_permission.cc:131:
> PermissionMessage::kBluetoothDevices,
> Corresponding permission in USB API is singular: kUsbDevice. You might want to
> match that.
> 
>
https://codereview.chromium.org/145663004/diff/1/chrome/common/extensions/api...
> chrome/common/extensions/api/bluetooth/bluetooth_manifest_permission.cc:134: }
> On 2014/01/24 21:59:38, kalman wrote:
> > On 2014/01/24 00:05:39, rpaquay wrote:
> > > On 2014/01/23 22:25:14, Mustafa Emre Acer wrote:
> > > > Let's be consistent with the USB API. It has a generic warning ("Access
> USB
> > > > devices") and a device specific ("Access USB device <Device Name>")
> warning
> > > with
> > > > the device name. The usage in chrome-app-samples is that the generic
> warning
> > > is
> > > > requested at install time and the device warning is requested during
> > runtime.
> > > > 
> > > > That said, I think install time warnings can be merged as Ben suggests
> (for
> > > both
> > > > Bluetooth and USB). If both bluetooth and bluetoothDevice permissions is
> > > > requested, can we show something like "Access USB Devices (<device
> name>)"?
> > > 
> > > +keybuk: Scott, a couple of questions:
> > > 1) Do you think it would be reasonable to add a "device name" in the
> bluetooth
> > > permission warning dialog? It is not clear to this can be done, as the
uuid
> > > prodived in the manifest file correspond to a profile the app registers.
> > > 2) Do you think it would make send to add additional permissions to
> bluetooth,
> > > such as "scan for devices" (or something else). The reason we are asking
is
> > > because when we change "bluetooth" from a permission to a manifest entry,
> the
> > > text in manifest.json looks weird by default:
> > > instead of
> > >   "permissions": ["bluetooth"]
> > > we now have
> > >   "bluetooth": {}
> > > It would be nicer to have something like
> > >     "bluetooth": {"scan": true}
> > > and it would look like this when adding profiles uuid:
> > >     "bluetooth": {"scan": true, "profiles": ["1234", "456"]}
> > > 
> > > 
> > > 
> > > 
> > 
> > Mustafa, what you say makes sense (install vs runtime permissions), in which
> > case they should be treated separately, but that's not what the code looks
> like
> > here, it looks like they're all install time?
> 
> The app can still choose to request bluetoothDevices at runtime though, right?
> It's just not mandatory to do so. For USB, you had to request usbDevice during
> runtime (via chrome.permissions.request), but apparently you can now request
it
> during install time too. So my suggestions is this:
> 
> bluetooth: Should be install time only.
> bluetoothDevice: Install time or runtime, whichever the app chooses.
> 
> If both are requested at install time, they should be collapsed into a single
> warning: "Access bluetooth devices including <device name>".
> But now I'm not sure if it's possible to request bluetoothDevice at runtime,
> since it won't be a permission but a top level key?

Indeed, it doesn't support being an optional permission in the manifest, though
I'm not sure it ever did support it for those rich permissions (at least not
per-device). For the string-only permission it would have, yes.

Incidentally the optional permission thing will need some thought just in
general, but, another time.

For now... ok, let's just submit this in a way which is compatible with the old
behaviour. Then worry about tweaking the wording etc afterwards to not hold this
up. Having this conversation in a bug is probably better.

[rpaquay, 2014-01-25 00:48:50]

PTAL.

I opened a http://crbug/338036 to track the permission messages issue.

https://codereview.chromium.org/145663004/diff/1/chrome/common/extensions/api...
File chrome/common/extensions/api/bluetooth/bluetooth_manifest_permission.cc
(right):

https://codereview.chromium.org/145663004/diff/1/chrome/common/extensions/api...
chrome/common/extensions/api/bluetooth/bluetooth_manifest_permission.cc:156:
return scoped_ptr<base::Value>(bluetooth.ToValue().release()).Pass();
On 2014/01/24 21:59:38, kalman wrote:
> On 2014/01/24 16:58:23, rpaquay wrote:
> > On 2014/01/23 20:29:12, kalman wrote:
> > > use bluetooth.ToValue().PassAs<base::Value>() here?
> > > 
> > > (btw you don't need the Pass() on the end there)
> > 
> > Done.
> 
> (Not confident in my C++ here but...) could you replace the std::copy with
> 
> bluetooth.profiles.reset(new std::vector<std::string>(
>     profile_uuids_.begin(), profile_uuids_.end()));
> 
> ?
Yep, done.

[not at google - send to devlin, 2014-01-27 19:47:59]

lgtm from extensions (I guess you'll need a base owner now)

[meacer, 2014-01-27 20:37:55]

LGTM with the "bluetoothDevices" vs "bluetoothDevice" nit.

[rpaquay, 2014-01-28 00:27:33]

willchan@chromium.org: Please review changes in 

base/

[willchan no longer on Chromium, 2014-01-28 21:46:05]

+jyasskin in case he has comments on the STL stuff

LGTM

[willchan no longer on Chromium, 2014-01-28 21:47:45]

Actually, do you mind splitting it out into a separate changelist? TBR to me.

[rpaquay, 2014-01-28 21:57:10]

On 2014/01/28 21:47:45, willchan wrote:
> Actually, do you mind splitting it out into a separate changelist? TBR to me.

will do.

[Jeffrey Yasskin, 2014-01-28 22:15:56]

STL stuff LGTM

[rpaquay, 2014-01-28 23:29:33]

FYI: CL with std_util only: https://codereview.chromium.org/148183012/

[rpaquay, 2014-01-29 19:19:52]

zelidrag@chromium.org: Please review changes in 
chrome/test/ext_auto/auto_provider/manifest.json

cdn@chromium.org: Please review changes in 
chrome/common/extensions/extension_messages.h

[zel, 2014-01-29 19:28:40]

LGTM

for chrome/test/ext_auto part

[Cris Neckar, 2014-02-04 19:54:50]

On 2014/01/29 19:28:40, zel wrote:
> LGTM
> 
> for chrome/test/ext_auto part

IPC changes LGTM

[rpaquay, 2014-02-07 16:31:42]

The CQ bit was checked by rpaquay@chromium.org

[commit-bot: I haz the power, 2014-02-07 16:33:07]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/rpaquay@chromium.org/145663004/470001

[commit-bot: I haz the power, 2014-02-07 16:57:25]

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

[commit-bot: I haz the power, 2014-02-07 16:57:26]

Retried try job too often on mac_rel for step(s) base_unittests, browser_tests,
interactive_ui_tests, net_unittests, unit_tests
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=mac_rel&nu...

[rpaquay, 2014-02-11 21:17:08]

The CQ bit was checked by rpaquay@chromium.org

[commit-bot: I haz the power, 2014-02-11 21:17:41]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/rpaquay@chromium.org/145663004/760001

[commit-bot: I haz the power, 2014-02-11 21:34:11]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/rpaquay@chromium.org/145663004/760001

[commit-bot: I haz the power, 2014-02-11 22:11:50]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/rpaquay@chromium.org/145663004/760001

[commit-bot: I haz the power, 2014-02-12 01:40:21]

Change committed as 250599