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

chrome/browser/ssl/chrome_security_state_model_client_unittest.cc

 // Copyright 2016 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/ssl/chrome_security_state_model_client.h"
 
 #include "base/command_line.h"
 #include "base/test/histogram_tester.h"
 #include "chrome/test/base/chrome_render_view_host_test_harness.h"
 #include "components/security_state/security_state_model.h"
 #include "components/security_state/switches.h"
 #include "content/public/browser/security_style_explanation.h"
 #include "content/public/browser/security_style_explanations.h"
 #include "net/cert/cert_status_flags.h"
 #include "net/ssl/ssl_cipher_suite_names.h"
 #include "net/ssl/ssl_connection_status_flags.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
 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.
 TEST(ChromeSecurityStateModelClientTest,
      GetSecurityStyleForContentWithCertErrors) {
   content::SecurityStyleExplanations explanations;
   security_state::SecurityStateModel::SecurityInfo security_info;
   security_info.cert_status = 0;
   security_info.scheme_is_cryptographic = true;
 
@@ skipping 237 matching lines @@
       public testing::WithParamInterface<bool> {
  public:
   ChromeSecurityStateModelClientHistogramTest() {}
   ~ChromeSecurityStateModelClientHistogramTest() override {}
 
   void SetUp() override {
     ChromeRenderViewHostTestHarness::SetUp();
 
     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
       web_contents()->OnCreditCardInputShownOnHttp();
     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"));
   }
 
  private:
   ChromeSecurityStateModelClient* client_;
   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,
        HTTPOmniboxWarningHistogram) {
   // Show Warning Chip.
   base::CommandLine::ForCurrentProcess()->AppendSwitchASCII(
       security_state::switches::kMarkHttpAs,
       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.
 TEST_P(ChromeSecurityStateModelClientHistogramTest,
        HTTPConsoleWarningHistogram) {
   // Show Neutral for HTTP
   base::CommandLine::ForCurrentProcess()->AppendSwitchASCII(
       security_state::switches::kMarkHttpAs,
       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,
                         ChromeSecurityStateModelClientHistogramTest,
                         // Here 'true' to test password field triggered
                         // histogram and 'false' to test credit card field.
                         testing::Bool());
 
 }  // namespace