Skip to content

GitLab

T
tangled
Commit f773967f authored by Yao Xiao's avatar Yao Xiao Committed by Commit Bot
Browse files
Options

Add a use counter for overlay popup 

We used to measure overlay-popup-ad. This CL adds another use counter to
measure the overlay-popup in general, including both ads & non-ads.

This is achieved by adding extra flags to OverlayInterstitialAdDetector
to denote whether the current candidate is an ad, as well as whether we
have encountered ad and/or general interstitials.

Bug: 1048136
Change-Id: I6dc809ceed51aa9b9747af44d8db6ed38721bc2f
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2250957
Commit-Queue: Yao Xiao <yaoxia@chromium.org>
Reviewed-by: default avatarBryan McQuade <bmcquade@chromium.org>
Reviewed-by: default avatarStephen Chenney <schenney@chromium.org>
Cr-Commit-Position: refs/heads/master@{#781231}
parent 715ceb2b
Show whitespace changes
Inline Side-by-side
Showing with 172 additions and 15 deletions
components/page_load_metrics/browser/observers/use_counter/ukm_features.cc
View file @ f773967f
......@@ -160,6 +160,7 @@ UseCounterPageLoadMetricsObserver::GetAllowedUkmFeatures() {
WebFeature::kThirdPartyCacheStorage,
WebFeature::kThirdPartyLocalStorage,
WebFeature::kThirdPartySessionStorage,
WebFeature::kOverlayPopup,
WebFeature::kOverlayPopupAd,
WebFeature::kTrustTokenXhr,
WebFeature::kTrustTokenFetch,
......
third_party/blink/public/mojom/web_feature/web_feature.mojom
View file @ f773967f
......@@ -2665,6 +2665,7 @@ enum WebFeature {
kWebBluetoothGetDevices = 3328,
kDialogWithNonZeroScrollOffset = 3329,
kDialogHeightLargerThanViewport = 3330,
kOverlayPopup = 3331,
// Add new features immediately above this line. Don't change assigned
// numbers of any item, and don't reuse removed slots.
......
third_party/blink/renderer/core/frame/overlay_interstitial_ad_detector.cc
View file @ f773967f
......@@ -27,10 +27,7 @@ constexpr double kLargeAdSizeToViewportSizeThreshold = 0.1;
// 2) <body> or <html> has style="overflow:hidden", and among its container
// ancestors (including itself), the 2nd to the top (where the top should always
// be the <body>) has absolute position.
bool IsOverlayAdCandidate(Element* element) {
if (!element->IsAdRelated())
return false;
bool IsOverlayCandidate(Element* element) {
const ComputedStyle* style = nullptr;
LayoutView* layout_view = element->GetDocument().GetLayoutView();
LayoutObject* object = element->GetLayoutObject();
......@@ -62,7 +59,7 @@ bool IsOverlayAdCandidate(Element* element) {
void OverlayInterstitialAdDetector::MaybeFireDetection(LocalFrame* main_frame) {
DCHECK(main_frame);
DCHECK(main_frame->IsMainFrame());
if (done_detection_)
if (popup_ad_detected_)
return;
DCHECK(main_frame->GetDocument());
......@@ -130,17 +127,22 @@ void OverlayInterstitialAdDetector::MaybeFireDetection(LocalFrame* main_frame) {
bool is_new_element = (element_id != candidate_id_);
// The popup candidate has just been dismissed.
if (is_new_element && candidate_id_ != kInvalidDOMNodeId) {
// If the main frame scrolling offset hasn't changed since the candidate's
// appearance, we consider it to be a overlay interstitial; otherwise, we
// skip that candidate because it could be a parallax/scroller ad.
if (main_frame->GetMainFrameScrollOffset().Y() ==
candidate_start_main_frame_scroll_offset_) {
OnPopupAdDetected(main_frame);
return;
OnPopupDetected(main_frame, candidate_is_ad_);
}
if (popup_ad_detected_)
return;
last_unqualified_element_id_ = candidate_id_;
candidate_id_ = kInvalidDOMNodeId;
candidate_is_ad_ = false;
}
if (!is_new_element)
......@@ -159,16 +161,22 @@ void OverlayInterstitialAdDetector::MaybeFireDetection(LocalFrame* main_frame) {
main_frame_size.Area() * kLargeAdSizeToViewportSizeThreshold);
bool has_gesture = LocalFrame::HasTransientUserActivation(main_frame);
bool is_ad = element->IsAdRelated();
if (!has_gesture && is_large && IsOverlayAdCandidate(element)) {
if (!has_gesture && is_large && (!popup_detected_ || is_ad) &&
IsOverlayCandidate(element)) {
// If main page is not scrollable, immediately determinine the overlay
// to be a popup. There's is no need to check any state at the dismissal
// time.
if (!main_frame->GetDocument()->GetLayoutView()->HasScrollableOverflowY()) {
OnPopupAdDetected(main_frame);
return;
OnPopupDetected(main_frame, is_ad);
}
if (popup_ad_detected_)
return;
candidate_id_ = element_id;
candidate_is_ad_ = is_ad;
candidate_start_main_frame_scroll_offset_ =
main_frame->GetMainFrameScrollOffset().Y();
} else {
......@@ -176,9 +184,18 @@ void OverlayInterstitialAdDetector::MaybeFireDetection(LocalFrame* main_frame) {
}
}
void OverlayInterstitialAdDetector::OnPopupAdDetected(LocalFrame* main_frame) {
void OverlayInterstitialAdDetector::OnPopupDetected(LocalFrame* main_frame,
bool is_ad) {
if (!popup_detected_) {
UseCounter::Count(main_frame->GetDocument(), WebFeature::kOverlayPopup);
popup_detected_ = true;
}
if (is_ad) {
DCHECK(!popup_ad_detected_);
UseCounter::Count(main_frame->GetDocument(), WebFeature::kOverlayPopupAd);
done_detection_ = true;
popup_ad_detected_ = true;
}
}
} // namespace blink
third_party/blink/renderer/core/frame/overlay_interstitial_ad_detector.h
View file @ f773967f
......@@ -54,7 +54,7 @@ class CORE_EXPORT OverlayInterstitialAdDetector {
void MaybeFireDetection(LocalFrame* main_frame);
private:
void OnPopupAdDetected(LocalFrame* main_frame);
void OnPopupDetected(LocalFrame* main_frame, bool is_ad);
bool started_detection_ = false;
bool main_content_has_loaded_ = false;
......@@ -64,6 +64,7 @@ class CORE_EXPORT OverlayInterstitialAdDetector {
IntSize last_detection_main_frame_size_;
DOMNodeId candidate_id_;
bool candidate_is_ad_ = false;
// The following members are valid only when |candidate_| is not nullptr.
int candidate_start_main_frame_scroll_offset_ = 0;
......@@ -82,7 +83,8 @@ class CORE_EXPORT OverlayInterstitialAdDetector {
// can skip it on its next occurrence without computing the style again.
DOMNodeId last_unqualified_element_id_;
bool done_detection_ = false;
bool popup_detected_ = false;
bool popup_ad_detected_ = false;
DISALLOW_COPY_AND_ASSIGN(OverlayInterstitialAdDetector);
};
......
third_party/blink/web_tests/SlowTests
View file @ f773967f
......@@ -539,3 +539,6 @@ crbug.com/1043669 [ Mac ] inspector-protocol/emulation/set-vision-deficiency.js
crbug.com/1092121 fast/css/large-list-of-rules-crash.html [ Slow ]
crbug.com/1042205 virtual/off-main-thread-css-paint/external/wpt/css/css-paint-api/paint2d-filter.https.html [ Slow ]
# Slow is the intended behavior
crbug.com/1048136 http/tests/subresource_filter/overlay-popup-non-ad-followed-by-ad.html [ Slow ]
third_party/blink/web_tests/http/tests/subresource_filter/overlay-popup-non-ad-followed-by-ad.html 0 → 100644
View file @ f773967f
<!DOCTYPE html>
<html>
<head>
<style>
div {
width: 100vw;
height: 100vh;
}
iframe {
position: fixed;
margin-left: 25vw;
width: 50vw;
height: 100vh;
border: 0px;
}
form {
position: fixed;
margin-left: 25vw;
width: 50vw;
height: 100vh;
border: 0px;
}
p {
position: fixed;
}
div.bottom {
position: absolute;
top: 10000px;
left: 0px;
width: 1px;
height: 1px;
}
</style>
<script src="../resources/testharness.js"></script>
<script src="../resources/testharnessreport.js"></script>
</head>
<body marginwidth="0" marginheight="0">
<!-- To trigger the first contentful paint at the very start -->
<p>some content</p>
<!-- To contain the overlay popup -->
<div></div>
<!-- To be positioned further down in the main page to make the page scrollable -->
<div class="bottom"></div>
<script>
if (window.testRunner) {
// Inject a subresource filter to mark 'overlay-interstitial-ad-testharness.js' as a would be disallowed resource.
testRunner.setDisallowedSubresourcePathSuffixes(["overlay-interstitial-ad-testharness.js"], false /* block_subresources */);
}
promise_test(() => {
return new Promise((resolve, reject) => {
let ad_script = document.createElement("script");
ad_script.async = false;
ad_script.src = "resources/overlay-interstitial-ad-testharness.js";
ad_script.onload = async() => {
// After 1500ms, force a layout update so that the interstitial detector
// is aware of the first meaningful paint, and future overlay candidates
// will be considered for pop-ups rather than for prestitials.
await timeout(1500);
await forceLayoutUpdate();
// Create a overlay pop-up <form> element.
let form = document.createElement('form');
document.getElementsByTagName('div')[0].appendChild(form);
// After 1500ms, force a layout update so that the interstitial detector
// is aware of the overlay candidate.
await timeout(1500);
await forceLayoutUpdate();
// Hide the pop-up.
document.getElementsByTagName('form')[0].style.display = 'none';
// After 1500ms, force a layout update so that the interstitial detector
// is aware of the overlay candidate's dismissal.
await timeout(1500);
await forceLayoutUpdate();
// Expect use counter kOverlayPopup.
if (!internals.isUseCounted(document, kOverlayPopup)) {
reject();
}
// Expect use counter kOverlayPopupAd is NOT recorded.
if (internals.isUseCounted(document, kOverlayPopupAd)) {
reject();
}
// Create the overlay pop-up ad.
appendAdFrameTo(document.getElementsByTagName('div')[0]);
// After 1500ms, force a layout update so that the interstitial detector
// is aware of the overlay ad candidate.
await timeout(1500);
await forceLayoutUpdate();
// Hide the pop-up ad.
document.getElementsByTagName('iframe')[0].style.display = 'none';
// After 1500ms, force a layout update so that the interstitial detector
// is aware of the overlay ad candidate's dismissal.
await timeout(1500);
await forceLayoutUpdate();
// Expect use counter kOverlayPopupAd.
if (!internals.isUseCounted(document, kOverlayPopupAd)) {
reject();
}
resolve();
};
document.body.appendChild(ad_script);
});
}, "Test UseCounter for overlay-popup non-ad element followed by an ad element.");
</script>
</body>
</html>
third_party/blink/web_tests/http/tests/subresource_filter/resources/overlay-interstitial-ad-testharness.js
View file @ f773967f
// |kOverlayPopupAd| from web_feature.mojom.
const kOverlayPopupAd = 3253;
// |kOverlayPopup| from web_feature.mojom.
const kOverlayPopup = 3331;
function timeout(ms) {
return new Promise(resolve => setTimeout(resolve, ms));
}
function forceLayoutUpdate() {
return new Promise((resolve) => requestAnimationFrame(() => { setTimeout(() => { resolve(); }) }));
}
// TODO(yaoxia): For those tests that still depend on this function, we'd want
// to convert them use Promise and async/await for better readability.
function verifyOverlayPopupUseCounterAfter1500ms(
test, expect_use_counter, post_verify_callback) {
// Force a lifecycle update after 1500ms, so that the final stable layout
......
tools/metrics/histograms/enums.xml
View file @ f773967f
......@@ -28066,6 +28066,7 @@ Called by update_use_counter_feature_enum.py.-->
<int value="3328" label="WebBluetoothGetDevices"/>
<int value="3329" label="DialogWithNonZeroScrollOffset"/>
<int value="3330" label="DialogHeightLargerThanViewport"/>
<int value="3331" label="OverlayPopup"/>
</enum>
<enum name="FeaturePolicyAllowlistType">
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