Expand WebsiteSettings histograms for HTTP-bad

This CL expands WebsiteSettings histograms so that we can track actions
broken down by each of the following omnibox states:
- valid HTTPS url
- HTTPS URL that has been downgraded because of e.g. mixed content
- HTTPS URL that is marked dangerous (e.g. broken HTTPS or malware)
- HTTP URL that is marked dangerous (e.g. malware)
- HTTP URL that has a "Not secure" warning in the omnibox

BUG=647566

Review-Url: https://codereview.chromium.org/2434083002
Cr-Commit-Position: refs/heads/master@{#427017}

chrome/browser/ui/website_settings/website_settings.cc

@@ -268,7 +268,8 @@ WebsiteSettings::WebsiteSettings(
       chrome_ssl_host_state_delegate_(
           ChromeSSLHostStateDelegateFactory::GetForProfile(profile)),
       did_revoke_user_ssl_decisions_(false),
-      profile_(profile) {
+      profile_(profile),
+      security_level_(security_state::SecurityStateModel::NONE) {
   Init(url, security_info);
 
   PresentSitePermissions();
@@ -289,20 +290,34 @@ void WebsiteSettings::RecordWebsiteSettingsAction(
                             action,
                             WEBSITE_SETTINGS_COUNT);
 
-  // Use a separate histogram to record actions if they are done on a page with
-  // an HTTPS URL. Note that this *disregards* security status.
-  //
-
-  // TODO(palmer): Consider adding a new histogram for
-  // GURL::SchemeIsCryptographic. (We don't want to replace this call with a
-  // call to that function because we don't want to change the meanings of
-  // existing metrics.) This would inform the decision to mark non-secure
-  // origins as Dubious or Non-Secure; the overall bug for that is
-  // crbug.com/454579.
-  if (site_url_.SchemeIs(url::kHttpsScheme)) {
-    UMA_HISTOGRAM_ENUMERATION("WebsiteSettings.Action.HttpsUrl",
-                              action,
-                              WEBSITE_SETTINGS_COUNT);
+  std::string histogram_name;
+
+  if (site_url_.SchemeIsCryptographic()) {
+    if (security_level_ == security_state::SecurityStateModel::SECURE ||
+        security_level_ == security_state::SecurityStateModel::EV_SECURE) {
+      UMA_HISTOGRAM_ENUMERATION("Security.PageInfo.Action.HttpsUrl.Valid",
+                                action, WEBSITE_SETTINGS_COUNT);
+    } else if (security_level_ == security_state::SecurityStateModel::NONE) {
+      UMA_HISTOGRAM_ENUMERATION("Security.PageInfo.Action.HttpsUrl.Downgraded",
+                                action, WEBSITE_SETTINGS_COUNT);
+    } else if (security_level_ ==
+               security_state::SecurityStateModel::DANGEROUS) {
+      UMA_HISTOGRAM_ENUMERATION("Security.PageInfo.Action.HttpsUrl.Dangerous",
+                                action, WEBSITE_SETTINGS_COUNT);
+    }
+    return;
+  }
+
+  if (security_level_ ==
+      security_state::SecurityStateModel::HTTP_SHOW_WARNING) {
+    UMA_HISTOGRAM_ENUMERATION("Security.PageInfo.Action.HttpUrl.Warning",
+                              action, WEBSITE_SETTINGS_COUNT);
+  } else if (security_level_ == security_state::SecurityStateModel::DANGEROUS) {
+    UMA_HISTOGRAM_ENUMERATION("Security.PageInfo.Action.HttpUrl.Dangerous",
+                              action, WEBSITE_SETTINGS_COUNT);
+  } else {
+    UMA_HISTOGRAM_ENUMERATION("Security.PageInfo.Action.HttpUrl.Neutral",
+                              action, WEBSITE_SETTINGS_COUNT);
   }
 }
 
@@ -412,6 +427,8 @@ void WebsiteSettings::Init(
   isChromeUINativeScheme = url.SchemeIs(chrome::kChromeUINativeScheme);
 #endif
 
+  security_level_ = security_info.security_level;
+
   if (url.SchemeIs(url::kAboutScheme)) {
     // All about: URLs except about:blank are redirected.
     DCHECK_EQ(url::kAboutBlankURL, url.spec());

chrome/browser/ui/website_settings/website_settings.h

@@ -236,6 +236,8 @@ class WebsiteSettings : public TabSpecificContentSettings::SiteDataObserver,
 
   Profile* profile_;
 
+  security_state::SecurityStateModel::SecurityLevel security_level_;
+
   DISALLOW_COPY_AND_ASSIGN(WebsiteSettings);
 };
 

chrome/browser/ui/website_settings/website_settings_unittest.cc

@@ -13,6 +13,7 @@
 #include "base/message_loop/message_loop.h"
 #include "base/strings/string16.h"
 #include "base/strings/utf_string_conversions.h"
+#include "base/test/histogram_tester.h"
 #include "build/build_config.h"
 #include "chrome/browser/content_settings/host_content_settings_map_factory.h"
 #include "chrome/browser/infobars/infobar_service.h"
@@ -893,3 +894,68 @@ TEST_F(WebsiteSettingsTest, InternalPage) {
   EXPECT_EQ(base::string16(), website_settings()->organization_name());
 }
 #endif
+
+// Tests that metrics are recorded on a WebsiteSettings for pages with
+// various security levels.
+TEST_F(WebsiteSettingsTest, SecurityLevelMetrics) {
+  struct TestCase {
+    const std::string url;
+    const SecurityStateModel::SecurityLevel security_level;
+    const std::string histogram_name;
+  };
+  const char kGenericHistogram[] = "WebsiteSettings.Action";
+
+  const TestCase kTestCases[] = {
+      {"https://example.test", SecurityStateModel::SECURE,
+       "Security.PageInfo.Action.HttpsUrl.Valid"},
+      {"https://example.test", SecurityStateModel::EV_SECURE,
+       "Security.PageInfo.Action.HttpsUrl.Valid"},
+      {"https://example2.test", SecurityStateModel::NONE,
+       "Security.PageInfo.Action.HttpsUrl.Downgraded"},
+      {"https://example.test", SecurityStateModel::DANGEROUS,
+       "Security.PageInfo.Action.HttpsUrl.Dangerous"},
+      {"http://example.test", SecurityStateModel::HTTP_SHOW_WARNING,
+       "Security.PageInfo.Action.HttpUrl.Warning"},
+      {"http://example.test", SecurityStateModel::DANGEROUS,
+       "Security.PageInfo.Action.HttpUrl.Dangerous"},
+      {"http://example.test", SecurityStateModel::NONE,
+       "Security.PageInfo.Action.HttpUrl.Neutral"},
+  };
+
+  for (const auto& test : kTestCases) {
+    base::HistogramTester histograms;
+    SetURL(test.url);
+    security_info_.security_level = test.security_level;
+    ResetMockUI();
+    ClearWebsiteSettings();
+    SetDefaultUIExpectations(mock_ui());
+
+    histograms.ExpectTotalCount(kGenericHistogram, 0);
+    histograms.ExpectTotalCount(test.histogram_name, 0);
+
+    website_settings()->RecordWebsiteSettingsAction(
+        WebsiteSettings::WebsiteSettingsAction::
+            WEBSITE_SETTINGS_PERMISSIONS_TAB_SELECTED);
+
+    // RecordWebsiteSettingsAction() is called during WebsiteSettings
+    // creation in addition to the explicit RecordWebsiteSettingsAction()
+    // call, so it is called twice in total.
+    histograms.ExpectTotalCount(kGenericHistogram, 2);
+    histograms.ExpectBucketCount(
+        kGenericHistogram,
+        WebsiteSettings::WebsiteSettingsAction::WEBSITE_SETTINGS_OPENED, 1);
+    histograms.ExpectBucketCount(kGenericHistogram,
+                                 WebsiteSettings::WebsiteSettingsAction::
+                                     WEBSITE_SETTINGS_PERMISSIONS_TAB_SELECTED,
+                                 1);
+
+    histograms.ExpectTotalCount(test.histogram_name, 2);
+    histograms.ExpectBucketCount(
+        test.histogram_name,
+        WebsiteSettings::WebsiteSettingsAction::WEBSITE_SETTINGS_OPENED, 1);
+    histograms.ExpectBucketCount(test.histogram_name,
+                                 WebsiteSettings::WebsiteSettingsAction::
+                                     WEBSITE_SETTINGS_PERMISSIONS_TAB_SELECTED,
+                                 1);
+  }
+}

tools/metrics/histograms/histograms.xml

@@ -56045,6 +56045,66 @@ http://cs/file:chrome/histograms.xml - but prefer this file for new entries.
   </summary>
 </histogram>
 
+<histogram name="Security.PageInfo.Action.HttpsUrl.Dangerous"
+    enum="WebsiteSettingsAction">
+  <owner>estark@chromium.org</owner>
+  <owner>lgarron@chromium.org</owner>
+  <summary>
+    Tracks Page Info bubble actions that take place on an HTTPS URL that has
+    been marked dangerous or not secure (such as for malware or broken HTTPS).
+  </summary>
+</histogram>
+
+<histogram name="Security.PageInfo.Action.HttpsUrl.Downgraded"
+    enum="WebsiteSettingsAction">
+  <owner>estark@chromium.org</owner>
+  <owner>lgarron@chromium.org</owner>
+  <summary>
+    Tracks Page Info bubble actions that take place on a valid HTTPS URL that
+    has a security issue (e.g. mixed content).
+  </summary>
+</histogram>
+
+<histogram name="Security.PageInfo.Action.HttpsUrl.Valid"
+    enum="WebsiteSettingsAction">
+  <owner>estark@chromium.org</owner>
+  <owner>lgarron@chromium.org</owner>
+  <summary>
+    Tracks Page Info bubble actions that take place on a valid HTTPS URL with no
+    security issues (e.g. no mixed content).
+  </summary>
+</histogram>
+
+<histogram name="Security.PageInfo.Action.HttpUrl.Dangerous"
+    enum="WebsiteSettingsAction">
+  <owner>estark@chromium.org</owner>
+  <owner>lgarron@chromium.org</owner>
+  <summary>
+    Tracks Page Info bubble actions that take place on an HTTP URL that has been
+    marked dangerous (such as for malware).
+  </summary>
+</histogram>
+
+<histogram name="Security.PageInfo.Action.HttpUrl.Neutral"
+    enum="WebsiteSettingsAction">
+  <owner>estark@chromium.org</owner>
+  <owner>lgarron@chromium.org</owner>
+  <summary>
+    Tracks Page Info bubble actions that take place on an HTTP URL that does not
+    have an omnibox security indicator warning associated with it.
+  </summary>
+</histogram>
+
+<histogram name="Security.PageInfo.Action.HttpUrl.Warning"
+    enum="WebsiteSettingsAction">
+  <owner>estark@chromium.org</owner>
+  <owner>lgarron@chromium.org</owner>
+  <summary>
+    Tracks Page Info bubble actions that take place on an HTTP URL that has been
+    given a &quot;Not secure&quot; warning in the omnibox.
+  </summary>
+</histogram>
+
 <histogram name="SequencedWorkerPool.ShutdownDelayTime" units="ms">
   <owner>gab@chromium.org</owner>
   <summary>
@@ -71750,6 +71810,11 @@ http://cs/file:chrome/histograms.xml - but prefer this file for new entries.
 </histogram>
 
 <histogram name="WebsiteSettings.Action.HttpsUrl" enum="WebsiteSettingsAction">
+  <obsolete>
+    Deprecated October 2016 in favor of Security.PageInfo.Action.HttpsUrl.Valid,
+    Security.PageInfo.Action.HttpsUrl.Dangerous, and
+    Security.PageInfo.Action.HttpsUrl.Downgraded.
+  </obsolete>
   <owner>lgarron@chromium.org</owner>
   <summary>
     Tracks WebsiteSettings actions that take place on an HTTPS URL. This