Commit 7e72794c authored by Yao Xiao's avatar Yao Xiao Committed by Commit Bot

Add UseCounter for ad click that results in navigation

Record for each window.open(), href link activation, and location=xxx. Those
calls should be mutually exclusive.

For this metric to be accurate, it has to assume that "the navigation was
initiated in a frame with transient gesture" == "the navigation was caused
by this gesture", which can be wrong. But it may just work in practice and
it seems to be a general assumption we have been making throughout Chrome.

Bug: 864147
Change-Id: If7f75bd8d17e1d2ea572607e6f8f4149c78222fd
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1488111
Commit-Queue: Yao Xiao <yaoxia@chromium.org>
Reviewed-by: default avatarBryan McQuade <bmcquade@chromium.org>
Reviewed-by: default avatarCamille Lamy <clamy@chromium.org>
Reviewed-by: default avatarDaniel Cheng <dcheng@chromium.org>
Reviewed-by: default avatarCharlie Harrison <csharrison@chromium.org>
Reviewed-by: default avatarMustaq Ahmed <mustaq@chromium.org>
Cr-Commit-Position: refs/heads/master@{#638631}
parent f2c92e29
......@@ -99,6 +99,7 @@ UseCounterPageLoadMetricsObserver::GetAllowedUkmFeatures() {
WebFeature::kV8MediaCapabilities_DecodingInfo_Method,
WebFeature::kOpenerNavigationDownloadCrossOrigin,
WebFeature::kLinkRelPrerender,
WebFeature::kAdClickNavigation,
}));
return *opt_in_features;
}
......@@ -65,6 +65,11 @@ bool PageLoadMetricsTestWaiter::DidObserveInPage(TimingField field) const {
return observed_page_fields_.IsSet(field);
}
bool PageLoadMetricsTestWaiter::DidObserveWebFeature(
blink::mojom::WebFeature feature) const {
return observed_web_features_.test(static_cast<size_t>(feature));
}
void PageLoadMetricsTestWaiter::Wait() {
if (ExpectationsSatisfied())
return;
......
......@@ -58,6 +58,9 @@ class PageLoadMetricsTestWaiter
// Whether the given TimingField was observed in the page.
bool DidObserveInPage(TimingField field) const;
// Whether the given WebFeature was observed in the page.
bool DidObserveWebFeature(blink::mojom::WebFeature feature) const;
// Waits for PageLoadMetrics events that match the fields set by the add
// expectation methods. All matching fields must be set to end this wait.
void Wait();
......
......@@ -15,6 +15,7 @@
#include "chrome/browser/subresource_filter/subresource_filter_browser_test_harness.h"
#include "chrome/browser/ui/browser.h"
#include "chrome/browser/ui/tabs/tab_strip_model.h"
#include "chrome/common/chrome_switches.h"
#include "chrome/test/base/ui_test_utils.h"
#include "components/subresource_filter/content/browser/subresource_filter_observer_test_utils.h"
#include "components/subresource_filter/core/common/test_ruleset_utils.h"
......@@ -387,6 +388,96 @@ void ExpectWindowOpenUmaEntry(const base::HistogramTester& histogram_tester,
1 /* expected_count */);
}
enum class NavigationInitiationType {
kWindowOpen,
kSetLocation,
kAnchorLinkActivate,
};
class AdClickNavigationBrowserTest
: public AdTaggingBrowserTest,
public ::testing::WithParamInterface<
std::tuple<NavigationInitiationType, bool /* gesture */>> {
void SetUpCommandLine(base::CommandLine* command_line) override {
// Popups without user gesture is blocked by default. Turn off the switch
// here to unblock that, so as to be able to test that the UseCounter not
// being recorded was due to the filtering in our recording function, rather
// than the default popup blocking.
command_line->AppendSwitch(::switches::kDisablePopupBlocking);
}
};
IN_PROC_BROWSER_TEST_P(AdClickNavigationBrowserTest, UseCounter) {
NavigationInitiationType type;
bool gesture;
std::tie(type, gesture) = GetParam();
auto web_feature_waiter =
std::make_unique<page_load_metrics::PageLoadMetricsTestWaiter>(
GetWebContents());
blink::mojom::WebFeature ad_click_navigation_feature =
blink::mojom::WebFeature::kAdClickNavigation;
web_feature_waiter->AddWebFeatureExpectation(ad_click_navigation_feature);
GURL url =
embedded_test_server()->GetURL("a.com", "/ad_tagging/frame_factory.html");
ui_test_utils::NavigateToURL(browser(), url);
content::WebContents* main_tab = GetWebContents();
RenderFrameHost* child = CreateSrcFrame(
main_tab, embedded_test_server()->GetURL(
"a.com", "/ad_tagging/frame_factory.html?1&ad=true"));
std::string script;
switch (type) {
case NavigationInitiationType::kWindowOpen:
script = "window.open('frame_factory.html')";
break;
case NavigationInitiationType::kSetLocation:
script = "location='frame_factory.html'";
break;
case NavigationInitiationType::kAnchorLinkActivate:
script =
"var a = document.createElement('a');"
"a.setAttribute('href', 'frame_factory.html');"
"a.click();";
break;
}
if (gesture) {
EXPECT_TRUE(ExecJs(child, script));
web_feature_waiter->Wait();
} else {
switch (type) {
case NavigationInitiationType::kSetLocation:
case NavigationInitiationType::kAnchorLinkActivate: {
content::TestNavigationObserver navigation_observer(web_contents());
EXPECT_TRUE(ExecuteScriptWithoutUserGesture(child, script));
// To report metrics.
navigation_observer.Wait();
break;
}
case NavigationInitiationType::kWindowOpen: {
EXPECT_TRUE(ExecuteScriptWithoutUserGesture(child, script));
// To report metrics.
ASSERT_EQ(2, browser()->tab_strip_model()->count());
browser()->tab_strip_model()->MoveSelectedTabsTo(0);
ui_test_utils::NavigateToURL(browser(), url);
break;
}
}
EXPECT_FALSE(
web_feature_waiter->DidObserveWebFeature(ad_click_navigation_feature));
}
}
INSTANTIATE_TEST_SUITE_P(
/* no prefix */,
AdClickNavigationBrowserTest,
::testing::Combine(
::testing::Values(NavigationInitiationType::kWindowOpen,
NavigationInitiationType::kSetLocation,
NavigationInitiationType::kAnchorLinkActivate),
::testing::Bool()));
class AdTaggingEventFromSubframeBrowserTest
: public AdTaggingBrowserTest,
public ::testing::WithParamInterface<
......
......@@ -2257,6 +2257,7 @@ enum WebFeature {
kCSSValueAppearanceTextFieldForTemporalRendered = 2823,
kBuiltInModuleKvStorage = 2824,
kBuiltInModuleVirtualScroller = 2825,
kAdClickNavigation = 2826,
// Add new features immediately above this line. Don't change assigned
// numbers of any item, and don't reuse removed slots.
......
......@@ -1828,4 +1828,9 @@ void LocalFrame::SetLifecycleState(mojom::FrameLifecycleState state) {
}
}
void LocalFrame::MaybeLogAdClickNavigation() {
if (HasTransientUserActivation() && IsAdSubframe())
UseCounter::Count(GetDocument(), WebFeature::kAdClickNavigation);
}
} // namespace blink
......@@ -437,6 +437,15 @@ class CORE_EXPORT LocalFrame final : public Frame,
void SetLifecycleState(mojom::FrameLifecycleState state);
// For a navigation initiated from this LocalFrame with user gesture, record
// the UseCounter AdClickNavigation if this frame is an adframe.
//
// TODO(crbug.com/939370): Currently this is called in a couple of sites,
// which is fragile and prone to break. If we have the ad status in
// RemoteFrame, we could call it at FrameLoader::StartNavigation where all
// navigations go through.
void MaybeLogAdClickNavigation();
private:
friend class FrameNavigationDisabler;
......
......@@ -321,6 +321,8 @@ void Location::SetLocation(const String& url,
WebFrameLoadType frame_load_type = WebFrameLoadType::kStandard;
if (set_location_policy == SetLocationPolicy::kReplaceThisFrame)
frame_load_type = WebFrameLoadType::kReplaceCurrentItem;
current_window->GetFrame()->MaybeLogAdClickNavigation();
dom_window_->GetFrame()->ScheduleNavigation(*current_window->document(),
completed_url, frame_load_type,
UserGestureStatus::kNone);
......
......@@ -462,6 +462,8 @@ void HTMLAnchorElement::HandleClick(Event& event) {
frame_request.SetInputStartTime(event.PlatformTimeStamp());
// TODO(japhet): Link clicks can be emulated via JS without a user gesture.
// Why doesn't this go through NavigationScheduler?
frame->MaybeLogAdClickNavigation();
frame->Loader().StartNavigation(frame_request, WebFrameLoadType::kStandard,
NavigationPolicyFromEvent(&event));
}
......
......@@ -424,6 +424,7 @@ DOMWindow* CreateWindow(const String& url_string,
// createWindow(LocalFrame& openerFrame, ...).
// This value will be set in ResourceRequest loaded in a new LocalFrame.
bool has_user_gesture = LocalFrame::HasTransientUserActivation(&opener_frame);
opener_frame.MaybeLogAdClickNavigation();
// We pass the opener frame for the lookupFrame in case the active frame is
// different from the opener frame, and the name references a frame relative
......
......@@ -21794,6 +21794,7 @@ Called by update_net_error_codes.py.-->
<int value="2823" label="CSSValueAppearanceTextFieldForTemporalRendered"/>
<int value="2824" label="BuiltInModuleKvStorage"/>
<int value="2825" label="BuiltInModuleVirtualScroller"/>
<int value="2826" label="AdClickNavigation"/>
</enum>
<enum name="FeaturePolicyFeature">
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