Skip to content

Commit

Permalink
Add UseCounter for download in AdFrame
Browse files Browse the repository at this point in the history 
Bug: 911674
Change-Id: Ia010b625ed72ca575fc1aeafdf64990bcd2171d8
Reviewed-on: https://chromium-review.googlesource.com/c/1366766
Commit-Queue: Yao Xiao <yaoxia@chromium.org>
Reviewed-by: Bryan McQuade <bmcquade@chromium.org>
Reviewed-by: Josh Karlin <jkarlin@chromium.org>
Reviewed-by: Jochen Eisinger <jochen@chromium.org>
Cr-Commit-Position: refs/heads/master@{#616021}
  • Loading branch information
@yaoxiachromium
yaoxiachromium authored and Commit Bot committed Dec 12, 2018
1 parent e463407 commit e6fb7a0
Show file tree
Hide file tree
Showing 8 changed files with 128 additions and 18 deletions.
19 changes: 16 additions & 3 deletions chrome/browser/page_load_metrics/observers/ads_page_load_metrics_observer.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -247,14 +247,27 @@ void AdsPageLoadMetricsObserver::OnDidFinishSubFrameNavigation(
value |= blink::DownloadStats::kGestureBit;
blink::DownloadStats::RecordSubframeSandboxOriginAdGesture(value);

std::vector<blink::mojom::WebFeature> web_features;
if (ad_types.any()) {
// Note: Here it covers download due to navigations to non-web-renderable
// content. These two features can also be logged from blink for download
// originated from clicking on <a download> link that results in direct
// download.
web_features.push_back(
gesture
? blink::mojom::WebFeature::kDownloadInAdFrameWithUserGesture
: blink::mojom::WebFeature::kDownloadInAdFrameWithoutUserGesture);
}
if (sandboxed) {
blink::mojom::WebFeature web_feature =
web_features.push_back(
gesture ? blink::mojom::WebFeature::
kNavigationDownloadInSandboxWithUserGesture
: blink::mojom::WebFeature::
kNavigationDownloadInSandboxWithoutUserGesture;
kNavigationDownloadInSandboxWithoutUserGesture);
}
if (!web_features.empty()) {
page_load_metrics::mojom::PageLoadFeatures page_load_features(
{web_feature}, {} /* css_properties */,
web_features, {} /* css_properties */,
{} /* animated_css_properties */);
page_load_metrics::MetricsWebContentsObserver::RecordFeatureUsage(
ad_host, page_load_features);
Expand Down
50 changes: 36 additions & 14 deletions chrome/browser/page_load_metrics/observers/ads_page_load_metrics_observer_browsertest.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -667,6 +667,10 @@ IN_PROC_BROWSER_TEST_P(SubframeDownloadSandboxOriginAdGestureBrowserTest,
sandbox_option == SandboxOption::kSandboxDisallowDownloads
? 0
: 1;
bool expected_sandbox_bit =
enable_blocking_downloads_in_sandbox
? sandbox_option == SandboxOption::kSandboxDisallowDownloads
: sandbox_option != SandboxOption::kNoSandbox;

std::unique_ptr<content::DownloadTestObserver> download_observer(
new content::DownloadTestObserverTerminal(
Expand All @@ -678,9 +682,39 @@ IN_PROC_BROWSER_TEST_P(SubframeDownloadSandboxOriginAdGestureBrowserTest,
"chrome/test/data/ad_tagging");
content::SetupCrossSiteRedirector(embedded_test_server());
ASSERT_TRUE(embedded_test_server()->Start());

content::WebContents* contents =
browser()->tab_strip_model()->GetActiveWebContents();

std::unique_ptr<AdsPageLoadMetricsTestWaiter> waiter;
if (expected_download_count > 0) {
if (ad_frame) {
if (!waiter)
waiter = std::make_unique<AdsPageLoadMetricsTestWaiter>(contents);
blink::mojom::WebFeature feature =
gesture
? blink::mojom::WebFeature::kDownloadInAdFrameWithUserGesture
: blink::mojom::WebFeature::kDownloadInAdFrameWithoutUserGesture;
waiter->AddWebFeatureExpectation(feature);
}
if (expected_sandbox_bit) {
if (!waiter)
waiter = std::make_unique<AdsPageLoadMetricsTestWaiter>(contents);
blink::mojom::WebFeature feature =
origin == Origin::kNavigation
? gesture ? blink::mojom::WebFeature::
kNavigationDownloadInSandboxWithUserGesture
: blink::mojom::WebFeature::
kNavigationDownloadInSandboxWithoutUserGesture
: gesture
? blink::mojom::WebFeature::
kHTMLAnchorElementDownloadInSandboxWithUserGesture
: blink::mojom::WebFeature::
kHTMLAnchorElementDownloadInSandboxWithoutUserGesture;
waiter->AddWebFeatureExpectation(feature);
}
}

std::string host_name = "foo.com";
ui_test_utils::NavigateToURL(
browser(),
Expand Down Expand Up @@ -715,6 +749,8 @@ IN_PROC_BROWSER_TEST_P(SubframeDownloadSandboxOriginAdGestureBrowserTest,
OpenLinkInFrame(rfh, link_id, gesture);

download_observer->WaitForFinished();
if (waiter)
waiter->Wait();
SubprocessMetricsProvider::MergeHistogramDeltasForTesting();

if (expected_download_count == 0) {
Expand All @@ -724,10 +760,6 @@ IN_PROC_BROWSER_TEST_P(SubframeDownloadSandboxOriginAdGestureBrowserTest,
}

unsigned expected_value = 0;
bool expected_sandbox_bit =
enable_blocking_downloads_in_sandbox
? sandbox_option == SandboxOption::kSandboxDisallowDownloads
: sandbox_option != SandboxOption::kNoSandbox;
if (expected_sandbox_bit)
expected_value |= blink::DownloadStats::kSandboxBit;
if (cross_origin)
Expand All @@ -740,16 +772,6 @@ IN_PROC_BROWSER_TEST_P(SubframeDownloadSandboxOriginAdGestureBrowserTest,
histogram_tester.ExpectUniqueSample(
"Download.Subframe.SandboxOriginAdGesture", expected_value,
1 /* expected_count */);

if (expected_sandbox_bit && origin == Origin::kNavigation) {
blink::mojom::WebFeature feature =
gesture ? blink::mojom::WebFeature::
kNavigationDownloadInSandboxWithUserGesture
: blink::mojom::WebFeature::
kNavigationDownloadInSandboxWithoutUserGesture;
histogram_tester.ExpectBucketCount(internal::kFeaturesHistogramName,
feature, 1 /* expected_count */);
}
}

INSTANTIATE_TEST_CASE_P(
Expand Down
2 changes: 2 additions & 0 deletions chrome/browser/page_load_metrics/observers/use_counter/ukm_features.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,8 @@ bool IsAllowedUkmFeature(blink::mojom::WebFeature feature) {
WebFeature::kHTMLAnchorElementDownloadInSandboxWithoutUserGesture,
WebFeature::kNavigationDownloadInSandboxWithUserGesture,
WebFeature::kNavigationDownloadInSandboxWithoutUserGesture,
WebFeature::kDownloadInAdFrameWithUserGesture,
WebFeature::kDownloadInAdFrameWithoutUserGesture,
}));
return opt_in_features->count(feature);
}
44 changes: 43 additions & 1 deletion chrome/browser/page_load_metrics/page_load_metrics_test_waiter.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,14 @@ void PageLoadMetricsTestWaiter::AddSubFrameExpectation(TimingField field) {
page_expected_fields_.Set(field);
}

void PageLoadMetricsTestWaiter::AddWebFeatureExpectation(
blink::mojom::WebFeature web_feature) {
size_t feature_idx = static_cast<size_t>(web_feature);
if (!expected_web_features_.test(feature_idx)) {
expected_web_features_.set(feature_idx);
}
}

void PageLoadMetricsTestWaiter::AddMinimumCompleteResourcesExpectation(
int expected_minimum_complete_resources) {
expected_minimum_complete_resources_ = expected_minimum_complete_resources;
Expand Down Expand Up @@ -128,6 +136,23 @@ void PageLoadMetricsTestWaiter::OnResourceDataUseObserved(
run_loop_->Quit();
}

void PageLoadMetricsTestWaiter::OnFeaturesUsageObserved(
const mojom::PageLoadFeatures& features,
const PageLoadExtraInfo& extra_info) {
if (WebFeaturesExpectationsSatisfied())
return;

for (blink::mojom::WebFeature feature : features.features) {
size_t feature_idx = static_cast<size_t>(feature);
if (observed_web_features_.test(feature_idx))
continue;
observed_web_features_.set(feature_idx);
}

if (ExpectationsSatisfied() && run_loop_)
run_loop_->Quit();
}

bool PageLoadMetricsTestWaiter::IsPageLevelField(TimingField field) {
switch (field) {
case TimingField::kFirstPaint:
Expand Down Expand Up @@ -193,9 +218,19 @@ bool PageLoadMetricsTestWaiter::ResourceUseExpectationsSatisfied() const {
current_network_bytes_ >= expected_minimum_network_bytes_);
}

bool PageLoadMetricsTestWaiter::WebFeaturesExpectationsSatisfied() const {
// We are only interested to see if all features being set in
// |expected_web_features_| are observed, but don't care about whether extra
// features are observed.
return (expected_web_features_ & observed_web_features_ ^
expected_web_features_)
.none();
}

bool PageLoadMetricsTestWaiter::ExpectationsSatisfied() const {
return subframe_expected_fields_.Empty() && page_expected_fields_.Empty() &&
ResourceUseExpectationsSatisfied();
ResourceUseExpectationsSatisfied() &&
WebFeaturesExpectationsSatisfied();
}

PageLoadMetricsTestWaiter::WaiterMetricsObserver::~WaiterMetricsObserver() {}
Expand Down Expand Up @@ -227,4 +262,11 @@ void PageLoadMetricsTestWaiter::WaiterMetricsObserver::
waiter_->OnResourceDataUseObserved(resources);
}

void PageLoadMetricsTestWaiter::WaiterMetricsObserver::OnFeaturesUsageObserved(
const mojom::PageLoadFeatures& features,
const PageLoadExtraInfo& extra_info) {
if (waiter_)
waiter_->OnFeaturesUsageObserved(features, extra_info);
}

} // namespace page_load_metrics
17 changes: 17 additions & 0 deletions chrome/browser/page_load_metrics/page_load_metrics_test_waiter.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,9 @@ class PageLoadMetricsTestWaiter
// Add a subframe-level expectation.
void AddSubFrameExpectation(TimingField field);

// Add a single WebFeature expectation.
void AddWebFeatureExpectation(blink::mojom::WebFeature web_feature);

// Add a minimum completed resource expectation.
void AddMinimumCompleteResourcesExpectation(
int expected_minimum_complete_resources);
Expand Down Expand Up @@ -92,6 +95,9 @@ class PageLoadMetricsTestWaiter
const std::vector<page_load_metrics::mojom::ResourceDataUpdatePtr>&
resources) override;

void OnFeaturesUsageObserved(const mojom::PageLoadFeatures& features,
const PageLoadExtraInfo& extra_info) override;

private:
const base::WeakPtr<PageLoadMetricsTestWaiter> waiter_;
};
Expand Down Expand Up @@ -153,18 +159,29 @@ class PageLoadMetricsTestWaiter
const std::vector<page_load_metrics::mojom::ResourceDataUpdatePtr>&
resources);

// Updates |observed_web_features_| to record any new feature observed.
// Stops waiting if expectations are satisfied after update.
void OnFeaturesUsageObserved(const mojom::PageLoadFeatures& features,
const PageLoadExtraInfo& extra_info);

void OnTrackerCreated(page_load_metrics::PageLoadTracker* tracker) override;

void OnCommit(page_load_metrics::PageLoadTracker* tracker) override;

bool ResourceUseExpectationsSatisfied() const;

bool WebFeaturesExpectationsSatisfied() const;

std::unique_ptr<base::RunLoop> run_loop_;

TimingFieldBitSet page_expected_fields_;
TimingFieldBitSet subframe_expected_fields_;
std::bitset<static_cast<size_t>(blink::mojom::WebFeature::kNumberOfFeatures)>
expected_web_features_;

TimingFieldBitSet observed_page_fields_;
std::bitset<static_cast<size_t>(blink::mojom::WebFeature::kNumberOfFeatures)>
observed_web_features_;

int current_complete_resources_ = 0;
int64_t current_network_bytes_ = 0;
Expand Down
2 changes: 2 additions & 0 deletions third_party/blink/public/platform/web_feature.mojom
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2090,6 +2090,8 @@ enum WebFeature {
kOpenerNavigationDownloadCrossOriginNoGesture = 2649,
kV8RegExpMatchIsTrueishOnNonJSRegExp = 2650,
kV8RegExpMatchIsFalseishOnJSRegExp = 2651,
kDownloadInAdFrameWithUserGesture = 2652,
kDownloadInAdFrameWithoutUserGesture = 2653,

// Add new features immediately above this line. Don't change assigned
// numbers of any item, and don't reuse removed slots.
Expand Down
10 changes: 10 additions & 0 deletions third_party/blink/renderer/core/html/html_anchor_element.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -404,6 +404,16 @@ void HTMLAnchorElement::HandleClick(Event& event) {
if (hasAttribute(kDownloadAttr) &&
NavigationPolicyFromEvent(&event) != kNavigationPolicyDownload &&
GetDocument().GetSecurityOrigin()->CanReadContent(completed_url)) {
if (frame->IsAdSubframe()) {
// Note: Here it covers download originated from clicking on <a download>
// link that results in direct download. These two features can also be
// logged from browser for download due to navigations to
// non-web-renderable content.
UseCounter::Count(GetDocument(),
LocalFrame::HasTransientUserActivation(frame)
? WebFeature::kDownloadInAdFrameWithUserGesture
: WebFeature::kDownloadInAdFrameWithoutUserGesture);
}
if (GetDocument().IsSandboxed(kSandboxDownloads)) {
if (RuntimeEnabledFeatures::BlockingDownloadsInSandboxEnabled())
return;
Expand Down
2 changes: 2 additions & 0 deletions tools/metrics/histograms/enums.xml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21007,6 +21007,8 @@ Called by update_net_error_codes.py.-->
<int value="2649" label="OpenerNavigationDownloadCrossOriginNoGesture"/>
<int value="2650" label="V8RegExpMatchIsTrueishOnNonJSRegExp"/>
<int value="2651" label="V8RegExpMatchIsFalseishOnJSRegExp"/>
<int value="2652" label="DownloadInAdFrameWithUserGesture"/>
<int value="2653" label="DownloadInAdFrameWithoutUserGesture"/>
</enum>

<enum name="FeaturePolicyFeature">
Expand Down

0 comments on commit e6fb7a0

Please sign in to comment.