diff --git a/chrome/browser/ui/blocked_content/popup_blocker_browsertest.cc b/chrome/browser/ui/blocked_content/popup_blocker_browsertest.cc index 3b2028774acd3..da155f07d7832 100644 --- a/chrome/browser/ui/blocked_content/popup_blocker_browsertest.cc +++ b/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(PopupBlockerTabHelper::Action::kInitiated), 2); + tester.ExpectBucketCount( + kPopupActions, static_cast(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(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(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); diff --git a/chrome/browser/ui/blocked_content/popup_blocker_tab_helper.cc b/chrome/browser/ui/blocked_content/popup_blocker_tab_helper.cc index d77b5886f0a36..2c100b83f7ff3 100644 --- a/chrome/browser/ui/blocked_content/popup_blocker_tab_helper.cc +++ b/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); +} diff --git a/chrome/browser/ui/blocked_content/popup_blocker_tab_helper.h b/chrome/browser/ui/blocked_content/popup_blocker_tab_helper.h index f2c17e8dcfa18..f5f077b8204a3 100644 --- a/chrome/browser/ui/blocked_content/popup_blocker_tab_helper.h +++ b/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> blocked_popups_; diff --git a/tools/metrics/histograms/enums.xml b/tools/metrics/histograms/enums.xml index e06f9e363b2ae..3b0d35f954b51 100644 --- a/tools/metrics/histograms/enums.xml +++ b/tools/metrics/histograms/enums.xml @@ -30706,6 +30706,12 @@ from previous Chrome versions. + + + + + + diff --git a/tools/metrics/histograms/histograms.xml b/tools/metrics/histograms/histograms.xml index 843a16672f0b3..40e02e6a72085 100644 --- a/tools/metrics/histograms/histograms.xml +++ b/tools/metrics/histograms/histograms.xml @@ -10218,6 +10218,17 @@ http://cs/file:chrome/histograms.xml - but prefer this file for new entries. + + csharrison@chromium.org + + 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. + + + csharrison@chromium.org