HTTP Bad: Split out UMA metrics for password vs credit card "Not secure" warnings

chrome/browser/ssl/chrome_security_state_model_client_unittest.cc

Index: chrome/browser/ssl/chrome_security_state_model_client_unittest.cc
diff --git a/chrome/browser/ssl/chrome_security_state_model_client_unittest.cc b/chrome/browser/ssl/chrome_security_state_model_client_unittest.cc
index 711c17cfcf9155fadbad38030854769bb2c2b182..1d4efe03cf27bd692ca1eadcf9d6c7424e0401dc 100644
--- a/chrome/browser/ssl/chrome_security_state_model_client_unittest.cc
+++ b/chrome/browser/ssl/chrome_security_state_model_client_unittest.cc
@@ -18,9 +18,6 @@
 
 namespace {
 
-const char kHTTPBadHistogram[] =
-    "Security.HTTPBad.UserWarnedAboutSensitiveInput";
-
 // Tests that SecurityInfo flags for subresources with certificate
 // errors are reflected in the SecurityStyleExplanations produced by
 // ChromeSecurityStateModelClient.
@@ -236,21 +233,39 @@ TEST(ChromeSecurityStateModelClientTest, HTTPWarning) {
 
 // Tests that a security level of NONE when there is a password or
 // credit card field on HTTP produces a content::SecurityStyle of
-// UNAUTHENTICATED, with an info explanation.
+// UNAUTHENTICATED, with an info explanation for each.
 TEST(ChromeSecurityStateModelClientTest, HTTPWarningInFuture) {
   security_state::SecurityStateModel::SecurityInfo security_info;
   content::SecurityStyleExplanations explanations;
   security_info.security_level = security_state::SecurityStateModel::NONE;
-  security_info.displayed_private_user_data_input_on_http = true;
+  security_info.displayed_password_field_on_http = true;
   blink::WebSecurityStyle security_style =
       ChromeSecurityStateModelClient::GetSecurityStyle(security_info,
                                                        &explanations);
   EXPECT_EQ(blink::WebSecurityStyleUnauthenticated, security_style);
   EXPECT_EQ(1u, explanations.info_explanations.size());
+
+  explanations.info_explanations.clear();
+  security_info.displayed_credit_card_field_on_http = true;
+  security_style = ChromeSecurityStateModelClient::GetSecurityStyle(
+      security_info, &explanations);
+  EXPECT_EQ(blink::WebSecurityStyleUnauthenticated, security_style);
+  EXPECT_EQ(1u, explanations.info_explanations.size());
+
+  // Check that when both password and credit card fields get displayed, only
+  // one explanation added.

[estark, 2016/11/10 05:19:29]

nit: added => is added

[lshang, 2016/11/10 05:33:52]

Done.

+  explanations.info_explanations.clear();
+  security_info.displayed_credit_card_field_on_http = true;
+  security_info.displayed_password_field_on_http = true;
+  security_style = ChromeSecurityStateModelClient::GetSecurityStyle(
+      security_info, &explanations);
+  EXPECT_EQ(blink::WebSecurityStyleUnauthenticated, security_style);
+  EXPECT_EQ(1u, explanations.info_explanations.size());
 }
 
 class ChromeSecurityStateModelClientHistogramTest
-    : public ChromeRenderViewHostTestHarness {
+    : public ChromeRenderViewHostTestHarness,
+      public testing::WithParamInterface<bool> {
  public:
   ChromeSecurityStateModelClientHistogramTest() {}
   ~ChromeSecurityStateModelClientHistogramTest() override {}
@@ -266,11 +281,21 @@ class ChromeSecurityStateModelClientHistogramTest
  protected:
   ChromeSecurityStateModelClient* client() { return client_; }
 
-  void signal_password() {
-    web_contents()->OnPasswordInputShownOnHttp();
+  void signal_sensitive_input() {
+    if (GetParam())
+      web_contents()->OnPasswordInputShownOnHttp();
+    else
+      web_contents()->OnCreditCardInputShownOnHttp();
     client_->VisibleSecurityStateChanged();
   }
 
+  const std::string histogram_name() {
+    if (GetParam())
+      return "Security.HTTPBad.UserWarnedAboutSensitiveInput.Password";
+    else
+      return "Security.HTTPBad.UserWarnedAboutSensitiveInput.CreditCard";
+  }
+
   void navigate_to_http() { NavigateAndCommit(GURL("http://example.test")); }
 
  private:
@@ -280,7 +305,7 @@ class ChromeSecurityStateModelClientHistogramTest
 
 // Tests that UMA logs the omnibox warning when security level is
 // HTTP_SHOW_WARNING.
-TEST_F(ChromeSecurityStateModelClientHistogramTest,
+TEST_P(ChromeSecurityStateModelClientHistogramTest,
        HTTPOmniboxWarningHistogram) {
   // Show Warning Chip.
   base::CommandLine::ForCurrentProcess()->AppendSwitchASCII(
@@ -288,22 +313,22 @@ TEST_F(ChromeSecurityStateModelClientHistogramTest,
       security_state::switches::kMarkHttpWithPasswordsOrCcWithChip);
 
   base::HistogramTester histograms;
-  signal_password();
-  histograms.ExpectUniqueSample(kHTTPBadHistogram, true, 1);
+  signal_sensitive_input();
+  histograms.ExpectUniqueSample(histogram_name(), true, 1);
 
   // Fire again and ensure no sample is recorded.
-  signal_password();
-  histograms.ExpectUniqueSample(kHTTPBadHistogram, true, 1);
+  signal_sensitive_input();
+  histograms.ExpectUniqueSample(histogram_name(), true, 1);
 
   // Navigate to a new page and ensure a sample is recorded.
   navigate_to_http();
-  histograms.ExpectUniqueSample(kHTTPBadHistogram, true, 1);
-  signal_password();
-  histograms.ExpectUniqueSample(kHTTPBadHistogram, true, 2);
+  histograms.ExpectUniqueSample(histogram_name(), true, 1);
+  signal_sensitive_input();
+  histograms.ExpectUniqueSample(histogram_name(), true, 2);
 }
 
 // Tests that UMA logs the console warning when security level is NONE.
-TEST_F(ChromeSecurityStateModelClientHistogramTest,
+TEST_P(ChromeSecurityStateModelClientHistogramTest,
        HTTPConsoleWarningHistogram) {
   // Show Neutral for HTTP
   base::CommandLine::ForCurrentProcess()->AppendSwitchASCII(
@@ -311,18 +336,25 @@ TEST_F(ChromeSecurityStateModelClientHistogramTest,
       security_state::switches::kMarkHttpAsNeutral);
 
   base::HistogramTester histograms;
-  signal_password();
-  histograms.ExpectUniqueSample(kHTTPBadHistogram, false, 1);
+  signal_sensitive_input();
+  histograms.ExpectUniqueSample(histogram_name(), false, 1);
 
   // Fire again and ensure no sample is recorded.
-  signal_password();
-  histograms.ExpectUniqueSample(kHTTPBadHistogram, false, 1);
+  signal_sensitive_input();
+  histograms.ExpectUniqueSample(histogram_name(), false, 1);
 
   // Navigate to a new page and ensure a sample is recorded.
   navigate_to_http();
-  histograms.ExpectUniqueSample(kHTTPBadHistogram, false, 1);
-  signal_password();
-  histograms.ExpectUniqueSample(kHTTPBadHistogram, false, 2);
+  histograms.ExpectUniqueSample(histogram_name(), false, 1);
+  signal_sensitive_input();
+  histograms.ExpectUniqueSample(histogram_name(), false, 2);
 }
 
+INSTANTIATE_TEST_CASE_P(ChromeSecurityStateModelClientHistogramTest,
+                        ChromeSecurityStateModelClientHistogramTest,
+                        // Here 'true' to test password field triggered
+                        // histogram and 'false' to test
+                        // credit catd field.

[estark, 2016/11/10 05:19:29]

nit: typo, catd => card

[lshang, 2016/11/10 05:33:52]

Done. Also correct the indent.

+                        testing::Bool());
+
 }  // namespace