Create Field Trial param for whitelisting UKM Entries.

Create Field Trial param for whitelisting UKM Entries.

This allows us to have finer grained controls over which UKM Entries are currently being logged. If the param is empty, then we default to logging all Entry types.

BUG=678682
Review-Url: https://codereview.chromium.org/2749433002
Cr-Commit-Position: refs/heads/master@{#457631}
Committed: https://chromium.googlesource.com/chromium/src/+/9e2fe00fdef6b5786d8077ceae63d92587b8110b

[rkaplow, 2017-03-10 23:12:29]

Description was changed from

==========
Create Field Trial param for whitelisting UKM Entries.

This allows us to have finer grained controls over which UKM Entries are
currently being logged. If the param is empty, then we default to logging all
Entry types.

BUG=678682
==========

to

==========
Create Field Trial param for whitelisting UKM Entries.

This allows us to have finer grained controls over which UKM Entries are
currently being logged. If the param is empty, then we default to logging all
Entry types.

BUG=678682
==========

[rkaplow, 2017-03-10 23:12:29]

rkaplow@chromium.org changed reviewers:
+ asvitkine@chromium.org, holte@chromium.org

[Alexei Svitkine (slow), 2017-03-14 15:40:56]

https://codereview.chromium.org/2749433002/diff/20001/components/ukm/ukm_serv...
File components/ukm/ukm_service.cc (right):

https://codereview.chromium.org/2749433002/diff/20001/components/ukm/ukm_serv...
components/ukm/ukm_service.cc:73: // Gets the list of whitelisted Entries as
string.
Is it comma-separated or something? Mention.

Or actually suggest doing the base::SplitString() here and returning the vector
- so the format of this param is localized to this function.

https://codereview.chromium.org/2749433002/diff/20001/components/ukm/ukm_serv...
components/ukm/ukm_service.cc:83: return GetWhitelistEntries() != std::string();
Nit: ! .empty()

https://codereview.chromium.org/2749433002/diff/20001/components/ukm/ukm_serv...
components/ukm/ukm_service.cc:514: if (UseWhitelistEntries() &&
Nit: I think it would be cleaner to just check
!whitelisted_entry_hashes_.empty() here and not use the extra helper.

[rkaplow, 2017-03-14 18:01:41]

https://codereview.chromium.org/2749433002/diff/20001/components/ukm/ukm_serv...
File components/ukm/ukm_service.cc (right):

https://codereview.chromium.org/2749433002/diff/20001/components/ukm/ukm_serv...
components/ukm/ukm_service.cc:73: // Gets the list of whitelisted Entries as
string.
On 2017/03/14 15:40:56, Alexei Svitkine (very slow) wrote:
> Is it comma-separated or something? Mention.
> 
> Or actually suggest doing the base::SplitString() here and returning the
vector
> - so the format of this param is localized to this function.

Done.

https://codereview.chromium.org/2749433002/diff/20001/components/ukm/ukm_serv...
components/ukm/ukm_service.cc:83: return GetWhitelistEntries() != std::string();
On 2017/03/14 15:40:56, Alexei Svitkine (very slow) wrote:
> Nit: ! .empty()

Acknowledged.

https://codereview.chromium.org/2749433002/diff/20001/components/ukm/ukm_serv...
components/ukm/ukm_service.cc:514: if (UseWhitelistEntries() &&
On 2017/03/14 15:40:56, Alexei Svitkine (very slow) wrote:
> Nit: I think it would be cleaner to just check
> !whitelisted_entry_hashes_.empty() here and not use the extra helper.

agreed

[Alexei Svitkine (slow), 2017-03-14 21:56:02]

lgtm

[rkaplow, 2017-03-14 23:14:07]

The CQ bit was checked by rkaplow@chromium.org

[commit-bot: I haz the power, 2017-03-14 23:15:02]

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, 2017-03-14 23:19:02]

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

[commit-bot: I haz the power, 2017-03-14 23:19:04]

Try jobs failed on following builders:
  ios-device on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-device/builds...)
  ios-device-xcode-clang on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-device-xcode-...)
  ios-simulator on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator/bui...)
  ios-simulator-xcode-clang on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator-xco...)
  mac_chromium_compile_dbg_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_comp...)
  mac_chromium_rel_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

[rkaplow, 2017-03-16 21:37:52]

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

[commit-bot: I haz the power, 2017-03-16 21:38:23]

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, 2017-03-17 00:03:22]

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

[commit-bot: I haz the power, 2017-03-17 00:03:24]

Dry run: This issue passed the CQ dry run.

[rkaplow, 2017-03-17 00:34:23]

The CQ bit was checked by rkaplow@chromium.org

[rkaplow, 2017-03-17 00:34:24]

The patchset sent to the CQ was uploaded after l-g-t-m from
asvitkine@chromium.org
Link to the patchset: https://codereview.chromium.org/2749433002/#ps80001
(title: "add missing } from merge")

[commit-bot: I haz the power, 2017-03-17 00:35:00]

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, 2017-03-17 00:45:01]

CQ is committing da patch.

Bot data: {"patchset_id": 80001, "attempt_start_ts": 1489710863634300,
"parent_rev": "1f9a36662a7b695f5d9d3023ad2be013901ebba3", "commit_rev":
"9e2fe00fdef6b5786d8077ceae63d92587b8110b"}

[commit-bot: I haz the power, 2017-03-17 00:45:39]

Description was changed from

==========
Create Field Trial param for whitelisting UKM Entries.

This allows us to have finer grained controls over which UKM Entries are
currently being logged. If the param is empty, then we default to logging all
Entry types.

BUG=678682
==========

to

==========
Create Field Trial param for whitelisting UKM Entries.

This allows us to have finer grained controls over which UKM Entries are
currently being logged. If the param is empty, then we default to logging all
Entry types.

BUG=678682

Review-Url: https://codereview.chromium.org/2749433002
Cr-Commit-Position: refs/heads/master@{#457631}
Committed:
https://chromium.googlesource.com/chromium/src/+/9e2fe00fdef6b5786d8077ceae63...
==========

[commit-bot: I haz the power, 2017-03-17 00:45:40]

Committed patchset #5 (id:80001) as
https://chromium.googlesource.com/chromium/src/+/9e2fe00fdef6b5786d8077ceae63...