Popup metrics: Add initiated/blocked/clicked through metric

This will allow us to calculate:
- % of popups blocked
- % of popups clicked through

Bug: 762617
Change-Id: Iaa850d03adb75e04b629b2d8623b0680b7dd095d
Reviewed-on: https://chromium-review.googlesource.com/658225
Reviewed-by: Steven Holte <[email protected]>
Reviewed-by: Avi Drissman <[email protected]>
Commit-Queue: Charlie Harrison <[email protected]>
Cr-Original-Commit-Position: refs/heads/master@{#501038}(cherry picked from commit afff863)
Reviewed-on: https://chromium-review.googlesource.com/667057
Cr-Commit-Position: refs/branch-heads/3202@{crosswalk-project#218}
Cr-Branched-From: fa6a5d8-refs/heads/master@{#499098}

Author: csharrison

chrome/browser/ui/blocked_content/popup_blocker_browsertest.cc

@@ -348,6 +348,45 @@ IN_PROC_BROWSER_TEST_F(PopupBlockerBrowserTest, PopupPositionMetrics) {
   tester.ExpectTotalCount(kClickThroughPosition, 4);
 }
 
+IN_PROC_BROWSER_TEST_F(PopupBlockerBrowserTest, PopupMetrics) {
+  const char kPopupActions[] = "ContentSettings.Popups.BlockerActions";
+  base::HistogramTester tester;
+
+  const GURL url(
+      embedded_test_server()->GetURL("/popup_blocker/popup-many.html"));
+  ui_test_utils::NavigateToURL(browser(), url);
+  EXPECT_EQ(2, GetBlockedContentsCount());
+
+  tester.ExpectBucketCount(
+      kPopupActions,
+      static_cast<int>(PopupBlockerTabHelper::Action::kInitiated), 2);
+  tester.ExpectBucketCount(
+      kPopupActions, static_cast<int>(PopupBlockerTabHelper::Action::kBlocked),
+      2);
+
+  // Click through one of them.
+  auto* popup_blocker = PopupBlockerTabHelper::FromWebContents(
+      browser()->tab_strip_model()->GetActiveWebContents());
+  popup_blocker->ShowBlockedPopup(
+      popup_blocker->GetBlockedPopupRequests().begin()->first,
+      WindowOpenDisposition::NEW_BACKGROUND_TAB);
+
+  tester.ExpectBucketCount(
+      kPopupActions,
+      static_cast<int>(PopupBlockerTabHelper::Action::kClickedThrough), 1);
+
+  // Whitelist the site and navigate again.
+  HostContentSettingsMapFactory::GetForProfile(browser()->profile())
+      ->SetContentSettingDefaultScope(url, GURL(), CONTENT_SETTINGS_TYPE_POPUPS,
+                                      std::string(), CONTENT_SETTING_ALLOW);
+  ui_test_utils::NavigateToURL(browser(), url);
+  tester.ExpectBucketCount(
+      kPopupActions,
+      static_cast<int>(PopupBlockerTabHelper::Action::kInitiated), 4);
+  // 4 initiated popups, 2 blocked, and 1 clicked through.
+  tester.ExpectTotalCount(kPopupActions, 4 + 2 + 1);
+}
+
 IN_PROC_BROWSER_TEST_F(PopupBlockerBrowserTest, MultiplePopups) {
   GURL url(embedded_test_server()->GetURL("/popup_blocker/popup-many.html"));
   ui_test_utils::NavigateToURL(browser(), url);

chrome/browser/ui/blocked_content/popup_blocker_tab_helper.cc

@@ -110,6 +110,8 @@ bool PopupBlockerTabHelper::MaybeBlockPopup(
   DCHECK(!open_url_params ||
          open_url_params->user_gesture == params.user_gesture);
 
+  LogAction(Action::kInitiated);
+
   const bool user_gesture = params.user_gesture;
   if (!web_contents)
     return false;
@@ -160,6 +162,7 @@ bool PopupBlockerTabHelper::MaybeBlockPopup(
 void PopupBlockerTabHelper::AddBlockedPopup(
     const chrome::NavigateParams& params,
     const blink::mojom::WindowFeatures& window_features) {
+  LogAction(Action::kBlocked);
   if (blocked_popups_.size() >= kMaximumNumberOfPopups)
     return;
 
@@ -208,6 +211,7 @@ void PopupBlockerTabHelper::ShowBlockedPopup(
   blocked_popups_.erase(id);
   if (blocked_popups_.empty())
     PopupNotificationVisibilityChanged(false);
+  LogAction(Action::kClickedThrough);
 }
 
 size_t PopupBlockerTabHelper::GetBlockedPopupsCount() const {
@@ -237,3 +241,9 @@ PopupBlockerTabHelper::PopupPosition PopupBlockerTabHelper::GetPopupPosition(
 
   return PopupPosition::kMiddlePopup;
 }
+
+// static
+void PopupBlockerTabHelper::LogAction(Action action) {
+  UMA_HISTOGRAM_ENUMERATION("ContentSettings.Popups.BlockerActions", action,
+                            Action::kLast);
+}

chrome/browser/ui/blocked_content/popup_blocker_tab_helper.h

@@ -48,6 +48,24 @@ class PopupBlockerTabHelper
     // Any new values should go before this one.
     kLast,
   };
+
+  // This enum is backed by a histogram. Make sure enums.xml is updated if this
+  // is updated.
+  enum class Action : int {
+    // A popup was initiated and was sent to the popup blocker for
+    // consideration.
+    kInitiated,
+
+    // A popup was blocked by the popup blocker.
+    kBlocked,
+
+    // A previously blocked popup was clicked through.
+    kClickedThrough,
+
+    // Add new elements before this value.
+    kLast
+  };
+
   class Observer {
    public:
     virtual void BlockedPopupAdded(int32_t id, const GURL& url) {}
@@ -108,6 +126,8 @@ class PopupBlockerTabHelper
 
   PopupPosition GetPopupPosition(int32_t id) const;
 
+  static void LogAction(Action action);
+
   // Note, this container should be sorted based on the position in the popup
   // list, so it is keyed by an id which is continually increased.
   std::map<int32_t, std::unique_ptr<BlockedRequest>> blocked_popups_;

tools/metrics/histograms/enums.xml

@@ -30706,6 +30706,12 @@ from previous Chrome versions.
   <int value="11" label="Bad Key Validation Signature"/>
 </enum>
 
+<enum name="PopupBlockerAction">
+  <int value="0" label="Popup initiated"/>
+  <int value="1" label="Popup blocked"/>
+  <int value="2" label="Popup clicked through"/>
+</enum>
+
 <enum name="PopupPosition">
   <int value="0" label="Only popup"/>
   <int value="1" label="First popup"/>

tools/metrics/histograms/histograms.xml

@@ -10218,6 +10218,17 @@ http://cs/file:chrome/histograms.xml - but prefer this file for new entries.
   </summary>
 </histogram>
 
+<histogram name="ContentSettings.Popups.BlockerActions"
+    enum="PopupBlockerAction">
+  <owner>[email protected]</owner>
+  <summary>
+    Counts of various events related to the popup blocker. Including blocked
+    popups and overridden (clicked through) popups. This is similar to the
+    ContentSettings.Popups but is at the per-popup layer rather than at the UI
+    layer.
+  </summary>
+</histogram>
+
 <histogram name="ContentSettings.Popups.ClickThroughPosition"
     enum="PopupPosition">
   <owner>[email protected]</owner>