Adding NetLog support to SafeBrowsingPingManager.

chrome/browser/safe_browsing/ping_manager.cc

Index: chrome/browser/safe_browsing/ping_manager.cc
diff --git a/chrome/browser/safe_browsing/ping_manager.cc b/chrome/browser/safe_browsing/ping_manager.cc
index 623a988f7388b29ea5575d275b480e804be75804..da7d7ae53164a559755d8f60a98676f131009291 100644
--- a/chrome/browser/safe_browsing/ping_manager.cc
+++ b/chrome/browser/safe_browsing/ping_manager.cc
@@ -10,15 +10,18 @@
 #include "base/memory/ptr_util.h"
 #include "base/strings/string_util.h"
 #include "base/strings/stringprintf.h"
+#include "base/values.h"
 #include "chrome/browser/safe_browsing/permission_reporter.h"
 #include "components/certificate_reporting/error_reporter.h"
 #include "content/public/browser/browser_thread.h"
 #include "google_apis/google_api_keys.h"
 #include "net/base/escape.h"
 #include "net/base/load_flags.h"
+#include "net/log/net_log_source_type.h"
 #include "net/ssl/ssl_info.h"
 #include "net/url_request/report_sender.h"
 #include "net/url_request/url_fetcher.h"
+#include "net/url_request/url_request_context.h"
 #include "net/url_request/url_request_context_getter.h"
 #include "net/url_request/url_request_status.h"
 #include "url/gurl.h"
@@ -32,6 +35,38 @@ namespace {
 const char kExtendedReportingUploadUrlInsecure[] =
     "http://safebrowsing.googleusercontent.com/safebrowsing/clientreport/"
     "chrome-certs";
+
+// Return a dictionary with "url"=|url-spec| and "data"=|payload| for
+// netlogging the start phase of a ping.
+std::unique_ptr<base::Value> NetLogPingStartCallback(
+    const net::NetLogWithSource& net_log,
+    const GURL& url,
+    const std::string& payload,
+    net::NetLogCaptureMode) {
+  std::unique_ptr<base::DictionaryValue> event_params(
+      new base::DictionaryValue());
+  event_params->SetString("url", url.spec());
+  event_params->SetString("payload", payload);
+  net_log.source().AddToEventParameters(event_params.get());
+  return std::move(event_params);
+}
+
+// Return a dictionary with "url"=|url-spec|, "status"=|status| and
+// "error"=|error| for netlogging the end phase of a ping.
+std::unique_ptr<base::Value> NetLogPingEndCallback(
+    const net::NetLogWithSource& net_log,
+    const GURL& url,
+    const net::URLRequestStatus& status,
+    net::NetLogCaptureMode) {
+  std::unique_ptr<base::DictionaryValue> event_params(
+      new base::DictionaryValue());
+  event_params->SetString("url", url.spec());
+  event_params->SetInteger("status", status.status());
+  event_params->SetInteger("error", status.error());
+  net_log.source().AddToEventParameters(event_params.get());
+  return std::move(event_params);
+}
+
 }  // namespace
 
 namespace safe_browsing {
@@ -67,6 +102,10 @@ SafeBrowsingPingManager::SafeBrowsingPingManager(
 
     permission_reporter_.reset(
         new PermissionReporter(request_context_getter->GetURLRequestContext()));
+
+    net_log_ = net::NetLogWithSource::Make(
+        request_context_getter->GetURLRequestContext()->net_log(),
+        net::NetLogSourceType::SAFE_BROWSING);
   }
 
   version_ = SafeBrowsingProtocolManagerHelper::Version();
@@ -87,6 +126,11 @@ void SafeBrowsingPingManager::OnURLFetchComplete(
                    });
   DCHECK(it != safebrowsing_reports_.end());
   safebrowsing_reports_.erase(it);
+
+  net_log_.EndEvent(
+      net::NetLogEventType::SAFE_BROWSING_PING,
+      base::Bind(&NetLogPingEndCallback, net_log_, source->GetURL(),
+                 source->GetStatus()));
 }
 
 // Sends a SafeBrowsing "hit" report.
@@ -104,6 +148,11 @@ void SafeBrowsingPingManager::ReportSafeBrowsingHit(
     report_ptr->SetUploadData("text/plain", hit_report.post_data);
   safebrowsing_reports_.insert(std::move(report_ptr));
   report->Start();
+
+  net_log_.BeginEvent(
+      net::NetLogEventType::SAFE_BROWSING_PING,
+      base::Bind(&NetLogPingStartCallback, net_log_, report_ptr->GetURL(),
+                 hit_report.post_data));

[Jialiu Lin, 2016/09/22 21:19:13]

It seems hit_report.post_data could be empty if report is sent via GET.  If
that's the case, should we at least send something to the netlog?

Seems only download_protection_service uses this post_data field of HitReport.
https://cs.chromium.org/chromium/src/chrome/browser/safe_browsing/download_pr...

[lpz, 2016/09/23 18:11:58]

Ahh good point. It looks like all the other data in HitReport is used to
generate the ping URL so it's represented there (of course in a much less
readable form). I could certainly pretty-print that struct into the payload, but
wondering if the URL params are good enough for what we need?

[Jialiu Lin, 2016/09/23 18:18:25]

either way is fine as long as human can understand it when displayed in
chrome://net-internal

[lpz, 2016/09/26 19:55:39]

It's somewhat usable since you can search within the net-internals window for
the url params (from SafeBrowsingHitURL). If it turns out to be too annoying I
think we can do a follow-up change to make it nicer for this use case.

[Jialiu Lin, 2016/09/26 20:13:56]

SG.

 }
 
 // Sends threat details for users who opt-in.
@@ -118,6 +167,11 @@ void SafeBrowsingPingManager::ReportThreatDetails(const std::string& report) {
   fetcher->SetAutomaticallyRetryOn5xx(false);
   fetcher->Start();
   safebrowsing_reports_.insert(std::move(fetcher));
+
+  net_log_.BeginEvent(
+      net::NetLogEventType::SAFE_BROWSING_PING,
+      base::Bind(&NetLogPingStartCallback, net_log_, fetcher->GetURL(),
+                 report));
 }
 
 void SafeBrowsingPingManager::ReportInvalidCertificateChain(