Commit b959e58a authored by Josh Karlin's avatar Josh Karlin Committed by Commit Bot

[AdTagging] Experiment with only looking at the top of the stack

What: Add a feature to cause AdTagging to only tag a v8 stack as an ad
stack if the top of the stack is an ad script. E.g., ignore the
bottom.

Why: Because sometimes an event on an ad script is used to create
content (e.g., the ad finishes showing so now reveal the page
content by calling content script).

Bug: 1022802
Change-Id: I54ef3f0930f886df01320a714e72e8e4554a0357
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1906467Reviewed-by: default avatarDaniel Cheng <dcheng@chromium.org>
Commit-Queue: Josh Karlin <jkarlin@chromium.org>
Cr-Commit-Position: refs/heads/master@{#714675}
parent 4160a16f
...@@ -38,6 +38,11 @@ namespace features { ...@@ -38,6 +38,11 @@ namespace features {
// the currently running stack is ad related. // the currently running stack is ad related.
const base::Feature kAsyncStackAdTagging{"AsyncStackAdTagging", const base::Feature kAsyncStackAdTagging{"AsyncStackAdTagging",
base::FEATURE_DISABLED_BY_DEFAULT}; base::FEATURE_DISABLED_BY_DEFAULT};
// Controls whether the AdTracker analyzes the whole pseudo-stack or just the
// top of the stack when detecting ads.
const base::Feature kTopOfStackAdTagging{"TopOfStackAdTagging",
base::FEATURE_DISABLED_BY_DEFAULT};
} // namespace features } // namespace features
// static // static
...@@ -57,7 +62,9 @@ AdTracker* AdTracker::FromExecutionContext( ...@@ -57,7 +62,9 @@ AdTracker* AdTracker::FromExecutionContext(
AdTracker::AdTracker(LocalFrame* local_root) AdTracker::AdTracker(LocalFrame* local_root)
: local_root_(local_root), : local_root_(local_root),
async_stack_enabled_( async_stack_enabled_(
base::FeatureList::IsEnabled(features::kAsyncStackAdTagging)) { base::FeatureList::IsEnabled(features::kAsyncStackAdTagging)),
top_of_stack_only_(
base::FeatureList::IsEnabled(features::kTopOfStackAdTagging)) {
local_root_->GetProbeSink()->AddAdTracker(this); local_root_->GetProbeSink()->AddAdTracker(this);
} }
...@@ -88,6 +95,9 @@ ExecutionContext* AdTracker::GetCurrentExecutionContext() { ...@@ -88,6 +95,9 @@ ExecutionContext* AdTracker::GetCurrentExecutionContext() {
void AdTracker::WillExecuteScript(ExecutionContext* execution_context, void AdTracker::WillExecuteScript(ExecutionContext* execution_context,
const String& script_url) { const String& script_url) {
if (top_of_stack_only_)
return;
bool is_ad = script_url.IsEmpty() bool is_ad = script_url.IsEmpty()
? false ? false
: IsKnownAdScript(execution_context, script_url); : IsKnownAdScript(execution_context, script_url);
...@@ -97,6 +107,9 @@ void AdTracker::WillExecuteScript(ExecutionContext* execution_context, ...@@ -97,6 +107,9 @@ void AdTracker::WillExecuteScript(ExecutionContext* execution_context,
} }
void AdTracker::DidExecuteScript() { void AdTracker::DidExecuteScript() {
if (top_of_stack_only_)
return;
if (stack_frame_is_ad_.back()) { if (stack_frame_is_ad_.back()) {
DCHECK_LT(0u, num_ads_in_stack_); DCHECK_LT(0u, num_ads_in_stack_);
num_ads_in_stack_ -= 1; num_ads_in_stack_ -= 1;
......
...@@ -28,6 +28,7 @@ class ExecuteScript; ...@@ -28,6 +28,7 @@ class ExecuteScript;
namespace features { namespace features {
CORE_EXPORT extern const base::Feature kAsyncStackAdTagging; CORE_EXPORT extern const base::Feature kAsyncStackAdTagging;
CORE_EXPORT extern const base::Feature kTopOfStackAdTagging;
} }
// Tracker for tagging resources as ads based on the call stack scripts. // Tracker for tagging resources as ads based on the call stack scripts.
...@@ -107,6 +108,7 @@ class CORE_EXPORT AdTracker : public GarbageCollected<AdTracker> { ...@@ -107,6 +108,7 @@ class CORE_EXPORT AdTracker : public GarbageCollected<AdTracker> {
uint32_t running_ad_async_tasks_ = 0; uint32_t running_ad_async_tasks_ = 0;
const bool async_stack_enabled_; const bool async_stack_enabled_;
const bool top_of_stack_only_;
DISALLOW_COPY_AND_ASSIGN(AdTracker); DISALLOW_COPY_AND_ASSIGN(AdTracker);
}; };
......
...@@ -136,6 +136,36 @@ TEST_F(AdTrackerTest, AnyExecutingScriptsTaggedAsAdResource) { ...@@ -136,6 +136,36 @@ TEST_F(AdTrackerTest, AnyExecutingScriptsTaggedAsAdResource) {
EXPECT_TRUE(AnyExecutingScriptsTaggedAsAdResource()); EXPECT_TRUE(AnyExecutingScriptsTaggedAsAdResource());
} }
TEST_F(AdTrackerTest, TopOfStackOnly_NoAdsOnTop) {
base::test::ScopedFeatureList feature_list;
feature_list.InitAndEnableFeature(features::kTopOfStackAdTagging);
CreateAdTracker();
String ad_script_url("https://example.com/bar.js");
AppendToKnownAdScripts(ad_script_url);
WillExecuteScript(ad_script_url);
WillExecuteScript("https://example.com/foo.js");
ad_tracker_->SetScriptAtTopOfStack("https://www.example.com/baz.js");
EXPECT_FALSE(AnyExecutingScriptsTaggedAsAdResource());
}
TEST_F(AdTrackerTest, TopOfStackOnly_AdsOnTop) {
base::test::ScopedFeatureList feature_list;
feature_list.InitAndEnableFeature(features::kTopOfStackAdTagging);
CreateAdTracker();
String ad_script_url("https://example.com/bar.js");
AppendToKnownAdScripts(ad_script_url);
WillExecuteScript(ad_script_url);
WillExecuteScript("https://example.com/foo.js");
ad_tracker_->SetScriptAtTopOfStack(ad_script_url);
EXPECT_TRUE(AnyExecutingScriptsTaggedAsAdResource());
}
// Tests that if neither script in the stack is an ad, // Tests that if neither script in the stack is an ad,
// AnyExecutingScriptsTaggedAsAdResource should return false. // AnyExecutingScriptsTaggedAsAdResource should return false.
TEST_F(AdTrackerTest, AnyExecutingScriptsTaggedAsAdResource_False) { TEST_F(AdTrackerTest, AnyExecutingScriptsTaggedAsAdResource_False) {
......
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