Commit fdc0bdda authored by Justin Miron's avatar Justin Miron Committed by Commit Bot

User's site level popup settings recorded in UKM.

This expands the popup_opener_tab_helper, which traditionally
tracks the pop-ups and navigation of the site for the purposes
of tab-unders, to include recording a user's pop up blocker content
settings to UKM.

This is done by querying the user's content settings when the
web contents associated with the site is destroyed and recording
it to a new UKM: Popup.Page.Allowed. Where the value of true
represents the user explicitly allowing all pop-ups on a site.

Change-Id: I33c729021511a154d944bdf45ca188971292640b
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2062488
Commit-Queue: Justin Miron <justinmiron@google.com>
Reviewed-by: default avatarRobert Kaplow <rkaplow@chromium.org>
Reviewed-by: default avatarCharlie Harrison <csharrison@chromium.org>
Cr-Commit-Position: refs/heads/master@{#743930}
parent b7ce6bb5
...@@ -10,9 +10,17 @@ ...@@ -10,9 +10,17 @@
#include "base/memory/ptr_util.h" #include "base/memory/ptr_util.h"
#include "base/metrics/histogram_macros.h" #include "base/metrics/histogram_macros.h"
#include "base/time/tick_clock.h" #include "base/time/tick_clock.h"
#include "chrome/browser/content_settings/host_content_settings_map_factory.h"
#include "chrome/browser/profiles/profile.h"
#include "chrome/browser/ui/blocked_content/tab_under_navigation_throttle.h" #include "chrome/browser/ui/blocked_content/tab_under_navigation_throttle.h"
#include "components/content_settings/core/browser/host_content_settings_map.h"
#include "components/content_settings/core/common/content_settings.h"
#include "components/ukm/content/source_url_recorder.h"
#include "content/public/browser/navigation_handle.h" #include "content/public/browser/navigation_handle.h"
#include "content/public/browser/web_contents.h" #include "content/public/browser/web_contents.h"
#include "services/metrics/public/cpp/metrics_utils.h"
#include "services/metrics/public/cpp/ukm_builders.h"
#include "services/metrics/public/cpp/ukm_recorder.h"
#include "ui/base/scoped_visibility_tracker.h" #include "ui/base/scoped_visibility_tracker.h"
// static // static
...@@ -43,6 +51,7 @@ PopupOpenerTabHelper::~PopupOpenerTabHelper() { ...@@ -43,6 +51,7 @@ PopupOpenerTabHelper::~PopupOpenerTabHelper() {
void PopupOpenerTabHelper::OnOpenedPopup(PopupTracker* popup_tracker) { void PopupOpenerTabHelper::OnOpenedPopup(PopupTracker* popup_tracker) {
has_opened_popup_since_last_user_gesture_ = true; has_opened_popup_since_last_user_gesture_ = true;
has_opened_popup_ = true;
last_popup_open_time_ = tick_clock_->NowTicks(); last_popup_open_time_ = tick_clock_->NowTicks();
} }
...@@ -67,6 +76,28 @@ PopupOpenerTabHelper::PopupOpenerTabHelper(content::WebContents* web_contents, ...@@ -67,6 +76,28 @@ PopupOpenerTabHelper::PopupOpenerTabHelper(content::WebContents* web_contents,
web_contents->GetVisibility() != content::Visibility::HIDDEN); web_contents->GetVisibility() != content::Visibility::HIDDEN);
} }
void PopupOpenerTabHelper::WebContentsDestroyed() {
// If the user has opened a popup, record the page popup settings ukm.
if (has_opened_popup_) {
const GURL& url = web_contents()->GetLastCommittedURL();
if (!url.is_valid()) {
return;
}
const ukm::SourceId source_id =
ukm::GetSourceIdForWebContentsDocument(web_contents());
Profile* profile =
Profile::FromBrowserContext(web_contents()->GetBrowserContext());
bool user_allows_popups =
HostContentSettingsMapFactory::GetForProfile(profile)
->GetContentSetting(url, url, ContentSettingsType::POPUPS,
std::string()) == CONTENT_SETTING_ALLOW;
ukm::builders::Popup_Page(source_id)
.SetAllowed(user_allows_popups)
.Record(ukm::UkmRecorder::Get());
}
}
void PopupOpenerTabHelper::OnVisibilityChanged(content::Visibility visibility) { void PopupOpenerTabHelper::OnVisibilityChanged(content::Visibility visibility) {
// TODO(csharrison): Consider handling OCCLUDED tabs the same way as HIDDEN // TODO(csharrison): Consider handling OCCLUDED tabs the same way as HIDDEN
// tabs. // tabs.
......
...@@ -59,6 +59,7 @@ class PopupOpenerTabHelper ...@@ -59,6 +59,7 @@ class PopupOpenerTabHelper
const base::TickClock* tick_clock); const base::TickClock* tick_clock);
// content::WebContentsObserver: // content::WebContentsObserver:
void WebContentsDestroyed() override;
void OnVisibilityChanged(content::Visibility visibility) override; void OnVisibilityChanged(content::Visibility visibility) override;
void DidStartNavigation( void DidStartNavigation(
content::NavigationHandle* navigation_handle) override; content::NavigationHandle* navigation_handle) override;
...@@ -80,6 +81,9 @@ class PopupOpenerTabHelper ...@@ -80,6 +81,9 @@ class PopupOpenerTabHelper
bool has_opened_popup_since_last_user_gesture_ = false; bool has_opened_popup_since_last_user_gesture_ = false;
// Whether this WebContents has opened a popup.
bool has_opened_popup_ = false;
WEB_CONTENTS_USER_DATA_KEY_DECL(); WEB_CONTENTS_USER_DATA_KEY_DECL();
DISALLOW_COPY_AND_ASSIGN(PopupOpenerTabHelper); DISALLOW_COPY_AND_ASSIGN(PopupOpenerTabHelper);
......
...@@ -17,6 +17,7 @@ ...@@ -17,6 +17,7 @@
#include "base/test/simple_test_tick_clock.h" #include "base/test/simple_test_tick_clock.h"
#include "base/time/time.h" #include "base/time/time.h"
#include "build/build_config.h" #include "build/build_config.h"
#include "chrome/browser/content_settings/host_content_settings_map_factory.h"
#include "chrome/browser/content_settings/tab_specific_content_settings.h" #include "chrome/browser/content_settings/tab_specific_content_settings.h"
#include "chrome/browser/infobars/infobar_service.h" #include "chrome/browser/infobars/infobar_service.h"
#include "chrome/browser/ui/blocked_content/list_item_position.h" #include "chrome/browser/ui/blocked_content/list_item_position.h"
...@@ -320,6 +321,73 @@ TEST_F(PopupOpenerTabHelperTest, SimulateTabUnderNavBeforePopup_LogsMetrics) { ...@@ -320,6 +321,73 @@ TEST_F(PopupOpenerTabHelperTest, SimulateTabUnderNavBeforePopup_LogsMetrics) {
histogram_tester()->ExpectTotalCount(kPopupToTabUnder, 0); histogram_tester()->ExpectTotalCount(kPopupToTabUnder, 0);
} }
// Navigate to a site without pop-ups, verify the user popup settings are not
// logged to ukm.
TEST_F(PopupOpenerTabHelperTest,
PageWithNoPopups_NoProfileSettingsLoggedInUkm) {
ukm::InitializeSourceUrlRecorderForWebContents(web_contents());
ukm::TestAutoSetUkmRecorder test_ukm_recorder;
const GURL url("https://first.test/");
NavigateAndCommitWithoutGesture(url);
DeleteContents();
auto entries =
test_ukm_recorder.GetEntriesByName(ukm::builders::Popup_Page::kEntryName);
EXPECT_EQ(0u, entries.size());
}
// Navigate to a site, verify histogram for user site pop up settings
// when there is at least one popup.
TEST_F(PopupOpenerTabHelperTest,
PageWithDefaultPopupBlocker_ProfileSettingsLoggedInUkm) {
ukm::InitializeSourceUrlRecorderForWebContents(web_contents());
ukm::TestAutoSetUkmRecorder test_ukm_recorder;
const GURL url("https://first.test/");
NavigateAndCommitWithoutGesture(url);
SimulatePopup();
DeleteContents();
auto entries =
test_ukm_recorder.GetEntriesByName(ukm::builders::Popup_Page::kEntryName);
EXPECT_EQ(1u, entries.size());
test_ukm_recorder.ExpectEntrySourceHasUrl(entries[0], url);
test_ukm_recorder.ExpectEntryMetric(
entries[0], ukm::builders::Popup_Page::kAllowedName, false);
}
// Same as the test with the default pop up blocker, however, with the user
// explicitly allowing popups on the site.
TEST_F(PopupOpenerTabHelperTest,
PageWithPopupsAllowed_ProfileSettingsLoggedInUkm) {
ukm::InitializeSourceUrlRecorderForWebContents(web_contents());
ukm::TestAutoSetUkmRecorder test_ukm_recorder;
const GURL url("https://first.test/");
// Allow popups on url for the test profile.
TestingProfile* test_profile = profile();
HostContentSettingsMap* host_content_settings_map =
HostContentSettingsMapFactory::GetForProfile(test_profile);
host_content_settings_map->SetContentSettingDefaultScope(
url, url, ContentSettingsType::POPUPS, std::string(),
CONTENT_SETTING_ALLOW);
NavigateAndCommitWithoutGesture(url);
SimulatePopup();
DeleteContents();
auto entries =
test_ukm_recorder.GetEntriesByName(ukm::builders::Popup_Page::kEntryName);
EXPECT_EQ(1u, entries.size());
test_ukm_recorder.ExpectEntrySourceHasUrl(entries[0], url);
test_ukm_recorder.ExpectEntryMetric(
entries[0], ukm::builders::Popup_Page::kAllowedName, true);
}
class BlockTabUnderTest : public PopupOpenerTabHelperTest { class BlockTabUnderTest : public PopupOpenerTabHelperTest {
public: public:
BlockTabUnderTest() {} BlockTabUnderTest() {}
......
...@@ -8304,6 +8304,21 @@ be describing additional metrics about the same event. ...@@ -8304,6 +8304,21 @@ be describing additional metrics about the same event.
</metric> </metric>
</event> </event>
<event name="Popup.Page">
<owner>justinmiron@google.com</owner>
<summary>
Page level pop-up statistics.
</summary>
<metric name="Allowed">
<summary>
Whether a user has, explicitly, allowed all popups on the last committed
url of a page. The user's site level popup content setting is queried when
a page that opened a popup is destroyed. Only emitted when at least one
popup has been opened and the last committed page url is valid.
</summary>
</metric>
</event>
<event name="PrefetchProxy" singular="True"> <event name="PrefetchProxy" singular="True">
<owner>robertogden@chromium.org</owner> <owner>robertogden@chromium.org</owner>
<owner>ryansturm@chromium.org</owner> <owner>ryansturm@chromium.org</owner>
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment