Get GWS ID from field trial

Get GWS ID from field trial

When valid matching rules are available, a synthetic field trial is
created with a GWS ID. This GWS ID should be obtained from field trial
so that sending GWS ID can be disabled.

BUG=642453
Committed: https://crrev.com/2cee8eac8b7cc7df6c0e03436588489bddf48c4b
Cr-Commit-Position: refs/heads/master@{#415424}

[Raj, 2016-08-30 19:31:15]

rajendrant@chromium.org changed reviewers:
+ sclittle@chromium.org

[Raj, 2016-08-30 19:31:16]

ptal

[sclittle, 2016-08-30 20:06:45]

https://codereview.chromium.org/2294923002/diff/1/chrome/browser/android/data...
File chrome/browser/android/data_usage/external_data_use_observer_bridge.cc
(right):

https://codereview.chromium.org/2294923002/diff/1/chrome/browser/android/data...
chrome/browser/android/data_usage/external_data_use_observer_bridge.cc:50:
static_cast<variations::VariationID>(
nit: this static_cast doesn't really do anything, since base::StringToInt
returns a bool. Could you remove it?

https://codereview.chromium.org/2294923002/diff/1/chrome/browser/android/data...
chrome/browser/android/data_usage/external_data_use_observer_bridge.cc:220:
should_register ? variation_id : variations::EMPTY_ID);
So if should_register is false, then this sends EMPTY_ID, but if variation_id is
EMPTY_ID, then send nothing? Could add a comment explaining this, e.g. why does
(variation_id == EMPTY_ID) lead to different behavior than (should_register ==
false)?

[Raj, 2016-08-30 20:21:10]

https://codereview.chromium.org/2294923002/diff/1/chrome/browser/android/data...
File chrome/browser/android/data_usage/external_data_use_observer_bridge.cc
(right):

https://codereview.chromium.org/2294923002/diff/1/chrome/browser/android/data...
chrome/browser/android/data_usage/external_data_use_observer_bridge.cc:50:
static_cast<variations::VariationID>(
On 2016/08/30 20:06:45, sclittle wrote:
> nit: this static_cast doesn't really do anything, since base::StringToInt
> returns a bool. Could you remove it?

Done.

https://codereview.chromium.org/2294923002/diff/1/chrome/browser/android/data...
chrome/browser/android/data_usage/external_data_use_observer_bridge.cc:220:
should_register ? variation_id : variations::EMPTY_ID);
On 2016/08/30 20:06:45, sclittle wrote:
> So if should_register is false, then this sends EMPTY_ID, but if variation_id
is
> EMPTY_ID, then send nothing? Could add a comment explaining this, e.g. why
does
> (variation_id == EMPTY_ID) lead to different behavior than (should_register ==
> false)?

Added comment. Setting EMPTY_ID, is actually clearing it and sending nothing.
Changing to Disabled group should be enough, and setting EMPTY_ID to Enabled
group is not needed. But there was some confusion what happens when disabling it
in the middle of a UMA log session. UMA records in that log session will be
treated as in a mixed mode w.r.t the synthetic trial. This only applies to UMA
records, and we do not plan to distinguish UMA based on this synthetic trial.

[sclittle, 2016-08-30 20:22:55]

lgtm

[Raj, 2016-08-30 20:24:53]

The CQ bit was checked by rajendrant@chromium.org

[commit-bot: I haz the power, 2016-08-30 20:25:22]

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-08-30 21:06:43]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2016-08-30 21:08:29]

Description was changed from

==========
Get GWS ID from field trial

When valid matching rules are available, a synthetic field trial is
created with a GWS ID. This GWS ID should be obtained from field trial
so that sending GWS ID can be disabled.

BUG=642453
==========

to

==========
Get GWS ID from field trial

When valid matching rules are available, a synthetic field trial is
created with a GWS ID. This GWS ID should be obtained from field trial
so that sending GWS ID can be disabled.

BUG=642453

Committed: https://crrev.com/2cee8eac8b7cc7df6c0e03436588489bddf48c4b
Cr-Commit-Position: refs/heads/master@{#415424}
==========

[commit-bot: I haz the power, 2016-08-30 21:08:31]

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