Refactor ChromeFraudulentCertReporter for code reuse by SSL reporting

chrome/browser/net/chrome_fraudulent_certificate_reporter_unittest.cc

 // Copyright (c) 2012 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 "chrome/browser/net/chrome_fraudulent_certificate_reporter.h"
 
 #include <string>
 
 #include "base/bind.h"
 #include "base/files/file_path.h"
@@ skipping 94 matching lines @@
 
   // Passes if invoked with a bad SSLInfo and for a hostname that is not a
   // Google pinned property.
   void SendReport(const std::string& hostname,
                   const SSLInfo& ssl_info) override {
     EXPECT_FALSE(IsGoodSSLInfo(ssl_info));
     EXPECT_FALSE(net::TransportSecurityState::IsGooglePinnedProperty(hostname));
   }
 };
 
-// A ChromeFraudulentCertificateReporter that uses a MockURLRequest, but is
+// A CertificateErrorReporter that uses a MockURLRequest, but is
 // otherwise normal: reports are constructed and sent in the usual way.
-class MockReporter : public ChromeFraudulentCertificateReporter {
+class MockReporter : public CertificateErrorReporter {
  public:
   explicit MockReporter(net::URLRequestContext* request_context)
-    : ChromeFraudulentCertificateReporter(request_context) {}
+      : CertificateErrorReporter(request_context, GURL("http://example.com")) {}
 
+  void SendReport(ReportType type,
+                  const std::string& hostname,
+                  const net::SSLInfo& ssl_info) override {
+    EXPECT_EQ(type, REPORT_TYPE_PINNING_VIOLATION);
+    EXPECT_FALSE(hostname.empty());
+    EXPECT_TRUE(ssl_info.is_valid());
+    CertificateErrorReporter::SendReport(type, hostname, ssl_info);

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

You don't actually need to forward this on, do you? It's just a mock - it
doesn't actually implement the functionality (beyond the minimum necessary)

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

To be honest I don't really understand what any of the tests in this file are
supposed to be testing other than that the certificate reporting code doesn't
crash, which is why I left it in (and that e.g. IsGooglePinnedProperty works,
but this file seems like a lot of unnecessary code just to test that). I'm
probably misunderstanding something, but for example, DoReportIsSent basically
just seems to test that IsGoodSSLInfo(GetGoodSSLInfo()) == true, which doesn't
really have anything to do with ChromeFraudulentCertificateReporter, and
DoMockReportIsSent seems to only test that GetGoodSSLInfo().is_valid() == true
and that if you pass a non-empty string to a method, the argument is non-empty.

+  }
+
+ private:
   scoped_ptr<net::URLRequest> CreateURLRequest(
       net::URLRequestContext* context) override {
     return context->CreateRequest(GURL(std::string()),
                                   net::DEFAULT_PRIORITY,
                                   NULL,
                                   NULL);
   }
-
-  void SendReport(const std::string& hostname,
-                  const net::SSLInfo& ssl_info) override {
-    DCHECK(!hostname.empty());
-    DCHECK(ssl_info.is_valid());
-    ChromeFraudulentCertificateReporter::SendReport(hostname, ssl_info);
-  }
 };
 
 static void DoReportIsSent() {
   net::TestURLRequestContext context;
   SendingTestReporter reporter(&context);
   SSLInfo info = GetGoodSSLInfo();
   reporter.SendReport("mail.google.com", info);
 }
 
 static void DoReportIsNotSent() {
   net::TestURLRequestContext context;
   NotSendingTestReporter reporter(&context);
   SSLInfo info = GetBadSSLInfo();
   reporter.SendReport("www.example.com", info);
 }
 
 static void DoMockReportIsSent() {
   net::TestURLRequestContext context;
-  MockReporter reporter(&context);
+  scoped_ptr<MockReporter> error_reporter(new MockReporter(&context));
+  ChromeFraudulentCertificateReporter reporter(error_reporter.Pass());
   SSLInfo info = GetGoodSSLInfo();
   reporter.SendReport("mail.google.com", info);
 }
 
 TEST(ChromeFraudulentCertificateReporterTest, GoodBadInfo) {
   SSLInfo good = GetGoodSSLInfo();
   EXPECT_TRUE(IsGoodSSLInfo(good));
 
   SSLInfo bad = GetBadSSLInfo();
   EXPECT_FALSE(IsGoodSSLInfo(bad));
@@ skipping 14 matching lines @@
 }
 
 TEST(ChromeFraudulentCertificateReporterTest, ReportIsNotSent) {
   base::MessageLoopForIO loop;
   content::TestBrowserThread io_thread(BrowserThread::IO, &loop);
   loop.PostTask(FROM_HERE, base::Bind(&DoReportIsNotSent));
   loop.RunUntilIdle();
 }
 
 }  // namespace chrome_browser_net