Add the chrome.bluetoothLowEnergy.setAdvertisingInterval API.

Add the chrome.bluetoothLowEnergy.setAdvertisingInterval API.

This API allows us to set the advertising interval on a device. This API is
restricted to an auto-launched app running in Kiosk mode since it can alter
the state of a device in a way that would effect other users.

Reviews requested,
xiyuan@ - overall
rdevlin.cronin@ - //extensions/OWNERS
ortuno@ -- //device/bluetooth/OWNERS
mpearson@ -- histograms

R=mpearson@chromium.org, ortuno@chromium.org, rdevlin.cronin@chromium.org, xiyuan@chromium.org
BUG=637231
Committed: https://crrev.com/ad68a0dabb2f35b89c304128d357272576586342
Cr-Commit-Position: refs/heads/master@{#420254}

[Mark P, 2016-09-20 23:08:26]

histograms.xml lgtm

[xiyuan, 2016-09-21 16:35:24]

https://codereview.chromium.org/2353133005/diff/1/chrome/browser/extensions/a...
File
chrome/browser/extensions/api/bluetooth_low_energy/bluetooth_low_energy_api.cc
(right):

https://codereview.chromium.org/2353133005/diff/1/chrome/browser/extensions/a...
chrome/browser/extensions/api/bluetooth_low_energy/bluetooth_low_energy_api.cc:1333:
return;
nit: remove since we have "break" ? Or keep this and remove "break" ?

https://codereview.chromium.org/2353133005/diff/1/device/bluetooth/bluez/blue...
File device/bluetooth/bluez/bluetooth_adapter_bluez.h (right):

https://codereview.chromium.org/2353133005/diff/1/device/bluetooth/bluez/blue...
device/bluetooth/bluez/bluetooth_adapter_bluez.h:37: class Time;
nit: TimeDelta ?

https://codereview.chromium.org/2353133005/diff/1/device/bluetooth/dbus/fake_...
File device/bluetooth/dbus/fake_bluetooth_le_advertising_manager_client.cc
(right):

https://codereview.chromium.org/2353133005/diff/1/device/bluetooth/dbus/fake_...
device/bluetooth/dbus/fake_bluetooth_le_advertising_manager_client.cc:23: }
nit: insert an empty line before and append "// namespace"

https://codereview.chromium.org/2353133005/diff/1/device/bluetooth/test/mock_...
File device/bluetooth/test/mock_bluetooth_adapter.h (right):

https://codereview.chromium.org/2353133005/diff/1/device/bluetooth/test/mock_...
device/bluetooth/test/mock_bluetooth_adapter.h:141: void SetAdvertisingInterval(
Should it be put under
  #if defined(OS_CHROMEOS) || defined(OS_LINUX)
?

[Devlin, 2016-09-21 17:07:03]

https://codereview.chromium.org/2353133005/diff/1/chrome/common/extensions/ap...
File chrome/common/extensions/api/bluetooth_low_energy.idl (right):

https://codereview.chromium.org/2353133005/diff/1/chrome/common/extensions/ap...
chrome/common/extensions/api/bluetooth_low_energy.idl:469: // from the specified
intervals. On some hardware, we will not even
s/we will not even attempt to have a minimum interval under 100ms/there is a
minimum interval of 100ms.

https://codereview.chromium.org/2353133005/diff/1/chrome/common/extensions/ap...
chrome/common/extensions/api/bluetooth_low_energy.idl:474: // milliseconds).
This cannot be more than 10240ms.
Out of curiosity, why 10240?

[Rahul Chaturvedi, 2016-09-21 18:54:13]

https://codereview.chromium.org/2353133005/diff/1/chrome/browser/extensions/a...
File
chrome/browser/extensions/api/bluetooth_low_energy/bluetooth_low_energy_api.cc
(right):

https://codereview.chromium.org/2353133005/diff/1/chrome/browser/extensions/a...
chrome/browser/extensions/api/bluetooth_low_energy/bluetooth_low_energy_api.cc:1333:
return;
On 2016/09/21 16:35:23, xiyuan wrote:
> nit: remove since we have "break" ? Or keep this and remove "break" ?

Done.

https://codereview.chromium.org/2353133005/diff/1/chrome/common/extensions/ap...
File chrome/common/extensions/api/bluetooth_low_energy.idl (right):

https://codereview.chromium.org/2353133005/diff/1/chrome/common/extensions/ap...
chrome/common/extensions/api/bluetooth_low_energy.idl:469: // from the specified
intervals. On some hardware, we will not even
On 2016/09/21 17:07:03, Devlin wrote:
> s/we will not even attempt to have a minimum interval under 100ms/there is a
> minimum interval of 100ms.

Done.

https://codereview.chromium.org/2353133005/diff/1/chrome/common/extensions/ap...
chrome/common/extensions/api/bluetooth_low_energy.idl:474: // milliseconds).
This cannot be more than 10240ms.
On 2016/09/21 17:07:03, Devlin wrote:
> Out of curiosity, why 10240?

These are limits required by the spec. Added a line to the header comment to
clarify.

https://codereview.chromium.org/2353133005/diff/1/device/bluetooth/bluez/blue...
File device/bluetooth/bluez/bluetooth_adapter_bluez.h (right):

https://codereview.chromium.org/2353133005/diff/1/device/bluetooth/bluez/blue...
device/bluetooth/bluez/bluetooth_adapter_bluez.h:37: class Time;
On 2016/09/21 16:35:23, xiyuan wrote:
> nit: TimeDelta ?

Done.

https://codereview.chromium.org/2353133005/diff/1/device/bluetooth/dbus/fake_...
File device/bluetooth/dbus/fake_bluetooth_le_advertising_manager_client.cc
(right):

https://codereview.chromium.org/2353133005/diff/1/device/bluetooth/dbus/fake_...
device/bluetooth/dbus/fake_bluetooth_le_advertising_manager_client.cc:23: }
On 2016/09/21 16:35:23, xiyuan wrote:
> nit: insert an empty line before and append "// namespace"

Done.

https://codereview.chromium.org/2353133005/diff/1/device/bluetooth/test/mock_...
File device/bluetooth/test/mock_bluetooth_adapter.h (right):

https://codereview.chromium.org/2353133005/diff/1/device/bluetooth/test/mock_...
device/bluetooth/test/mock_bluetooth_adapter.h:141: void SetAdvertisingInterval(
On 2016/09/21 16:35:23, xiyuan wrote:
> Should it be put under
>   #if defined(OS_CHROMEOS) || defined(OS_LINUX)
> ?

Yep :)
Done.

[Devlin, 2016-09-21 18:56:35]

idl lgtm

https://codereview.chromium.org/2353133005/diff/20001/chrome/common/extension...
File chrome/common/extensions/api/bluetooth_low_energy.idl (right):

https://codereview.chromium.org/2353133005/diff/20001/chrome/common/extension...
chrome/common/extensions/api/bluetooth_low_energy.idl:469: // from the specified
intervals. On some hardware, there is a minimum
nit: s/specified/requested

[Rahul Chaturvedi, 2016-09-21 19:18:22]

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

[Rahul Chaturvedi, 2016-09-21 19:18:23]

Adding Vincent, since I think ortuno@ is travelling.

https://codereview.chromium.org/2353133005/diff/20001/chrome/common/extension...
File chrome/common/extensions/api/bluetooth_low_energy.idl (right):

https://codereview.chromium.org/2353133005/diff/20001/chrome/common/extension...
chrome/common/extensions/api/bluetooth_low_energy.idl:469: // from the specified
intervals. On some hardware, there is a minimum
On 2016/09/21 18:56:35, Devlin wrote:
> nit: s/specified/requested

Done.

[xiyuan, 2016-09-21 19:25:53]

lgtm

[scheib, 2016-09-21 19:57:00]

device/bluetooth LGTM with optional:

https://codereview.chromium.org/2353133005/diff/40001/device/bluetooth/blueto...
File device/bluetooth/bluetooth_advertisement.h (right):

https://codereview.chromium.org/2353133005/diff/40001/device/bluetooth/blueto...
device/bluetooth/bluetooth_advertisement.h:40:
ERROR_INVALID_ADVERTISEMENT_INTERVAL,  // Advertisement interval specified
This is forever ChromeOS only, right? Wrap in #ifdef as the
SetAdvertisingInterval's have done?

[Rahul Chaturvedi, 2016-09-21 20:09:22]

https://codereview.chromium.org/2353133005/diff/40001/device/bluetooth/blueto...
File device/bluetooth/bluetooth_advertisement.h (right):

https://codereview.chromium.org/2353133005/diff/40001/device/bluetooth/blueto...
device/bluetooth/bluetooth_advertisement.h:40:
ERROR_INVALID_ADVERTISEMENT_INTERVAL,  // Advertisement interval specified
On 2016/09/21 19:57:00, scheib wrote:
> This is forever ChromeOS only, right? Wrap in #ifdef as the
> SetAdvertisingInterval's have done?

Done.

[Rahul Chaturvedi, 2016-09-21 20:10:00]

The CQ bit was checked by steel@chromium.org

[Rahul Chaturvedi, 2016-09-21 20:10:00]

The patchset sent to the CQ was uploaded after l-g-t-m from
mpearson@chromium.org, rdevlin.cronin@chromium.org, xiyuan@chromium.org,
scheib@chromium.org
Link to the patchset: https://codereview.chromium.org/2353133005/#ps60001
(title: ".")

[commit-bot: I haz the power, 2016-09-21 20:10:35]

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, 2016-09-21 20:40:56]

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

[commit-bot: I haz the power, 2016-09-21 20:40:57]

Try jobs failed on following builders:
  mac_chromium_rel_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

[Rahul Chaturvedi, 2016-09-21 20:46:14]

The CQ bit was checked by steel@chromium.org

[Rahul Chaturvedi, 2016-09-21 20:46:15]

The patchset sent to the CQ was uploaded after l-g-t-m from xiyuan@chromium.org,
mpearson@chromium.org, rdevlin.cronin@chromium.org, scheib@chromium.org
Link to the patchset: https://codereview.chromium.org/2353133005/#ps80001
(title: "build_fix")

[commit-bot: I haz the power, 2016-09-21 20:46:36]

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, 2016-09-21 21:35:15]

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

[commit-bot: I haz the power, 2016-09-21 21:35:17]

Try jobs failed on following builders:
  win_clang on master.tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_clang/builds/...)

[Rahul Chaturvedi, 2016-09-21 21:44:30]

The CQ bit was checked by steel@chromium.org

[Rahul Chaturvedi, 2016-09-21 21:44:31]

The patchset sent to the CQ was uploaded after l-g-t-m from xiyuan@chromium.org,
mpearson@chromium.org, rdevlin.cronin@chromium.org, scheib@chromium.org
Link to the patchset: https://codereview.chromium.org/2353133005/#ps20002
(title: "moar build fixes")

[commit-bot: I haz the power, 2016-09-21 21:44:59]

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

[Rahul Chaturvedi, 2016-09-21 22:05:38]

The CQ bit was checked by steel@chromium.org

[Rahul Chaturvedi, 2016-09-21 22:05:38]

The patchset sent to the CQ was uploaded after l-g-t-m from xiyuan@chromium.org,
mpearson@chromium.org, rdevlin.cronin@chromium.org, scheib@chromium.org
Link to the patchset: https://codereview.chromium.org/2353133005/#ps110001
(title: ".")

[commit-bot: I haz the power, 2016-09-21 22:06:19]

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

[ortuno, 2016-09-21 23:26:07]

The CQ bit was unchecked by ortuno@chromium.org

[ortuno, 2016-09-21 23:26:12]

https://codereview.chromium.org/2353133005/diff/110001/device/bluetooth/bluet...
File device/bluetooth/bluetooth_adapter.h (right):

https://codereview.chromium.org/2353133005/diff/110001/device/bluetooth/bluet...
device/bluetooth/bluetooth_adapter.h:447: virtual void SetAdvertisingInterval(
Wouldn't this fit better as an argument of RegisterAdvertisement?

[Rahul Chaturvedi, 2016-09-21 23:35:43]

On 2016/09/21 23:26:12, ortuno wrote:
>
https://codereview.chromium.org/2353133005/diff/110001/device/bluetooth/bluet...
> File device/bluetooth/bluetooth_adapter.h (right):
> 
>
https://codereview.chromium.org/2353133005/diff/110001/device/bluetooth/bluet...
> device/bluetooth/bluetooth_adapter.h:447: virtual void SetAdvertisingInterval(
> Wouldn't this fit better as an argument of RegisterAdvertisement?

[Rahul Chaturvedi, 2016-09-21 23:35:51]

On 2016/09/21 23:26:12, ortuno wrote:
>
https://codereview.chromium.org/2353133005/diff/110001/device/bluetooth/bluet...
> File device/bluetooth/bluetooth_adapter.h (right):
> 
>
https://codereview.chromium.org/2353133005/diff/110001/device/bluetooth/bluet...
> device/bluetooth/bluetooth_adapter.h:447: virtual void SetAdvertisingInterval(
> Wouldn't this fit better as an argument of RegisterAdvertisement?

[ortuno, 2016-09-22 03:02:25]

https://codereview.chromium.org/2353133005/diff/110001/device/bluetooth/bluet...
File device/bluetooth/bluetooth_adapter.h (right):

https://codereview.chromium.org/2353133005/diff/110001/device/bluetooth/bluet...
device/bluetooth/bluetooth_adapter.h:444: #if defined(OS_CHROMEOS) ||
defined(OS_LINUX)
I think we still want device/bluetooth tests. It doesn't make sense for only
clients of the API to be testing parts of it.

https://codereview.chromium.org/2353133005/diff/110001/device/bluetooth/bluet...
device/bluetooth/bluetooth_adapter.h:446: // for the interval are from 20ms to
10.24 seconds, with min <= max.
nit: Could you add the same caveats as the chrome apps documentation?

https://codereview.chromium.org/2353133005/diff/110001/device/bluetooth/dbus/...
File device/bluetooth/dbus/bluetooth_le_advertising_manager_client.cc (right):

https://codereview.chromium.org/2353133005/diff/110001/device/bluetooth/dbus/...
device/bluetooth/dbus/bluetooth_le_advertising_manager_client.cc:118: //
constant in service_constants.h once cros_system_api DEPS are rolled.
nit: There are a lot of TODO(username) that are leftover and forgotten. If you
don't plan on doing this immediately could you link to an issue about it or a
related issue an mention this work is still missing.

https://codereview.chromium.org/2353133005/diff/110001/device/bluetooth/dbus/...
File device/bluetooth/dbus/fake_bluetooth_le_advertising_manager_client.cc
(right):

https://codereview.chromium.org/2353133005/diff/110001/device/bluetooth/dbus/...
device/bluetooth/dbus/fake_bluetooth_le_advertising_manager_client.cc:21:
constexpr uint16_t kMinIntervalMs = 20;
nit: Please add an explanation and mention the section of the spec this refers
to.

[Rahul Chaturvedi, 2016-09-22 03:35:02]

The CQ bit was checked by steel@chromium.org

[commit-bot: I haz the power, 2016-09-22 03:35:15]

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, 2016-09-22 03:41:47]

Committed patchset #7 (id:110001)

[commit-bot: I haz the power, 2016-09-22 03:45:49]

Description was changed from

==========
Add the chrome.bluetoothLowEnergy.setAdvertisingInterval API.

This API allows us to set the advertising interval on a device. This API is
restricted to an auto-launched app running in Kiosk mode since it can alter
the state of a device in a way that would effect other users.

Reviews requested,
xiyuan@ - overall
rdevlin.cronin@ - //extensions/OWNERS
ortuno@ -- //device/bluetooth/OWNERS
mpearson@ -- histograms

R=mpearson@chromium.org, ortuno@chromium.org, rdevlin.cronin@chromium.org,
xiyuan@chromium.org
BUG=637231
==========

to

==========
Add the chrome.bluetoothLowEnergy.setAdvertisingInterval API.

This API allows us to set the advertising interval on a device. This API is
restricted to an auto-launched app running in Kiosk mode since it can alter
the state of a device in a way that would effect other users.

Reviews requested,
xiyuan@ - overall
rdevlin.cronin@ - //extensions/OWNERS
ortuno@ -- //device/bluetooth/OWNERS
mpearson@ -- histograms

R=mpearson@chromium.org, ortuno@chromium.org, rdevlin.cronin@chromium.org,
xiyuan@chromium.org
BUG=637231

Committed: https://crrev.com/ad68a0dabb2f35b89c304128d357272576586342
Cr-Commit-Position: refs/heads/master@{#420254}
==========

[commit-bot: I haz the power, 2016-09-22 03:45:51]

Patchset 7 (id:??) landed as
https://crrev.com/ad68a0dabb2f35b89c304128d357272576586342
Cr-Commit-Position: refs/heads/master@{#420254}