Restrict sending GWS ID only to signed in users

Restrict sending GWS ID only to signed in users

When valid matching rules are available and user is signed-in to Chrome,
a synthetic field trial is created with GWS ID retrieved from field trial.

BUG=648651
Review-Url: https://codereview.chromium.org/2505343002
Cr-Commit-Position: refs/heads/master@{#443185}
Committed: https://chromium.googlesource.com/chromium/src/+/94ce96bd62731841d2470f352ec40d56f5041f09

[Raj, 2016-11-17 06:34:14]

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

[commit-bot: I haz the power, 2016-11-17 06:34:41]

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-11-17 08:25:40]

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

[commit-bot: I haz the power, 2016-11-17 08:25:41]

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

[Raj, 2016-11-17 17:47:39]

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

[Raj, 2016-11-17 17:47:40]

ptal

[tbansal1, 2016-11-17 18:09:31]

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

https://codereview.chromium.org/2505343002/diff/1/chrome/browser/android/data...
chrome/browser/android/data_usage/external_data_use_observer_bridge.cc:218:
Profile* profile = ProfileManager::GetActiveUserProfile();
Comment for this method says "As such avoid
  // using it at all cost."

https://cs.chromium.org/chromium/src/chrome/browser/profiles/profile_manager....

Is there an alternative method?

[Raj, 2016-11-17 18:46:31]

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

https://codereview.chromium.org/2505343002/diff/1/chrome/browser/android/data...
chrome/browser/android/data_usage/external_data_use_observer_bridge.cc:218:
Profile* profile = ProfileManager::GetActiveUserProfile();
On 2016/11/17 18:09:31, tbansal1 wrote:
> Comment for this method says "As such avoid
>   // using it at all cost."
> 
>
https://cs.chromium.org/chromium/src/chrome/browser/profiles/profile_manager....
> 
> Is there an alternative method?

Using GetLastUsedProfile() instead, which is very similar for Android chrome.

[tbansal1, 2016-11-18 00:05:25]

https://codereview.chromium.org/2505343002/diff/20001/chrome/browser/android/...
File chrome/browser/android/data_usage/external_data_use_observer_bridge.cc
(right):

https://codereview.chromium.org/2505343002/diff/20001/chrome/browser/android/...
chrome/browser/android/data_usage/external_data_use_observer_bridge.cc:218:
Profile* profile = ProfileManager::GetLastUsedProfile();
Is there a need to check if profile is incognito?

[Raj, 2016-12-12 18:51:11]

https://codereview.chromium.org/2505343002/diff/20001/chrome/browser/android/...
File chrome/browser/android/data_usage/external_data_use_observer_bridge.cc
(right):

https://codereview.chromium.org/2505343002/diff/20001/chrome/browser/android/...
chrome/browser/android/data_usage/external_data_use_observer_bridge.cc:218:
Profile* profile = ProfileManager::GetLastUsedProfile();
On 2016/11/18 00:05:25, tbansal1 wrote:
> Is there a need to check if profile is incognito?

Done. ExternalDataUseObserver and other objects should not get created for
incognito profiles.
But still doing the check.

[Raj, 2016-12-12 18:51:22]

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

[commit-bot: I haz the power, 2016-12-12 18:52:10]

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

[tbansal1, 2016-12-12 18:55:23]

lgtm % nit.

https://codereview.chromium.org/2505343002/diff/40001/chrome/browser/android/...
File chrome/browser/android/data_usage/external_data_use_observer_bridge.cc
(right):

https://codereview.chromium.org/2505343002/diff/40001/chrome/browser/android/...
chrome/browser/android/data_usage/external_data_use_observer_bridge.cc:219:
DCHECK(profile);
nit: DCHECK is not required since if the |profile| is null, Chromium will
anyways crash on the next line where profile is dereferenced.

[Raj, 2016-12-12 18:58:54]

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

[commit-bot: I haz the power, 2016-12-12 19:00: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-12-12 21:05:17]

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

[commit-bot: I haz the power, 2016-12-12 21:05:18]

Dry run: Try jobs failed on following builders:
  cast_shell_linux on master.tryserver.chromium.linux (JOB_TIMED_OUT, no build
URL)
  chromeos_amd64-generic_chromium_compile_only_ng on
master.tryserver.chromium.linux (JOB_TIMED_OUT, no build URL)
  chromeos_daisy_chromium_compile_only_ng on master.tryserver.chromium.linux
(JOB_TIMED_OUT, no build URL)
  chromium_presubmit on master.tryserver.chromium.linux (JOB_TIMED_OUT, no build
URL)
  linux_chromium_asan_rel_ng on master.tryserver.chromium.linux (JOB_TIMED_OUT,
no build URL)
  linux_chromium_chromeos_ozone_rel_ng on master.tryserver.chromium.linux
(JOB_TIMED_OUT, no build URL)
  linux_chromium_chromeos_rel_ng on master.tryserver.chromium.linux
(JOB_TIMED_OUT, no build URL)
  linux_chromium_compile_dbg_ng on master.tryserver.chromium.linux
(JOB_TIMED_OUT, no build URL)
  linux_chromium_rel_ng on master.tryserver.chromium.linux (JOB_TIMED_OUT, no
build URL)

[Raj, 2016-12-13 19:45:03]

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

[commit-bot: I haz the power, 2016-12-13 19:45:50]

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-12-13 21:30:16]

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

[commit-bot: I haz the power, 2016-12-13 21:30:16]

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...)

[Raj, 2016-12-13 22:00:05]

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

[commit-bot: I haz the power, 2016-12-13 22:00:51]

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-12-13 23:47:34]

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

[commit-bot: I haz the power, 2016-12-13 23:47:35]

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...)

[Raj, 2017-01-12 07:43:48]

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

[commit-bot: I haz the power, 2017-01-12 07:44:02]

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

[Raj, 2017-01-12 08:12:09]

The CQ bit was unchecked by rajendrant@chromium.org

[Raj, 2017-01-12 08:12:09]

The CQ bit was checked by rajendrant@chromium.org

[Raj, 2017-01-12 08:12:09]

The patchset sent to the CQ was uploaded after l-g-t-m from tbansal@chromium.org
Link to the patchset: https://codereview.chromium.org/2505343002/#ps120001
(title: "Rebased")

[commit-bot: I haz the power, 2017-01-12 08:12:32]

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-01-12 08:21:28]

CQ is committing da patch.

Bot data: {"patchset_id": 120001, "attempt_start_ts": 1484208729694660,
"parent_rev": "19f85676701186e6ac1de7c625fffcab4a83d701", "commit_rev":
"94ce96bd62731841d2470f352ec40d56f5041f09"}

[commit-bot: I haz the power, 2017-01-12 08:22:07]

Description was changed from

==========
Restrict sending GWS ID only to signed in users

When valid matching rules are available and user is signed-in to Chrome,
a synthetic field trial is created with GWS ID retrieved from field trial.

BUG=648651
==========

to

==========
Restrict sending GWS ID only to signed in users

When valid matching rules are available and user is signed-in to Chrome,
a synthetic field trial is created with GWS ID retrieved from field trial.

BUG=648651

Review-Url: https://codereview.chromium.org/2505343002
Cr-Commit-Position: refs/heads/master@{#443185}
Committed:
https://chromium.googlesource.com/chromium/src/+/94ce96bd62731841d2470f352ec4...
==========

[commit-bot: I haz the power, 2017-01-12 08:22:08]

Committed patchset #7 (id:120001) as
https://chromium.googlesource.com/chromium/src/+/94ce96bd62731841d2470f352ec4...