Chromium Code Reviews| Index: components/rappor/rappor_service.cc |
| diff --git a/components/rappor/rappor_service.cc b/components/rappor/rappor_service.cc |
| index 11419f5baaf583af9995ba352ec41b326d73a041..c329ef4b52bbcac421698042cb94520509dbc222 100644 |
| --- a/components/rappor/rappor_service.cc |
| +++ b/components/rappor/rappor_service.cc |
| @@ -37,18 +37,10 @@ const char kRapporRolloutFieldTrialName[] = "RapporRollout"; |
| // Constant for the finch parameter name for the server URL |
| const char kRapporRolloutServerUrlParam[] = "ServerUrl"; |
| -// Constant for the finch parameter name for the server URL |
| -const char kRapporRolloutRequireUmaParam[] = "RequireUma"; |
| - |
| // The rappor server's URL. |
| const char kDefaultServerUrl[] = "https://clients4.google.com/rappor"; |
| -GURL GetServerUrl(bool metrics_enabled) { |
| - bool require_uma = variations::GetVariationParamValue( |
| - kRapporRolloutFieldTrialName, |
| - kRapporRolloutRequireUmaParam) != "False"; |
| - if (!metrics_enabled && require_uma) |
| - return GURL(); // Invalid URL disables Rappor. |
| +GURL GetServerUrl() { |
| std::string server_url = variations::GetVariationParamValue( |
| kRapporRolloutFieldTrialName, |
| kRapporRolloutServerUrlParam); |
| @@ -66,7 +58,17 @@ const RapporParameters kRapporParametersForType[NUM_RAPPOR_TYPES] = { |
| rappor::PROBABILITY_50 /* Fake data probability */, |
| rappor::PROBABILITY_50 /* Fake one probability */, |
| rappor::PROBABILITY_75 /* One coin probability */, |
| - rappor::PROBABILITY_25 /* Zero coin probability */}, |
| + rappor::PROBABILITY_25 /* Zero coin probability */, |
| + false /* Require UMA */}, |
| + // FULL_POPULATION_RAPPOR_TYPE |
| + {128 /* Num cohorts */, |
| + 1 /* Bloom filter size bytes */, |
| + 2 /* Bloom filter hash count */, |
| + rappor::PROBABILITY_50 /* Fake data probability */, |
| + rappor::PROBABILITY_50 /* Fake one probability */, |
| + rappor::PROBABILITY_75 /* One coin probability */, |
| + rappor::PROBABILITY_25 /* Zero coin probability */, |
| + true /* Don't require UMA */}, |
| }; |
| } // namespace |
| @@ -75,8 +77,9 @@ RapporService::RapporService(PrefService* pref_service) |
| : pref_service_(pref_service), |
| cohort_(-1), |
| daily_event_(pref_service, |
| - prefs::kRapporLastDailySample, |
| - kRapporDailyEventHistogram) { |
| + prefs::kRapporLastDailySample, |
| + kRapporDailyEventHistogram), |
| + all_metrics_enabled_(false) { |
| } |
| RapporService::~RapporService() { |
| @@ -90,12 +93,14 @@ void RapporService::AddDailyObserver( |
| void RapporService::Start(net::URLRequestContextGetter* request_context, |
| bool metrics_enabled) { |
|
Alexei Svitkine (slow)
2014/11/03 21:48:21
Can you document the 2nd param better in the heade
Steven Holte
2014/11/04 02:44:19
Done.
|
| - const GURL server_url = GetServerUrl(metrics_enabled); |
| + const GURL server_url = GetServerUrl(); |
| if (!server_url.is_valid()) { |
| DVLOG(1) << server_url.spec() << " is invalid. " |
| << "RapporService not started."; |
| return; |
| } |
| + all_metrics_enabled_ = metrics_enabled; |
| + DVLOG(1) << "RapporService all_metrics_enabled_? " << all_metrics_enabled_; |
| DVLOG(1) << "RapporService started. Reporting to " << server_url.spec(); |
| DCHECK(!uploader_); |
| LoadSecret(); |
| @@ -203,10 +208,15 @@ void RapporService::RecordSample(const std::string& metric_name, |
| if (!IsInitialized()) |
| return; |
| DCHECK_LT(type, NUM_RAPPOR_TYPES); |
| + const RapporParameters& parameters = kRapporParametersForType[type]; |
| + // Skip this metric if all metrics aren't enabled and this isn't a |
| + // whitelisted metric. |
| + if (!all_metrics_enabled_ && !parameters.always_enabled) |
|
Alexei Svitkine (slow)
2014/11/03 21:48:21
Can you add a test for this?
Steven Holte
2014/11/04 02:44:20
Done.
|
| + return; |
| DVLOG(2) << "Recording sample \"" << sample |
| << "\" for metric \"" << metric_name |
| << "\" of type: " << type; |
| - RecordSampleInternal(metric_name, kRapporParametersForType[type], sample); |
| + RecordSampleInternal(metric_name, parameters, sample); |
| } |
| void RapporService::RecordSampleInternal(const std::string& metric_name, |