Record time to navigation/tab-closed after HTTP-bad warning

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 c8e6e626410f7757283fccde4808a3ff6c237f98..fc214851568244f3eb758bebe37fe93c9d000a88 100644
--- a/chrome/browser/ssl/chrome_security_state_model_client_unittest.cc
+++ b/chrome/browser/ssl/chrome_security_state_model_client_unittest.cc
@@ -18,6 +18,11 @@
 
 namespace {
 
+const char kHTTPBadNavigationHistogram[] =
+    "Security.HTTPBad.NavigationStartedAfterUserWarnedAboutSensitiveInput";
+const char kHTTPBadWebContentsDestroyedHistogram[] =
+    "Security.HTTPBad.WebContentsDestroyedAfterUserWarnedAboutSensitiveInput";
+
 // Tests that SecurityInfo flags for subresources with certificate
 // errors are reflected in the SecurityStyleExplanations produced by
 // ChromeSecurityStateModelClient.
@@ -275,13 +280,13 @@ class ChromeSecurityStateModelClientHistogramTest
 
     ChromeSecurityStateModelClient::CreateForWebContents(web_contents());
     client_ = ChromeSecurityStateModelClient::FromWebContents(web_contents());
-    navigate_to_http();
+    NavigateToHTTP();
   }
 
  protected:
   ChromeSecurityStateModelClient* client() { return client_; }
 
-  void signal_sensitive_input() {
+  void SignalSensitiveInput() {
     if (GetParam())
       web_contents()->OnPasswordInputShownOnHttp();
     else
@@ -289,16 +294,16 @@ class ChromeSecurityStateModelClientHistogramTest
     client_->VisibleSecurityStateChanged();
   }
 
-  const std::string histogram_name() {
+  const std::string GetHistogramName() {
     if (GetParam())
       return "Security.HTTPBad.UserWarnedAboutSensitiveInput.Password";
     else
       return "Security.HTTPBad.UserWarnedAboutSensitiveInput.CreditCard";
   }
 
-  void navigate_to_http() { NavigateAndCommit(GURL("http://example.test")); }
+  void NavigateToHTTP() { NavigateAndCommit(GURL("http://example.test")); }
 
-  void navigate_to_different_http_page() {
+  void NavigateToDifferentHTTPPage() {
     NavigateAndCommit(GURL("http://example2.test"));
   }
 
@@ -307,6 +312,76 @@ class ChromeSecurityStateModelClientHistogramTest
   DISALLOW_COPY_AND_ASSIGN(ChromeSecurityStateModelClientHistogramTest);
 };
 
+// Tests that an UMA histogram is recorded after setting the security
+// level to HTTP_SHOW_WARNING and navigating away.
+TEST_P(ChromeSecurityStateModelClientHistogramTest,
+       HTTPOmniboxWarningNavigationHistogram) {
+  base::CommandLine::ForCurrentProcess()->AppendSwitchASCII(
+      security_state::switches::kMarkHttpAs,
+      security_state::switches::kMarkHttpWithPasswordsOrCcWithChip);
+
+  base::HistogramTester histograms;
+  SignalSensitiveInput();
+  // Make sure that if the omnibox warning gets dynamically hidden, the
+  // histogram still gets recorded.
+  if (GetParam())
+    web_contents()->OnAllPasswordInputsHiddenOnHttp();
+  NavigateToDifferentHTTPPage();
+  // Destroy the WebContents to simulate the tab being closed after a
+  // navigation.
+  SetContents(nullptr);
+  histograms.ExpectTotalCount(kHTTPBadNavigationHistogram, 1);
+  histograms.ExpectTotalCount(kHTTPBadWebContentsDestroyedHistogram, 0);
+}
+
+// Tests that an UMA histogram is recorded after showing a console
+// warning for a sensitive input on HTTP and navigating away.
+TEST_P(ChromeSecurityStateModelClientHistogramTest,
+       HTTPConsoleWarningNavigationHistogram) {
+  // Same as the test above, but ensuring that the histogram gets

[elawrence, 2016/11/15 16:29:53]

"test above"

This comment feels fragile. Is it safe to assume ordering of the file will not
change?

[estark, 2016/11/16 05:16:14]

Done.

+  // recorded even if the command-line switch to show the omnibox
+  // warning is not set.
+  base::HistogramTester histograms;
+  SignalSensitiveInput();
+  NavigateToDifferentHTTPPage();
+  // Destroy the WebContents to simulate the tab being closed after a
+  // navigation.
+  SetContents(nullptr);
+  histograms.ExpectTotalCount(kHTTPBadNavigationHistogram, 1);
+  histograms.ExpectTotalCount(kHTTPBadWebContentsDestroyedHistogram, 0);
+}
+
+// Tests that an UMA histogram is recorded after setting the security
+// level to HTTP_SHOW_WARNING and closing the tab.
+TEST_P(ChromeSecurityStateModelClientHistogramTest,
+       HTTPOmniboxWarningTabClosedHistogram) {
+  base::CommandLine::ForCurrentProcess()->AppendSwitchASCII(
+      security_state::switches::kMarkHttpAs,
+      security_state::switches::kMarkHttpWithPasswordsOrCcWithChip);
+
+  base::HistogramTester histograms;
+  SignalSensitiveInput();
+  // Destroy the WebContents to simulate the tab being closed.
+  SetContents(nullptr);
+  histograms.ExpectTotalCount(kHTTPBadNavigationHistogram, 0);
+  histograms.ExpectTotalCount(kHTTPBadWebContentsDestroyedHistogram, 1);
+}
+
+// Tests that an UMA histogram is recorded after showing a console
+// warning for a sensitive input on HTTP and closing the tab.
+TEST_P(ChromeSecurityStateModelClientHistogramTest,
+       HTTPConsoleWarningTabClosedHistogram) {
+  // Same as the test above, but ensuring that the histogram gets

[elawrence, 2016/11/15 16:29:53]

"test above"

[estark, 2016/11/16 05:16:14]

Done.

+  // recorded even if the command-line switch to show the omnibox
+  // warning is not set.

[elawrence, 2016/11/15 16:29:53]

"warning is not set"

Do we explicitly need to set the desired flag value, because presumably the
command line default will eventually change?

[estark, 2016/11/16 05:16:13]

Done.

+  base::HistogramTester histograms;
+  SignalSensitiveInput();
+  // Destroy the WebContents to simulate the tab being closed.
+  SetContents(nullptr);
+  histograms.ExpectTotalCount(kHTTPBadNavigationHistogram, 0);
+  histograms.ExpectTotalCount(kHTTPBadWebContentsDestroyedHistogram, 1);
+}
+
 // Tests that UMA logs the omnibox warning when security level is
 // HTTP_SHOW_WARNING.
 TEST_P(ChromeSecurityStateModelClientHistogramTest,
@@ -317,18 +392,18 @@ TEST_P(ChromeSecurityStateModelClientHistogramTest,
       security_state::switches::kMarkHttpWithPasswordsOrCcWithChip);
 
   base::HistogramTester histograms;
-  signal_sensitive_input();
-  histograms.ExpectUniqueSample(histogram_name(), true, 1);
+  SignalSensitiveInput();
+  histograms.ExpectUniqueSample(GetHistogramName(), true, 1);
 
   // Fire again and ensure no sample is recorded.
-  signal_sensitive_input();
-  histograms.ExpectUniqueSample(histogram_name(), true, 1);
+  SignalSensitiveInput();
+  histograms.ExpectUniqueSample(GetHistogramName(), true, 1);
 
   // Navigate to a new page and ensure a sample is recorded.
-  navigate_to_different_http_page();
-  histograms.ExpectUniqueSample(histogram_name(), true, 1);
-  signal_sensitive_input();
-  histograms.ExpectUniqueSample(histogram_name(), true, 2);
+  NavigateToDifferentHTTPPage();
+  histograms.ExpectUniqueSample(GetHistogramName(), true, 1);
+  SignalSensitiveInput();
+  histograms.ExpectUniqueSample(GetHistogramName(), true, 2);
 }
 
 // Tests that UMA logs the console warning when security level is NONE.
@@ -340,18 +415,18 @@ TEST_P(ChromeSecurityStateModelClientHistogramTest,
       security_state::switches::kMarkHttpAsNeutral);
 
   base::HistogramTester histograms;
-  signal_sensitive_input();
-  histograms.ExpectUniqueSample(histogram_name(), false, 1);
+  SignalSensitiveInput();
+  histograms.ExpectUniqueSample(GetHistogramName(), false, 1);
 
   // Fire again and ensure no sample is recorded.
-  signal_sensitive_input();
-  histograms.ExpectUniqueSample(histogram_name(), false, 1);
+  SignalSensitiveInput();
+  histograms.ExpectUniqueSample(GetHistogramName(), false, 1);
 
   // Navigate to a new page and ensure a sample is recorded.
-  navigate_to_different_http_page();
-  histograms.ExpectUniqueSample(histogram_name(), false, 1);
-  signal_sensitive_input();
-  histograms.ExpectUniqueSample(histogram_name(), false, 2);
+  NavigateToDifferentHTTPPage();
+  histograms.ExpectUniqueSample(GetHistogramName(), false, 1);
+  SignalSensitiveInput();
+  histograms.ExpectUniqueSample(GetHistogramName(), false, 2);
 }
 
 INSTANTIATE_TEST_CASE_P(ChromeSecurityStateModelClientHistogramTest,