diff options
-rw-r--r-- | chrome/browser/extensions/activity_log/activity_actions.cc | 4 | ||||
-rw-r--r-- | chrome/browser/prefs/pref_metrics_service.cc | 2 | ||||
-rw-r--r-- | components/rappor/byte_vector_utils.cc | 14 | ||||
-rw-r--r-- | components/rappor/byte_vector_utils.h | 4 | ||||
-rw-r--r-- | components/rappor/byte_vector_utils_unittest.cc | 9 | ||||
-rw-r--r-- | components/rappor/proto/rappor_metric.proto | 3 | ||||
-rw-r--r-- | components/rappor/rappor_metric.cc | 8 | ||||
-rw-r--r-- | components/rappor/rappor_metric_unittest.cc | 2 | ||||
-rw-r--r-- | components/rappor/rappor_parameters.cc | 6 | ||||
-rw-r--r-- | components/rappor/rappor_parameters.h | 7 | ||||
-rw-r--r-- | components/rappor/rappor_pref_names.cc | 7 | ||||
-rw-r--r-- | components/rappor/rappor_pref_names.h | 3 | ||||
-rw-r--r-- | components/rappor/rappor_service.cc | 24 | ||||
-rw-r--r-- | components/rappor/rappor_service_unittest.cc | 1 |
14 files changed, 71 insertions, 23 deletions
diff --git a/chrome/browser/extensions/activity_log/activity_actions.cc b/chrome/browser/extensions/activity_log/activity_actions.cc index 143de24..899c7d0 100644 --- a/chrome/browser/extensions/activity_log/activity_actions.cc +++ b/chrome/browser/extensions/activity_log/activity_actions.cc @@ -46,8 +46,10 @@ const char* kApisForRapporMetric[] = { ad_injection_constants::kHtmlAnchorHrefApiName }; +// The "Extensions.PossibleAdInjection2" metric uses different Rappor +// parameters than the original metric. const char* kExtensionAdInjectionRapporMetricName = - "Extensions.PossibleAdInjection"; + "Extensions.PossibleAdInjection2"; // The names of different types of HTML elements we check for ad injection. const char* kIframeElementType = "HTMLIFrameElement"; diff --git a/chrome/browser/prefs/pref_metrics_service.cc b/chrome/browser/prefs/pref_metrics_service.cc index 6d84569..8886554 100644 --- a/chrome/browser/prefs/pref_metrics_service.cc +++ b/chrome/browser/prefs/pref_metrics_service.cc @@ -82,7 +82,7 @@ void PrefMetricsService::RecordLaunchPrefs() { SEARCH_ENGINE_MAX); if (g_browser_process->rappor_service()) { g_browser_process->rappor_service()->RecordSample( - "Settings.HomePage", + "Settings.HomePage2", rappor::ETLD_PLUS_ONE_RAPPOR_TYPE, net::registry_controlled_domains::GetDomainAndRegistry(homepage_url, net::registry_controlled_domains::INCLUDE_PRIVATE_REGISTRIES)); diff --git a/components/rappor/byte_vector_utils.cc b/components/rappor/byte_vector_utils.cc index 57f2574..69abcd4 100644 --- a/components/rappor/byte_vector_utils.cc +++ b/components/rappor/byte_vector_utils.cc @@ -78,9 +78,17 @@ bool HMAC_DRBG_Update(const std::string& provided_data, } // namespace +ByteVector* ByteVectorAnd(const ByteVector& lhs, ByteVector* rhs) { + DCHECK_EQ(lhs.size(), rhs->size()); + for (size_t i = 0; i < lhs.size(); ++i) { + (*rhs)[i] = lhs[i] & (*rhs)[i]; + } + return rhs; +} + ByteVector* ByteVectorOr(const ByteVector& lhs, ByteVector* rhs) { DCHECK_EQ(lhs.size(), rhs->size()); - for (size_t i = 0, len = lhs.size(); i < len; ++i) { + for (size_t i = 0; i < lhs.size(); ++i) { (*rhs)[i] = lhs[i] | (*rhs)[i]; } return rhs; @@ -90,7 +98,7 @@ ByteVector* ByteVectorMerge(const ByteVector& mask, const ByteVector& lhs, ByteVector* rhs) { DCHECK_EQ(lhs.size(), rhs->size()); - for (size_t i = 0, len = lhs.size(); i < len; ++i) { + for (size_t i = 0; i < lhs.size(); ++i) { (*rhs)[i] = (lhs[i] & ~mask[i]) | ((*rhs)[i] & mask[i]); } return rhs; @@ -127,6 +135,8 @@ ByteVector ByteVectorGenerator::GetWeightedRandomByteVector( return *ByteVectorOr(GetRandomByteVector(), &bytes); case PROBABILITY_50: return bytes; + case PROBABILITY_25: + return *ByteVectorAnd(GetRandomByteVector(), &bytes); } NOTREACHED(); return bytes; diff --git a/components/rappor/byte_vector_utils.h b/components/rappor/byte_vector_utils.h index 0e33041..2f2d8c0 100644 --- a/components/rappor/byte_vector_utils.h +++ b/components/rappor/byte_vector_utils.h @@ -17,6 +17,10 @@ namespace rappor { // A vector of 8-bit integers used to store a set of binary bits. typedef std::vector<uint8_t> ByteVector; +// Computes a bitwise AND of byte vectors and stores the result in rhs. +// Returns rhs for chaining. +ByteVector* ByteVectorAnd(const ByteVector& lhs, ByteVector* rhs); + // Computes a bitwise OR of byte vectors and stores the result in rhs. // Returns rhs for chaining. ByteVector* ByteVectorOr(const ByteVector& lhs, ByteVector* rhs); diff --git a/components/rappor/byte_vector_utils_unittest.cc b/components/rappor/byte_vector_utils_unittest.cc index 54df9ea..672fa8d 100644 --- a/components/rappor/byte_vector_utils_unittest.cc +++ b/components/rappor/byte_vector_utils_unittest.cc @@ -26,6 +26,15 @@ std::string HexToString(const char* hex) { } // namespace +TEST(ByteVectorTest, ByteVectorAnd) { + ByteVector lhs(2); + lhs[1] = 0x12; + ByteVector rhs(2); + rhs[1] = 0x03; + + EXPECT_EQ(0x02, (*ByteVectorAnd(lhs, &rhs))[1]); +} + TEST(ByteVectorTest, ByteVectorOr) { ByteVector lhs(2); lhs[1] = 0x12; diff --git a/components/rappor/proto/rappor_metric.proto b/components/rappor/proto/rappor_metric.proto index e5efe21..c1d9cbf 100644 --- a/components/rappor/proto/rappor_metric.proto +++ b/components/rappor/proto/rappor_metric.proto @@ -20,7 +20,8 @@ message RapporReports { // partioned into cohorts in different ways, to allow better statistics and // increased coverage. In particular, the cohort will serve to choose the // hash functions used for Bloom-filter-based reports. The cohort is - // generated randomly by the client and is currently in the range [0,32). + // generated randomly by the client and is currently in the range [0,512). + // Was in range [0,32) in chrome versions before M37. optional int32 cohort = 1; // Each Report contains the values generated by the RAPPOR process for one diff --git a/components/rappor/rappor_metric.cc b/components/rappor/rappor_metric.cc index f8ff162..b4bd950 100644 --- a/components/rappor/rappor_metric.cc +++ b/components/rappor/rappor_metric.cc @@ -10,13 +10,15 @@ namespace rappor { RapporMetric::RapporMetric(const std::string& metric_name, const RapporParameters& parameters, - int32_t cohort) + int32_t cohort_seed) : metric_name_(metric_name), parameters_(parameters), bloom_filter_(parameters.bloom_filter_size_bytes, parameters.bloom_filter_hash_function_count, - cohort * parameters.bloom_filter_hash_function_count) { - DCHECK_GE(cohort, 0); + (cohort_seed % parameters.num_cohorts) * + parameters.bloom_filter_hash_function_count) { + DCHECK_GE(cohort_seed, 0); + DCHECK_LT(cohort_seed, RapporParameters::kMaxCohorts); } RapporMetric::~RapporMetric() {} diff --git a/components/rappor/rappor_metric_unittest.cc b/components/rappor/rappor_metric_unittest.cc index 7c50fd4..b5707cf 100644 --- a/components/rappor/rappor_metric_unittest.cc +++ b/components/rappor/rappor_metric_unittest.cc @@ -13,6 +13,7 @@ namespace rappor { const RapporParameters kTestRapporParameters = { + 1 /* Num cohorts */, 16 /* Bloom filter size bytes */, 4 /* Bloom filter hash count */, PROBABILITY_75 /* Fake data probability */, @@ -21,6 +22,7 @@ const RapporParameters kTestRapporParameters = { PROBABILITY_50 /* Zero coin probability */}; const RapporParameters kTestStatsRapporParameters = { + 1 /* Num cohorts */, 50 /* Bloom filter size bytes */, 4 /* Bloom filter hash count */, PROBABILITY_75 /* Fake data probability */, diff --git a/components/rappor/rappor_parameters.cc b/components/rappor/rappor_parameters.cc index ede7636..ae5b7ba 100644 --- a/components/rappor/rappor_parameters.cc +++ b/components/rappor/rappor_parameters.cc @@ -4,12 +4,14 @@ #include "components/rappor/rappor_parameters.h" +#include "base/compiler_specific.h" #include "base/strings/stringprintf.h" namespace rappor { std::string RapporParameters::ToString() const { - return base::StringPrintf("{ %d, %d, %d, %d, %d, %d }", + return base::StringPrintf("{ %d, %d, %d, %d, %d, %d, %d }", + num_cohorts, bloom_filter_size_bytes, bloom_filter_hash_function_count, fake_prob, @@ -18,4 +20,6 @@ std::string RapporParameters::ToString() const { zero_coin_prob); } +const int RapporParameters::kMaxCohorts = 512; + } // namespace rappor diff --git a/components/rappor/rappor_parameters.h b/components/rappor/rappor_parameters.h index 77b1614..ca625ba 100644 --- a/components/rappor/rappor_parameters.h +++ b/components/rappor/rappor_parameters.h @@ -12,6 +12,7 @@ namespace rappor { enum Probability { PROBABILITY_75, // 75% PROBABILITY_50, // 50% + PROBABILITY_25, // 25% }; // An object describing a rappor metric and the parameters used to generate it. @@ -22,6 +23,12 @@ struct RapporParameters { // Get a string representing the parameters, for DCHECK_EQ. std::string ToString() const; + // The maximum number of cohorts we divide clients into. + static const int kMaxCohorts; + + // The number of cohorts to divide the reports for this metric into. + int num_cohorts; + // The number of bytes stored in the Bloom filter. int bloom_filter_size_bytes; // The number of hash functions used in the Bloom filter. diff --git a/components/rappor/rappor_pref_names.cc b/components/rappor/rappor_pref_names.cc index e62c764..ff444ef 100644 --- a/components/rappor/rappor_pref_names.cc +++ b/components/rappor/rappor_pref_names.cc @@ -7,8 +7,11 @@ namespace rappor { namespace prefs { -// A randomly generated number, which determines cohort data is reported for. -const char kRapporCohort[] = "rappor.cohort"; +// Deprecated: Old cohort selection pref, should be cleared. +const char kRapporCohortDeprecated[] = "rappor.cohort"; + +// A randomly generated number, which determines cohorts data is reported for. +const char kRapporCohortSeed[] = "rappor.cohort_seed"; // A base-64 encoded, randomly generated byte string, which is used as a seed // for redacting collected data. diff --git a/components/rappor/rappor_pref_names.h b/components/rappor/rappor_pref_names.h index ec42b76..9cb60ca 100644 --- a/components/rappor/rappor_pref_names.h +++ b/components/rappor/rappor_pref_names.h @@ -10,7 +10,8 @@ namespace prefs { // Alphabetical list of preference names specific to the Rappor // component. Keep alphabetized, and document each in the .cc file. -extern const char kRapporCohort[]; +extern const char kRapporCohortDeprecated[]; +extern const char kRapporCohortSeed[]; extern const char kRapporSecret[]; } // namespace prefs diff --git a/components/rappor/rappor_service.cc b/components/rappor/rappor_service.cc index 01e6ab0..67bc4aa 100644 --- a/components/rappor/rappor_service.cc +++ b/components/rappor/rappor_service.cc @@ -22,9 +22,6 @@ namespace rappor { namespace { -// The number of cohorts we divide clients into. -const int kNumCohorts = 32; - // Seconds before the initial log is generated. const int kInitialLogIntervalSeconds = 15; // Interval between ongoing logs. @@ -46,12 +43,13 @@ GURL GetServerUrl() { const RapporParameters kRapporParametersForType[NUM_RAPPOR_TYPES] = { // ETLD_PLUS_ONE_RAPPOR_TYPE - {16 /* Bloom filter size bytes */, + {128 /* Num cohorts */, + 16 /* Bloom filter size bytes */, 2 /* Bloom filter hash count */, - rappor::PROBABILITY_75 /* Fake data probability */, + rappor::PROBABILITY_50 /* Fake data probability */, rappor::PROBABILITY_50 /* Fake one probability */, rappor::PROBABILITY_75 /* One coin probability */, - rappor::PROBABILITY_50 /* Zero coin probability */}, + rappor::PROBABILITY_25 /* Zero coin probability */}, }; } // namespace @@ -96,20 +94,24 @@ void RapporService::OnLogInterval() { // static void RapporService::RegisterPrefs(PrefRegistrySimple* registry) { registry->RegisterStringPref(prefs::kRapporSecret, std::string()); - registry->RegisterIntegerPref(prefs::kRapporCohort, -1); + registry->RegisterIntegerPref(prefs::kRapporCohortDeprecated, -1); + registry->RegisterIntegerPref(prefs::kRapporCohortSeed, -1); } void RapporService::LoadCohort(PrefService* pref_service) { DCHECK(!IsInitialized()); - cohort_ = pref_service->GetInteger(prefs::kRapporCohort); + // Ignore and delete old cohort parameter. + pref_service->ClearPref(prefs::kRapporCohortDeprecated); + + cohort_ = pref_service->GetInteger(prefs::kRapporCohortSeed); // If the user is already assigned to a valid cohort, we're done. - if (cohort_ >= 0 && cohort_ < kNumCohorts) + if (cohort_ >= 0 && cohort_ < RapporParameters::kMaxCohorts) return; // This is the first time the client has started the service (or their // preferences were corrupted). Randomly assign them to a cohort. - cohort_ = base::RandGenerator(kNumCohorts); - pref_service->SetInteger(prefs::kRapporCohort, cohort_); + cohort_ = base::RandGenerator(RapporParameters::kMaxCohorts); + pref_service->SetInteger(prefs::kRapporCohortSeed, cohort_); } void RapporService::LoadSecret(PrefService* pref_service) { diff --git a/components/rappor/rappor_service_unittest.cc b/components/rappor/rappor_service_unittest.cc index a36e960..a91db8c 100644 --- a/components/rappor/rappor_service_unittest.cc +++ b/components/rappor/rappor_service_unittest.cc @@ -25,6 +25,7 @@ class TestRapporService : public RapporService { TEST(RapporServiceTest, RecordAndExportMetrics) { const RapporParameters kTestRapporParameters = { + 1 /* Num cohorts */, 16 /* Bloom filter size bytes */, 4 /* Bloom filter hash count */, PROBABILITY_75 /* Fake data probability */, |