Add metrics tracking how long Page Info is open

Per a suggestion of martinshelton@, this adds a new set of histograms to
track how long users have the Page Info bubble open. This will help us
better understand users who open Page Info but take no actions in it
(e.g., are these opens likely to be more accidental or incidental).

This metric is split by the security level of the page to allow us to
see differences between different security indicators. Two sub-histograms
("Action" and "NoAction") further split up the metric by whether the user
performed an action in the Page Info bubble before closing it (including
actions that cause the bubble to close).

Bug: 859116
Change-Id: I630a1f37ecff13bc7ee44b3e8e5a5c34337c9267
Reviewed-on: https://chromium-review.googlesource.com/1096336
Commit-Queue: Christopher Thompson <cthomp@chromium.org>
Reviewed-by: Steven Holte <holte@chromium.org>
Reviewed-by: Emily Stark <estark@chromium.org>
Cr-Commit-Position: refs/heads/master@{#574381}

chrome/browser/ui/page_info/page_info.cc

@@ -16,6 +16,7 @@
 #include "base/i18n/time_formatting.h"
 #include "base/macros.h"
 #include "base/metrics/field_trial.h"
+#include "base/metrics/histogram_functions.h"
 #include "base/metrics/histogram_macros.h"
 #include "base/metrics/user_metrics.h"
 #include "base/strings/string_number_conversions.h"
@@ -302,6 +303,37 @@ const PageInfo::ChooserUIInfo kChooserUIInfo[] = {
      IDS_PAGE_INFO_DELETE_USB_DEVICE, "name"},
 };
 
+// Time open histogram prefixes.
+const char kPageInfoTimePrefix[] = "Security.PageInfo.TimeOpen";
+const char kPageInfoTimeActionPrefix[] = "Security.PageInfo.TimeOpen.Action";
+const char kPageInfoTimeNoActionPrefix[] =
+    "Security.PageInfo.TimeOpen.NoAction";
+
+std::string GetHistogramSuffixForSecurityLevel(
+    security_state::SecurityLevel level) {
+  switch (level) {
+    case security_state::EV_SECURE:
+      return "EV_SECURE";
+    case security_state::SECURE:
+      return "SECURE";
+    case security_state::NONE:
+      return "NONE";
+    case security_state::HTTP_SHOW_WARNING:
+      return "HTTP_SHOW_WARNING";
+    case security_state::SECURE_WITH_POLICY_INSTALLED_CERT:
+      return "SECURE_WITH_POLICY_INSTALLED_CERT";
+    case security_state::DANGEROUS:
+      return "DANGEROUS";
+    default:
+      return "OTHER";
+  }
+}
+
+std::string GetHistogramName(const char* prefix,
+                             security_state::SecurityLevel level) {
+  return std::string(prefix) + "." + GetHistogramSuffixForSecurityLevel(level);
+}
+
 }  // namespace
 
 PageInfo::PageInfo(PageInfoUI* ui,
@@ -330,7 +362,8 @@ PageInfo::PageInfo(PageInfoUI* ui,
           safe_browsing::ChromePasswordProtectionService::
               GetPasswordProtectionService(profile_)),
 #endif
-      show_change_password_buttons_(false) {
+      show_change_password_buttons_(false),
+      did_perform_action_(false) {
   Init(url, security_info);
 
   PresentSitePermissions();
@@ -340,6 +373,10 @@ PageInfo::PageInfo(PageInfoUI* ui,
   // Every time the Page Info UI is opened a |PageInfo| object is
   // created. So this counts how ofter the Page Info UI is opened.
   RecordPageInfoAction(PAGE_INFO_OPENED);
+
+  // Record the time when the Page Info UI is opened so the total time it is
+  // open can be measured.
+  start_time_ = base::TimeTicks::Now();
 }
 
 PageInfo::~PageInfo() {
@@ -353,9 +390,32 @@ PageInfo::~PageInfo() {
                               user_decision,
                               END_OF_SSL_CERTIFICATE_DECISIONS_DID_REVOKE_ENUM);
   }
+
+  // Record the total time the Page Info UI was open for all opens as well as
+  // split between whether any action was taken.
+  base::UmaHistogramCustomTimes(
+      GetHistogramName(kPageInfoTimePrefix, security_level_),
+      base::TimeTicks::Now() - start_time_,
+      base::TimeDelta::FromMilliseconds(1), base::TimeDelta::FromHours(1), 100);
+  if (did_perform_action_) {
+    base::UmaHistogramCustomTimes(
+        GetHistogramName(kPageInfoTimeActionPrefix, security_level_),
+        base::TimeTicks::Now() - start_time_,
+        base::TimeDelta::FromMilliseconds(1), base::TimeDelta::FromHours(1),
+        100);
+  } else {
+    base::UmaHistogramCustomTimes(
+        GetHistogramName(kPageInfoTimeNoActionPrefix, security_level_),
+        base::TimeTicks::Now() - start_time_,
+        base::TimeDelta::FromMilliseconds(1), base::TimeDelta::FromHours(1),
+        100);
+  }
 }
 
 void PageInfo::RecordPageInfoAction(PageInfoAction action) {
+  if (action != PAGE_INFO_OPENED)
+    did_perform_action_ = true;
+
   UMA_HISTOGRAM_ENUMERATION("WebsiteSettings.Action", action, PAGE_INFO_COUNT);
 
   std::string histogram_name;

chrome/browser/ui/page_info/page_info.h

@@ -289,6 +289,9 @@ class PageInfo : public TabSpecificContentSettings::SiteDataObserver,
   // whitelist current site.
   bool show_change_password_buttons_;
 
+  base::TimeTicks start_time_;
+  bool did_perform_action_;
+
   DISALLOW_COPY_AND_ASSIGN(PageInfo);
 };
 

chrome/browser/ui/page_info/page_info_unittest.cc

@@ -983,6 +983,71 @@ TEST_F(PageInfoTest, SecurityLevelMetrics) {
   }
 }
 
+// Tests that the duration of time the PageInfo is open is recorded for pages
+// with various security levels.
+TEST_F(PageInfoTest, TimeOpenMetrics) {
+  struct TestCase {
+    const std::string url;
+    const security_state::SecurityLevel security_level;
+    const std::string security_level_name;
+    const PageInfo::PageInfoAction action;
+  };
+
+  const std::string kHistogramPrefix("Security.PageInfo.TimeOpen.");
+
+  const TestCase kTestCases[] = {
+      // PAGE_INFO_COUNT used as shorthand for "take no action".
+      {"https://example.test", security_state::SECURE, "SECURE",
+       PageInfo::PAGE_INFO_COUNT},
+      {"https://example.test", security_state::EV_SECURE, "EV_SECURE",
+       PageInfo::PAGE_INFO_COUNT},
+      {"http://example.test", security_state::NONE, "NONE",
+       PageInfo::PAGE_INFO_COUNT},
+      {"https://example.test", security_state::SECURE, "SECURE",
+       PageInfo::PAGE_INFO_SITE_SETTINGS_OPENED},
+      {"https://example.test", security_state::EV_SECURE, "EV_SECURE",
+       PageInfo::PAGE_INFO_SITE_SETTINGS_OPENED},
+      {"http://example.test", security_state::NONE, "NONE",
+       PageInfo::PAGE_INFO_SITE_SETTINGS_OPENED},
+  };
+
+  for (const auto& test : kTestCases) {
+    base::HistogramTester histograms;
+    SetURL(test.url);
+    security_info_.security_level = test.security_level;
+    ResetMockUI();
+    ClearPageInfo();
+    SetDefaultUIExpectations(mock_ui());
+
+    histograms.ExpectTotalCount(kHistogramPrefix + test.security_level_name, 0);
+    histograms.ExpectTotalCount(
+        kHistogramPrefix + "Action." + test.security_level_name, 0);
+    histograms.ExpectTotalCount(
+        kHistogramPrefix + "NoAction." + test.security_level_name, 0);
+
+    PageInfo* test_page_info = page_info();
+    if (test.action != PageInfo::PAGE_INFO_COUNT) {
+      test_page_info->RecordPageInfoAction(test.action);
+    }
+    ClearPageInfo();
+
+    histograms.ExpectTotalCount(kHistogramPrefix + test.security_level_name, 1);
+
+    if (test.action != PageInfo::PAGE_INFO_COUNT) {
+      histograms.ExpectTotalCount(
+          kHistogramPrefix + "Action." + test.security_level_name, 1);
+    } else {
+      histograms.ExpectTotalCount(
+          kHistogramPrefix + "NoAction." + test.security_level_name, 1);
+    }
+  }
+
+  // PageInfoTest expects a valid PageInfo instance to exist at end of test.
+  ResetMockUI();
+  SetDefaultUIExpectations(mock_ui());
+  page_info();
+}
+
 // Tests that the SubresourceFilter setting is omitted correctly.
 TEST_F(PageInfoTest, SubresourceFilterSetting_MatchesActivation) {
   auto showing_setting = [](const PermissionInfoList& permissions) {

tools/metrics/histograms/histograms.xml

@@ -88953,6 +88953,30 @@ uploading your change for review.
   </summary>
 </histogram>
 
+<histogram name="Security.PageInfo.TimeOpen">
+  <owner>cthomp@chromium.org</owner>
+  <summary>
+    Records the amount of time the Page Info bubble is open before the user
+    closes it or takes an action which closes it.
+  </summary>
+</histogram>
+
+<histogram name="Security.PageInfo.TimeOpen.Action">
+  <owner>cthomp@chromium.org</owner>
+  <summary>
+    Records the amount of time the Page Info bubble is open before the user
+    closes it, for cases where the user has performed an action inside it.
+  </summary>
+</histogram>
+
+<histogram name="Security.PageInfo.TimeOpen.NoAction">
+  <owner>cthomp@chromium.org</owner>
+  <summary>
+    Records the amount of time the Page Info bubble is open before the user
+    closes it, for cases where the user performed no action inside it.
+  </summary>
+</histogram>
+
 <histogram name="Security.SecurityLevel.CryptographicScheme"
     enum="SecurityLevel">
   <owner>estark@chromium.org</owner>
@@ -124791,6 +124815,9 @@ uploading your change for review.
   <affected-histogram name="Autofill.UserHappiness.Password"/>
   <affected-histogram name="Autofill.UserHappiness.Unknown"/>
   <affected-histogram name="Security.PageEndReason"/>
+  <affected-histogram name="Security.PageInfo.TimeOpen"/>
+  <affected-histogram name="Security.PageInfo.TimeOpen.Action"/>
+  <affected-histogram name="Security.PageInfo.TimeOpen.NoAction"/>
   <affected-histogram name="Security.SiteEngagement"/>
   <affected-histogram name="Security.SiteEngagementDelta"/>
   <affected-histogram name="Security.TimeOnPage"/>