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

chrome/browser/ssl/chrome_security_state_model_client.cc

 // Copyright 2015 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 <vector>
 
 #include "base/command_line.h"
 #include "base/metrics/field_trial.h"
@@ skipping 177 matching lines @@
       security_state::SecurityStateModel::HTTP_SHOW_WARNING) {
     // If the HTTP_SHOW_WARNING field trial is in use, display an
     // unauthenticated explanation explaining why the omnibox warning is
     // present.
     security_style_explanations->unauthenticated_explanations.push_back(
         content::SecurityStyleExplanation(
             l10n_util::GetStringUTF8(IDS_PRIVATE_USER_DATA_INPUT),
             l10n_util::GetStringUTF8(IDS_PRIVATE_USER_DATA_INPUT_DESCRIPTION)));
   } else if (security_info.security_level ==
                  security_state::SecurityStateModel::NONE &&
-             security_info.displayed_private_user_data_input_on_http) {
+             (security_info.displayed_password_field_on_http ||
+              security_info.displayed_credit_card_field_on_http)) {
     // If the HTTP_SHOW_WARNING field trial isn't in use yet, display an
     // informational note that the omnibox will contain a warning for
     // this site in a future version of Chrome.
     security_style_explanations->info_explanations.push_back(
         content::SecurityStyleExplanation(
             l10n_util::GetStringUTF8(IDS_PRIVATE_USER_DATA_INPUT),
             l10n_util::GetStringUTF8(
                 IDS_PRIVATE_USER_DATA_INPUT_FUTURE_DESCRIPTION)));
   }
 
@@ skipping 117 matching lines @@
     SecurityStateModel::SecurityInfo* result) const {
   security_state_model_->GetSecurityInfo(result);
 }
 
 void ChromeSecurityStateModelClient::VisibleSecurityStateChanged() {
   if (logged_http_warning_on_current_navigation_)
     return;
 
   security_state::SecurityStateModel::SecurityInfo security_info;
   GetSecurityInfo(&security_info);
-  if (!security_info.displayed_private_user_data_input_on_http)
+  if (!security_info.displayed_password_field_on_http &&
+      !security_info.displayed_credit_card_field_on_http)

[estark, 2016/11/09 15:56:46]

nit: now that the conditional spans two lines, can you please add curly braces
around the body?

[lshang, 2016/11/10 04:53:54]

Done.

     return;
 
   std::string warning;
   bool warning_is_user_visible = false;
   switch (security_info.security_level) {
     case security_state::SecurityStateModel::HTTP_SHOW_WARNING:
       warning =
           "This page includes a password or credit card input in a non-secure "
           "context. A warning has been added to the URL bar. For more "
           "information, see https://goo.gl/zmWq3m.";
       warning_is_user_visible = true;
       break;
     case security_state::SecurityStateModel::NONE:
     case security_state::SecurityStateModel::DANGEROUS:
       warning =
           "This page includes a password or credit card input in a non-secure "
           "context. A warning will be added to the URL bar in Chrome 56 (Jan "
           "2017). For more information, see https://goo.gl/zmWq3m.";
       break;
     default:
       return;
   }
 
   logged_http_warning_on_current_navigation_ = true;
   web_contents_->GetMainFrame()->AddMessageToConsole(
       content::CONSOLE_MESSAGE_LEVEL_WARNING, warning);
-  UMA_HISTOGRAM_BOOLEAN("Security.HTTPBad.UserWarnedAboutSensitiveInput",
-                        warning_is_user_visible);
+
+  UMA_HISTOGRAM_BOOLEAN(
+      "Security.HTTPBad.UserWarnedAboutSensitiveInput.CreditCard",
+      security_info.displayed_credit_card_field_on_http &&

[estark, 2016/11/09 15:56:46]

Hmm. I think we ought to only record the histograms conditionally, like this:

if (security_info.displayed_credit_card_field_on_http) {
  UMA_HISTOGRAM_BOOLEAN("...CreditCard", warning_is_user_visible);
}
if (security_info.displayed_password_field_on_http) {
  UMA_HISTOGRAM_BOOLEAN("...Password", warning_is_user_visible);
}

That way a histogram value of false means "there was a password/credit card
field and we only logged a console message." As it is right now, with recording
the histogram unconditionally, a value of false could mean multiple things: it
could mean there was a password/cc field and we only logged a console message,
or it could mean there was no password/cc field.

[lshang, 2016/11/10 04:53:54]

Done.

+          warning_is_user_visible);
+  UMA_HISTOGRAM_BOOLEAN(
+      "Security.HTTPBad.UserWarnedAboutSensitiveInput.Password",
+      security_info.displayed_password_field_on_http &&
+          warning_is_user_visible);
 }
 
 void ChromeSecurityStateModelClient::DidFinishNavigation(
     content::NavigationHandle* navigation_handle) {
   if (navigation_handle->IsInMainFrame() && !navigation_handle->IsSamePage()) {
     // Only reset the console message flag for main-frame navigations,
     // and not for same-page navigations like reference fragments and pushState.
     logged_http_warning_on_current_navigation_ = false;
   }
 }
@@ skipping 54 matching lines @@
       !!(ssl.content_status & content::SSLStatus::RAN_CONTENT_WITH_CERT_ERRORS);
   state->displayed_password_field_on_http =
       !!(ssl.content_status &
          content::SSLStatus::DISPLAYED_PASSWORD_FIELD_ON_HTTP);
   state->displayed_credit_card_field_on_http =
       !!(ssl.content_status &
          content::SSLStatus::DISPLAYED_CREDIT_CARD_FIELD_ON_HTTP);
 
   CheckSafeBrowsingStatus(entry, web_contents_, state);
 }