Refactor SecurityStateModel/Clients for simplicity and reusability.

components/security_state/core/security_state_model_unittest.cc

Index: components/security_state/core/security_state_model_unittest.cc
diff --git a/components/security_state/security_state_model_unittest.cc b/components/security_state/core/security_state_model_unittest.cc
similarity index 73%
rename from components/security_state/security_state_model_unittest.cc
rename to components/security_state/core/security_state_model_unittest.cc
index 7776a0c5ba5e492df8a36efef7d189ad3a2db7c9..45e66a224a07c33ae86997c5bb2e0bf04f4a6192 100644
--- a/components/security_state/security_state_model_unittest.cc
+++ b/components/security_state/core/security_state_model_unittest.cc
@@ -2,14 +2,15 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
-#include "components/security_state/security_state_model.h"
+#include "components/security_state/core/security_state_model.h"
 
 #include <stdint.h>
 
+#include "base/bind.h"
 #include "base/command_line.h"
+#include "base/memory/ptr_util.h"
 #include "base/test/histogram_tester.h"
-#include "components/security_state/security_state_model_client.h"
-#include "components/security_state/switches.h"
+#include "components/security_state/core/switches.h"
 #include "net/cert/x509_certificate.h"
 #include "net/ssl/ssl_cipher_suite_names.h"
 #include "net/ssl/ssl_connection_status_flags.h"
@@ -25,9 +26,13 @@ namespace {
 const char kHttpsUrl[] = "https://foo.test";
 const char kHttpUrl[] = "http://foo.test";
 
-class TestSecurityStateModelClient : public SecurityStateModelClient {
+bool IsOriginSecure(const GURL& url) {
+  return url == GURL(kHttpsUrl);
+}
+
+class TestSecurityStateModelHelper {
  public:
-  TestSecurityStateModelClient()
+  TestSecurityStateModelHelper()
       : url_(kHttpsUrl),
         connection_status_(net::SSL_CONNECTION_VERSION_TLS1_2
                            << net::SSL_CONNECTION_VERSION_SHIFT),
@@ -40,7 +45,7 @@ class TestSecurityStateModelClient : public SecurityStateModelClient {
     cert_ =
         net::ImportCertFromFile(net::GetTestCertsDirectory(), "sha1_2016.pem");
   }
-  ~TestSecurityStateModelClient() override {}
+  virtual ~TestSecurityStateModelHelper() {}
 
   void set_connection_status(int connection_status) {
     connection_status_ = connection_status;
@@ -71,9 +76,9 @@ class TestSecurityStateModelClient : public SecurityStateModelClient {
 
   void UseHttpUrl() { url_ = GURL(kHttpUrl); }
 
-  // SecurityStateModelClient:
-  void GetVisibleSecurityState(
-      SecurityStateModel::VisibleSecurityState* state) override {
+  std::unique_ptr<SecurityStateModel::VisibleSecurityState>
+  GetVisibleSecurityState() {
+    auto state = base::MakeUnique<SecurityStateModel::VisibleSecurityState>();
     state->connection_info_initialized = true;
     state->url = url_;
     state->certificate = cert_;
@@ -86,15 +91,19 @@ class TestSecurityStateModelClient : public SecurityStateModelClient {
     state->displayed_password_field_on_http = displayed_password_field_on_http_;
     state->displayed_credit_card_field_on_http =
         displayed_credit_card_field_on_http_;
+    return state;
   }
 
-  bool UsedPolicyInstalledCertificate() override { return false; }
+  bool UsedPolicyInstalledCertificate() { return false; }
 
-  bool IsOriginSecure(const GURL& url) override {
-    return url_ == GURL(kHttpsUrl);
+  void GetSecurityInfo(SecurityStateModel::SecurityInfo* security_info) {
+    model_.GetSecurityInfo(security_info, GetVisibleSecurityState(),
+                           UsedPolicyInstalledCertificate(),

[estark, 2016/11/03 04:38:45]

nit:
`false /* used policy installed certificate */`
instead of a helper method

[Eric Seckler, 2016/11/03 17:01:06]

Done.

+                           base::Bind(&IsOriginSecure));
   }
 
  private:
+  SecurityStateModel model_;
   GURL url_;
   scoped_refptr<net::X509Certificate> cert_;
   int connection_status_;
@@ -106,14 +115,14 @@ class TestSecurityStateModelClient : public SecurityStateModelClient {
   bool displayed_credit_card_field_on_http_;
 };
 
+}  // namespace
+
 // Tests that SHA1-signed certificates expiring in 2016 downgrade the
 // security state of the page.
 TEST(SecurityStateModelTest, SHA1Warning) {
-  TestSecurityStateModelClient client;
-  SecurityStateModel model;
-  model.SetClient(&client);
+  TestSecurityStateModelHelper helper;
   SecurityStateModel::SecurityInfo security_info;
-  model.GetSecurityInfo(&security_info);
+  helper.GetSecurityInfo(&security_info);
   EXPECT_EQ(SecurityStateModel::DEPRECATED_SHA1_MINOR,
             security_info.sha1_deprecation_status);
   EXPECT_EQ(SecurityStateModel::NONE, security_info.security_level);
@@ -122,22 +131,20 @@ TEST(SecurityStateModelTest, SHA1Warning) {
 // Tests that SHA1 warnings don't interfere with the handling of mixed
 // content.
 TEST(SecurityStateModelTest, SHA1WarningMixedContent) {
-  TestSecurityStateModelClient client;
-  SecurityStateModel model;
-  model.SetClient(&client);
-  client.SetDisplayedMixedContent(true);
+  TestSecurityStateModelHelper helper;
+  helper.SetDisplayedMixedContent(true);
   SecurityStateModel::SecurityInfo security_info1;
-  model.GetSecurityInfo(&security_info1);
+  helper.GetSecurityInfo(&security_info1);
   EXPECT_EQ(SecurityStateModel::DEPRECATED_SHA1_MINOR,
             security_info1.sha1_deprecation_status);
   EXPECT_EQ(SecurityStateModel::CONTENT_STATUS_DISPLAYED,
             security_info1.mixed_content_status);
   EXPECT_EQ(SecurityStateModel::NONE, security_info1.security_level);
 
-  client.SetDisplayedMixedContent(false);
-  client.SetRanMixedContent(true);
+  helper.SetDisplayedMixedContent(false);
+  helper.SetRanMixedContent(true);
   SecurityStateModel::SecurityInfo security_info2;
-  model.GetSecurityInfo(&security_info2);
+  helper.GetSecurityInfo(&security_info2);
   EXPECT_EQ(SecurityStateModel::DEPRECATED_SHA1_MINOR,
             security_info2.sha1_deprecation_status);
   EXPECT_EQ(SecurityStateModel::CONTENT_STATUS_RAN,
@@ -148,12 +155,10 @@ TEST(SecurityStateModelTest, SHA1WarningMixedContent) {
 // Tests that SHA1 warnings don't interfere with the handling of major
 // cert errors.
 TEST(SecurityStateModelTest, SHA1WarningBrokenHTTPS) {
-  TestSecurityStateModelClient client;
-  SecurityStateModel model;
-  model.SetClient(&client);
-  client.AddCertStatus(net::CERT_STATUS_DATE_INVALID);
+  TestSecurityStateModelHelper helper;
+  helper.AddCertStatus(net::CERT_STATUS_DATE_INVALID);
   SecurityStateModel::SecurityInfo security_info;
-  model.GetSecurityInfo(&security_info);
+  helper.GetSecurityInfo(&security_info);
   EXPECT_EQ(SecurityStateModel::DEPRECATED_SHA1_MINOR,
             security_info.sha1_deprecation_status);
   EXPECT_EQ(SecurityStateModel::DANGEROUS, security_info.security_level);
@@ -162,65 +167,57 @@ TEST(SecurityStateModelTest, SHA1WarningBrokenHTTPS) {
 // Tests that |security_info.is_secure_protocol_and_ciphersuite| is
 // computed correctly.
 TEST(SecurityStateModelTest, SecureProtocolAndCiphersuite) {
-  TestSecurityStateModelClient client;
-  SecurityStateModel model;
-  model.SetClient(&client);
+  TestSecurityStateModelHelper helper;
   // TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256 from
   // http://www.iana.org/assignments/tls-parameters/tls-parameters.xml#tls-parameters-4
   const uint16_t ciphersuite = 0xc02f;
-  client.set_connection_status(net::SSL_CONNECTION_VERSION_TLS1_2
+  helper.set_connection_status(net::SSL_CONNECTION_VERSION_TLS1_2
                                << net::SSL_CONNECTION_VERSION_SHIFT);
-  client.SetCipherSuite(ciphersuite);
+  helper.SetCipherSuite(ciphersuite);
   SecurityStateModel::SecurityInfo security_info;
-  model.GetSecurityInfo(&security_info);
+  helper.GetSecurityInfo(&security_info);
   EXPECT_EQ(net::OBSOLETE_SSL_NONE, security_info.obsolete_ssl_status);
 }
 
 TEST(SecurityStateModelTest, NonsecureProtocol) {
-  TestSecurityStateModelClient client;
-  SecurityStateModel model;
-  model.SetClient(&client);
+  TestSecurityStateModelHelper helper;
   // TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256 from
   // http://www.iana.org/assignments/tls-parameters/tls-parameters.xml#tls-parameters-4
   const uint16_t ciphersuite = 0xc02f;
-  client.set_connection_status(net::SSL_CONNECTION_VERSION_TLS1_1
+  helper.set_connection_status(net::SSL_CONNECTION_VERSION_TLS1_1
                                << net::SSL_CONNECTION_VERSION_SHIFT);
-  client.SetCipherSuite(ciphersuite);
+  helper.SetCipherSuite(ciphersuite);
   SecurityStateModel::SecurityInfo security_info;
-  model.GetSecurityInfo(&security_info);
+  helper.GetSecurityInfo(&security_info);
   EXPECT_EQ(net::OBSOLETE_SSL_MASK_PROTOCOL, security_info.obsolete_ssl_status);
 }
 
 TEST(SecurityStateModelTest, NonsecureCiphersuite) {
-  TestSecurityStateModelClient client;
-  SecurityStateModel model;
-  model.SetClient(&client);
+  TestSecurityStateModelHelper helper;
   // TLS_RSA_WITH_AES_128_CCM_8 from
   // http://www.iana.org/assignments/tls-parameters/tls-parameters.xml#tls-parameters-4
   const uint16_t ciphersuite = 0xc0a0;
-  client.set_connection_status(net::SSL_CONNECTION_VERSION_TLS1_2
+  helper.set_connection_status(net::SSL_CONNECTION_VERSION_TLS1_2
                                << net::SSL_CONNECTION_VERSION_SHIFT);
-  client.SetCipherSuite(ciphersuite);
+  helper.SetCipherSuite(ciphersuite);
   SecurityStateModel::SecurityInfo security_info;
-  model.GetSecurityInfo(&security_info);
+  helper.GetSecurityInfo(&security_info);
   EXPECT_EQ(net::OBSOLETE_SSL_MASK_KEY_EXCHANGE | net::OBSOLETE_SSL_MASK_CIPHER,
             security_info.obsolete_ssl_status);
 }
 
 // Tests that the malware/phishing status is set, and it overrides valid HTTPS.
 TEST(SecurityStateModelTest, MalwareOverride) {
-  TestSecurityStateModelClient client;
-  SecurityStateModel model;
-  model.SetClient(&client);
+  TestSecurityStateModelHelper helper;
   // TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256 from
   // http://www.iana.org/assignments/tls-parameters/tls-parameters.xml#tls-parameters-4
   const uint16_t ciphersuite = 0xc02f;
-  client.set_connection_status(net::SSL_CONNECTION_VERSION_TLS1_2
+  helper.set_connection_status(net::SSL_CONNECTION_VERSION_TLS1_2
                                << net::SSL_CONNECTION_VERSION_SHIFT);
-  client.SetCipherSuite(ciphersuite);
-  client.set_fails_malware_check(true);
+  helper.SetCipherSuite(ciphersuite);
+  helper.set_fails_malware_check(true);
   SecurityStateModel::SecurityInfo security_info;
-  model.GetSecurityInfo(&security_info);
+  helper.GetSecurityInfo(&security_info);
   EXPECT_TRUE(security_info.fails_malware_check);
   EXPECT_EQ(SecurityStateModel::DANGEROUS, security_info.security_level);
 }
@@ -228,12 +225,10 @@ TEST(SecurityStateModelTest, MalwareOverride) {
 // Tests that the malware/phishing status is set, even if other connection info
 // is not available.
 TEST(SecurityStateModelTest, MalwareWithoutCOnnectionState) {
-  TestSecurityStateModelClient client;
-  SecurityStateModel model;
-  model.SetClient(&client);
-  client.set_fails_malware_check(true);
+  TestSecurityStateModelHelper helper;
+  helper.set_fails_malware_check(true);
   SecurityStateModel::SecurityInfo security_info;
-  model.GetSecurityInfo(&security_info);
+  helper.GetSecurityInfo(&security_info);
   EXPECT_TRUE(security_info.fails_malware_check);
   EXPECT_EQ(SecurityStateModel::DANGEROUS, security_info.security_level);
 }
@@ -244,13 +239,11 @@ TEST(SecurityStateModelTest, PasswordFieldWarning) {
   base::CommandLine::ForCurrentProcess()->AppendSwitchASCII(
       switches::kMarkHttpAs,
       switches::kMarkHttpWithPasswordsOrCcWithChip);
-  TestSecurityStateModelClient client;
-  client.UseHttpUrl();
-  SecurityStateModel model;
-  model.SetClient(&client);
-  client.set_displayed_password_field_on_http(true);
+  TestSecurityStateModelHelper helper;
+  helper.UseHttpUrl();
+  helper.set_displayed_password_field_on_http(true);
   SecurityStateModel::SecurityInfo security_info;
-  model.GetSecurityInfo(&security_info);
+  helper.GetSecurityInfo(&security_info);
   EXPECT_TRUE(security_info.displayed_private_user_data_input_on_http);
   EXPECT_EQ(SecurityStateModel::HTTP_SHOW_WARNING,
             security_info.security_level);
@@ -262,13 +255,11 @@ TEST(SecurityStateModelTest, CreditCardFieldWarning) {
   base::CommandLine::ForCurrentProcess()->AppendSwitchASCII(
       switches::kMarkHttpAs,
       switches::kMarkHttpWithPasswordsOrCcWithChip);
-  TestSecurityStateModelClient client;
-  client.UseHttpUrl();
-  SecurityStateModel model;
-  model.SetClient(&client);
-  client.set_displayed_credit_card_field_on_http(true);
+  TestSecurityStateModelHelper helper;
+  helper.UseHttpUrl();
+  helper.set_displayed_credit_card_field_on_http(true);
   SecurityStateModel::SecurityInfo security_info;
-  model.GetSecurityInfo(&security_info);
+  helper.GetSecurityInfo(&security_info);
   EXPECT_TRUE(security_info.displayed_private_user_data_input_on_http);
   EXPECT_EQ(SecurityStateModel::HTTP_SHOW_WARNING,
             security_info.security_level);
@@ -278,14 +269,12 @@ TEST(SecurityStateModelTest, CreditCardFieldWarning) {
 // level to be downgraded to HTTP_SHOW_WARNING when the command-line switch
 // is NOT set.
 TEST(SecurityStateModelTest, HttpWarningNotSetWithoutSwitch) {
-  TestSecurityStateModelClient client;
-  client.UseHttpUrl();
-  SecurityStateModel model;
-  model.SetClient(&client);
-  client.set_displayed_password_field_on_http(true);
-  client.set_displayed_credit_card_field_on_http(true);
+  TestSecurityStateModelHelper helper;
+  helper.UseHttpUrl();
+  helper.set_displayed_password_field_on_http(true);
+  helper.set_displayed_credit_card_field_on_http(true);
   SecurityStateModel::SecurityInfo security_info;
-  model.GetSecurityInfo(&security_info);
+  helper.GetSecurityInfo(&security_info);
   EXPECT_TRUE(security_info.displayed_private_user_data_input_on_http);
   EXPECT_EQ(SecurityStateModel::NONE, security_info.security_level);
 }
@@ -293,12 +282,10 @@ TEST(SecurityStateModelTest, HttpWarningNotSetWithoutSwitch) {
 // Tests that |displayed_private_user_data_input_on_http| is not set
 // when the corresponding VisibleSecurityState flags are not set.
 TEST(SecurityStateModelTest, PrivateUserDataNotSet) {
-  TestSecurityStateModelClient client;
-  client.UseHttpUrl();
-  SecurityStateModel model;
-  model.SetClient(&client);
+  TestSecurityStateModelHelper helper;
+  helper.UseHttpUrl();
   SecurityStateModel::SecurityInfo security_info;
-  model.GetSecurityInfo(&security_info);
+  helper.GetSecurityInfo(&security_info);
   EXPECT_FALSE(security_info.displayed_private_user_data_input_on_http);
   EXPECT_EQ(SecurityStateModel::NONE, security_info.security_level);
 }
@@ -310,25 +297,21 @@ TEST(SecurityStateModelTest, MarkHttpAsStatusHistogram) {
   base::HistogramTester histograms;
   base::CommandLine::ForCurrentProcess()->AppendSwitchASCII(
       switches::kMarkHttpAs, switches::kMarkHttpWithPasswordsOrCcWithChip);
-  TestSecurityStateModelClient client;
-  client.UseHttpUrl();
-  SecurityStateModel model;
-  model.SetClient(&client);
+  TestSecurityStateModelHelper helper;
+  helper.UseHttpUrl();
 
   // Ensure histogram recorded correctly when a non-secure password input is
   // found on the page.
-  client.set_displayed_password_field_on_http(true);
+  helper.set_displayed_password_field_on_http(true);
   SecurityStateModel::SecurityInfo security_info;
   histograms.ExpectTotalCount(kHistogramName, 0);
-  model.GetSecurityInfo(&security_info);
+  helper.GetSecurityInfo(&security_info);
   histograms.ExpectUniqueSample(kHistogramName, 2 /* HTTP_SHOW_WARNING */, 1);
 
   // Ensure histogram recorded correctly even without a password input.
-  client.set_displayed_password_field_on_http(false);
-  model.GetSecurityInfo(&security_info);
+  helper.set_displayed_password_field_on_http(false);
+  helper.GetSecurityInfo(&security_info);
   histograms.ExpectUniqueSample(kHistogramName, 2 /* HTTP_SHOW_WARNING */, 2);
 }
 
-}  // namespace
-
 }  // namespace security_state