Skip to content

Commit

Permalink
Popup metrics: Add initiated/blocked/clicked through metric
Browse files Browse the repository at this point in the history
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}
  • Loading branch information
csharrison authored and Avi Drissman committed Sep 14, 2017
1 parent 78f58ba commit a82b4ad
Show file tree
Hide file tree
Showing 5 changed files with 86 additions and 0 deletions.
39 changes: 39 additions & 0 deletions chrome/browser/ui/blocked_content/popup_blocker_browsertest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
10 changes: 10 additions & 0 deletions chrome/browser/ui/blocked_content/popup_blocker_tab_helper.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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;

Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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);
}
20 changes: 20 additions & 0 deletions chrome/browser/ui/blocked_content/popup_blocker_tab_helper.h
Original file line number Diff line number Diff line change
Expand Up @@ -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) {}
Expand Down Expand Up @@ -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_;
Expand Down
6 changes: 6 additions & 0 deletions tools/metrics/histograms/enums.xml
Original file line number Diff line number Diff line change
Expand Up @@ -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"/>
Expand Down
11 changes: 11 additions & 0 deletions tools/metrics/histograms/histograms.xml
Original file line number Diff line number Diff line change
Expand Up @@ -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>
Expand Down

0 comments on commit a82b4ad

Please sign in to comment.