Skip to content

Commit afff863

Browse files
csharrisonCommit Bot
authored and
Commit Bot
committed
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-Commit-Position: refs/heads/master@{#501038}
1 parent 2556416 commit afff863

File tree

5 files changed

+86
-0
lines changed

5 files changed

+86
-0
lines changed

chrome/browser/ui/blocked_content/popup_blocker_browsertest.cc

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -348,6 +348,45 @@ IN_PROC_BROWSER_TEST_F(PopupBlockerBrowserTest, PopupPositionMetrics) {
348348
tester.ExpectTotalCount(kClickThroughPosition, 4);
349349
}
350350

351+
IN_PROC_BROWSER_TEST_F(PopupBlockerBrowserTest, PopupMetrics) {
352+
const char kPopupActions[] = "ContentSettings.Popups.BlockerActions";
353+
base::HistogramTester tester;
354+
355+
const GURL url(
356+
embedded_test_server()->GetURL("/popup_blocker/popup-many.html"));
357+
ui_test_utils::NavigateToURL(browser(), url);
358+
EXPECT_EQ(2, GetBlockedContentsCount());
359+
360+
tester.ExpectBucketCount(
361+
kPopupActions,
362+
static_cast<int>(PopupBlockerTabHelper::Action::kInitiated), 2);
363+
tester.ExpectBucketCount(
364+
kPopupActions, static_cast<int>(PopupBlockerTabHelper::Action::kBlocked),
365+
2);
366+
367+
// Click through one of them.
368+
auto* popup_blocker = PopupBlockerTabHelper::FromWebContents(
369+
browser()->tab_strip_model()->GetActiveWebContents());
370+
popup_blocker->ShowBlockedPopup(
371+
popup_blocker->GetBlockedPopupRequests().begin()->first,
372+
WindowOpenDisposition::NEW_BACKGROUND_TAB);
373+
374+
tester.ExpectBucketCount(
375+
kPopupActions,
376+
static_cast<int>(PopupBlockerTabHelper::Action::kClickedThrough), 1);
377+
378+
// Whitelist the site and navigate again.
379+
HostContentSettingsMapFactory::GetForProfile(browser()->profile())
380+
->SetContentSettingDefaultScope(url, GURL(), CONTENT_SETTINGS_TYPE_POPUPS,
381+
std::string(), CONTENT_SETTING_ALLOW);
382+
ui_test_utils::NavigateToURL(browser(), url);
383+
tester.ExpectBucketCount(
384+
kPopupActions,
385+
static_cast<int>(PopupBlockerTabHelper::Action::kInitiated), 4);
386+
// 4 initiated popups, 2 blocked, and 1 clicked through.
387+
tester.ExpectTotalCount(kPopupActions, 4 + 2 + 1);
388+
}
389+
351390
IN_PROC_BROWSER_TEST_F(PopupBlockerBrowserTest, MultiplePopups) {
352391
GURL url(embedded_test_server()->GetURL("/popup_blocker/popup-many.html"));
353392
ui_test_utils::NavigateToURL(browser(), url);

chrome/browser/ui/blocked_content/popup_blocker_tab_helper.cc

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -110,6 +110,8 @@ bool PopupBlockerTabHelper::MaybeBlockPopup(
110110
DCHECK(!open_url_params ||
111111
open_url_params->user_gesture == params.user_gesture);
112112

113+
LogAction(Action::kInitiated);
114+
113115
const bool user_gesture = params.user_gesture;
114116
if (!web_contents)
115117
return false;
@@ -160,6 +162,7 @@ bool PopupBlockerTabHelper::MaybeBlockPopup(
160162
void PopupBlockerTabHelper::AddBlockedPopup(
161163
const chrome::NavigateParams& params,
162164
const blink::mojom::WindowFeatures& window_features) {
165+
LogAction(Action::kBlocked);
163166
if (blocked_popups_.size() >= kMaximumNumberOfPopups)
164167
return;
165168

@@ -208,6 +211,7 @@ void PopupBlockerTabHelper::ShowBlockedPopup(
208211
blocked_popups_.erase(id);
209212
if (blocked_popups_.empty())
210213
PopupNotificationVisibilityChanged(false);
214+
LogAction(Action::kClickedThrough);
211215
}
212216

213217
size_t PopupBlockerTabHelper::GetBlockedPopupsCount() const {
@@ -237,3 +241,9 @@ PopupBlockerTabHelper::PopupPosition PopupBlockerTabHelper::GetPopupPosition(
237241

238242
return PopupPosition::kMiddlePopup;
239243
}
244+
245+
// static
246+
void PopupBlockerTabHelper::LogAction(Action action) {
247+
UMA_HISTOGRAM_ENUMERATION("ContentSettings.Popups.BlockerActions", action,
248+
Action::kLast);
249+
}

chrome/browser/ui/blocked_content/popup_blocker_tab_helper.h

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -48,6 +48,24 @@ class PopupBlockerTabHelper
4848
// Any new values should go before this one.
4949
kLast,
5050
};
51+
52+
// This enum is backed by a histogram. Make sure enums.xml is updated if this
53+
// is updated.
54+
enum class Action : int {
55+
// A popup was initiated and was sent to the popup blocker for
56+
// consideration.
57+
kInitiated,
58+
59+
// A popup was blocked by the popup blocker.
60+
kBlocked,
61+
62+
// A previously blocked popup was clicked through.
63+
kClickedThrough,
64+
65+
// Add new elements before this value.
66+
kLast
67+
};
68+
5169
class Observer {
5270
public:
5371
virtual void BlockedPopupAdded(int32_t id, const GURL& url) {}
@@ -108,6 +126,8 @@ class PopupBlockerTabHelper
108126

109127
PopupPosition GetPopupPosition(int32_t id) const;
110128

129+
static void LogAction(Action action);
130+
111131
// Note, this container should be sorted based on the position in the popup
112132
// list, so it is keyed by an id which is continually increased.
113133
std::map<int32_t, std::unique_ptr<BlockedRequest>> blocked_popups_;

tools/metrics/histograms/enums.xml

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -30799,6 +30799,12 @@ from previous Chrome versions.
3079930799
<int value="11" label="Bad Key Validation Signature"/>
3080030800
</enum>
3080130801

30802+
<enum name="PopupBlockerAction">
30803+
<int value="0" label="Popup initiated"/>
30804+
<int value="1" label="Popup blocked"/>
30805+
<int value="2" label="Popup clicked through"/>
30806+
</enum>
30807+
3080230808
<enum name="PopupPosition">
3080330809
<int value="0" label="Only popup"/>
3080430810
<int value="1" label="First popup"/>

tools/metrics/histograms/histograms.xml

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10268,6 +10268,17 @@ http://cs/file:chrome/histograms.xml - but prefer this file for new entries.
1026810268
</summary>
1026910269
</histogram>
1027010270

10271+
<histogram name="ContentSettings.Popups.BlockerActions"
10272+
enum="PopupBlockerAction">
10273+
<owner>[email protected]</owner>
10274+
<summary>
10275+
Counts of various events related to the popup blocker. Including blocked
10276+
popups and overridden (clicked through) popups. This is similar to the
10277+
ContentSettings.Popups but is at the per-popup layer rather than at the UI
10278+
layer.
10279+
</summary>
10280+
</histogram>
10281+
1027110282
<histogram name="ContentSettings.Popups.ClickThroughPosition"
1027210283
enum="PopupPosition">
1027310284
<owner>[email protected]</owner>

0 commit comments

Comments
 (0)