Skip to content

Commit

Permalink
Reland "Add UKM for LazyAds / LazyEmbeds Interventions"
Browse files Browse the repository at this point in the history
This is a reland of commit 2d85d0e

The previous CL was reverted because of the flaky tests on
Windows. Back/forward cache caused UKM recording problems on
Windows. So this CL disables back/forward cache to avoid UKM recording
problems.

Original change's description:
> Add UKM for LazyAds / LazyEmbeds Interventions
>
> Collection Review (Googlers only):
> https://docs.google.com/document/d/18igOKn3CjqukHDU1L_xYyFH0w9ZVixkyQrRbddkrN_4
>
> This CL adds Blink.AutomaticLazyLoadFrame UKM event for LazyAds /
> LazyEmbeds Interventions. This event includes the following metrics.
>
> - LazyAdsFrameCount
>
> Records the total number of per-page ad frames that are eligible for
> the LazyFrame interventions by AutomaticLazyFrameLoadingToAds feature
> flag.
>
> - LazyEmbedsFrameCount
>
> Records the total number of per-page frames that are eligible for the
> LazyEmbeds interventions by AutomaticLazyFrameLoadingToEmbeds feature
> flag.
>
> These metrics will be recorded by using exponential bucketing with
> ukm::GetExponentialBucketMinForCounts1000().
>
> This CL also updates the existing behavior of LazyAds and
> LazyEmbeds. Previously, LazyAds and LazyEmbeds ignored the following
> conditions.
>
>  - RuntimeEnabledFeatures::LazyFrameLoadingEnabled()
>  - settings of document (Document::GetSettings())
>  - backgrounded tabs (lazy loading is disabled for backgrounded tabs.)
>
> This CL starts checking these conditions so that the behavior aligns
> the behavior of lazy loading (<iframe loading="lazy">).
>
> Bug: 1247131, 1265343
> Change-Id: Ibd93a556a99fb3bc59ed62e5f8b281e15bd4d948
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3554238
> Reviewed-by: Kent Tamura <tkent@chromium.org>
> Reviewed-by: Alex Turner <alexmt@chromium.org>
> Reviewed-by: Robert Kaplow <rkaplow@chromium.org>
> Reviewed-by: Kouhei Ueno <kouhei@chromium.org>
> Commit-Queue: Minoru Chikamune <chikamune@chromium.org>
> Cr-Commit-Position: refs/heads/main@{#991942}

(cherry picked from commit a01cd72)

Bug: 1316004, 1247131, 1265343
Change-Id: Iddf77b0934f8e858cf6ab20ddddb5529324ad961
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3582251
Reviewed-by: Kouhei Ueno <kouhei@chromium.org>
Reviewed-by: Robert Kaplow <rkaplow@chromium.org>
Reviewed-by: Alex Turner <alexmt@chromium.org>
Reviewed-by: Kent Tamura <tkent@chromium.org>
Commit-Queue: Minoru Chikamune <chikamune@chromium.org>
Cr-Original-Commit-Position: refs/heads/main@{#992781}
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3590312
Bot-Commit: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
Cr-Commit-Position: refs/branch-heads/5005@{#41}
Cr-Branched-From: 5b4d945-refs/heads/main@{#992738}
  • Loading branch information
chikamune-cr authored and Chromium LUCI CQ committed Apr 19, 2022
1 parent 6b940e3 commit 5e437c0
Show file tree
Hide file tree
Showing 12 changed files with 363 additions and 42 deletions.
210 changes: 210 additions & 0 deletions chrome/browser/subresource_filter/subresource_filter_browsertest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,7 @@
#include "components/subresource_filter/core/common/test_ruleset_creator.h"
#include "components/subresource_filter/core/common/test_ruleset_utils.h"
#include "components/subresource_filter/core/mojom/subresource_filter.mojom.h"
#include "components/ukm/test_ukm_recorder.h"
#include "components/url_pattern_index/proto/rules.pb.h"
#include "content/public/browser/navigation_handle.h"
#include "content/public/browser/page_navigator.h"
Expand All @@ -61,15 +62,18 @@
#include "content/public/common/content_features.h"
#include "content/public/common/content_switches.h"
#include "content/public/common/referrer.h"
#include "content/public/test/back_forward_cache_util.h"
#include "content/public/test/browser_test.h"
#include "content/public/test/browser_test_utils.h"
#include "content/public/test/no_renderer_crashes_assertion.h"
#include "content/public/test/test_navigation_observer.h"
#include "content/public/test/test_utils.h"
#include "net/test/embedded_test_server/embedded_test_server.h"
#include "services/metrics/public/cpp/ukm_builders.h"
#include "testing/gmock/include/gmock/gmock.h"
#include "testing/gtest/include/gtest/gtest.h"
#include "third_party/blink/public/common/chrome_debug_urls.h"
#include "third_party/blink/public/common/features.h"
#include "url/gurl.h"

namespace subresource_filter {
Expand Down Expand Up @@ -1181,4 +1185,210 @@ IN_PROC_BROWSER_TEST_F(SubresourceFilterBrowserTest,
EXPECT_FALSE(WasParsedScriptElementLoaded(child_rfh));
}

struct AutomaticLazyLoadFrameBrowserTestParam {
bool enabled_lazy_ads_and_embeds;
bool enable_lazy_embed_urls;
int number_of_ads;
int number_of_embeds;
};

class AutomaticLazyLoadFrameBrowserTest
: public SubresourceFilterBrowserTest,
public ::testing::WithParamInterface<
AutomaticLazyLoadFrameBrowserTestParam> {
public:
AutomaticLazyLoadFrameBrowserTest() {
if (GetParam().enabled_lazy_ads_and_embeds) {
// kAutomaticLazyFrameLoadingToEmbedUrls should be enabled when
// kAutomaticLazyFrameLoadingToEmbeds is enabled.
EXPECT_TRUE(GetParam().enable_lazy_embed_urls);
feature_list_.InitWithFeaturesAndParameters(
/*enabled_features=*/
{{blink::features::kAutomaticLazyFrameLoadingToEmbedUrls,
{{"allowed_websites", "http://embed.com|/title1.html"}}},
{blink::features::kAutomaticLazyFrameLoadingToAds, {}},
{blink::features::kAutomaticLazyFrameLoadingToEmbeds, {}}},
/*disabled_features=*/
{});
} else if (GetParam().enable_lazy_embed_urls) {
// kAutomaticLazyFrameLoadingToEmbedUrls should be enabled when we want
// to record LazyEmbedFrameCount UKM even when
// kAutomaticLazyFrameLoadingToEmbeds is disabled.
feature_list_.InitWithFeaturesAndParameters(
/*enabled_features=*/
{{blink::features::kAutomaticLazyFrameLoadingToEmbedUrls,
{{"allowed_websites", "http://embed.com|/title1.html"}}}},
/*disabled_features=*/
{blink::features::kAutomaticLazyFrameLoadingToAds,
blink::features::kAutomaticLazyFrameLoadingToEmbeds});
} else {
feature_list_.InitWithFeaturesAndParameters(
/*enabled_features=*/
{},
/*disabled_features=*/
{blink::features::kAutomaticLazyFrameLoadingToAds,
blink::features::kAutomaticLazyFrameLoadingToEmbeds,
blink::features::kAutomaticLazyFrameLoadingToEmbedUrls});
}
}

protected:
void SetUpOnMainThread() override {
SubresourceFilterBrowserTest::SetUpOnMainThread();
SetRulesetWithRules(
{subresource_filter::testing::CreateSuffixRule("ad_iframe_writer.js")});
}

void AddAdIframe(content::RenderFrameHost* render_frame_host,
const GURL& url) {
EXPECT_TRUE(ExecJs(render_frame_host,
content::JsReplace("createAdIframeWithSrc($1);", url)));
}

void AddLazyAdIframe(content::RenderFrameHost* render_frame_host,
const GURL& url) {
EXPECT_TRUE(
ExecJs(render_frame_host,
content::JsReplace("createLazyAdIframeWithSrc($1);", url)));
}

void AddIframe(content::RenderFrameHost* render_frame_host, const GURL& url) {
const base::StringPiece script = R"(
const iframeElement = document.createElement("iframe");
iframeElement.src = $1;
document.body.appendChild(iframeElement);
)";
EXPECT_TRUE(ExecJs(render_frame_host, content::JsReplace(script, url)));
}

void AddLazyIframe(content::RenderFrameHost* render_frame_host,
const GURL& url) {
const base::StringPiece script = R"(
const iframeElement = document.createElement("iframe");
iframeElement.src = $1;
iframeElement.loading = 'lazy';
document.body.appendChild(iframeElement);
)";
EXPECT_TRUE(ExecJs(render_frame_host, content::JsReplace(script, url)));
}

private:
base::test::ScopedFeatureList feature_list_;
};

IN_PROC_BROWSER_TEST_P(AutomaticLazyLoadFrameBrowserTest, UKM) {
// Ensure that the previous page won't be stored in the back/forward cache, so
// that the histogram will be recorded when the previous page is unloaded.
DisableBackForwardCacheForTesting(
web_contents(), content::BackForwardCache::TEST_REQUIRES_NO_CACHING);

base::RunLoop ukm_loop;
ukm::TestAutoSetUkmRecorder ukm_recorder;
ukm_recorder.SetOnAddEntryCallback(
ukm::builders::Blink_AutomaticLazyLoadFrame::kEntryName,
ukm_loop.QuitClosure());

const GURL kMainFrameUrl(embedded_test_server()->GetURL(
"a_main_frame.com", "/ads_observer/blank_with_adiframe_writer.html"));
const GURL kAdUrl(embedded_test_server()->GetURL("ad.com", "/title1.html"));
const GURL kEmbedUrl(
embedded_test_server()->GetURL("embed.com", "/title1.html"));
const GURL kNonAdNonEmbed(
embedded_test_server()->GetURL("non_ad_non_embed.com", "/title1.html"));

content::RenderFrameHost* render_frame_host =
ui_test_utils::NavigateToURL(browser(), kMainFrameUrl);
ASSERT_TRUE(render_frame_host);

for (int i = 0; i < GetParam().number_of_ads; i++)
AddAdIframe(render_frame_host, kAdUrl);

for (int i = 0; i < GetParam().number_of_embeds; i++)
AddIframe(render_frame_host, kEmbedUrl);

// Add ad-iframe that is already specified to lazy-load which should not be
// counted as LazyAdsFrameCount.
AddLazyAdIframe(render_frame_host, kEmbedUrl);

// Add embed-iframe that is already specified to lazy-load which should not be
// counted as LazyEmbedFrameCount.
AddLazyIframe(render_frame_host, kEmbedUrl);

// Add iframe that is not detected as an ad-frame nor an embed.
AddIframe(render_frame_host, kNonAdNonEmbed);

// Navigating away from the test page (kMainFrameUrl) causes the document to
// be unloaded. That will cause any buffered metrics to be flushed.
content::NavigateToURLBlockUntilNavigationsComplete(web_contents(),
GURL("about:blank"), 1);

// Waits until UKM data is recorded.
ukm_loop.Run();

// Checks merged metrics by singular="True".
auto merged_entries = ukm_recorder.GetMergedEntriesByName(
ukm::builders::Blink_AutomaticLazyLoadFrame::kEntryName);
EXPECT_EQ(1u, merged_entries.size());
for (auto& entry : merged_entries) {
const ukm::mojom::UkmEntry* ukm_entry = entry.second.get();
ukm_recorder.ExpectEntrySourceHasUrl(ukm_entry, kMainFrameUrl);
ukm::TestAutoSetUkmRecorder::ExpectEntryMetric(
ukm_entry, "LazyAdsFrameCount", GetParam().number_of_ads);
ukm::TestAutoSetUkmRecorder::ExpectEntryMetric(
ukm_entry, "LazyEmbedsFrameCount",
GetParam().enable_lazy_embed_urls ? GetParam().number_of_embeds : 0);
}
}

const AutomaticLazyLoadFrameBrowserTestParam
kAutomaticLazyLoadFrameBrowserTestParams[] = {
{
.enabled_lazy_ads_and_embeds = false,
.enable_lazy_embed_urls = true,
.number_of_ads = 2,
.number_of_embeds = 0,
},
{
.enabled_lazy_ads_and_embeds = false,
.enable_lazy_embed_urls = true,
.number_of_ads = 0,
.number_of_embeds = 2,
},
{
.enabled_lazy_ads_and_embeds = false,
.enable_lazy_embed_urls = true,
.number_of_ads = 2,
.number_of_embeds = 2,
},
{
.enabled_lazy_ads_and_embeds = false,
.enable_lazy_embed_urls = false,
.number_of_ads = 2,
.number_of_embeds = 2,
},
{
.enabled_lazy_ads_and_embeds = true,
.enable_lazy_embed_urls = true,
.number_of_ads = 2,
.number_of_embeds = 0,
},
{
.enabled_lazy_ads_and_embeds = true,
.enable_lazy_embed_urls = true,
.number_of_ads = 0,
.number_of_embeds = 2,
},
{
.enabled_lazy_ads_and_embeds = true,
.enable_lazy_embed_urls = true,
.number_of_ads = 2,
.number_of_embeds = 2,
},
};

INSTANTIATE_TEST_SUITE_P(
All,
AutomaticLazyLoadFrameBrowserTest,
::testing::ValuesIn(kAutomaticLazyLoadFrameBrowserTestParams));

} // namespace subresource_filter
8 changes: 8 additions & 0 deletions components/test/data/ads_observer/ad_iframe_writer.js
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,14 @@ function createAdIframeWithSrc(src) {
return frame;
}

function createLazyAdIframeWithSrc(src) {
const frame = document.createElement('iframe');
frame.src = src;
frame.loading = 'lazy';
document.body.appendChild(frame);
return frame;
}

function createAdIframeAtRect(x, y, width, height) {
const frame = document.createElement('iframe');
frame.style.border = '0px none transparent';
Expand Down
1 change: 1 addition & 0 deletions testing/variations/fieldtrial_testing_config.json
Original file line number Diff line number Diff line change
Expand Up @@ -1366,6 +1366,7 @@
"allowed_websites": "https://www.youtube.com|/embed,https://www.google.com|/maps/embed,https://platform.twitter.com|/embed,https://www.redditmedia.com|embed=true,https://www.instagram.com|/embed"
},
"enable_features": [
"AutomaticLazyFrameLoadingToEmbedUrls",
"AutomaticLazyFrameLoadingToEmbeds"
]
}
Expand Down
7 changes: 7 additions & 0 deletions third_party/blink/common/features.cc
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,13 @@ const base::Feature kAutomaticLazyFrameLoadingToAds{
const base::Feature kAutomaticLazyFrameLoadingToEmbeds{
"AutomaticLazyFrameLoadingToEmbeds", base::FEATURE_DISABLED_BY_DEFAULT};

// Define the allowed websites to use LazyEmbeds. The allowed websites need to
// be defined separately from kAutomaticLazyFrameLoadingToEmbeds because we want
// to gather Blink.AutomaticLazyLoadFrame.LazyEmbedFrameCount UKM data even when
// kAutomaticLazyFrameLoadingToEmbeds is disabled.
const base::Feature kAutomaticLazyFrameLoadingToEmbedUrls{
"AutomaticLazyFrameLoadingToEmbedUrls", base::FEATURE_DISABLED_BY_DEFAULT};

// Allows pages with DedicatedWorker to stay eligible for the back/forward
// cache.
const base::Feature kBackForwardCacheDedicatedWorker{
Expand Down
2 changes: 2 additions & 0 deletions third_party/blink/public/common/features.h
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,8 @@ namespace features {
BLINK_COMMON_EXPORT extern const base::Feature kAutomaticLazyFrameLoadingToAds;
BLINK_COMMON_EXPORT extern const base::Feature
kAutomaticLazyFrameLoadingToEmbeds;
BLINK_COMMON_EXPORT extern const base::Feature
kAutomaticLazyFrameLoadingToEmbedUrls;
BLINK_COMMON_EXPORT extern const base::Feature kBackForwardCacheDedicatedWorker;
BLINK_COMMON_EXPORT extern const base::Feature
kBlockingDownloadsInAdFrameWithoutUserActivation;
Expand Down
21 changes: 21 additions & 0 deletions third_party/blink/renderer/core/dom/document.cc
Original file line number Diff line number Diff line change
Expand Up @@ -2805,6 +2805,19 @@ void Document::Shutdown() {
if (num_canvases_ > 0)
UMA_HISTOGRAM_COUNTS_100("Blink.Canvas.NumCanvasesPerPage", num_canvases_);

if (!data_->already_sent_automatic_lazy_load_frame_ukm_) {
data_->already_sent_automatic_lazy_load_frame_ukm_ = true;
if (data_->lazy_ads_frame_count_ > 0 ||
data_->lazy_embeds_frame_count_ > 0) {
ukm::builders::Blink_AutomaticLazyLoadFrame(UkmSourceID())
.SetLazyAdsFrameCount(ukm::GetExponentialBucketMinForCounts1000(
data_->lazy_ads_frame_count_))
.SetLazyEmbedsFrameCount(ukm::GetExponentialBucketMinForCounts1000(
data_->lazy_embeds_frame_count_))
.Record(UkmRecorder());
}
}

GetFontMatchingMetrics()->PublishAllMetrics();

GetViewportData().Shutdown();
Expand Down Expand Up @@ -6610,6 +6623,14 @@ HTMLCollection* Document::DocumentAllNamedItems(const AtomicString& name) {
kDocumentAllNamedItems, name);
}

void Document::IncrementLazyAdsFrameCount() {
data_->lazy_ads_frame_count_++;
}

void Document::IncrementLazyEmbedsFrameCount() {
data_->lazy_embeds_frame_count_++;
}

DOMWindow* Document::defaultView() const {
return dom_window_;
}
Expand Down
3 changes: 3 additions & 0 deletions third_party/blink/renderer/core/dom/document.h
Original file line number Diff line number Diff line change
Expand Up @@ -1796,6 +1796,9 @@ class CORE_EXPORT Document : public ContainerNode,

bool RenderingHasBegun() const { return rendering_has_begun_; }

void IncrementLazyAdsFrameCount();
void IncrementLazyEmbedsFrameCount();

enum class DeclarativeShadowRootAllowState : uint8_t {
kNotSet,
kAllow,
Expand Down
12 changes: 12 additions & 0 deletions third_party/blink/renderer/core/dom/document_data.h
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,18 @@ class DocumentData final : public GarbageCollected<DocumentData> {
// To do email regex checks.
Member<ScriptRegexp> email_regexp_;

// The total number of per-page ad frames that are eligible for the LazyAds
// interventions by AutomaticLazyFrameLoadingToAds. This is used to report
// UKM.
int64_t lazy_ads_frame_count_ = 0;
// The total number of per-page frames that are eligible for the LazyEmbeds
// interventions by AutomaticLazyFrameLoadingToEmbeds. This is used to report
// UKM.
int64_t lazy_embeds_frame_count_ = 0;
// |Document::Shutdown()| is called multiple times. The following flag
// prevents sending UKM multiple times.
bool already_sent_automatic_lazy_load_frame_ukm_ = false;

friend class Document;
};

Expand Down

0 comments on commit 5e437c0

Please sign in to comment.