Popups: Breakout "click through" metric by block type

We've launched abusive experience enforcement (aka the safe browsing
triggered popup blocker). However, we don't have great metrics moving
forward to discern click-through metrics from that feature vs the
standard popup blocker.

This CL breaks that out into another PopupBlockerTabHelper::Action.

Change-Id: I2f8277619ddc90532d3fef5136f746bc63130a62
Reviewed-on: https://chromium-review.googlesource.com/938585
Reviewed-by: Avi Drissman <avi@chromium.org>
Commit-Queue: Charlie Harrison <csharrison@chromium.org>
Cr-Commit-Position: refs/heads/master@{#539325}

chrome/browser/ui/blocked_content/popup_blocker_browsertest.cc

@@ -370,7 +370,8 @@ IN_PROC_BROWSER_TEST_F(PopupBlockerBrowserTest, PopupMetrics) {
 
   tester.ExpectBucketCount(
       kPopupActions,
-      static_cast<int>(PopupBlockerTabHelper::Action::kClickedThrough), 1);
+      static_cast<int>(PopupBlockerTabHelper::Action::kClickedThroughNoGesture),
+      1);
 
   // Whitelist the site and navigate again.
   HostContentSettingsMapFactory::GetForProfile(browser()->profile())

chrome/browser/ui/blocked_content/popup_blocker_tab_helper.cc

@@ -43,11 +43,15 @@ DEFINE_WEB_CONTENTS_USER_DATA_KEY(PopupBlockerTabHelper);
 
 struct PopupBlockerTabHelper::BlockedRequest {
   BlockedRequest(const NavigateParams& params,
-                 const blink::mojom::WindowFeatures& window_features)
-      : params(params), window_features(window_features) {}
+                 const blink::mojom::WindowFeatures& window_features,
+                 PopupBlockType block_type)
+      : params(params),
+        window_features(window_features),
+        block_type(block_type) {}
 
   NavigateParams params;
   blink::mojom::WindowFeatures window_features;
+  PopupBlockType block_type;
 };
 
 PopupBlockerTabHelper::PopupBlockerTabHelper(content::WebContents* web_contents)
@@ -144,6 +148,7 @@ bool PopupBlockerTabHelper::MaybeBlockPopup(
     return false;
   }
 
+  PopupBlockType block_type = PopupBlockType::kNoGesture;
   if (user_gesture) {
     auto* safe_browsing_blocker =
         popup_blocker->safe_browsing_triggered_popup_blocker_.get();
@@ -152,23 +157,25 @@ bool PopupBlockerTabHelper::MaybeBlockPopup(
             open_url_params)) {
       return false;
     }
+    block_type = PopupBlockType::kAbusive;
   }
 
-  popup_blocker->AddBlockedPopup(params, window_features);
+  popup_blocker->AddBlockedPopup(params, window_features, block_type);
   return true;
 }
 
 void PopupBlockerTabHelper::AddBlockedPopup(
     const NavigateParams& params,
-    const blink::mojom::WindowFeatures& window_features) {
+    const blink::mojom::WindowFeatures& window_features,
+    PopupBlockType block_type) {
   LogAction(Action::kBlocked);
   if (blocked_popups_.size() >= kMaximumNumberOfPopups)
     return;
 
   int id = next_id_;
   next_id_++;
   blocked_popups_[id] =
-      std::make_unique<BlockedRequest>(params, window_features);
+      std::make_unique<BlockedRequest>(params, window_features, block_type);
   TabSpecificContentSettings::FromWebContents(web_contents())->
       OnContentBlocked(CONTENT_SETTINGS_TYPE_POPUPS);
   for (auto& observer : observers_)
@@ -214,10 +221,18 @@ void PopupBlockerTabHelper::ShowBlockedPopup(
     }
   }
 
+  switch (popup->block_type) {
+    case PopupBlockType::kNoGesture:
+      LogAction(Action::kClickedThroughNoGesture);
+      break;
+    case PopupBlockType::kAbusive:
+      LogAction(Action::kClickedThroughAbusive);
+      break;
+  }
+
   blocked_popups_.erase(id);
   if (blocked_popups_.empty())
     PopupNotificationVisibilityChanged(false);
-  LogAction(Action::kClickedThrough);
 }
 
 size_t PopupBlockerTabHelper::GetBlockedPopupsCount() const {

chrome/browser/ui/blocked_content/popup_blocker_tab_helper.h

@@ -36,6 +36,14 @@ class PopupBlockerTabHelper
   // Mapping from popup IDs to blocked popup requests.
   typedef std::map<int32_t, GURL> PopupIdMap;
 
+  // Classifies what caused a popup to be blocked.
+  enum class PopupBlockType {
+    // Popup blocked due to no user gesture.
+    kNoGesture,
+    // Popup blocked due to the abusive popup blocker.
+    kAbusive,
+  };
+
   // This enum is backed by a histogram. Make sure enums.xml is updated if this
   // is updated.
   enum class Action : int {
@@ -46,8 +54,13 @@ class PopupBlockerTabHelper
     // A popup was blocked by the popup blocker.
     kBlocked,
 
-    // A previously blocked popup was clicked through.
-    kClickedThrough,
+    // A previously blocked popup was clicked through. For popups blocked
+    // without a user gesture.
+    kClickedThroughNoGesture,
+
+    // A previously blocked popup was clicked through. For popups blocked
+    // due to the abusive popup blocker.
+    kClickedThroughAbusive,
 
     // Add new elements before this value.
     kLast
@@ -106,7 +119,8 @@ class PopupBlockerTabHelper
   explicit PopupBlockerTabHelper(content::WebContents* web_contents);
 
   void AddBlockedPopup(const NavigateParams& params,
-                       const blink::mojom::WindowFeatures& window_features);
+                       const blink::mojom::WindowFeatures& window_features,
+                       PopupBlockType block_type);
 
   // Called when the blocked popup notification is shown or hidden.
   void PopupNotificationVisibilityChanged(bool visible);

chrome/browser/ui/blocked_content/safe_browsing_triggered_popup_blocker_browsertest.cc

@@ -19,7 +19,7 @@
 #include "chrome/browser/content_settings/tab_specific_content_settings.h"
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/safe_browsing/test_safe_browsing_database_helper.h"
-#include "chrome/browser/ui/blocked_content/safe_browsing_triggered_popup_blocker.h"
+#include "chrome/browser/ui/blocked_content/popup_blocker_tab_helper.h"
 #include "chrome/browser/ui/browser.h"
 #include "chrome/browser/ui/browser_commands.h"
 #include "chrome/browser/ui/tabs/tab_strip_model.h"
@@ -436,6 +436,46 @@ IN_PROC_BROWSER_TEST_F(SafeBrowsingTriggeredPopupBlockerBrowserTest,
                    ->IsContentBlocked(CONTENT_SETTINGS_TYPE_POPUPS));
 }
 
+IN_PROC_BROWSER_TEST_F(SafeBrowsingTriggeredPopupBlockerBrowserTest,
+                       ShowBlockedPopup) {
+  base::HistogramTester tester;
+
+  const char kWindowOpenPath[] = "/subresource_filter/window_open.html";
+  GURL a_url(embedded_test_server()->GetURL("a.com", kWindowOpenPath));
+  ConfigureAsAbusive(a_url);
+
+  // Navigate to a_url, should trigger the popup blocker.
+  ui_test_utils::NavigateToURL(browser(), a_url);
+  bool opened_window = false;
+  content::WebContents* web_contents =
+      browser()->tab_strip_model()->GetActiveWebContents();
+  EXPECT_TRUE(content::ExecuteScriptAndExtractBool(web_contents, "openWindow()",
+                                                   &opened_window));
+  EXPECT_FALSE(opened_window);
+
+  // Make sure the popup UI was shown.
+  EXPECT_TRUE(TabSpecificContentSettings::FromWebContents(web_contents)
+                  ->IsContentBlocked(CONTENT_SETTINGS_TYPE_POPUPS));
+
+  // Click through.
+  content::TestNavigationObserver navigation_observer(nullptr, 1);
+  navigation_observer.StartWatchingNewWebContents();
+  auto* popup_blocker = PopupBlockerTabHelper::FromWebContents(
+      browser()->tab_strip_model()->GetActiveWebContents());
+  popup_blocker->ShowBlockedPopup(
+      popup_blocker->GetBlockedPopupRequests().begin()->first,
+      WindowOpenDisposition::NEW_BACKGROUND_TAB);
+
+  const char kPopupActions[] = "ContentSettings.Popups.BlockerActions";
+  tester.ExpectBucketCount(
+      kPopupActions,
+      static_cast<int>(PopupBlockerTabHelper::Action::kClickedThroughAbusive),
+      1);
+
+  navigation_observer.Wait();
+  EXPECT_TRUE(navigation_observer.last_navigation_succeeded());
+}
+
 IN_PROC_BROWSER_TEST_F(SafeBrowsingTriggeredPopupBlockerBrowserTest,
                        BlockCreatingNewWindows_LogsToConsole) {
   content::ConsoleObserverDelegate console_observer(web_contents(),

tools/metrics/histograms/enums.xml

@@ -35213,7 +35213,8 @@ Called by update_net_trust_anchors.py.-->
 <enum name="PopupBlockerAction">
   <int value="0" label="Popup initiated"/>
   <int value="1" label="Popup blocked"/>
-  <int value="2" label="Popup clicked through"/>
+  <int value="2" label="Popup clicked through (no gesture)"/>
+  <int value="3" label="Popup clicked through (abusive)"/>
 </enum>
 
 <enum name="Ports">