Create variations param for RapporRollout to non-UMA users.

Create variations param for RapporRollout to non-UMA users.

BUG=400861
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=288127

[Alexei Svitkine (slow), 2014-07-30 16:52:51]

Please file a crbug for this work.

https://codereview.chromium.org/430703002/diff/1/components/rappor/rappor_ser...
File components/rappor/rappor_service.cc (right):

https://codereview.chromium.org/430703002/diff/1/components/rappor/rappor_ser...
components/rappor/rappor_service.cc:65: bool IsRapporEnabled(const GURL&
server_url, bool metrics_enabled) {
Can this be part of GetServerUrl() given that already has a way to disable UMA
via returning an invalid URL?

Seems hard to follow if the logic is to split into different places.

https://codereview.chromium.org/430703002/diff/1/components/rappor/rappor_ser...
components/rappor/rappor_service.cc:70: if (metrics_enabled) {
Nit: No {}'s.

https://codereview.chromium.org/430703002/diff/1/components/rappor/rappor_ser...
components/rappor/rappor_service.cc:73: return
chrome_variations::GetVariationParamValue(
Nit: Please rebase, this is now in namespace variations

[Steven Holte, 2014-08-05 03:09:37]

https://codereview.chromium.org/430703002/diff/1/components/rappor/rappor_ser...
File components/rappor/rappor_service.cc (right):

https://codereview.chromium.org/430703002/diff/1/components/rappor/rappor_ser...
components/rappor/rappor_service.cc:65: bool IsRapporEnabled(const GURL&
server_url, bool metrics_enabled) {
On 2014/07/30 16:52:51, Alexei Svitkine wrote:
> Can this be part of GetServerUrl() given that already has a way to disable UMA
> via returning an invalid URL?
> 
> Seems hard to follow if the logic is to split into different places.

Done.

https://codereview.chromium.org/430703002/diff/1/components/rappor/rappor_ser...
components/rappor/rappor_service.cc:70: if (metrics_enabled) {
On 2014/07/30 16:52:51, Alexei Svitkine wrote:
> Nit: No {}'s.

Done.

https://codereview.chromium.org/430703002/diff/1/components/rappor/rappor_ser...
components/rappor/rappor_service.cc:73: return
chrome_variations::GetVariationParamValue(
On 2014/07/30 16:52:51, Alexei Svitkine wrote:
> Nit: Please rebase, this is now in namespace variations

Done.

[Alexei Svitkine (slow), 2014-08-05 13:26:15]

LGTM, but please file a crbug for this work.

https://codereview.chromium.org/430703002/diff/20001/components/rappor/rappor...
File components/rappor/rappor_service.cc (right):

https://codereview.chromium.org/430703002/diff/20001/components/rappor/rappor...
components/rappor/rappor_service.cc:45: std::string server_url =
variations::GetVariationParamValue(
Nit: Move this after the require_uma check to avoid querying this on early
return.

[Steven Holte, 2014-08-05 21:21:09]

https://codereview.chromium.org/430703002/diff/20001/components/rappor/rappor...
File components/rappor/rappor_service.cc (right):

https://codereview.chromium.org/430703002/diff/20001/components/rappor/rappor...
components/rappor/rappor_service.cc:45: std::string server_url =
variations::GetVariationParamValue(
On 2014/08/05 13:26:15, Alexei Svitkine wrote:
> Nit: Move this after the require_uma check to avoid querying this on early
> return.

Done.

[Steven Holte, 2014-08-05 22:04:40]

The CQ bit was checked by holte@chromium.org

[commit-bot: I haz the power, 2014-08-05 22:08:21]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/holte@chromium.org/430703002/40001

[commit-bot: I haz the power, 2014-08-06 06:11:44]

FYI, CQ is re-trying this CL (attempt #1).
The failing builders are:
  win_gpu on tryserver.chromium.gpu
(http://build.chromium.org/p/tryserver.chromium.gpu/builders/win_gpu/builds/42886)
  chromium_presubmit on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)

[commit-bot: I haz the power, 2014-08-06 06:16:59]

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

[commit-bot: I haz the power, 2014-08-06 06:17:00]

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

[Steven Holte, 2014-08-06 23:17:11]

Forgot review for chrome_browser_main.cc

+jam

[jam, 2014-08-07 05:51:30]

lgtm

[Steven Holte, 2014-08-07 19:36:01]

The CQ bit was checked by holte@chromium.org

[commit-bot: I haz the power, 2014-08-07 19:46:41]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/holte@chromium.org/430703002/40001

[commit-bot: I haz the power, 2014-08-07 20:02:48]

Change committed as 288127