Disable chrome.bluetoothLowEnergy on non linux platforms

Disable chrome.bluetoothLowEnergy on non linux platforms

BUG=648212
Committed: https://crrev.com/ff04ae6fa1e2205b096566aabab1fffbf81090e5
Cr-Commit-Position: refs/heads/master@{#421505}

[François Beaufort, 2016-09-20 11:18:27]

beaufort.francois@gmail.com changed reviewers:
+ rdevlin.cronin@chromium.org

[François Beaufort, 2016-09-20 11:18:28]

Hello,

This patch is about disabling properly chrome.bluetoothLowEnergy on non linux
platforms as it is not officially supported.

[Devlin, 2016-09-20 21:42:02]

https://codereview.chromium.org/2353483002/diff/1/extensions/common/api/_api_...
File extensions/common/api/_api_features.json (right):

https://codereview.chromium.org/2353483002/diff/1/extensions/common/api/_api_...
extensions/common/api/_api_features.json:98: "bluetoothLowEnergy.createService":
{
Can we get rid of all these child features, since they now match the parent?

[François Beaufort, 2016-09-21 07:34:04]

beaufort.francois@gmail.com changed reviewers:
+ steel@chromium.org

[François Beaufort, 2016-09-21 07:34:05]

I've got rid of all the child features since they now match the parent.
Adding steel@ as well then.

[Rahul Chaturvedi, 2016-09-21 07:58:57]

lgtm

[François Beaufort, 2016-09-21 07:59:38]

The CQ bit was checked by beaufort.francois@gmail.com

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

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 08:08:38]

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

[commit-bot: I haz the power, 2016-09-21 08:08:39]

Try jobs failed on following builders:
  chromium_presubmit on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)

[Devlin, 2016-09-21 16:18:19]

Looks like a number of bluetooth tests need to be updated to only run on
supported platforms.

[fbeaufortchromium, 2016-09-22 08:59:48]

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

[commit-bot: I haz the power, 2016-09-22 09:00:07]

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, 2016-09-22 10:09:12]

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

[commit-bot: I haz the power, 2016-09-22 10:09:13]

Dry run: This issue passed the CQ dry run.

[François Beaufort, 2016-09-22 10:37:28]

On 2016/09/21 16:18:19, Devlin (OOO until sep 26) wrote:
> Looks like a number of bluetooth tests need to be updated to only run on
> supported platforms.

I've updated tests.
steeel@ WDYT?

[Rahul Chaturvedi, 2016-09-23 18:42:33]

This will only run the API tests on Chrome OS, which is not what we want. This
needs to be fixed by adding build specific rules to the BUILD.gn to only add the
_apitest.cc file when we're on either Chrome OS, or on linux with use_dbus==1

[fbeaufortchromium, 2016-09-26 07:04:21]

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

[commit-bot: I haz the power, 2016-09-26 07:04:35]

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, 2016-09-26 07:21:23]

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

[commit-bot: I haz the power, 2016-09-26 07:21:24]

Dry run: Try jobs failed on following builders:
  linux_chromium_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[fbeaufortchromium, 2016-09-26 08:02:13]

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

[commit-bot: I haz the power, 2016-09-26 08:02: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, 2016-09-26 10:35:30]

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

[commit-bot: I haz the power, 2016-09-26 10:35:31]

Dry run: Try jobs failed on following builders:
  linux_android_rel_ng on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/linux_androi...)

[fbeaufortchromium, 2016-09-26 10:37:53]

On 2016/09/23 18:42:33, Rahul Chaturvedi wrote:
> This will only run the API tests on Chrome OS, which is not what we want. This
> needs to be fixed by adding build specific rules to the BUILD.gn to only add
the
> _apitest.cc file when we're on either Chrome OS, or on linux with use_dbus==1

Rahul, Devlin does that latest patch look good to you?

[Devlin, 2016-09-26 15:58:05]

lgtm.  Rahul's more familiar with build files nowadays, so I'll let him comment
definitively on that.

https://codereview.chromium.org/2353483002/diff/80001/chrome/test/BUILD.gn
File chrome/test/BUILD.gn (right):

https://codereview.chromium.org/2353483002/diff/80001/chrome/test/BUILD.gn#ne...
chrome/test/BUILD.gn:1830: if ((is_chromeos || is_linux) && use_dbus) {
nit: it looks like most of the platform-specific additions are below this block.
 Maybe add this there?

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

https://codereview.chromium.org/2353483002/diff/80001/chrome/test/BUILD.gn
File chrome/test/BUILD.gn (right):

https://codereview.chromium.org/2353483002/diff/80001/chrome/test/BUILD.gn#ne...
chrome/test/BUILD.gn:1830: if ((is_chromeos || is_linux) && use_dbus) {
Should be,
if (is_chromeos || (is_linux && use_dbus)) {

I am not particular where this gets places - this entire file is such a mess
anyway. Additionally, bluetooth tests are one of the first few we're looking to
move into //extensions anyway.

[François Beaufort, 2016-09-28 12:05:18]

On 2016/09/26 18:21:22, Rahul Chaturvedi wrote:
> https://codereview.chromium.org/2353483002/diff/80001/chrome/test/BUILD.gn
> File chrome/test/BUILD.gn (right):
> 
>
https://codereview.chromium.org/2353483002/diff/80001/chrome/test/BUILD.gn#ne...
> chrome/test/BUILD.gn:1830: if ((is_chromeos || is_linux) && use_dbus) {
> Should be,
> if (is_chromeos || (is_linux && use_dbus)) {
> 
> I am not particular where this gets places - this entire file is such a mess
> anyway. Additionally, bluetooth tests are one of the first few we're looking
to
> move into //extensions anyway.

Feedback is addressed.
Thanks everyone!

[François Beaufort, 2016-09-28 12:05:23]

The CQ bit was checked by beaufort.francois@gmail.com

[François Beaufort, 2016-09-28 12:05:24]

The patchset sent to the CQ was uploaded after l-g-t-m from steel@chromium.org,
rdevlin.cronin@chromium.org
Link to the patchset: https://codereview.chromium.org/2353483002/#ps100001
(title: "Address steel@ feedback")

[commit-bot: I haz the power, 2016-09-28 12:05: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-28 12:58:58]

Committed patchset #6 (id:100001)

[commit-bot: I haz the power, 2016-09-28 13:01:48]

Description was changed from

==========
Disable chrome.bluetoothLowEnergy on non linux platforms

BUG=648212
==========

to

==========
Disable chrome.bluetoothLowEnergy on non linux platforms

BUG=648212

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

[commit-bot: I haz the power, 2016-09-28 13:01:49]

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

[Rahul Chaturvedi, 2016-09-28 17:32:00]

Next time, please wait for re-l g t m after major changes before committing.

[François Beaufort, 2016-09-29 07:48:27]

On 2016/09/28 17:32:00, Rahul Chaturvedi wrote:
> Next time, please wait for re-l g t m after major changes before committing.

Sorry for that.
It won't happen again.