Domain Reliability: Simplify configs and reports

components/domain_reliability/monitor.cc

 // Copyright 2014 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "components/domain_reliability/monitor.h"
 
 #include "base/command_line.h"
 #include "base/logging.h"
 #include "base/profiler/scoped_tracker.h"
 #include "base/single_thread_task_runner.h"
 #include "base/task_runner.h"
 #include "components/domain_reliability/baked_in_configs.h"
+#include "components/domain_reliability/google_configs.h"
+#include "net/base/ip_endpoint.h"
 #include "net/base/load_flags.h"
 #include "net/base/net_errors.h"
 #include "net/base/net_util.h"
 #include "net/http/http_response_headers.h"
 #include "net/url_request/url_request.h"
 #include "net/url_request/url_request_context.h"
 #include "net/url_request/url_request_context_getter.h"
 
 namespace domain_reliability {
 
@@ skipping 12 matching lines @@
     default:
       NOTREACHED();
       return net::ERR_UNEXPECTED;
   }
 }
 
 // Updates the status, chrome_error, and server_ip fields of |beacon| from
 // the endpoint and result of |attempt|. If there is no matching status for
 // the result, returns false (which means the attempt should not result in a
 // beacon being reported).
-bool UpdateBeaconFromAttempt(DomainReliabilityBeacon* beacon,
-                             const net::ConnectionAttempt& attempt) {
+scoped_ptr<DomainReliabilityBeacon> CreateBeaconFromAttempt(
+    const DomainReliabilityBeacon& beacon_template,
+    const net::ConnectionAttempt& attempt) {
+  std::string status;
   if (!GetDomainReliabilityBeaconStatus(
-          attempt.result, beacon->http_response_code, &beacon->status)) {
-    return false;
+          attempt.result, beacon_template.http_response_code, &status)) {
+    return scoped_ptr<DomainReliabilityBeacon>();
   }
+
+  scoped_ptr<DomainReliabilityBeacon> beacon(
+      new DomainReliabilityBeacon(beacon_template));
+  beacon->status = status;
   beacon->chrome_error = attempt.result;
   if (!attempt.endpoint.address().empty())
     beacon->server_ip = attempt.endpoint.ToString();
   else
     beacon->server_ip = "";
-  return true;
+  return beacon.Pass();
 }
 
 }  // namespace
 
 DomainReliabilityMonitor::DomainReliabilityMonitor(
     const std::string& upload_reporter_string,
     const scoped_refptr<base::SingleThreadTaskRunner>& pref_thread,
     const scoped_refptr<base::SingleThreadTaskRunner>& network_thread)
     : time_(new ActualTime()),
       upload_reporter_string_(upload_reporter_string),
@@ skipping 78 matching lines @@
       FROM_HERE_WITH_EXPLICIT_FUNCTION(
           "436671 DomainReliabilityMonitor::AddBakedInConfigs"));
 
   DCHECK(OnNetworkThread());
   DCHECK(moved_to_network_thread_);
 
   for (size_t i = 0; kBakedInJsonConfigs[i]; ++i) {
     base::StringPiece json(kBakedInJsonConfigs[i]);
     scoped_ptr<const DomainReliabilityConfig> config =
         DomainReliabilityConfig::FromJSON(json);
-    if (!config)
+    if (!config) {
+      LOG(WARNING) << "Baked-in Domain Reliability config failed to parse: "
+                   << json;
       continue;
+    }
     context_manager_.AddContextForConfig(config.Pass());
   }
+
+  GoogleConfigs google_configs = GoogleConfigs::GetAllConfigs();
+  while (google_configs.HasMoreConfigs())
+    context_manager_.AddContextForConfig(google_configs.GetNextConfig());

[davidben, 2015/10/20 23:35:54]

Since you're calling AddContextForConfig which still holds onto the config at
the end of the day. Any reason you're doing this as an iterator and not just
having it fill out a std::vector for you to iterate over? That seems simpler.

[Deprecated (see juliatuttle), 2015/10/26 19:08:41]

Yeah, it is.

What's the best way to convey a vector of scoped pointers? vector can't take a
scoped_ptr because there's no copy, and ScopedVector is scoped at the vector
level, not the element level (and doesn't have, like, "release this element as a
scoped_ptr", even though it has "insert this scoped_ptr as an element").

 }
 
 void DomainReliabilityMonitor::SetDiscardUploads(bool discard_uploads) {
   DCHECK(OnNetworkThread());
   DCHECK(moved_to_network_thread_);
   DCHECK(uploader_);
 
   uploader_->set_discard_uploads(discard_uploads);
   discard_uploads_set_ = true;
 }
@@ skipping 122 matching lines @@
 
   int response_code;
   if (request.response_info.headers.get())
     response_code = request.response_info.headers->response_code();
   else
     response_code = -1;
 
   net::ConnectionAttempt url_request_attempt(
       request.remote_endpoint, URLRequestStatusToNetError(request.status));
 
-  DomainReliabilityBeacon beacon;
-  beacon.protocol = GetDomainReliabilityProtocol(
-      request.response_info.connection_info,
-      request.response_info.ssl_info.is_valid());
-  beacon.http_response_code = response_code;
-  beacon.start_time = request.load_timing_info.request_start;
-  beacon.elapsed = time_->NowTicks() - beacon.start_time;
-  beacon.was_proxied = request.response_info.was_fetched_via_proxy;
-  beacon.domain = request.url.host();
+  DomainReliabilityBeacon beacon_template;
+  beacon_template.protocol =
+      GetDomainReliabilityProtocol(request.response_info.connection_info,
+                                   request.response_info.ssl_info.is_valid());
+  beacon_template.http_response_code = response_code;
+  beacon_template.start_time = request.load_timing_info.request_start;
+  beacon_template.elapsed = time_->NowTicks() - beacon_template.start_time;
+  beacon_template.was_proxied = request.response_info.was_fetched_via_proxy;
+  beacon_template.url = request.url;
 
   // This is not foolproof -- it's possible that we'll see the same error twice
   // (e.g. an SSL error during connection on one attempt, and then an error
   // that maps to the same code during a read).
   // TODO(ttuttle): Find a way for this code to reliably tell whether we
   // eventually established a connection or not.
   bool url_request_attempt_is_duplicate = false;
   for (const auto& attempt : request.connection_attempts) {
     if (attempt.result == url_request_attempt.result)
       url_request_attempt_is_duplicate = true;
-    if (!UpdateBeaconFromAttempt(&beacon, attempt))
-      continue;
-    context_manager_.RouteBeacon(request.url, beacon);
+
+    scoped_ptr<DomainReliabilityBeacon> beacon =
+        CreateBeaconFromAttempt(beacon_template, attempt);
+    if (beacon)
+      context_manager_.RouteBeacon(beacon.Pass());
   }
 
   if (url_request_attempt_is_duplicate)
     return;
-  if (!UpdateBeaconFromAttempt(&beacon, url_request_attempt))
-    return;
-  context_manager_.RouteBeacon(request.url, beacon);
+
+  scoped_ptr<DomainReliabilityBeacon> beacon =
+      CreateBeaconFromAttempt(beacon_template, url_request_attempt);
+  if (beacon)
+    context_manager_.RouteBeacon(beacon.Pass());
 }
 
 base::WeakPtr<DomainReliabilityMonitor>
 DomainReliabilityMonitor::MakeWeakPtr() {
   return weak_factory_.GetWeakPtr();
 }
 
 }  // namespace domain_reliability