Add checkbox for reporting invalid TLS/SSL cert chains

net/url_request/fraudulent_certificate_reporter.h

Index: net/url_request/fraudulent_certificate_reporter.h
diff --git a/net/url_request/fraudulent_certificate_reporter.h b/net/url_request/fraudulent_certificate_reporter.h
index 8d5d60a0ef151769355e0995ffc01714b65c8884..eaf1f16546fb6ee29e13004f8ad53fb64d0830f5 100644
--- a/net/url_request/fraudulent_certificate_reporter.h
+++ b/net/url_request/fraudulent_certificate_reporter.h
@@ -16,15 +16,22 @@ class SSLInfo;
 // check.
 class FraudulentCertificateReporter {
  public:
+  // The type of a report: either a pinning violation on a google.com

[Ryan Sleevi, 2015/03/03 19:16:39]

This is a bit of a layering violation (or more aptly, documenting the layering
violation)

enum ReportType {
  // The report is an HPKP pinning violation
  REPORT_TYPE_PIN_VIOLATION,

  // The report is being manually sent
  REPORT_TYPE_EXTENDED_REPORTING
};

From a layering perspective, you shouldn't be talking about who/what is being
sent - it seems like all HPKP reports are equal from the API

Having REPORT_TYPE_EXTENDED_REPORTING does feel somewhat weird as well, from an
API layer, since it's really not something //net is using. In this respect, in
looking at it again, it almost feels like SendReport() should NOT have
*ReportType* at all. After all, the comments on line 14-16 indicate this is just
for pinning errors.

A way to accomplish what you want - reuse of code - without doing what gets me
grumpy - layering violations - would be to first (in a separate CL) split the
FraudulentCertificateReporter up.

That is, one class is responsible for building reports, and another class is
responsible for *sending* reports. I suspect the build-report code lives in
//chrome (since the report format is specific to the fraudulent reporting), and
the send report code lives here (as a base interface). Refactor the //chrome
implementation of this interface (without your report type changes) to call the
build report to build the report when SendReport is called, then sending to the
appropriate endpoints.

You could then reuse the BuildReport code (and possibly the SendReport) code
specifically in //chrome, without reaching into this //net interface.

Does that logic make sense?

+  // site, or a report of an invalid SSL certificate chain (sent as part
+  // of the extended reporting program).
+  enum ReportType { REPORT_TYPE_PIN_VIOLATION, REPORT_TYPE_EXTENDED_REPORTING };
+
   virtual ~FraudulentCertificateReporter() {}
 
-  // Sends a report to the report collection server containing the |ssl_info|
-  // associated with a connection to |hostname|.
-  virtual void SendReport(const std::string& hostname,
+  // Sends a report of the given |type| to the report collection server,
+  // containing the |ssl_info| associated with a connection to
+  // |hostname|. |type| determines the format of the report to be sent
+  // and the endpoint that it gets sent to.
+  virtual void SendReport(ReportType type,
+                          const std::string& hostname,
                           const SSLInfo& ssl_info) = 0;
 };
 
 }  // namespace net
 
 #endif  // NET_URL_REQUEST_FRAUDULENT_CERTIFICATE_REPORTER_H_
-