Refactor ChromeFraudulentCertReporter for code reuse by SSL reporting

chrome/browser/net/chrome_fraudulent_certificate_reporter.h

 // Copyright (c) 2011 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.
 
 #ifndef CHROME_BROWSER_NET_CHROME_FRAUDULENT_CERTIFICATE_REPORTER_H_
 #define CHROME_BROWSER_NET_CHROME_FRAUDULENT_CERTIFICATE_REPORTER_H_
 
 #include <set>
 #include <string>
 
+#include "chrome/browser/net/certificate_error_reporter.h"
 #include "net/url_request/fraudulent_certificate_reporter.h"
 #include "net/url_request/url_request.h"
 
 namespace net {
 class URLRequestContext;
 }
 
 namespace chrome_browser_net {
 
 class ChromeFraudulentCertificateReporter
     : public net::FraudulentCertificateReporter,
       public net::URLRequest::Delegate {

[Ryan Sleevi, 2015/03/06 19:23:13]

You no longer need to be a net::URLRequest::Delegate, since nothing ever passes
this to URLRequests

[estark, 2015/03/06 20:10:03]

Done.

  public:
   explicit ChromeFraudulentCertificateReporter(
       net::URLRequestContext* request_context);
 
-  ~ChromeFraudulentCertificateReporter() override;
+  // A constructor used by tests to set certificate_reporter_ to a mock
+  // reporter.
+  explicit ChromeFraudulentCertificateReporter(
+      scoped_ptr<CertificateErrorReporter> certificate_reporter);

[Ryan Sleevi, 2015/03/06 19:23:14]

Then you can make this constructor private and friend test again.

Note that again, the comment is describing "why", rather than "how"

// Useful for tests to use a mock reporter.

[estark, 2015/03/06 19:52:06]

Friending ChromeFraudulentCertificateReporterTest doesn't work because the
constructor is actually called from the helper function DoMockReportIsSent (or
at least I think that's why). It's not safe to construct a MockReporter (which
requires a TestURLRequestContext) from ChromeFraudulentCertificateReporterTest
not on the IO thread, right?

 
-  // Allows users of this class to override this and set their own URLRequest
-  // type. Used by SendReport.
-  virtual scoped_ptr<net::URLRequest> CreateURLRequest(
-      net::URLRequestContext* context);
+  ~ChromeFraudulentCertificateReporter() override {}
 
   // net::FraudulentCertificateReporter
   void SendReport(const std::string& hostname,
                   const net::SSLInfo& ssl_info) override;
 
   // net::URLRequest::Delegate
   void OnResponseStarted(net::URLRequest* request) override;
   void OnReadCompleted(net::URLRequest* request, int bytes_read) override;

[Ryan Sleevi, 2015/03/06 19:23:13]

You can delete these two methods

[estark, 2015/03/06 20:10:03]

Done.

 
- protected:
-  net::URLRequestContext* const request_context_;
-
  private:
-  // Performs post-report cleanup.
-  void RequestComplete(net::URLRequest* request);
-
-  const GURL upload_url_;
-  // Owns the contained requests.
-  std::set<net::URLRequest*> inflight_requests_;
+  scoped_ptr<CertificateErrorReporter> certificate_reporter_;
 
   DISALLOW_COPY_AND_ASSIGN(ChromeFraudulentCertificateReporter);
 };
 
 }  // namespace chrome_browser_net
 
 #endif  // CHROME_BROWSER_NET_CHROME_FRAUDULENT_CERTIFICATE_REPORTER_H_
-