Fix memory leak and crash when parsing baked-in configs in domain reliability monitor.

Invalid baked-in configs would be leaked (in FromJSON) and then crash
due to a null deref (in AddContext).

This change also makes two minor improvements:
- pass scoped_refptrs by const ref
- use a StringPiece rather than a std::string when parsing bake-in configs

BUG=460483
R=ttuttle@chromium.org

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

Cr-Commit-Position: refs/heads/master@{#317375}

components/domain_reliability/config.cc

@@ -94,13 +94,12 @@ scoped_ptr<const DomainReliabilityConfig> DomainReliabilityConfig::FromJSON(
     const base::StringPiece& json) {
   scoped_ptr<base::Value> value(base::JSONReader::Read(json));
   base::JSONValueConverter<DomainReliabilityConfig> converter;
-  DomainReliabilityConfig* config = new DomainReliabilityConfig();
+  scoped_ptr<DomainReliabilityConfig> config(new DomainReliabilityConfig());
 
   // 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>();
+  if (value && converter.Convert(*value, config.get()) && config->IsValid())
+    return config.Pass();
+  return scoped_ptr<const DomainReliabilityConfig>();
 }
 
 bool DomainReliabilityConfig::IsValid() const {

components/domain_reliability/monitor.cc

@@ -21,8 +21,8 @@ namespace domain_reliability {
 
 DomainReliabilityMonitor::DomainReliabilityMonitor(
     const std::string& upload_reporter_string,
-    scoped_refptr<base::SingleThreadTaskRunner> pref_thread,
-    scoped_refptr<base::SingleThreadTaskRunner> network_thread)
+    const scoped_refptr<base::SingleThreadTaskRunner>& pref_thread,
+    const scoped_refptr<base::SingleThreadTaskRunner>& network_thread)
     : time_(new ActualTime()),
       upload_reporter_string_(upload_reporter_string),
       scheduler_params_(
@@ -39,8 +39,8 @@ DomainReliabilityMonitor::DomainReliabilityMonitor(
 
 DomainReliabilityMonitor::DomainReliabilityMonitor(
     const std::string& upload_reporter_string,
-    scoped_refptr<base::SingleThreadTaskRunner> pref_thread,
-    scoped_refptr<base::SingleThreadTaskRunner> network_thread,
+    const scoped_refptr<base::SingleThreadTaskRunner>& pref_thread,
+    const scoped_refptr<base::SingleThreadTaskRunner>& network_thread,
     scoped_ptr<MockableTime> time)
     : time_(time.Pass()),
       upload_reporter_string_(upload_reporter_string),
@@ -90,7 +90,8 @@ void DomainReliabilityMonitor::InitURLRequestContext(
 }
 
 void DomainReliabilityMonitor::InitURLRequestContext(
-    scoped_refptr<net::URLRequestContextGetter> url_request_context_getter) {
+    const scoped_refptr<net::URLRequestContextGetter>&
+        url_request_context_getter) {
   DCHECK(OnNetworkThread());
   DCHECK(moved_to_network_thread_);
 
@@ -114,10 +115,12 @@ void DomainReliabilityMonitor::AddBakedInConfigs() {
 
   base::Time now = base::Time::Now();
   for (size_t i = 0; kBakedInJsonConfigs[i]; ++i) {
-    std::string json(kBakedInJsonConfigs[i]);
+    base::StringPiece json(kBakedInJsonConfigs[i]);
     scoped_ptr<const DomainReliabilityConfig> config =
         DomainReliabilityConfig::FromJSON(json);
-    if (config && config->IsExpired(now)) {
+    if (!config) {
+      continue;
+    } else if (config->IsExpired(now)) {
       LOG(WARNING) << "Baked-in Domain Reliability config for "
                    << config->domain << " is expired.";
       continue;

components/domain_reliability/monitor.h

@@ -49,14 +49,14 @@ class DOMAIN_RELIABILITY_EXPORT DomainReliabilityMonitor
   // on which requests will actually be monitored and reported.
   DomainReliabilityMonitor(
       const std::string& upload_reporter_string,
-      scoped_refptr<base::SingleThreadTaskRunner> pref_thread,
-      scoped_refptr<base::SingleThreadTaskRunner> network_thread);
+      const scoped_refptr<base::SingleThreadTaskRunner>& pref_thread,
+      const scoped_refptr<base::SingleThreadTaskRunner>& network_thread);
 
   // Same, but specifies a mock interface for time functions for testing.
   DomainReliabilityMonitor(
       const std::string& upload_reporter_string,
-      scoped_refptr<base::SingleThreadTaskRunner> pref_thread,
-      scoped_refptr<base::SingleThreadTaskRunner> network_thread,
+      const scoped_refptr<base::SingleThreadTaskRunner>& pref_thread,
+      const scoped_refptr<base::SingleThreadTaskRunner>& network_thread,
       scoped_ptr<MockableTime> time);
 
   // Must be called from the pref thread if |MoveToNetworkThread| was not
@@ -78,7 +78,8 @@ class DOMAIN_RELIABILITY_EXPORT DomainReliabilityMonitor
 
   // Same, but for unittests where the Getter is readily available.
   void InitURLRequestContext(
-      scoped_refptr<net::URLRequestContextGetter> url_request_context_getter);
+      const scoped_refptr<net::URLRequestContextGetter>&
+          url_request_context_getter);
 
   // Populates the monitor with contexts that were configured at compile time.
   void AddBakedInConfigs();