From 84d2a495da80a3d4b029d62a69be8323fd69008e Mon Sep 17 00:00:00 2001
From: "ttuttle@chromium.org"
 <ttuttle@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98>
Date: Fri, 9 May 2014 22:18:50 +0000
Subject: Domain Reliability: More security review.

Change some trivial stuff so sleevi can do more security review.
Also, add rdsmith as an OWNER.

BUG=356791

Review URL: https://codereview.chromium.org/252613002

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@269434 0039d316-1c4b-4281-b951-d872f2087c98
---
 components/domain_reliability/DEPS                 |   1 +
 components/domain_reliability/OWNERS               |   1 +
 components/domain_reliability/bake_in_configs.py   |   3 +-
 components/domain_reliability/baked_in_configs.h   |   2 +
 components/domain_reliability/beacon.cc            |   2 -
 components/domain_reliability/beacon.h             |   4 +-
 components/domain_reliability/config.cc            |  50 +++---
 components/domain_reliability/config.h             |  15 +-
 components/domain_reliability/config_unittest.cc   |  91 +++++++---
 components/domain_reliability/context.cc           | 193 +++++++++++----------
 components/domain_reliability/context.h            |  71 +++-----
 components/domain_reliability/context_unittest.cc  |  26 ++-
 components/domain_reliability/dispatcher.cc        |  62 ++++---
 components/domain_reliability/dispatcher.h         |  35 ++--
 .../domain_reliability/domain_reliability_export.h |   7 +-
 components/domain_reliability/monitor.cc           |  31 ++--
 components/domain_reliability/monitor.h            |  30 ++--
 components/domain_reliability/monitor_unittest.cc  |   6 +-
 components/domain_reliability/scheduler.cc         |  64 +++----
 components/domain_reliability/scheduler.h          |  27 +--
 .../domain_reliability/scheduler_unittest.cc       |  18 +-
 components/domain_reliability/test_util.cc         |   1 +
 components/domain_reliability/test_util.h          |   2 +
 components/domain_reliability/uploader.cc          |   8 +-
 components/domain_reliability/uploader.h           |  11 +-
 components/domain_reliability/uploader_unittest.cc |  10 +-
 components/domain_reliability/util.cc              |  18 +-
 components/domain_reliability/util.h               |  21 ++-
 components/domain_reliability/util_unittest.cc     |   2 +-
 29 files changed, 450 insertions(+), 362 deletions(-)

(limited to 'components/domain_reliability')

diff --git a/components/domain_reliability/DEPS b/components/domain_reliability/DEPS
index 70cc3c5..faf5b87 100644
--- a/components/domain_reliability/DEPS
+++ b/components/domain_reliability/DEPS
@@ -7,3 +7,4 @@ include_rules = [
   "+content/public/test",
   "+net",
 ]
+
diff --git a/components/domain_reliability/OWNERS b/components/domain_reliability/OWNERS
index 07a5e14..695ce9d 100644
--- a/components/domain_reliability/OWNERS
+++ b/components/domain_reliability/OWNERS
@@ -2,3 +2,4 @@ davidben@chromium.org
 mmenke@chromium.org
 szym@chromium.org
 ttuttle@chromium.org
+rdsmith@chromium.org
diff --git a/components/domain_reliability/bake_in_configs.py b/components/domain_reliability/bake_in_configs.py
index 65f8a0a..5cc784c 100755
--- a/components/domain_reliability/bake_in_configs.py
+++ b/components/domain_reliability/bake_in_configs.py
@@ -78,7 +78,6 @@ def main():
     return 1
 
   cpp_code = CC_HEADER
-  cpp_file = sys.argv[-1]
   for json_file in sys.argv[1:-1]:
     with open(json_file, 'r') as f:
       json_text = f.read()
@@ -96,7 +95,7 @@ def main():
     cpp_code += "\n"
   cpp_code += CC_FOOTER
 
-  with open(cpp_file, 'wb') as f:
+  with open(sys.argv[-1], 'wb') as f:
     f.write(cpp_code)
 
   return 0
diff --git a/components/domain_reliability/baked_in_configs.h b/components/domain_reliability/baked_in_configs.h
index 1e6fe82..31f1d5d 100644
--- a/components/domain_reliability/baked_in_configs.h
+++ b/components/domain_reliability/baked_in_configs.h
@@ -9,6 +9,8 @@
 
 namespace domain_reliability {
 
+// NULL-terminated array of pointers to JSON-encoded Domain Reliability
+// configurations. Read by DomainReliabilityMonitor::AddBakedInConfigs.
 DOMAIN_RELIABILITY_EXPORT extern const char* const kBakedInJsonConfigs[];
 
 }  // namespace domain_reliability
diff --git a/components/domain_reliability/beacon.cc b/components/domain_reliability/beacon.cc
index 8b73811..f826a20 100644
--- a/components/domain_reliability/beacon.cc
+++ b/components/domain_reliability/beacon.cc
@@ -31,8 +31,6 @@ Value* DomainReliabilityBeacon::ToValue(base::TimeTicks upload_time) const {
                            elapsed.InMilliseconds());
   beacon_value->SetInteger("request_age_ms",
                            (upload_time - start_time).InMilliseconds());
-  // TODO(ttuttle): Implement protocol and dns_resolver_ip[s] fields.
-
   return beacon_value;
 }
 
diff --git a/components/domain_reliability/beacon.h b/components/domain_reliability/beacon.h
index 42cfb1c..36bddf8 100644
--- a/components/domain_reliability/beacon.h
+++ b/components/domain_reliability/beacon.h
@@ -17,7 +17,7 @@ class Value;
 namespace domain_reliability {
 
 // The per-request data that is uploaded to the Domain Reliability collector.
-class DOMAIN_RELIABILITY_EXPORT DomainReliabilityBeacon {
+struct DOMAIN_RELIABILITY_EXPORT DomainReliabilityBeacon {
  public:
   DomainReliabilityBeacon();
   ~DomainReliabilityBeacon();
@@ -39,7 +39,7 @@ class DOMAIN_RELIABILITY_EXPORT DomainReliabilityBeacon {
   // Start time of the request.  Encoded as the request age in the final JSON.
   base::TimeTicks start_time;
 
-  // Okay to copy and assign :)
+  // Okay to copy and assign.
 };
 
 }  // namespace domain_reliability
diff --git a/components/domain_reliability/config.cc b/components/domain_reliability/config.cc
index 675f319..c8e9793 100644
--- a/components/domain_reliability/config.cc
+++ b/components/domain_reliability/config.cc
@@ -2,8 +2,15 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
+// Make sure stdint.h includes SIZE_MAX. (See C89, p259, footnote 221.)
+#ifndef __STDC_LIMIT_MACROS
+#define __STDC_LIMIT_MACROS 1
+#endif
+
 #include "components/domain_reliability/config.h"
 
+#include <stdint.h>
+
 #include "base/json/json_reader.h"
 #include "base/json/json_value_converter.h"
 #include "base/rand_util.h"
@@ -22,16 +29,19 @@ bool IsValidSampleRate(double p) { return p >= 0.0 && p <= 1.0; }
 
 namespace domain_reliability {
 
-DomainReliabilityConfig::Resource::Resource() {}
+// static
+const size_t DomainReliabilityConfig::kInvalidResourceIndex = SIZE_MAX;
 
+DomainReliabilityConfig::Resource::Resource() {
+}
 DomainReliabilityConfig::Resource::~Resource() {}
 
-bool DomainReliabilityConfig::Resource::MatchesUrlString(
-    const std::string& url_string) const {
-  ScopedVector<std::string>::const_iterator it;
+bool DomainReliabilityConfig::Resource::MatchesUrl(const GURL& url) const {
+  const std::string& spec = url.spec();
 
+  ScopedVector<std::string>::const_iterator it;
   for (it = url_patterns.begin(); it != url_patterns.end(); it++) {
-    if (MatchPattern(url_string, **it))
+    if (MatchPattern(spec, **it))
       return true;
   }
 
@@ -41,6 +51,7 @@ bool DomainReliabilityConfig::Resource::MatchesUrlString(
 bool DomainReliabilityConfig::Resource::DecideIfShouldReportRequest(
     bool success) const {
   double sample_rate = success ? success_sample_rate : failure_sample_rate;
+  DCHECK(IsValidSampleRate(sample_rate));
   return base::RandDouble() < sample_rate;
 }
 
@@ -62,7 +73,6 @@ bool DomainReliabilityConfig::Resource::IsValid() const {
 }
 
 DomainReliabilityConfig::Collector::Collector() {}
-
 DomainReliabilityConfig::Collector::~Collector() {}
 
 // static
@@ -77,26 +87,20 @@ bool DomainReliabilityConfig::Collector::IsValid() const {
 }
 
 DomainReliabilityConfig::DomainReliabilityConfig() : valid_until(0.0) {}
-
 DomainReliabilityConfig::~DomainReliabilityConfig() {}
 
 // static
 scoped_ptr<const DomainReliabilityConfig> DomainReliabilityConfig::FromJSON(
     const base::StringPiece& json) {
   scoped_ptr<base::Value> value(base::JSONReader::Read(json));
-  if (!value)
-    return scoped_ptr<const DomainReliabilityConfig>();
-
-  DomainReliabilityConfig* config = new DomainReliabilityConfig();
   base::JSONValueConverter<DomainReliabilityConfig> converter;
-  if (!converter.Convert(*value, config)) {
-    return scoped_ptr<const DomainReliabilityConfig>();
-  }
+  DomainReliabilityConfig* config = new DomainReliabilityConfig();
 
-  if (!config->IsValid())
+  // If we can parse and convert the JSON into a valid config, return that.
+  if (value && converter.Convert(*value, config) && config->IsValid())
+    return scoped_ptr<const DomainReliabilityConfig>(config);
+  else
     return scoped_ptr<const DomainReliabilityConfig>();
-
-  return scoped_ptr<const DomainReliabilityConfig>(config);
 }
 
 bool DomainReliabilityConfig::IsValid() const {
@@ -119,19 +123,21 @@ bool DomainReliabilityConfig::IsValid() const {
 }
 
 bool DomainReliabilityConfig::IsExpired(base::Time now) const {
+  DCHECK_NE(0.0, valid_until);
   base::Time valid_until_time = base::Time::FromDoubleT(valid_until);
   return now > valid_until_time;
 }
 
-int DomainReliabilityConfig::GetResourceIndexForUrl(const GURL& url) const {
-  const std::string& url_string = url.spec();
+size_t DomainReliabilityConfig::GetResourceIndexForUrl(const GURL& url) const {
+  // Removes username, password, and fragment.
+  GURL sanitized_url = url.GetAsReferrer();
 
   for (size_t i = 0; i < resources.size(); ++i) {
-    if (resources[i]->MatchesUrlString(url_string))
-      return static_cast<int>(i);
+    if (resources[i]->MatchesUrl(sanitized_url))
+      return i;
   }
 
-  return -1;
+  return kInvalidResourceIndex;
 }
 
 // static
diff --git a/components/domain_reliability/config.h b/components/domain_reliability/config.h
index 8fd1ce8..1ba3c7a 100644
--- a/components/domain_reliability/config.h
+++ b/components/domain_reliability/config.h
@@ -23,6 +23,8 @@ namespace domain_reliability {
 // with what frequency, and where the beacons are uploaded.
 class DOMAIN_RELIABILITY_EXPORT DomainReliabilityConfig {
  public:
+  static const size_t kInvalidResourceIndex;
+
   // A particular resource named in the config -- includes a set of URL
   // patterns that the resource will match, along with sample rates for
   // successful and unsuccessful requests.
@@ -33,12 +35,11 @@ class DOMAIN_RELIABILITY_EXPORT DomainReliabilityConfig {
 
     // Returns whether |url_string| matches at least one of the |url_patterns|
     // in this Resource.
-    bool MatchesUrlString(const std::string& url_string) const;
+    bool MatchesUrl(const GURL& url) const;
 
     // Returns whether a request (that was successful if |success| is true)
-    // should be reported (with a full beacon). (The output is random; it
-    // compares a random number to |success_sample_rate| or
-    // |failure_sample_rate|.)
+    // should be reported with a full beacon. (The output is non-deterministic;
+    // it |success_sample_rate| or |failure_sample_rate| to a random number.)
     bool DecideIfShouldReportRequest(bool success) const;
 
     // Registers with the JSONValueConverter so it will know how to convert the
@@ -93,11 +94,13 @@ class DOMAIN_RELIABILITY_EXPORT DomainReliabilityConfig {
 
   bool IsValid() const;
 
+  // Checks whether |now| is past the expiration time provided in the config.
   bool IsExpired(base::Time now) const;
 
   // Finds the index (in resources) of the first Resource that matches a
-  // particular URL. Returns -1 if the URL is not matched by any Resources.
-  int GetResourceIndexForUrl(const GURL& url) const;
+  // particular URL. Returns kInvalidResourceIndex if it is not matched by any
+  // Resources.
+  size_t GetResourceIndexForUrl(const GURL& url) const;
 
   // Registers with the JSONValueConverter so it will know how to convert the
   // JSON for a config into the struct.
diff --git a/components/domain_reliability/config_unittest.cc b/components/domain_reliability/config_unittest.cc
index 6d58efa..11aa39a 100644
--- a/components/domain_reliability/config_unittest.cc
+++ b/components/domain_reliability/config_unittest.cc
@@ -11,15 +11,25 @@
 #include "testing/gtest/include/gtest/gtest.h"
 
 namespace domain_reliability {
-
 namespace {
 
-scoped_ptr<DomainReliabilityConfig> MakeValidConfig() {
+scoped_ptr<DomainReliabilityConfig> MakeBaseConfig() {
   DomainReliabilityConfig* config = new DomainReliabilityConfig();
   config->domain = "example";
   config->valid_until = 1234567890.0;
   config->version = "1";
 
+  DomainReliabilityConfig::Collector* collector =
+      new DomainReliabilityConfig::Collector();
+  collector->upload_url = GURL("https://example/upload");
+  config->collectors.push_back(collector);
+
+  return scoped_ptr<DomainReliabilityConfig>(config);
+}
+
+scoped_ptr<DomainReliabilityConfig> MakeSampleConfig() {
+  scoped_ptr<DomainReliabilityConfig> config(MakeBaseConfig());
+
   DomainReliabilityConfig::Resource* resource =
       new DomainReliabilityConfig::Resource();
   resource->name = "home";
@@ -45,13 +55,25 @@ scoped_ptr<DomainReliabilityConfig> MakeValidConfig() {
   resource->failure_sample_rate = 1.0;
   config->resources.push_back(resource);
 
-  DomainReliabilityConfig::Collector* collector =
-      new DomainReliabilityConfig::Collector();
-  collector->upload_url = GURL("https://example/upload");
-  config->collectors.push_back(collector);
+  EXPECT_TRUE(config->IsValid());
+  return config.Pass();
+}
+
+scoped_ptr<DomainReliabilityConfig> MakeConfigWithResource(
+    const std::string& name,
+    const std::string& pattern) {
+  scoped_ptr<DomainReliabilityConfig> config(MakeBaseConfig());
+
+  DomainReliabilityConfig::Resource* resource =
+      new DomainReliabilityConfig::Resource();
+  resource->name = name;
+  resource->url_patterns.push_back(new std::string(pattern));
+  resource->success_sample_rate = 1.0;
+  resource->failure_sample_rate = 1.0;
+  config->resources.push_back(resource);
 
   EXPECT_TRUE(config->IsValid());
-  return scoped_ptr<DomainReliabilityConfig>(config);
+  return config.Pass();
 }
 
 int GetIndex(DomainReliabilityConfig* config, const char* url_string) {
@@ -65,46 +87,46 @@ class DomainReliabilityConfigTest : public testing::Test { };
 TEST_F(DomainReliabilityConfigTest, IsValid) {
   scoped_ptr<DomainReliabilityConfig> config;
 
-  config = MakeValidConfig();
+  config = MakeSampleConfig();
   EXPECT_TRUE(config->IsValid());
 
-  config = MakeValidConfig();
+  config = MakeSampleConfig();
   config->domain = "";
   EXPECT_FALSE(config->IsValid());
 
-  config = MakeValidConfig();
+  config = MakeSampleConfig();
   config->valid_until = 0.0;
   EXPECT_FALSE(config->IsValid());
 
-  config = MakeValidConfig();
+  config = MakeSampleConfig();
   config->version = "";
   EXPECT_FALSE(config->IsValid());
 
-  config = MakeValidConfig();
+  config = MakeSampleConfig();
   config->resources.clear();
   EXPECT_FALSE(config->IsValid());
 
-  config = MakeValidConfig();
+  config = MakeSampleConfig();
   config->resources[0]->name.clear();
   EXPECT_FALSE(config->IsValid());
 
-  config = MakeValidConfig();
+  config = MakeSampleConfig();
   config->resources[0]->url_patterns.clear();
   EXPECT_FALSE(config->IsValid());
 
-  config = MakeValidConfig();
+  config = MakeSampleConfig();
   config->resources[0]->success_sample_rate = 2.0;
   EXPECT_FALSE(config->IsValid());
 
-  config = MakeValidConfig();
+  config = MakeSampleConfig();
   config->resources[0]->failure_sample_rate = 2.0;
   EXPECT_FALSE(config->IsValid());
 
-  config = MakeValidConfig();
+  config = MakeSampleConfig();
   config->collectors.clear();
   EXPECT_FALSE(config->IsValid());
 
-  config = MakeValidConfig();
+  config = MakeSampleConfig();
   config->collectors[0]->upload_url = GURL();
   EXPECT_FALSE(config->IsValid());
 }
@@ -123,13 +145,34 @@ TEST_F(DomainReliabilityConfigTest, IsExpired) {
 }
 
 TEST_F(DomainReliabilityConfigTest, GetResourceIndexForUrl) {
-  scoped_ptr<DomainReliabilityConfig> config = MakeValidConfig();
+  scoped_ptr<DomainReliabilityConfig> config = MakeSampleConfig();
+
+  EXPECT_EQ(0, GetIndex(config.get(), "http://example/"));
+  EXPECT_EQ(1, GetIndex(config.get(), "http://example/css/foo.css"));
+  EXPECT_EQ(1, GetIndex(config.get(), "http://example/js/bar.js"));
+  EXPECT_EQ(2, GetIndex(config.get(), "http://example/test.html"));
+  EXPECT_EQ(-1, GetIndex(config.get(), "http://example/no-resource"));
+}
+
+TEST_F(DomainReliabilityConfigTest, UrlPatternCantMatchUsername) {
+  scoped_ptr<DomainReliabilityConfig> config =
+      MakeConfigWithResource("username", "*username*");
+
+  EXPECT_EQ(-1, GetIndex(config.get(), "http://username:password@example/"));
+}
+
+TEST_F(DomainReliabilityConfigTest, UrlPatternCantMatchPassword) {
+  scoped_ptr<DomainReliabilityConfig> config =
+      MakeConfigWithResource("password", "*password*");
+
+  EXPECT_EQ(-1, GetIndex(config.get(), "http://username:password@example/"));
+}
+
+TEST_F(DomainReliabilityConfigTest, UrlPatternCantMatchFragment) {
+  scoped_ptr<DomainReliabilityConfig> config =
+      MakeConfigWithResource("fragment", "*fragment*");
 
-  EXPECT_EQ(0, GetIndex(&*config, "http://example/"));
-  EXPECT_EQ(1, GetIndex(&*config, "http://example/css/foo.css"));
-  EXPECT_EQ(1, GetIndex(&*config, "http://example/js/bar.js"));
-  EXPECT_EQ(2, GetIndex(&*config, "http://example/test.html"));
-  EXPECT_EQ(-1, GetIndex(&*config, "http://example/no-resource"));
+  EXPECT_EQ(-1, GetIndex(config.get(), "http://example/#fragment"));
 }
 
 TEST_F(DomainReliabilityConfigTest, FromJSON) {
diff --git a/components/domain_reliability/context.cc b/components/domain_reliability/context.cc
index 24ff329..3805c1d9 100644
--- a/components/domain_reliability/context.cc
+++ b/components/domain_reliability/context.cc
@@ -12,6 +12,7 @@
 #include "base/metrics/histogram.h"
 #include "base/metrics/sparse_histogram.h"
 #include "base/values.h"
+#include "components/domain_reliability/beacon.h"
 #include "components/domain_reliability/dispatcher.h"
 #include "net/base/net_errors.h"
 #include "net/url_request/url_request_context_getter.h"
@@ -23,23 +24,105 @@ using base::Value;
 namespace domain_reliability {
 
 namespace {
-const char* kReporter = "chrome";
 typedef std::deque<DomainReliabilityBeacon> BeaconDeque;
 typedef BeaconDeque::iterator BeaconIterator;
 typedef BeaconDeque::const_iterator BeaconConstIterator;
 }  // namespace
 
-const int DomainReliabilityContext::kMaxQueuedBeacons = 150;
+class DomainReliabilityContext::ResourceState {
+ public:
+  ResourceState(DomainReliabilityContext* context,
+                const DomainReliabilityConfig::Resource* config)
+      : context(context),
+        config(config),
+        successful_requests(0),
+        failed_requests(0) {}
+  ~ResourceState() {}
+
+  scoped_ptr<base::Value> ToValue(base::TimeTicks upload_time) const {
+    ListValue* beacons_value = new ListValue();
+    for (BeaconConstIterator it = beacons.begin(); it != beacons.end(); ++it)
+      beacons_value->Append(it->ToValue(upload_time));
+
+    DictionaryValue* resource_value = new DictionaryValue();
+    resource_value->SetString("resource_name", config->name);
+    resource_value->SetInteger("successful_requests", successful_requests);
+    resource_value->SetInteger("failed_requests", failed_requests);
+    resource_value->Set("beacons", beacons_value);
+
+    return scoped_ptr<Value>(resource_value);
+  }
+
+  // Remembers the current state of the resource data when an upload starts.
+  void MarkUpload() {
+    uploading_beacons_size = beacons.size();
+    uploading_successful_requests = successful_requests;
+    uploading_failed_requests = failed_requests;
+  }
+
+  // Uses the state remembered by |MarkUpload| to remove successfully uploaded
+  // data but keep beacons and request counts added after the upload started.
+  void CommitUpload() {
+    BeaconIterator begin = beacons.begin();
+    BeaconIterator end = begin + uploading_beacons_size;
+    beacons.erase(begin, end);
+    successful_requests -= uploading_successful_requests;
+    failed_requests -= uploading_failed_requests;
+  }
+
+  // Gets the start time of the oldest beacon, if there are any. Returns true
+  // and sets |oldest_start_out| if so; otherwise, returns false.
+  bool GetOldestBeaconStart(base::TimeTicks* oldest_start_out) const {
+    if (beacons.empty())
+      return false;
+    *oldest_start_out = beacons[0].start_time;
+    return true;
+  }
+
+  // Removes the oldest beacon. DCHECKs if there isn't one.
+  void RemoveOldestBeacon() {
+    DCHECK(!beacons.empty());
+    beacons.erase(beacons.begin());
+    // If that just removed a beacon counted in uploading_beacons_size,
+    // decrement
+    // that.
+    if (uploading_beacons_size > 0)
+      --uploading_beacons_size;
+  }
+
+  DomainReliabilityContext* context;
+  const DomainReliabilityConfig::Resource* config;
+
+  std::deque<DomainReliabilityBeacon> beacons;
+  uint32 successful_requests;
+  uint32 failed_requests;
+
+  // State saved during uploads; if an upload succeeds, these are used to
+  // remove uploaded data from the beacon list and request counters.
+  size_t uploading_beacons_size;
+  uint32 uploading_successful_requests;
+  uint32 uploading_failed_requests;
+
+ private:
+  DISALLOW_COPY_AND_ASSIGN(ResourceState);
+};
+
+// static
+const size_t DomainReliabilityContext::kMaxQueuedBeacons = 150;
 
 DomainReliabilityContext::DomainReliabilityContext(
     MockableTime* time,
     const DomainReliabilityScheduler::Params& scheduler_params,
+    const std::string& upload_reporter_string,
     DomainReliabilityDispatcher* dispatcher,
     DomainReliabilityUploader* uploader,
     scoped_ptr<const DomainReliabilityConfig> config)
     : config_(config.Pass()),
       time_(time),
-      scheduler_(time, config_->collectors.size(), scheduler_params,
+      upload_reporter_string_(upload_reporter_string),
+      scheduler_(time,
+                 config_->collectors.size(),
+                 scheduler_params,
                  base::Bind(&DomainReliabilityContext::ScheduleUpload,
                             base::Unretained(this))),
       dispatcher_(dispatcher),
@@ -51,13 +134,12 @@ DomainReliabilityContext::DomainReliabilityContext(
 
 DomainReliabilityContext::~DomainReliabilityContext() {}
 
-void DomainReliabilityContext::AddBeacon(
-    const DomainReliabilityBeacon& beacon,
-    const GURL& url) {
-  int index = config_->GetResourceIndexForUrl(url);
-  if (index < 0)
+void DomainReliabilityContext::OnBeacon(const GURL& url,
+                                        const DomainReliabilityBeacon& beacon) {
+  size_t index = config_->GetResourceIndexForUrl(url);
+  if (index == DomainReliabilityConfig::kInvalidResourceIndex)
     return;
-  DCHECK_GT(states_.size(), static_cast<size_t>(index));
+  DCHECK_GT(states_.size(), index);
 
   ResourceState* state = states_[index];
   bool success = beacon.http_response_code >= 200 &&
@@ -67,14 +149,6 @@ void DomainReliabilityContext::AddBeacon(
   else
     ++state->failed_requests;
 
-  VLOG(1) << "Received Beacon: "
-          << state->config->name << " "
-          << beacon.status << " "
-          << beacon.chrome_error << " "
-          << beacon.http_response_code << " "
-          << beacon.server_ip << " "
-          << beacon.elapsed.InMilliseconds() << "ms";
-
   bool reported = false;
   bool evicted = false;
   if (state->config->DecideIfShouldReportRequest(success)) {
@@ -92,84 +166,25 @@ void DomainReliabilityContext::AddBeacon(
   }
 
   UMA_HISTOGRAM_BOOLEAN("DomainReliability.BeaconReported", reported);
-  UMA_HISTOGRAM_BOOLEAN("DomainReliability.AddBeaconDidEvict", evicted);
+  UMA_HISTOGRAM_BOOLEAN("DomainReliability.OnBeaconDidEvict", evicted);
 }
 
 void DomainReliabilityContext::GetQueuedDataForTesting(
-    int resource_index,
+    size_t resource_index,
     std::vector<DomainReliabilityBeacon>* beacons_out,
-    int* successful_requests_out,
-    int* failed_requests_out) const {
-  DCHECK_LE(0, resource_index);
-  DCHECK_GT(static_cast<int>(states_.size()), resource_index);
+    uint32* successful_requests_out,
+    uint32* failed_requests_out) const {
+  DCHECK_NE(DomainReliabilityConfig::kInvalidResourceIndex, resource_index);
+  DCHECK_GT(states_.size(), resource_index);
   const ResourceState& state = *states_[resource_index];
-  if (beacons_out) {
-    beacons_out->resize(state.beacons.size());
-    std::copy(state.beacons.begin(), state.beacons.end(), beacons_out->begin());
-  }
+  if (beacons_out)
+    beacons_out->assign(state.beacons.begin(), state.beacons.end());
   if (successful_requests_out)
     *successful_requests_out = state.successful_requests;
   if (failed_requests_out)
     *failed_requests_out = state.failed_requests;
 }
 
-DomainReliabilityContext::ResourceState::ResourceState(
-    DomainReliabilityContext* context,
-    const DomainReliabilityConfig::Resource* config)
-    : context(context),
-      config(config),
-      successful_requests(0),
-      failed_requests(0) {}
-
-DomainReliabilityContext::ResourceState::~ResourceState() {}
-
-scoped_ptr<Value> DomainReliabilityContext::ResourceState::ToValue(
-    base::TimeTicks upload_time) const {
-  ListValue* beacons_value = new ListValue();
-  for (BeaconConstIterator it = beacons.begin(); it != beacons.end(); ++it)
-    beacons_value->Append(it->ToValue(upload_time));
-
-  DictionaryValue* resource_value = new DictionaryValue();
-  resource_value->SetString("resource_name", config->name);
-  resource_value->SetInteger("successful_requests", successful_requests);
-  resource_value->SetInteger("failed_requests", failed_requests);
-  resource_value->Set("beacons", beacons_value);
-
-  return scoped_ptr<Value>(resource_value);
-}
-
-void DomainReliabilityContext::ResourceState::MarkUpload() {
-  uploading_beacons_size = beacons.size();
-  uploading_successful_requests = successful_requests;
-  uploading_failed_requests = failed_requests;
-}
-
-void DomainReliabilityContext::ResourceState::CommitUpload() {
-  BeaconIterator begin = beacons.begin();
-  BeaconIterator end = begin + uploading_beacons_size;
-  beacons.erase(begin, end);
-  successful_requests -= uploading_successful_requests;
-  failed_requests -= uploading_failed_requests;
-}
-
-bool DomainReliabilityContext::ResourceState::GetOldestBeaconStart(
-    base::TimeTicks* oldest_start_out) const {
-  if (beacons.empty())
-    return false;
-
-  *oldest_start_out = beacons[0].start_time;
-  return true;
-}
-
-void DomainReliabilityContext::ResourceState::RemoveOldestBeacon() {
-  DCHECK(!beacons.empty());
-  beacons.erase(beacons.begin());
-  // If that just removed a beacon counted in uploading_beacons_size, decrement
-  // that.
-  if (uploading_beacons_size > 0)
-    --uploading_beacons_size;
-}
-
 void DomainReliabilityContext::InitializeResourceStates() {
   ScopedVector<DomainReliabilityConfig::Resource>::const_iterator it;
   for (it = config_->resources.begin(); it != config_->resources.end(); ++it)
@@ -197,8 +212,7 @@ void DomainReliabilityContext::StartUpload() {
   base::JSONWriter::Write(report_value.get(), &report_json);
   report_value.reset();
 
-  int collector_index;
-  scheduler_.OnUploadStart(&collector_index);
+  size_t collector_index = scheduler_.OnUploadStart();
 
   uploader_->UploadReport(
       report_json,
@@ -234,7 +248,7 @@ scoped_ptr<const Value> DomainReliabilityContext::CreateReport(
     resources_value->Append((*it)->ToValue(upload_time).release());
 
   DictionaryValue* report_value = new DictionaryValue();
-  report_value->SetString("reporter", kReporter);
+  report_value->SetString("reporter", upload_reporter_string_);
   report_value->Set("resource_reports", resources_value);
 
   return scoped_ptr<const Value>(report_value);
@@ -253,7 +267,7 @@ void DomainReliabilityContext::CommitUpload() {
 }
 
 void DomainReliabilityContext::RemoveOldestBeacon() {
-  DCHECK_LT(0, beacon_count_);
+  DCHECK_LT(0u, beacon_count_);
 
   base::TimeTicks min_time;
   ResourceState* min_resource = NULL;
@@ -268,7 +282,8 @@ void DomainReliabilityContext::RemoveOldestBeacon() {
   }
   DCHECK(min_resource);
 
-  VLOG(1) << "Removing oldest beacon from " << min_resource->config->name;
+  VLOG(1) << "Beacon queue for " << config().domain << " full; "
+          << "removing oldest beacon from " << min_resource->config->name;
 
   min_resource->RemoveOldestBeacon();
   --beacon_count_;
diff --git a/components/domain_reliability/context.h b/components/domain_reliability/context.h
index 2a8a9e1..c980937 100644
--- a/components/domain_reliability/context.h
+++ b/components/domain_reliability/context.h
@@ -6,13 +6,11 @@
 #define COMPONENTS_DOMAIN_RELIABILITY_CONTEXT_H_
 
 #include <deque>
-#include <map>
+#include <vector>
 
-#include "base/memory/ref_counted.h"
 #include "base/memory/scoped_ptr.h"
 #include "base/memory/weak_ptr.h"
 #include "base/time/time.h"
-#include "components/domain_reliability/beacon.h"
 #include "components/domain_reliability/config.h"
 #include "components/domain_reliability/domain_reliability_export.h"
 #include "components/domain_reliability/scheduler.h"
@@ -23,6 +21,7 @@ class GURL;
 
 namespace domain_reliability {
 
+struct DomainReliabilityBeacon;
 class DomainReliabilityDispatcher;
 class MockableTime;
 
@@ -33,64 +32,31 @@ class DOMAIN_RELIABILITY_EXPORT DomainReliabilityContext {
   DomainReliabilityContext(
       MockableTime* time,
       const DomainReliabilityScheduler::Params& scheduler_params,
+      const std::string& upload_reporter_string,
       DomainReliabilityDispatcher* dispatcher,
       DomainReliabilityUploader* uploader,
       scoped_ptr<const DomainReliabilityConfig> config);
-  virtual ~DomainReliabilityContext();
+  ~DomainReliabilityContext();
 
-  void AddBeacon(const DomainReliabilityBeacon& beacon, const GURL& url);
+  // Notifies the context of a beacon on its domain(s); may or may not save the
+  // actual beacon to be uploaded, depending on the sample rates in the config,
+  // but will increment one of the request counters in any case.
+  void OnBeacon(const GURL& url, const DomainReliabilityBeacon& beacon);
 
   void GetQueuedDataForTesting(
-      int resource_index,
+      size_t resource_index,
       std::vector<DomainReliabilityBeacon>* beacons_out,
-      int* successful_requests_out,
-      int* failed_requests_out) const;
+      uint32* successful_requests_out,
+      uint32* failed_requests_out) const;
 
   const DomainReliabilityConfig& config() { return *config_.get(); }
 
   // Maximum number of beacons queued per context; if more than this many are
   // queued; the oldest beacons will be removed.
-  static const int kMaxQueuedBeacons;
+  static const size_t kMaxQueuedBeacons;
 
  private:
-  // Resource-specific state (queued beacons and request counts).
-  class ResourceState {
-   public:
-    ResourceState(DomainReliabilityContext* context,
-                  const DomainReliabilityConfig::Resource* config);
-    ~ResourceState();
-
-    scoped_ptr<base::Value> ToValue(base::TimeTicks upload_time) const;
-
-    // Remembers the current state of the resource data when an upload starts.
-    void MarkUpload();
-
-    // Uses the state remembered by |MarkUpload| to remove successfully uploaded
-    // data but keep beacons and request counts added after the upload started.
-    void CommitUpload();
-
-    // Gets the start time of the oldest beacon, if there are any. Returns true
-    // and sets |oldest_start_out| if so; otherwise, returns false.
-    bool GetOldestBeaconStart(base::TimeTicks* oldest_start_out) const;
-    // Removes the oldest beacon. DCHECKs if there isn't one.
-    void RemoveOldestBeacon();
-
-    DomainReliabilityContext* context;
-    const DomainReliabilityConfig::Resource* config;
-
-    std::deque<DomainReliabilityBeacon> beacons;
-    int successful_requests;
-    int failed_requests;
-
-    // State saved during uploads; if an upload succeeds, these are used to
-    // remove uploaded data from the beacon list and request counters.
-    size_t uploading_beacons_size;
-    int uploading_successful_requests;
-    int uploading_failed_requests;
-
-   private:
-    DISALLOW_COPY_AND_ASSIGN(ResourceState);
-  };
+  class ResourceState;
 
   typedef ScopedVector<ResourceState> ResourceStateVector;
   typedef ResourceStateVector::const_iterator ResourceStateIterator;
@@ -102,11 +68,15 @@ class DOMAIN_RELIABILITY_EXPORT DomainReliabilityContext {
 
   scoped_ptr<const base::Value> CreateReport(base::TimeTicks upload_time) const;
 
-  // Remembers the current state of the context when an upload starts.
+  // Remembers the current state of the context when an upload starts. Can be
+  // called multiple times in a row (without |CommitUpload|) if uploads fail
+  // and are retried.
   void MarkUpload();
 
   // Uses the state remembered by |MarkUpload| to remove successfully uploaded
   // data but keep beacons and request counts added after the upload started.
+  // N.B.: There is no equivalent "RollbackUpload" that needs to be called on a
+  // failed upload.
   void CommitUpload();
 
   // Finds and removes the oldest beacon. DCHECKs if there is none. (Called
@@ -115,6 +85,7 @@ class DOMAIN_RELIABILITY_EXPORT DomainReliabilityContext {
 
   scoped_ptr<const DomainReliabilityConfig> config_;
   MockableTime* time_;
+  const std::string& upload_reporter_string_;
   DomainReliabilityScheduler scheduler_;
   DomainReliabilityDispatcher* dispatcher_;
   DomainReliabilityUploader* uploader_;
@@ -122,8 +93,8 @@ class DOMAIN_RELIABILITY_EXPORT DomainReliabilityContext {
   // Each ResourceState in |states_| corresponds to the Resource of the same
   // index in the config.
   ResourceStateVector states_;
-  int beacon_count_;
-  int uploading_beacon_count_;
+  size_t beacon_count_;
+  size_t uploading_beacon_count_;
   base::TimeTicks upload_time_;
   base::TimeTicks last_upload_time_;
 
diff --git a/components/domain_reliability/context_unittest.cc b/components/domain_reliability/context_unittest.cc
index 56b62a1..2bdf2e6 100644
--- a/components/domain_reliability/context_unittest.cc
+++ b/components/domain_reliability/context_unittest.cc
@@ -18,7 +18,6 @@
 #include "testing/gtest/include/gtest/gtest.h"
 
 namespace domain_reliability {
-
 namespace {
 
 typedef std::vector<DomainReliabilityBeacon> BeaconVector;
@@ -43,8 +42,10 @@ class DomainReliabilityContextTest : public testing::Test {
         params_(CreateParams()),
         uploader_(base::Bind(&DomainReliabilityContextTest::OnUploadRequest,
                              base::Unretained(this))),
+        upload_reporter_string_("test-reporter"),
         context_(&time_,
                  params_,
+                 upload_reporter_string_,
                  &dispatcher_,
                  &uploader_,
                  CreateConfig().Pass()),
@@ -73,14 +74,16 @@ class DomainReliabilityContextTest : public testing::Test {
     upload_pending_ = false;
   }
 
-  bool CheckNoBeacons(int index) {
+  bool CheckNoBeacons(size_t index) {
     BeaconVector beacons;
     context_.GetQueuedDataForTesting(index, &beacons, NULL, NULL);
     return beacons.empty();
   }
 
-  bool CheckCounts(int index, int expected_successful, int expected_failed) {
-    int successful, failed;
+  bool CheckCounts(size_t index,
+                   unsigned expected_successful,
+                   unsigned expected_failed) {
+    unsigned successful, failed;
     context_.GetQueuedDataForTesting(index, NULL, &successful, &failed);
     return successful == expected_successful && failed == expected_failed;
   }
@@ -89,6 +92,7 @@ class DomainReliabilityContextTest : public testing::Test {
   DomainReliabilityDispatcher dispatcher_;
   DomainReliabilityScheduler::Params params_;
   MockUploader uploader_;
+  std::string upload_reporter_string_;
   DomainReliabilityContext context_;
 
  private:
@@ -153,8 +157,9 @@ TEST_F(DomainReliabilityContextTest, Create) {
 }
 
 TEST_F(DomainReliabilityContextTest, NoResource) {
+  GURL url("http://example/no_resource");
   DomainReliabilityBeacon beacon = MakeBeacon(&time_);
-  context_.AddBeacon(beacon, GURL("http://example/no_resource"));
+  context_.OnBeacon(url, beacon);
 
   EXPECT_TRUE(CheckNoBeacons(0));
   EXPECT_TRUE(CheckCounts(0, 0, 0));
@@ -163,8 +168,9 @@ TEST_F(DomainReliabilityContextTest, NoResource) {
 }
 
 TEST_F(DomainReliabilityContextTest, NeverReport) {
+  GURL url("http://example/never_report");
   DomainReliabilityBeacon beacon = MakeBeacon(&time_);
-  context_.AddBeacon(beacon, GURL("http://example/never_report"));
+  context_.OnBeacon(url, beacon);
 
   EXPECT_TRUE(CheckNoBeacons(0));
   EXPECT_TRUE(CheckCounts(0, 0, 0));
@@ -173,8 +179,9 @@ TEST_F(DomainReliabilityContextTest, NeverReport) {
 }
 
 TEST_F(DomainReliabilityContextTest, AlwaysReport) {
+  GURL url("http://example/always_report");
   DomainReliabilityBeacon beacon = MakeBeacon(&time_);
-  context_.AddBeacon(beacon, GURL("http://example/always_report"));
+  context_.OnBeacon(url, beacon);
 
   BeaconVector beacons;
   context_.GetQueuedDataForTesting(0, &beacons, NULL, NULL);
@@ -185,10 +192,11 @@ TEST_F(DomainReliabilityContextTest, AlwaysReport) {
 }
 
 TEST_F(DomainReliabilityContextTest, ReportUpload) {
+  GURL url("http://example/always_report");
   DomainReliabilityBeacon beacon = MakeBeacon(&time_);
-  context_.AddBeacon(beacon, GURL("http://example/always_report"));
+  context_.OnBeacon(url, beacon);
 
-  const char* kExpectedReport = "{\"reporter\":\"chrome\","
+  const char* kExpectedReport = "{\"reporter\":\"test-reporter\","
       "\"resource_reports\":[{\"beacons\":[{\"http_response_code\":200,"
       "\"request_age_ms\":300250,\"request_elapsed_ms\":250,\"server_ip\":"
       "\"127.0.0.1\",\"status\":\"ok\"}],\"failed_requests\":0,"
diff --git a/components/domain_reliability/dispatcher.cc b/components/domain_reliability/dispatcher.cc
index ceb4da0..e5e11cc 100644
--- a/components/domain_reliability/dispatcher.cc
+++ b/components/domain_reliability/dispatcher.cc
@@ -12,6 +12,32 @@
 
 namespace domain_reliability {
 
+struct DomainReliabilityDispatcher::Task {
+  Task(const base::Closure& closure,
+       scoped_ptr<MockableTime::Timer> timer,
+       base::TimeDelta min_delay,
+       base::TimeDelta max_delay);
+  ~Task();
+
+  base::Closure closure;
+  scoped_ptr<MockableTime::Timer> timer;
+  base::TimeDelta min_delay;
+  base::TimeDelta max_delay;
+  bool eligible;
+};
+
+DomainReliabilityDispatcher::Task::Task(const base::Closure& closure,
+                                        scoped_ptr<MockableTime::Timer> timer,
+                                        base::TimeDelta min_delay,
+                                        base::TimeDelta max_delay)
+    : closure(closure),
+      timer(timer.Pass()),
+      min_delay(min_delay),
+      max_delay(max_delay),
+      eligible(false) {}
+
+DomainReliabilityDispatcher::Task::~Task() {}
+
 DomainReliabilityDispatcher::DomainReliabilityDispatcher(MockableTime* time)
     : time_(time) {}
 
@@ -56,29 +82,15 @@ void DomainReliabilityDispatcher::RunEligibleTasks() {
   }
 }
 
-DomainReliabilityDispatcher::Task::Task(const base::Closure& closure,
-                                        scoped_ptr<MockableTime::Timer> timer,
-                                        base::TimeDelta min_delay,
-                                        base::TimeDelta max_delay)
-    : closure(closure),
-      timer(timer.Pass()),
-      min_delay(min_delay),
-      max_delay(max_delay),
-      eligible(false) {}
-
-DomainReliabilityDispatcher::Task::~Task() {}
-
 void DomainReliabilityDispatcher::MakeTaskWaiting(Task* task) {
   DCHECK(task);
   DCHECK(!task->eligible);
   DCHECK(!task->timer->IsRunning());
-  task->timer->Start(
-      FROM_HERE,
-      task->min_delay,
-      base::Bind(
-          &DomainReliabilityDispatcher::MakeTaskEligible,
-          base::Unretained(this),
-          task));
+  task->timer->Start(FROM_HERE,
+                     task->min_delay,
+                     base::Bind(&DomainReliabilityDispatcher::MakeTaskEligible,
+                                base::Unretained(this),
+                                task));
 }
 
 void
@@ -87,13 +99,11 @@ DomainReliabilityDispatcher::MakeTaskEligible(Task* task) {
   DCHECK(!task->eligible);
   task->eligible = true;
   eligible_tasks_.insert(task);
-  task->timer->Start(
-      FROM_HERE,
-      task->max_delay - task->min_delay,
-      base::Bind(
-          &DomainReliabilityDispatcher::RunAndDeleteTask,
-          base::Unretained(this),
-          task));
+  task->timer->Start(FROM_HERE,
+                     task->max_delay - task->min_delay,
+                     base::Bind(&DomainReliabilityDispatcher::RunAndDeleteTask,
+                                base::Unretained(this),
+                                task));
 }
 
 void DomainReliabilityDispatcher::RunAndDeleteTask(Task* task) {
diff --git a/components/domain_reliability/dispatcher.h b/components/domain_reliability/dispatcher.h
index 52309e7..58c2b27 100644
--- a/components/domain_reliability/dispatcher.h
+++ b/components/domain_reliability/dispatcher.h
@@ -8,7 +8,6 @@
 #include <set>
 
 #include "base/callback.h"
-#include "base/memory/scoped_vector.h"
 #include "base/time/time.h"
 #include "components/domain_reliability/domain_reliability_export.h"
 #include "components/domain_reliability/util.h"
@@ -26,33 +25,31 @@ namespace domain_reliability {
 // (See scheduler.h for an explanation of how the intervals are chosen.)
 class DOMAIN_RELIABILITY_EXPORT DomainReliabilityDispatcher {
  public:
-  DomainReliabilityDispatcher(MockableTime* time);
+  explicit DomainReliabilityDispatcher(MockableTime* time);
   ~DomainReliabilityDispatcher();
 
-  void ScheduleTask(
-      const base::Closure& task,
-      base::TimeDelta min_delay,
-      base::TimeDelta max_delay);
+  // Schedules |task| to be executed between |min_delay| and |max_delay| from
+  // now. The task will be run at most |max_delay| from now; once |min_delay|
+  // has passed, any call to |RunEligibleTasks| will run the task earlier than
+  // that.
+  void ScheduleTask(const base::Closure& task,
+                    base::TimeDelta min_delay,
+                    base::TimeDelta max_delay);
 
+  // Runs all tasks whose minimum delay has already passed.
   void RunEligibleTasks();
 
  private:
-  struct Task {
-    Task(const base::Closure& closure_p,
-         scoped_ptr<MockableTime::Timer> timer_p,
-         base::TimeDelta min_delay_p,
-         base::TimeDelta max_delay_p);
-    ~Task();
-
-    base::Closure closure;
-    scoped_ptr<MockableTime::Timer> timer;
-    base::TimeDelta min_delay;
-    base::TimeDelta max_delay;
-    bool eligible;
-  };
+  struct Task;
 
+  // Adds |task| to the set of all tasks, but not the set of eligible tasks.
   void MakeTaskWaiting(Task* task);
+
+  // Adds |task| to the set of eligible tasks, and also the set of all tasks
+  // if not already there.
   void MakeTaskEligible(Task* task);
+
+  // Runs |task|'s callback, removes it from both sets, and deletes it.
   void RunAndDeleteTask(Task* task);
 
   MockableTime* time_;
diff --git a/components/domain_reliability/domain_reliability_export.h b/components/domain_reliability/domain_reliability_export.h
index d0951cc..e37571c 100644
--- a/components/domain_reliability/domain_reliability_export.h
+++ b/components/domain_reliability/domain_reliability_export.h
@@ -12,18 +12,21 @@
 #define DOMAIN_RELIABILITY_EXPORT __declspec(dllexport)
 #else
 #define DOMAIN_RELIABILITY_EXPORT __declspec(dllimport)
-#endif  // defined(DOMAIN_RELIABILITY_IMPLEMENTATION)
+#endif
 
 #else  // defined(WIN32)
+
 #if defined(DOMAIN_RELIABILITY_IMPLEMENTATION)
 #define DOMAIN_RELIABILITY_EXPORT __attribute__((visibility("default")))
 #else
 #define DOMAIN_RELIABILITY_EXPORT
 #endif
-#endif
 
+#endif  // defined(WIN32)
 #else  // defined(COMPONENT_BUILD)
+
 #define DOMAIN_RELIABILITY_EXPORT
+
 #endif
 
 #endif  // COMPONENTS_DOMAIN_RELIABILITY_DOMAIN_RELIABILITY_EXPORT_H_
diff --git a/components/domain_reliability/monitor.cc b/components/domain_reliability/monitor.cc
index 06d5f55..29b276d 100644
--- a/components/domain_reliability/monitor.cc
+++ b/components/domain_reliability/monitor.cc
@@ -54,13 +54,15 @@ class TrivialURLRequestContextGetter : public net::URLRequestContextGetter {
 namespace domain_reliability {
 
 DomainReliabilityMonitor::DomainReliabilityMonitor(
-    net::URLRequestContext* url_request_context)
+    net::URLRequestContext* url_request_context,
+    const std::string& upload_reporter_string)
     : time_(new ActualTime()),
       url_request_context_getter_(scoped_refptr<net::URLRequestContextGetter>(
           new TrivialURLRequestContextGetter(
               url_request_context,
               content::BrowserThread::GetMessageLoopProxyForThread(
                   content::BrowserThread::IO)))),
+      upload_reporter_string_(upload_reporter_string),
       scheduler_params_(
           DomainReliabilityScheduler::Params::GetFromFieldTrialsOrDefaults()),
       dispatcher_(time_.get()),
@@ -71,6 +73,7 @@ DomainReliabilityMonitor::DomainReliabilityMonitor(
 
 DomainReliabilityMonitor::DomainReliabilityMonitor(
     net::URLRequestContext* url_request_context,
+    const std::string& upload_reporter_string,
     scoped_ptr<MockableTime> time)
     : time_(time.Pass()),
       url_request_context_getter_(scoped_refptr<net::URLRequestContextGetter>(
@@ -78,6 +81,7 @@ DomainReliabilityMonitor::DomainReliabilityMonitor(
               url_request_context,
               content::BrowserThread::GetMessageLoopProxyForThread(
                   content::BrowserThread::IO)))),
+      upload_reporter_string_(upload_reporter_string),
       scheduler_params_(
           DomainReliabilityScheduler::Params::GetFromFieldTrialsOrDefaults()),
       dispatcher_(time_.get()),
@@ -88,8 +92,7 @@ DomainReliabilityMonitor::DomainReliabilityMonitor(
 
 DomainReliabilityMonitor::~DomainReliabilityMonitor() {
   DCHECK(OnIOThread());
-  STLDeleteContainerPairSecondPointers(
-      contexts_.begin(), contexts_.end());
+  STLDeleteContainerPairSecondPointers(contexts_.begin(), contexts_.end());
 }
 
 void DomainReliabilityMonitor::AddBakedInConfigs() {
@@ -109,9 +112,8 @@ void DomainReliabilityMonitor::AddBakedInConfigs() {
 
 void DomainReliabilityMonitor::OnBeforeRedirect(net::URLRequest* request) {
   DCHECK(OnIOThread());
-  RequestInfo request_info(*request);
   // Record the redirect itself in addition to the final request.
-  OnRequestLegComplete(request_info);
+  OnRequestLegComplete(RequestInfo(*request));
 }
 
 void DomainReliabilityMonitor::OnCompleted(net::URLRequest* request,
@@ -161,15 +163,16 @@ DomainReliabilityContext* DomainReliabilityMonitor::AddContext(
   DCHECK(config);
   DCHECK(config->IsValid());
 
-  // Grab domain before we config.Pass().
+  // Grab a copy of the domain before transferring ownership of |config|.
   std::string domain = config->domain;
 
-  DomainReliabilityContext* context = new DomainReliabilityContext(
-      time_.get(),
-      scheduler_params_,
-      &dispatcher_,
-      uploader_.get(),
-      config.Pass());
+  DomainReliabilityContext* context =
+      new DomainReliabilityContext(time_.get(),
+                                   scheduler_params_,
+                                   upload_reporter_string_,
+                                   &dispatcher_,
+                                   uploader_.get(),
+                                   config.Pass());
 
   std::pair<ContextMap::iterator, bool> map_it =
       contexts_.insert(make_pair(domain, context));
@@ -201,7 +204,7 @@ void DomainReliabilityMonitor::OnRequestLegComplete(
   DomainReliabilityContext* context = it->second;
 
   std::string beacon_status;
-  bool got_status = DomainReliabilityUtil::GetBeaconStatus(
+  bool got_status = GetDomainReliabilityBeaconStatus(
       request.status.error(),
       request.response_code,
       &beacon_status);
@@ -215,7 +218,7 @@ void DomainReliabilityMonitor::OnRequestLegComplete(
   beacon.http_response_code = request.response_code;
   beacon.start_time = request.load_timing_info.request_start;
   beacon.elapsed = time_->NowTicks() - beacon.start_time;
-  context->AddBeacon(beacon, request.url);
+  context->OnBeacon(request.url, beacon);
 }
 
 }  // namespace domain_reliability
diff --git a/components/domain_reliability/monitor.h b/components/domain_reliability/monitor.h
index 003a42c..e001a4b 100644
--- a/components/domain_reliability/monitor.h
+++ b/components/domain_reliability/monitor.h
@@ -29,26 +29,29 @@ class URLRequestContextGetter;
 
 namespace domain_reliability {
 
-// The top-level per-profile object that measures requests and hands off the
-// measurements to the proper |DomainReliabilityContext|.  Referenced by the
-// |ChromeNetworkDelegate|, which calls the On* methods.
+// The top-level object that measures requests and hands off the measurements
+// to the proper |DomainReliabilityContext|. Lives on the I/O thread, so the
+// constructor accepts a URLRequestContext directly instead of a
+// URLRequestContextGetter.
 class DOMAIN_RELIABILITY_EXPORT DomainReliabilityMonitor {
  public:
-  // NB: We don't take a URLRequestContextGetter because we already live on the
-  // I/O thread.
-  explicit DomainReliabilityMonitor(
-      net::URLRequestContext* url_request_context);
-  DomainReliabilityMonitor(
-      net::URLRequestContext* url_request_context,
-      scoped_ptr<MockableTime> time);
+  DomainReliabilityMonitor(net::URLRequestContext* url_request_context,
+                           const std::string& upload_reporter_string);
+  DomainReliabilityMonitor(net::URLRequestContext* url_request_context,
+                           const std::string& upload_reporter_string,
+                           scoped_ptr<MockableTime> time);
   ~DomainReliabilityMonitor();
 
-  // Adds the "baked-in" configuration(s) for Google sites.
+  // Populates the monitor with contexts that were configured at compile time.
   void AddBakedInConfigs();
 
-  // Should be called from the profile's NetworkDelegate on the corresponding
-  // events:
+  // Should be called when |request| is about to follow a redirect. Will
+  // examine and possibly log the redirect request.
   void OnBeforeRedirect(net::URLRequest* request);
+
+  // Should be called when |request| is complete. Will examine and possibly
+  // log the (final) request. (|started| should be true if the request was
+  // actually started before it was terminated.)
   void OnCompleted(net::URLRequest* request, bool started);
 
   DomainReliabilityContext* AddContextForTesting(
@@ -86,6 +89,7 @@ class DOMAIN_RELIABILITY_EXPORT DomainReliabilityMonitor {
 
   scoped_ptr<MockableTime> time_;
   scoped_refptr<net::URLRequestContextGetter> url_request_context_getter_;
+  const std::string upload_reporter_string_;
   DomainReliabilityScheduler::Params scheduler_params_;
   DomainReliabilityDispatcher dispatcher_;
   scoped_ptr<DomainReliabilityUploader> uploader_;
diff --git a/components/domain_reliability/monitor_unittest.cc b/components/domain_reliability/monitor_unittest.cc
index b04755f..1366f8d 100644
--- a/components/domain_reliability/monitor_unittest.cc
+++ b/components/domain_reliability/monitor_unittest.cc
@@ -36,6 +36,7 @@ class DomainReliabilityMonitorTest : public testing::Test {
             base::MessageLoopProxy::current())),
         time_(new MockTime()),
         monitor_(url_request_context_getter_->GetURLRequestContext(),
+                 "test-reporter",
                  scoped_ptr<MockableTime>(time_)),
         context_(monitor_.AddContextForTesting(CreateConfig())) {}
 
@@ -86,9 +87,9 @@ class DomainReliabilityMonitorTest : public testing::Test {
     return request;
   }
 
-  bool CheckNoBeacons(int index) {
+  bool CheckNoBeacons(size_t index) {
     BeaconVector beacons;
-    int successful, failed;
+    unsigned successful, failed;
     context_->GetQueuedDataForTesting(index, &beacons, &successful, &failed);
     return beacons.empty() && successful == 0 && failed == 0;
   }
@@ -156,6 +157,7 @@ TEST_F(DomainReliabilityMonitorTest, AddBakedInConfigs) {
   // source tree.
   monitor_.AddBakedInConfigs();
 
+  // Count the number of baked-in configs.
   size_t num_baked_in_configs = 0;
   for (const char* const* p = kBakedInJsonConfigs; *p; ++p)
     ++num_baked_in_configs;
diff --git a/components/domain_reliability/scheduler.cc b/components/domain_reliability/scheduler.cc
index 3321d71..b114d4b 100644
--- a/components/domain_reliability/scheduler.cc
+++ b/components/domain_reliability/scheduler.cc
@@ -13,27 +13,27 @@
 
 namespace {
 
-const int kInvalidCollectorIndex = -1;
+const unsigned kInvalidCollectorIndex = -1;
 
-const int kDefaultMinimumUploadDelaySec = 60;
-const int kDefaultMaximumUploadDelaySec = 300;
-const int kDefaultUploadRetryIntervalSec = 60;
+const unsigned kDefaultMinimumUploadDelaySec = 60;
+const unsigned kDefaultMaximumUploadDelaySec = 300;
+const unsigned kDefaultUploadRetryIntervalSec = 60;
 
 const char* kMinimumUploadDelayFieldTrialName = "DomRel-MinimumUploadDelay";
 const char* kMaximumUploadDelayFieldTrialName = "DomRel-MaximumUploadDelay";
 const char* kUploadRetryIntervalFieldTrialName = "DomRel-UploadRetryInterval";
 
-int GetIntegerFieldTrialValueOrDefault(
-    std::string field_trial_name,
-    int default_value) {
+unsigned GetUnsignedFieldTrialValueOrDefault(std::string field_trial_name,
+                                             unsigned default_value) {
   if (!base::FieldTrialList::TrialExists(field_trial_name))
     return default_value;
 
   std::string group_name = base::FieldTrialList::FindFullName(field_trial_name);
-  int value;
-  if (!base::StringToInt(group_name, &value)) {
-    LOG(ERROR) << "Expected integer for field trial " << field_trial_name
-               << " group name, but got \"" << group_name << "\".";
+  unsigned value;
+  if (!base::StringToUint(group_name, &value)) {
+    LOG(ERROR) << "Expected unsigned integer for field trial "
+               << field_trial_name << " group name, but got \"" << group_name
+               << "\".";
     return default_value;
   }
 
@@ -49,15 +49,15 @@ DomainReliabilityScheduler::Params
 DomainReliabilityScheduler::Params::GetFromFieldTrialsOrDefaults() {
   DomainReliabilityScheduler::Params params;
 
-  params.minimum_upload_delay = base::TimeDelta::FromSeconds(
-      GetIntegerFieldTrialValueOrDefault(kMinimumUploadDelayFieldTrialName,
-                                         kDefaultMinimumUploadDelaySec));
-  params.maximum_upload_delay = base::TimeDelta::FromSeconds(
-      GetIntegerFieldTrialValueOrDefault(kMaximumUploadDelayFieldTrialName,
-                                         kDefaultMaximumUploadDelaySec));
-  params.upload_retry_interval = base::TimeDelta::FromSeconds(
-      GetIntegerFieldTrialValueOrDefault(kUploadRetryIntervalFieldTrialName,
-                                         kDefaultUploadRetryIntervalSec));
+  params.minimum_upload_delay =
+      base::TimeDelta::FromSeconds(GetUnsignedFieldTrialValueOrDefault(
+          kMinimumUploadDelayFieldTrialName, kDefaultMinimumUploadDelaySec));
+  params.maximum_upload_delay =
+      base::TimeDelta::FromSeconds(GetUnsignedFieldTrialValueOrDefault(
+          kMaximumUploadDelayFieldTrialName, kDefaultMaximumUploadDelaySec));
+  params.upload_retry_interval =
+      base::TimeDelta::FromSeconds(GetUnsignedFieldTrialValueOrDefault(
+          kUploadRetryIntervalFieldTrialName, kDefaultUploadRetryIntervalSec));
 
   return params;
 }
@@ -83,12 +83,10 @@ void DomainReliabilityScheduler::OnBeaconAdded() {
   if (!upload_pending_)
     first_beacon_time_ = time_->NowTicks();
   upload_pending_ = true;
-  VLOG(2) << "OnBeaconAdded";
   MaybeScheduleUpload();
 }
 
-void DomainReliabilityScheduler::OnUploadStart(int* collector_index_out) {
-  DCHECK(collector_index_out);
+size_t DomainReliabilityScheduler::OnUploadStart() {
   DCHECK(upload_scheduled_);
   DCHECK_EQ(kInvalidCollectorIndex, collector_index_);
   upload_scheduled_ = false;
@@ -99,9 +97,9 @@ void DomainReliabilityScheduler::OnUploadStart(int* collector_index_out) {
   GetNextUploadTimeAndCollector(now, &min_upload_time, &collector_index_);
   DCHECK(min_upload_time <= now);
 
-  *collector_index_out = collector_index_;
-
   VLOG(1) << "Starting upload to collector " << collector_index_ << ".";
+
+  return collector_index_;
 }
 
 void DomainReliabilityScheduler::OnUploadComplete(bool success) {
@@ -153,7 +151,7 @@ void DomainReliabilityScheduler::MaybeScheduleUpload() {
   DCHECK(min_by_deadline <= max_by_deadline);
 
   base::TimeTicks min_by_backoff;
-  int collector_index;
+  size_t collector_index;
   GetNextUploadTimeAndCollector(now, &min_by_backoff, &collector_index);
 
   base::TimeDelta min_delay = std::max(min_by_deadline, min_by_backoff) - now;
@@ -166,19 +164,20 @@ void DomainReliabilityScheduler::MaybeScheduleUpload() {
 }
 
 // TODO(ttuttle): Add min and max interval to config, use that instead.
+
 // TODO(ttuttle): Cap min and max intervals received from config.
 
 void DomainReliabilityScheduler::GetNextUploadTimeAndCollector(
     base::TimeTicks now,
     base::TimeTicks* upload_time_out,
-    int* collector_index_out) {
+    size_t* collector_index_out) {
   DCHECK(upload_time_out);
   DCHECK(collector_index_out);
 
   base::TimeTicks min_time;
-  int min_index = kInvalidCollectorIndex;
+  size_t min_index = kInvalidCollectorIndex;
 
-  for (unsigned i = 0; i < collectors_.size(); ++i) {
+  for (size_t i = 0; i < collectors_.size(); ++i) {
     CollectorState* collector = &collectors_[i];
     // If a collector is usable, use the first one in the list.
     if (collector->failures == 0 || collector->next_upload <= now) {
@@ -199,11 +198,14 @@ void DomainReliabilityScheduler::GetNextUploadTimeAndCollector(
 }
 
 base::TimeDelta DomainReliabilityScheduler::GetUploadRetryInterval(
-    int failures) {
+    unsigned failures) {
   if (failures == 0)
     return base::TimeDelta::FromSeconds(0);
   else {
-    return params_.upload_retry_interval * (1 << std::min(failures - 1, 5));
+    // Don't back off more than 64x the original delay.
+    if (failures > 7)
+      failures = 7;
+    return params_.upload_retry_interval * (1 << (failures - 1));
   }
 }
 
diff --git a/components/domain_reliability/scheduler.h b/components/domain_reliability/scheduler.h
index ee924d7..c62fe58 100644
--- a/components/domain_reliability/scheduler.h
+++ b/components/domain_reliability/scheduler.h
@@ -49,16 +49,20 @@ class DOMAIN_RELIABILITY_EXPORT DomainReliabilityScheduler {
                              const ScheduleUploadCallback& callback);
   ~DomainReliabilityScheduler();
 
-  // Called when a beacon is added to the context. May call the provided
-  // ScheduleUploadCallback (if all previous beacons have been uploaded).
+  // If there is no upload pending, schedules an upload based on the provided
+  // parameters (some time between the minimum and maximum delay from now).
+  // May call the ScheduleUploadCallback.
   void OnBeaconAdded();
 
-  // Called when an upload is about to start.
-  void OnUploadStart(int* collector_index_out);
+  // Returns which collector to use for an upload that is about to start. Must
+  // be called exactly once during or after the ScheduleUploadCallback but
+  // before OnUploadComplete is called. (Also records the upload start time for
+  // future retries, if the upload ends up failing.)
+  size_t OnUploadStart();
 
-  // Called when an upload has finished. May call the provided
-  // ScheduleUploadCallback (if the upload failed, or if beacons arrived
-  // during the upload).
+  // Updates the scheduler state based on the result of an upload. Must be
+  // called exactly once after |OnUploadStart|. |success| should be true if the
+  // upload was successful, and false otherwise.
   void OnUploadComplete(bool success);
 
  private:
@@ -67,7 +71,7 @@ class DOMAIN_RELIABILITY_EXPORT DomainReliabilityScheduler {
 
     // The number of consecutive failures to upload to this collector, or 0 if
     // the most recent upload succeeded.
-    int failures;
+    unsigned failures;
     base::TimeTicks next_upload;
   };
 
@@ -75,8 +79,9 @@ class DOMAIN_RELIABILITY_EXPORT DomainReliabilityScheduler {
 
   void GetNextUploadTimeAndCollector(base::TimeTicks now,
                                      base::TimeTicks* upload_time_out,
-                                     int* collector_index_out);
-  base::TimeDelta GetUploadRetryInterval(int failures);
+                                     size_t* collector_index_out);
+
+  base::TimeDelta GetUploadRetryInterval(unsigned failures);
 
   MockableTime* time_;
   std::vector<CollectorState> collectors_;
@@ -98,7 +103,7 @@ class DOMAIN_RELIABILITY_EXPORT DomainReliabilityScheduler {
 
   // Index of the collector selected for the next upload.  (Set in
   // |OnUploadStart| and cleared in |OnUploadComplete|.)
-  int collector_index_;
+  size_t collector_index_;
 
   // Time of the first beacon that was not included in the last successful
   // upload.
diff --git a/components/domain_reliability/scheduler_unittest.cc b/components/domain_reliability/scheduler_unittest.cc
index 1d4ef7b..9eb96a3 100644
--- a/components/domain_reliability/scheduler_unittest.cc
+++ b/components/domain_reliability/scheduler_unittest.cc
@@ -6,6 +6,7 @@
 
 #include "base/bind.h"
 #include "base/time/time.h"
+#include "components/domain_reliability/config.h"
 #include "components/domain_reliability/test_util.h"
 #include "components/domain_reliability/util.h"
 #include "testing/gtest/include/gtest/gtest.h"
@@ -17,11 +18,10 @@ namespace domain_reliability {
 
 class DomainReliabilitySchedulerTest : public testing::Test {
  public:
-  DomainReliabilitySchedulerTest() :
-      num_collectors_(0),
-      params_(CreateDefaultParams()),
-      callback_called_(false) {
-  }
+  DomainReliabilitySchedulerTest()
+      : num_collectors_(0),
+        params_(CreateDefaultParams()),
+        callback_called_(false) {}
 
   void CreateScheduler(int num_collectors) {
     DCHECK_LT(0, num_collectors);
@@ -80,13 +80,11 @@ class DomainReliabilitySchedulerTest : public testing::Test {
     }
   }
 
-  ::testing::AssertionResult CheckStartingUpload(int expected_collector) {
+  ::testing::AssertionResult CheckStartingUpload(size_t expected_collector) {
     DCHECK(scheduler_);
-    DCHECK_LE(0, expected_collector);
     DCHECK_GT(num_collectors_, expected_collector);
 
-    int collector;
-    scheduler_->OnUploadStart(&collector);
+    size_t collector = scheduler_->OnUploadStart();
     if (collector == expected_collector)
       return ::testing::AssertionSuccess();
 
@@ -108,7 +106,7 @@ class DomainReliabilitySchedulerTest : public testing::Test {
   }
 
   MockTime time_;
-  int num_collectors_;
+  size_t num_collectors_;
   DomainReliabilityScheduler::Params params_;
   scoped_ptr<DomainReliabilityScheduler> scheduler_;
 
diff --git a/components/domain_reliability/test_util.cc b/components/domain_reliability/test_util.cc
index 389ab56..9bc5a7b 100644
--- a/components/domain_reliability/test_util.cc
+++ b/components/domain_reliability/test_util.cc
@@ -21,6 +21,7 @@ class MockTimer : public MockableTime::Timer {
         weak_factory_(this) {
     DCHECK(time);
   }
+
   virtual ~MockTimer() {}
 
   // MockableTime::Timer implementation:
diff --git a/components/domain_reliability/test_util.h b/components/domain_reliability/test_util.h
index b1dfeef..4c968ed 100644
--- a/components/domain_reliability/test_util.h
+++ b/components/domain_reliability/test_util.h
@@ -43,6 +43,7 @@ class MockUploader : public DomainReliabilityUploader {
       UploadRequestCallback;
 
   MockUploader(const UploadRequestCallback& callback);
+
   virtual ~MockUploader();
 
   // DomainReliabilityUploader implementation:
@@ -57,6 +58,7 @@ class MockUploader : public DomainReliabilityUploader {
 class MockTime : public MockableTime {
  public:
   MockTime();
+
   // N.B.: Tasks (and therefore Timers) scheduled to run in the future will
   // never be run if MockTime is destroyed before the mock time is advanced
   // to their scheduled time.
diff --git a/components/domain_reliability/uploader.cc b/components/domain_reliability/uploader.cc
index 8293bb7..7a67a77 100644
--- a/components/domain_reliability/uploader.cc
+++ b/components/domain_reliability/uploader.cc
@@ -40,8 +40,8 @@ const void* UploadUserData::kUserDataKey =
 class DomainReliabilityUploaderImpl
     : public DomainReliabilityUploader, net::URLFetcherDelegate {
  public:
-  DomainReliabilityUploaderImpl(
-      scoped_refptr<net::URLRequestContextGetter> url_request_context_getter)
+  DomainReliabilityUploaderImpl(const scoped_refptr<
+      net::URLRequestContextGetter>& url_request_context_getter)
       : url_request_context_getter_(url_request_context_getter) {}
 
   virtual ~DomainReliabilityUploaderImpl() {
@@ -104,12 +104,12 @@ class DomainReliabilityUploaderImpl
 }  // namespace
 
 DomainReliabilityUploader::DomainReliabilityUploader() {}
-
 DomainReliabilityUploader::~DomainReliabilityUploader() {}
 
 // static
 scoped_ptr<DomainReliabilityUploader> DomainReliabilityUploader::Create(
-    scoped_refptr<net::URLRequestContextGetter> url_request_context_getter) {
+    const scoped_refptr<net::URLRequestContextGetter>&
+        url_request_context_getter) {
   return scoped_ptr<DomainReliabilityUploader>(
       new DomainReliabilityUploaderImpl(url_request_context_getter));
 }
diff --git a/components/domain_reliability/uploader.h b/components/domain_reliability/uploader.h
index ede3d2c..a9c5211 100644
--- a/components/domain_reliability/uploader.h
+++ b/components/domain_reliability/uploader.h
@@ -20,16 +20,23 @@ class URLRequestContextGetter;
 
 namespace domain_reliability {
 
+// Uploads Domain Reliability reports to collectors.
 class DOMAIN_RELIABILITY_EXPORT DomainReliabilityUploader {
  public:
   typedef base::Callback<void(bool success)> UploadCallback;
 
   DomainReliabilityUploader();
+
   virtual ~DomainReliabilityUploader();
 
-  static scoped_ptr<DomainReliabilityUploader> Create(
-      scoped_refptr<net::URLRequestContextGetter> url_request_context_getter);
+  // Creates an uploader that uses the given |url_request_context_getter| to
+  // get a URLRequestContext to use for uploads. (See test_util.h for a mock
+  // version.)
+  static scoped_ptr<DomainReliabilityUploader> Create(const scoped_refptr<
+      net::URLRequestContextGetter>& url_request_context_getter);
 
+  // Uploads |report_json| to |upload_url| and calls |callback| when the upload
+  // has either completed or failed.
   virtual void UploadReport(const std::string& report_json,
                             const GURL& upload_url,
                             const UploadCallback& callback) = 0;
diff --git a/components/domain_reliability/uploader_unittest.cc b/components/domain_reliability/uploader_unittest.cc
index e9614a4..e7734e0 100644
--- a/components/domain_reliability/uploader_unittest.cc
+++ b/components/domain_reliability/uploader_unittest.cc
@@ -29,13 +29,13 @@ class DomainReliabilityUploaderTest : public testing::Test {
         uploader_(DomainReliabilityUploader::Create(
             url_request_context_getter_)) {}
 
-  DomainReliabilityUploader::UploadCallback MakeUploadCallback(int index) {
+  DomainReliabilityUploader::UploadCallback MakeUploadCallback(size_t index) {
     return base::Bind(&DomainReliabilityUploaderTest::OnUploadComplete,
                       base::Unretained(this),
                       index);
   }
 
-  void OnUploadComplete(int index, bool success) {
+  void OnUploadComplete(size_t index, bool success) {
     EXPECT_FALSE(upload_complete_[index]);
     upload_complete_[index] = true;
     upload_successful_[index] = success;
@@ -46,8 +46,10 @@ class DomainReliabilityUploaderTest : public testing::Test {
   scoped_refptr<net::TestURLRequestContextGetter> url_request_context_getter_;
   scoped_ptr<DomainReliabilityUploader> uploader_;
 
-  std::map<int, bool> upload_complete_;
-  std::map<int, bool> upload_successful_;
+  // Whether the upload callback was called for a particular collector index.
+  std::map<size_t, bool> upload_complete_;
+  // Whether the upload to a particular collector was successful.
+  std::map<size_t, bool> upload_successful_;
 };
 
 TEST_F(DomainReliabilityUploaderTest, Create) {
diff --git a/components/domain_reliability/util.cc b/components/domain_reliability/util.cc
index d4632bf..25d5c72 100644
--- a/components/domain_reliability/util.cc
+++ b/components/domain_reliability/util.cc
@@ -20,6 +20,7 @@ class ActualTimer : public MockableTime::Timer {
  public:
   // Initialize base timer with retain_user_info and is_repeating false.
   ActualTimer() : base_timer_(false, false) {}
+
   virtual ~ActualTimer() {}
 
   // MockableTime::Timer implementation:
@@ -77,7 +78,7 @@ const struct NetErrorMapping {
 }  // namespace
 
 // static
-bool DomainReliabilityUtil::GetBeaconStatus(
+bool GetDomainReliabilityBeaconStatus(
     int net_error,
     int http_response_code,
     std::string* beacon_status_out) {
@@ -87,15 +88,16 @@ bool DomainReliabilityUtil::GetBeaconStatus(
     else
       *beacon_status_out = "ok";
     return true;
-  } else {
-    for (size_t i = 0; i < arraysize(net_error_map); i++) {
-      if (net_error_map[i].net_error == net_error) {
-        *beacon_status_out = net_error_map[i].beacon_status;
-        return true;
-      }
+  }
+
+  // TODO(ttuttle): Consider sorting and using binary search?
+  for (size_t i = 0; i < arraysize(net_error_map); i++) {
+    if (net_error_map[i].net_error == net_error) {
+      *beacon_status_out = net_error_map[i].beacon_status;
+      return true;
     }
-    return false;
   }
+  return false;
 }
 
 MockableTime::Timer::~Timer() {}
diff --git a/components/domain_reliability/util.h b/components/domain_reliability/util.h
index 85c68aa..9f49618 100644
--- a/components/domain_reliability/util.h
+++ b/components/domain_reliability/util.h
@@ -15,18 +15,20 @@
 
 namespace domain_reliability {
 
-class DOMAIN_RELIABILITY_EXPORT DomainReliabilityUtil {
- public:
-  // Attempts to convert a net error and an HTTP response code into the status
-  // string that should be recorded in a beacon. Returns true if it could.
-  static bool GetBeaconStatus(
-      int net_error,
-      int http_response_code,
-      std::string* beacon_status_out);
-};
+// Attempts to convert a net error and an HTTP response code into the status
+// string that should be recorded in a beacon. Returns true if it could.
+//
+// N.B.: This functions as the whitelist of "safe" errors to report; network-
+//       local errors are purposefully not converted to avoid revealing
+//       information about the local network to the remote server.
+bool GetDomainReliabilityBeaconStatus(
+    int net_error,
+    int http_response_code,
+    std::string* beacon_status_out);
 
 // Mockable wrapper around TimeTicks::Now and Timer. Mock version is in
 // test_util.h.
+// TODO(ttuttle): Rename to Time{Provider,Source,?}.
 class DOMAIN_RELIABILITY_EXPORT MockableTime {
  public:
   // Mockable wrapper around (a subset of) base::Timer.
@@ -65,6 +67,7 @@ class DOMAIN_RELIABILITY_EXPORT MockableTime {
 class DOMAIN_RELIABILITY_EXPORT ActualTime : public MockableTime {
  public:
   ActualTime();
+
   virtual ~ActualTime();
 
   // MockableTime implementation:
diff --git a/components/domain_reliability/util_unittest.cc b/components/domain_reliability/util_unittest.cc
index 2a16a83..a082fd2 100644
--- a/components/domain_reliability/util_unittest.cc
+++ b/components/domain_reliability/util_unittest.cc
@@ -18,7 +18,7 @@ class DomainReliabilityMockTimeTest : public testing::Test {
   MockTime time_;
 };
 
-TEST_F(DomainReliabilityMockTimeTest, Null) {
+TEST_F(DomainReliabilityMockTimeTest, Create) {
 }
 
 TEST_F(DomainReliabilityMockTimeTest, NowAndAdvance) {
-- 
cgit v1.1