summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
-rw-r--r--components/rappor/bloom_filter.h1
-rw-r--r--components/rappor/byte_vector_utils.h1
-rw-r--r--components/rappor/log_uploader.cc6
-rw-r--r--components/rappor/log_uploader.h20
-rw-r--r--components/rappor/log_uploader_unittest.cc1
-rw-r--r--components/rappor/rappor_metric.cc2
-rw-r--r--components/rappor/rappor_metric.h23
-rw-r--r--components/rappor/rappor_metric_unittest.cc25
-rw-r--r--components/rappor/rappor_service.cc38
-rw-r--r--components/rappor/rappor_service.h26
-rw-r--r--components/rappor/rappor_service_unittest.cc6
11 files changed, 90 insertions, 59 deletions
diff --git a/components/rappor/bloom_filter.h b/components/rappor/bloom_filter.h
index 0bb9a57..d9acfa4 100644
--- a/components/rappor/bloom_filter.h
+++ b/components/rappor/bloom_filter.h
@@ -8,6 +8,7 @@
#include <string>
#include "base/basictypes.h"
+#include "base/macros.h"
#include "components/rappor/byte_vector_utils.h"
namespace rappor {
diff --git a/components/rappor/byte_vector_utils.h b/components/rappor/byte_vector_utils.h
index 9154cab..0e33041 100644
--- a/components/rappor/byte_vector_utils.h
+++ b/components/rappor/byte_vector_utils.h
@@ -8,6 +8,7 @@
#include <vector>
#include "base/basictypes.h"
+#include "base/macros.h"
#include "components/rappor/rappor_parameters.h"
#include "crypto/hmac.h"
diff --git a/components/rappor/log_uploader.cc b/components/rappor/log_uploader.cc
index 4837ba1..ead1a97 100644
--- a/components/rappor/log_uploader.cc
+++ b/components/rappor/log_uploader.cc
@@ -102,12 +102,12 @@ void LogUploader::OnURLFetchComplete(const net::URLFetcher* source) {
DCHECK_EQ(current_fetch_.get(), source);
scoped_ptr<net::URLFetcher> fetch(current_fetch_.Pass());
- int response_code = source->GetResponseCode();
+ const int response_code = source->GetResponseCode();
// Log a histogram to track response success vs. failure rates.
UMA_HISTOGRAM_SPARSE_SLOWLY("Rappor.UploadResponseCode", response_code);
- bool upload_succeeded = response_code == 200;
+ const bool upload_succeeded = response_code == 200;
// Determine whether this log should be retransmitted.
DiscardReason reason = NUM_DISCARD_REASONS;
@@ -128,7 +128,7 @@ void LogUploader::OnURLFetchComplete(const net::URLFetcher* source) {
// Error 400 indicates a problem with the log, not with the server, so
// don't consider that a sign that the server is in trouble.
- bool server_is_healthy = upload_succeeded || response_code == 400;
+ const bool server_is_healthy = upload_succeeded || response_code == 400;
OnUploadFinished(server_is_healthy, !queued_logs_.empty());
}
diff --git a/components/rappor/log_uploader.h b/components/rappor/log_uploader.h
index 30a129a..a446a34 100644
--- a/components/rappor/log_uploader.h
+++ b/components/rappor/log_uploader.h
@@ -8,6 +8,8 @@
#include <queue>
#include <string>
+#include "base/compiler_specific.h"
+#include "base/macros.h"
#include "base/memory/scoped_ptr.h"
#include "base/time/time.h"
#include "base/timer/timer.h"
@@ -21,16 +23,22 @@ class URLFetcher;
namespace rappor {
-// Handles uploading logs to an external server.
+// Uploads logs from RapporService. Logs are passed in via QueueLog(), stored
+// internally, and uploaded one at a time. A queued log will be uploaded at a
+// fixed interval after the successful upload of the previous logs. If an
+// upload fails, the uploader will keep retrying the upload with an exponential
+// backoff interval.
class LogUploader : public net::URLFetcherDelegate {
public:
- // Constructor takes the server_url that logs should be uploaded to, the
- // mime_type of the uploaded data, and request_context to create uploads
+ // Constructor takes the |server_url| that logs should be uploaded to, the
+ // |mime_type| of the uploaded data, and |request_context| to create uploads
// with.
LogUploader(const GURL& server_url,
const std::string& mime_type,
net::URLRequestContextGetter* request_context);
+ // If the object is destroyed (or the program terminates) while logs are
+ // queued, the logs are lost.
virtual ~LogUploader();
// Adds an entry to the queue of logs to be uploaded to the server. The
@@ -39,7 +47,7 @@ class LogUploader : public net::URLFetcherDelegate {
void QueueLog(const std::string& log);
protected:
- // Check if an upload has been scheduled.
+ // Checks if an upload has been scheduled.
virtual bool IsUploadScheduled() const;
// Schedules a future call to StartScheduledUpload if one isn't already
@@ -54,8 +62,8 @@ class LogUploader : public net::URLFetcherDelegate {
static base::TimeDelta BackOffUploadInterval(base::TimeDelta);
private:
- // Implementation of net::URLFetcherDelegate. Called after transmission
- // completes (either successfully or with failure).
+ // Implements net::URLFetcherDelegate. Called after transmission completes
+ // (whether successful or not).
virtual void OnURLFetchComplete(const net::URLFetcher* source) OVERRIDE;
// Called when the upload is completed.
diff --git a/components/rappor/log_uploader_unittest.cc b/components/rappor/log_uploader_unittest.cc
index 46db7db..8c175b4 100644
--- a/components/rappor/log_uploader_unittest.cc
+++ b/components/rappor/log_uploader_unittest.cc
@@ -4,6 +4,7 @@
#include "components/rappor/log_uploader.h"
+#include "base/compiler_specific.h"
#include "base/message_loop/message_loop_proxy.h"
#include "net/url_request/test_url_fetcher_factory.h"
#include "net/url_request/url_request_test_util.h"
diff --git a/components/rappor/rappor_metric.cc b/components/rappor/rappor_metric.cc
index 2da3b31..f8ff162 100644
--- a/components/rappor/rappor_metric.cc
+++ b/components/rappor/rappor_metric.cc
@@ -30,7 +30,7 @@ ByteVector RapporMetric::GetReport(const std::string& secret) const {
// client's secret key + real data as a seed. The inclusion of the secret
// in the seed avoids correlations between real and fake data.
// The seed isn't a human-readable string.
- std::string personalization_string = metric_name_ +
+ const std::string personalization_string = metric_name_ +
std::string(bytes().begin(), bytes().end());
HmacByteVectorGenerator hmac_generator(bytes().size(), secret,
personalization_string);
diff --git a/components/rappor/rappor_metric.h b/components/rappor/rappor_metric.h
index 30147c4..07f422d 100644
--- a/components/rappor/rappor_metric.h
+++ b/components/rappor/rappor_metric.h
@@ -2,11 +2,13 @@
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
-#ifndef COMPONENTS_RAPPOR_RAPPOR_H_
-#define COMPONENTS_RAPPOR_RAPPOR_H_
+#ifndef COMPONENTS_RAPPOR_RAPPOR_METRIC_H_
+#define COMPONENTS_RAPPOR_RAPPOR_METRIC_H_
#include <string>
+#include "base/basictypes.h"
+#include "base/macros.h"
#include "components/rappor/bloom_filter.h"
#include "components/rappor/byte_vector_utils.h"
#include "components/rappor/rappor_parameters.h"
@@ -16,17 +18,20 @@ namespace rappor {
// A RapporMetric is an object that collects string samples into a Bloom filter,
// and generates randomized reports about the collected data.
//
+// This class should not be used directly by metrics clients. Record metrics
+// using RapporService::RecordSample instead.
+//
// For a full description of the rappor metrics, see
// http://www.chromium.org/developers/design-documents/rappor
class RapporMetric {
public:
// Takes the |metric_name| that this will be reported to the server with,
- // a |parameters| describing size and probability weights to be used in
- // recording this metric, and cohort value, which modifies the hash
- // functions and used in the bloom filter.
- explicit RapporMetric(const std::string& metric_name,
- const RapporParameters& parameters,
- int32_t cohort);
+ // a |parameters| describing size and probability weights used in recording
+ // this metric, and a |cohort| value, which determines the hash functions
+ // used in the Bloom filter.
+ RapporMetric(const std::string& metric_name,
+ const RapporParameters& parameters,
+ int32_t cohort);
~RapporMetric();
// Records an additional sample in the Bloom filter.
@@ -53,4 +58,4 @@ class RapporMetric {
} // namespace rappor
-#endif // COMPONENTS_RAPPOR_RAPPOR_H_
+#endif // COMPONENTS_RAPPOR_RAPPOR_METRIC_H_
diff --git a/components/rappor/rappor_metric_unittest.cc b/components/rappor/rappor_metric_unittest.cc
index bbcb1ab..7c50fd4 100644
--- a/components/rappor/rappor_metric_unittest.cc
+++ b/components/rappor/rappor_metric_unittest.cc
@@ -18,8 +18,7 @@ const RapporParameters kTestRapporParameters = {
PROBABILITY_75 /* Fake data probability */,
PROBABILITY_50 /* Fake one probability */,
PROBABILITY_75 /* One coin probability */,
- PROBABILITY_50 /* Zero coin probability */
-};
+ PROBABILITY_50 /* Zero coin probability */};
const RapporParameters kTestStatsRapporParameters = {
50 /* Bloom filter size bytes */,
@@ -27,8 +26,7 @@ const RapporParameters kTestStatsRapporParameters = {
PROBABILITY_75 /* Fake data probability */,
PROBABILITY_50 /* Fake one probability */,
PROBABILITY_75 /* One coin probability */,
- PROBABILITY_50 /* Zero coin probability */
-};
+ PROBABILITY_50 /* Zero coin probability */};
// Check for basic syntax and use.
TEST(RapporMetricTest, BasicMetric) {
@@ -41,7 +39,7 @@ TEST(RapporMetricTest, BasicMetric) {
TEST(RapporMetricTest, GetReport) {
RapporMetric metric("MyRappor", kTestRapporParameters, 0);
- ByteVector report = metric.GetReport(
+ const ByteVector report = metric.GetReport(
HmacByteVectorGenerator::GenerateEntropyInput());
EXPECT_EQ(16u, report.size());
}
@@ -52,25 +50,26 @@ TEST(RapporMetricTest, GetReportStatistics) {
for (char i = 0; i < 50; i++) {
metric.AddSample(base::StringPrintf("%d", i));
}
- ByteVector real_bits = metric.bytes();
- int real_bit_count = CountBits(real_bits);
+ const ByteVector real_bits = metric.bytes();
+ const int real_bit_count = CountBits(real_bits);
EXPECT_EQ(real_bit_count, 152);
- std::string secret = HmacByteVectorGenerator::GenerateEntropyInput();
- ByteVector report = metric.GetReport(secret);
+ const std::string secret = HmacByteVectorGenerator::GenerateEntropyInput();
+ const ByteVector report = metric.GetReport(secret);
- // For the bits we actually set in the bloom filter, get a count of how
+ // For the bits we actually set in the Bloom filter, get a count of how
// many of them reported true.
ByteVector from_true_reports = report;
// Real bits AND report bits.
ByteVectorMerge(real_bits, real_bits, &from_true_reports);
- int true_from_true_count = CountBits(from_true_reports);
+ const int true_from_true_count = CountBits(from_true_reports);
- // For the bits we didn't set in the bloom filter, get a count of how
+ // For the bits we didn't set in the Bloom filter, get a count of how
// many of them reported true.
ByteVector from_false_reports = report;
ByteVectorOr(real_bits, &from_false_reports);
- int true_from_false_count = CountBits(from_false_reports) - real_bit_count;
+ const int true_from_false_count =
+ CountBits(from_false_reports) - real_bit_count;
// The probability of a true bit being true after redaction =
// [fake_prob]*[fake_true_prob] + (1-[fake_prob]) =
diff --git a/components/rappor/rappor_service.cc b/components/rappor/rappor_service.cc
index 493e7b9..01e6ab0 100644
--- a/components/rappor/rappor_service.cc
+++ b/components/rappor/rappor_service.cc
@@ -10,8 +10,11 @@
#include "base/prefs/pref_service.h"
#include "base/rand_util.h"
#include "base/stl_util.h"
+#include "base/time/time.h"
#include "components/metrics/metrics_hashes.h"
+#include "components/rappor/log_uploader.h"
#include "components/rappor/proto/rappor_metric.pb.h"
+#include "components/rappor/rappor_metric.h"
#include "components/rappor/rappor_pref_names.h"
#include "components/variations/variations_associated_data.h"
@@ -42,14 +45,13 @@ GURL GetServerUrl() {
}
const RapporParameters kRapporParametersForType[NUM_RAPPOR_TYPES] = {
- { // ETLD_PLUS_ONE_RAPPOR_TYPE
- 16 /* Bloom filter size bytes */,
- 2 /* Bloom filter hash count */,
- rappor::PROBABILITY_75 /* Fake data probability */,
- rappor::PROBABILITY_50 /* Fake one probability */,
- rappor::PROBABILITY_75 /* One coin probability */,
- rappor::PROBABILITY_50 /* Zero coin probability */
- },
+ // ETLD_PLUS_ONE_RAPPOR_TYPE
+ {16 /* Bloom filter size bytes */,
+ 2 /* Bloom filter hash count */,
+ rappor::PROBABILITY_75 /* Fake data probability */,
+ rappor::PROBABILITY_50 /* Fake one probability */,
+ rappor::PROBABILITY_75 /* One coin probability */,
+ rappor::PROBABILITY_50 /* Zero coin probability */},
};
} // namespace
@@ -62,7 +64,7 @@ RapporService::~RapporService() {
void RapporService::Start(PrefService* pref_service,
net::URLRequestContextGetter* request_context) {
- GURL server_url = GetServerUrl();
+ const GURL server_url = GetServerUrl();
if (!server_url.is_valid())
return;
DCHECK(!uploader_);
@@ -98,19 +100,21 @@ void RapporService::RegisterPrefs(PrefRegistrySimple* registry) {
}
void RapporService::LoadCohort(PrefService* pref_service) {
- DCHECK_EQ(cohort_, -1);
+ DCHECK(!IsInitialized());
cohort_ = pref_service->GetInteger(prefs::kRapporCohort);
+ // If the user is already assigned to a valid cohort, we're done.
if (cohort_ >= 0 && cohort_ < kNumCohorts)
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_);
}
void RapporService::LoadSecret(PrefService* pref_service) {
DCHECK(secret_.empty());
- std::string secret_base64 =
- pref_service->GetString(prefs::kRapporSecret);
+ std::string secret_base64 = pref_service->GetString(prefs::kRapporSecret);
if (!secret_base64.empty()) {
bool decoded = base::Base64Decode(secret_base64, &secret_);
if (decoded && secret_.size() == HmacByteVectorGenerator::kEntropyInputSize)
@@ -132,8 +136,9 @@ bool RapporService::ExportMetrics(RapporReports* reports) {
DCHECK_GE(cohort_, 0);
reports->set_cohort(cohort_);
- for (std::map<std::string, RapporMetric*>::iterator it = metrics_map_.begin();
- metrics_map_.end() != it;
+ for (std::map<std::string, RapporMetric*>::const_iterator it =
+ metrics_map_.begin();
+ it != metrics_map_.end();
++it) {
const RapporMetric* metric = it->second;
RapporReports::Report* report = reports->add_report();
@@ -163,7 +168,6 @@ void RapporService::RecordSampleInternal(const std::string& metric_name,
const RapporParameters& parameters,
const std::string& sample) {
DCHECK(IsInitialized());
-
RapporMetric* metric = LookUpMetric(metric_name, parameters);
metric->AddSample(sample);
}
@@ -171,9 +175,9 @@ void RapporService::RecordSampleInternal(const std::string& metric_name,
RapporMetric* RapporService::LookUpMetric(const std::string& metric_name,
const RapporParameters& parameters) {
DCHECK(IsInitialized());
- std::map<std::string, RapporMetric*>::iterator it =
+ std::map<std::string, RapporMetric*>::const_iterator it =
metrics_map_.find(metric_name);
- if (metrics_map_.end() != it) {
+ if (it != metrics_map_.end()) {
RapporMetric* metric = it->second;
DCHECK_EQ(parameters.ToString(), metric->parameters().ToString());
return metric;
diff --git a/components/rappor/rappor_service.h b/components/rappor/rappor_service.h
index 9e2cd9b..323e9ba 100644
--- a/components/rappor/rappor_service.h
+++ b/components/rappor/rappor_service.h
@@ -5,22 +5,31 @@
#ifndef COMPONENTS_RAPPOR_RAPPOR_SERVICE_H_
#define COMPONENTS_RAPPOR_RAPPOR_SERVICE_H_
+#include <map>
#include <string>
#include "base/basictypes.h"
-#include "base/prefs/pref_service.h"
-#include "base/time/time.h"
+#include "base/macros.h"
+#include "base/memory/scoped_ptr.h"
#include "base/timer/timer.h"
-#include "components/rappor/log_uploader.h"
-#include "components/rappor/proto/rappor_metric.pb.h"
-#include "components/rappor/rappor_metric.h"
class PrefRegistrySimple;
+class PrefService;
+
+namespace net {
+class URLRequestContextGetter;
+}
namespace rappor {
+class LogUploader;
+class RapporMetric;
+class RapporReports;
+struct RapporParameters;
+
// The type of data stored in a metric.
enum RapporType {
+ // For sampling the eTLD+1 of a URL.
ETLD_PLUS_ONE_RAPPOR_TYPE = 0,
NUM_RAPPOR_TYPES
};
@@ -52,8 +61,9 @@ class RapporService {
static void RegisterPrefs(PrefRegistrySimple* registry);
protected:
- // Logs all of the collected metrics to the reports proto message. Exposed
- // for tests. Returns true if any metrics were recorded.
+ // Logs all of the collected metrics to the reports proto message and clears
+ // the internal map. Exposed for tests. Returns true if any metrics were
+ // recorded.
bool ExportMetrics(RapporReports* reports);
// Records a sample of the rappor metric specified by |parameters|.
@@ -91,7 +101,7 @@ class RapporService {
// The cohort this client is assigned to. -1 is uninitialized.
int32_t cohort_;
- // Timer which schedules calls to OnLogInterval()
+ // Timer which schedules calls to OnLogInterval().
base::OneShotTimer<RapporService> log_rotation_timer_;
// A private LogUploader instance for sending reports to the server.
diff --git a/components/rappor/rappor_service_unittest.cc b/components/rappor/rappor_service_unittest.cc
index 790eae7..a36e960 100644
--- a/components/rappor/rappor_service_unittest.cc
+++ b/components/rappor/rappor_service_unittest.cc
@@ -4,6 +4,9 @@
#include "components/rappor/rappor_service.h"
+#include "components/rappor/byte_vector_utils.h"
+#include "components/rappor/proto/rappor_metric.pb.h"
+#include "components/rappor/rappor_parameters.h"
#include "testing/gtest/include/gtest/gtest.h"
namespace rappor {
@@ -27,8 +30,7 @@ TEST(RapporServiceTest, RecordAndExportMetrics) {
PROBABILITY_75 /* Fake data probability */,
PROBABILITY_50 /* Fake one probability */,
PROBABILITY_75 /* One coin probability */,
- PROBABILITY_50 /* Zero coin probability */
- };
+ PROBABILITY_50 /* Zero coin probability */};
TestRapporService rappor_service;
rappor_service.SetCohortForTesting(0);