bluetooth: web: Update Blacklist field trial with Cypress & FIDO services.

bluetooth: web: Update Blacklist field trial with Cypress & FIDO services.

In preparation for deploying the Web Bluetooth Blacklist update via
field trial parameter this patch adds the payload to the
Field Trial Testing Config.

To support a browser test for this field trial parameter,
a test specific UUID is also added to the configs.

To enable readable comments in the json, the PRESUBMIT is
modified to accept comment keys.

BUG=570121, 643013
Committed: https://crrev.com/45e66c790f6b93dec90df68aaaf826c09ae114a1
Cr-Commit-Position: refs/heads/master@{#416332}

[scheib, 2016-09-01 01:24:37]

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

[commit-bot: I haz the power, 2016-09-01 01:25:10]

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

[scheib, 2016-09-01 01:28:41]

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

[scheib, 2016-09-01 01:28:42]

jyasskin for parameter payload review "00060000:e,fffd:e"
After, I will ask a field trial owner for review.

See BluetoothBlacklistTest, Add_StringWithValidEntries to sanity check this type
of string parses correctly. 

 
https://cs.chromium.org/chromium/src/content/browser/bluetooth/bluetooth_blac...

[commit-bot: I haz the power, 2016-09-01 02:04:33]

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

[commit-bot: I haz the power, 2016-09-01 02:04:34]

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

[Jeffrey Yasskin, 2016-09-01 21:48:23]

The extra blacklisted IDs lgtm, but:
1) Why's this causing WebBluetoothTest.BlacklistShouldBlock to fail, apparently
by not throwing the SecurityError?
2) In the CL description, could you say when we want to remove these fields from
the trial? Probably when M55 gets to stable.

[scheib, 2016-09-02 02:00:44]

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

[commit-bot: I haz the power, 2016-09-02 02:00:57]

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

[scheib, 2016-09-02 02:02:34]

Description was changed from

==========
bluetooth: web: Update Blacklist field trial with Cypress & FIDO services.

In preparation for deploying the Web Bluetooth Blacklist update via
field trial parameter this patch adds the payload to the
Field Trial Testing Config.

BUG=570121, 643013
==========

to

==========
bluetooth: web: Update Blacklist field trial with Cypress & FIDO services.

In preparation for deploying the Web Bluetooth Blacklist update via
field trial parameter this patch adds the payload to the
Field Trial Testing Config.

To support a browser test for this field trial parameter,
a test specific UUID is also added to the configs.

To enable readable comments in the json, the PRESUBMIT is
modified to accept comment keys.

BUG=570121, 643013
==========

[scheib, 2016-09-02 02:05:00]

Thanks. 

Ha, the test was failing because it added a field trial, but only one applies
and so the newly added configs precluded the test's trial group.

It didn't error cleanly, because it attempted to show the chooser dialog and
simply timed out.

I've added the removal notes in change description and in the configs.

[scheib, 2016-09-02 02:05:49]

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

[scheib, 2016-09-02 02:05:50]

holte, PTAL

[commit-bot: I haz the power, 2016-09-02 02:51:51]

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

[commit-bot: I haz the power, 2016-09-02 02:51:52]

Dry run: This issue passed the CQ dry run.

[Steven Holte, 2016-09-02 18:57:57]

lgtm

[scheib, 2016-09-02 20:15:18]

The CQ bit was checked by scheib@chromium.org

[scheib, 2016-09-02 20:15:19]

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/2306473002/#ps20001
(title: "fix test")

[commit-bot: I haz the power, 2016-09-02 20:15:48]

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-02 20:20:10]

Description was changed from

==========
bluetooth: web: Update Blacklist field trial with Cypress & FIDO services.

In preparation for deploying the Web Bluetooth Blacklist update via
field trial parameter this patch adds the payload to the
Field Trial Testing Config.

To support a browser test for this field trial parameter,
a test specific UUID is also added to the configs.

To enable readable comments in the json, the PRESUBMIT is
modified to accept comment keys.

BUG=570121, 643013
==========

to

==========
bluetooth: web: Update Blacklist field trial with Cypress & FIDO services.

In preparation for deploying the Web Bluetooth Blacklist update via
field trial parameter this patch adds the payload to the
Field Trial Testing Config.

To support a browser test for this field trial parameter,
a test specific UUID is also added to the configs.

To enable readable comments in the json, the PRESUBMIT is
modified to accept comment keys.

BUG=570121, 643013
==========

[commit-bot: I haz the power, 2016-09-02 20:20:11]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2016-09-02 20:23:11]

Description was changed from

==========
bluetooth: web: Update Blacklist field trial with Cypress & FIDO services.

In preparation for deploying the Web Bluetooth Blacklist update via
field trial parameter this patch adds the payload to the
Field Trial Testing Config.

To support a browser test for this field trial parameter,
a test specific UUID is also added to the configs.

To enable readable comments in the json, the PRESUBMIT is
modified to accept comment keys.

BUG=570121, 643013
==========

to

==========
bluetooth: web: Update Blacklist field trial with Cypress & FIDO services.

In preparation for deploying the Web Bluetooth Blacklist update via
field trial parameter this patch adds the payload to the
Field Trial Testing Config.

To support a browser test for this field trial parameter,
a test specific UUID is also added to the configs.

To enable readable comments in the json, the PRESUBMIT is
modified to accept comment keys.

BUG=570121, 643013

Committed: https://crrev.com/45e66c790f6b93dec90df68aaaf826c09ae114a1
Cr-Commit-Position: refs/heads/master@{#416332}
==========

[commit-bot: I haz the power, 2016-09-02 20:23:11]

Patchset 2 (id:??) landed as
https://crrev.com/45e66c790f6b93dec90df68aaaf826c09ae114a1
Cr-Commit-Position: refs/heads/master@{#416332}

[Michael Moss, 2016-09-06 14:43:15]

I think this is causing a new WebBluetoothTest.BlacklistShouldBlock test failure
on the Linux official builder (M55 builds), starting with:

https://uberchromegw.corp.google.com/i/official.desktop/builders/precise64/bu...

Could someone take a look?

[chromium-reviews, 2016-09-06 15:01:49]

I will look, still need to commute into office

On Sep 6, 2016 7:43 AM, <mmoss@chromium.org> wrote:

> I think this is causing a new WebBluetoothTest.BlacklistShouldBlock test
> failure
> on the Linux official builder (M55 builds), starting with:
>
> https://uberchromegw.corp.google.com/i/official.desktop/
> builders/precise64/builds/1017
>
> Could someone take a look?
>
> https://codereview.chromium.org/2306473002/
>

-- 
You received this message because you are subscribed to the Google Groups
"Chromium-reviews" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[scheib, 2016-09-06 17:16:19]

The official builders are not using the field trial configs - and so the tests
aren't picking up the required data. I'll reach out to field trials team to see
if we should be enabling those configs on official builders - or if the test
needs to work both with and without finch configs enabled.

[Steven Holte, 2016-09-06 18:15:04]

On 2016/09/06 17:16:19, scheib wrote:
> The official builders are not using the field trial configs - and so the tests
> aren't picking up the required data. I'll reach out to field trials team to
see
> if we should be enabling those configs on official builders - or if the test
> needs to work both with and without finch configs enabled.

It sounds like the tests should work with/without the field trial configs. 
Checked with Alexei, and it sounds like the official builders are supposed to
work like actual Chrome, and actually talk to finch servers (though they won't
reuse local state, so they won't actually enable any field trials).

[scheib, 2016-09-06 19:35:37]

On 2016/09/06 18:15:04, Steven Holte wrote:
> On 2016/09/06 17:16:19, scheib wrote:
> > The official builders are not using the field trial configs - and so the
tests
> > aren't picking up the required data. I'll reach out to field trials team to
> see
> > if we should be enabling those configs on official builders - or if the test
> > needs to work both with and without finch configs enabled.
> 
> It sounds like the tests should work with/without the field trial configs. 
> Checked with Alexei, and it sounds like the official builders are supposed to
> work like actual Chrome, and actually talk to finch servers (though they won't
> reuse local state, so they won't actually enable any field trials).

Test now detects if configs are active 
and handles either scenario https://codereview.chromium.org/2314953002/