Commit 7c66a5db authored by Josh Karlin's avatar Josh Karlin Committed by Commit Bot

[AdTagging] Optimize IsAdScriptInStack

When scanning the stack we're really just checking if any element in
the vector is true. We can replace this O(n) check with O(1) if we
keep track of the number of true elements in the vector.

Bug: 894505
Change-Id: I378cffe80212277a7fa8ad414e15a5cf3c9824c8
Reviewed-on: https://chromium-review.googlesource.com/c/1289329
Commit-Queue: Josh Karlin <jkarlin@chromium.org>
Reviewed-by: default avatarNate Chapin <japhet@chromium.org>
Cr-Commit-Position: refs/heads/master@{#601002}
parent 98c62bdc
...@@ -67,9 +67,15 @@ void AdTracker::WillExecuteScript(ExecutionContext* execution_context, ...@@ -67,9 +67,15 @@ void AdTracker::WillExecuteScript(ExecutionContext* execution_context,
? false ? false
: IsKnownAdScript(execution_context, script_url); : IsKnownAdScript(execution_context, script_url);
stack_frame_is_ad_.push_back(is_ad); stack_frame_is_ad_.push_back(is_ad);
if (is_ad)
num_ads_in_stack_ += 1;
} }
void AdTracker::DidExecuteScript() { void AdTracker::DidExecuteScript() {
if (stack_frame_is_ad_.back()) {
DCHECK_LT(0u, num_ads_in_stack_);
num_ads_in_stack_ -= 1;
}
stack_frame_is_ad_.pop_back(); stack_frame_is_ad_.pop_back();
} }
...@@ -127,6 +133,9 @@ void AdTracker::WillSendRequest(ExecutionContext* execution_context, ...@@ -127,6 +133,9 @@ void AdTracker::WillSendRequest(ExecutionContext* execution_context,
} }
bool AdTracker::IsAdScriptInStack() { bool AdTracker::IsAdScriptInStack() {
if (num_ads_in_stack_ > 0)
return true;
ExecutionContext* execution_context = GetCurrentExecutionContext(); ExecutionContext* execution_context = GetCurrentExecutionContext();
if (!execution_context) if (!execution_context)
return false; return false;
...@@ -144,11 +153,6 @@ bool AdTracker::IsAdScriptInStack() { ...@@ -144,11 +153,6 @@ bool AdTracker::IsAdScriptInStack() {
if (!top_script.IsEmpty() && IsKnownAdScript(execution_context, top_script)) if (!top_script.IsEmpty() && IsKnownAdScript(execution_context, top_script))
return true; return true;
// Scan the pseudo-stack for ad scripts.
for (bool is_ad : stack_frame_is_ad_) {
if (is_ad)
return true;
}
return false; return false;
} }
......
...@@ -85,6 +85,8 @@ class CORE_EXPORT AdTracker : public GarbageCollectedFinalized<AdTracker> { ...@@ -85,6 +85,8 @@ class CORE_EXPORT AdTracker : public GarbageCollectedFinalized<AdTracker> {
// an ad script. Each time the script or function finishes, it pops the stack. // an ad script. Each time the script or function finishes, it pops the stack.
Vector<bool> stack_frame_is_ad_; Vector<bool> stack_frame_is_ad_;
uint32_t num_ads_in_stack_ = 0;
// The set of ad scripts detected outside of ad-frame contexts. // The set of ad scripts detected outside of ad-frame contexts.
HeapHashMap<WeakMember<ExecutionContext>, HashSet<String>> known_ad_scripts_; HeapHashMap<WeakMember<ExecutionContext>, HashSet<String>> known_ad_scripts_;
......
...@@ -94,6 +94,8 @@ class AdTrackerTest : public testing::Test { ...@@ -94,6 +94,8 @@ class AdTrackerTest : public testing::Test {
String(script_url)); String(script_url));
} }
void DidExecuteScript() { ad_tracker_->DidExecuteScript(); }
bool AnyExecutingScriptsTaggedAsAdResource() { bool AnyExecutingScriptsTaggedAsAdResource() {
return ad_tracker_->IsAdScriptInStack(); return ad_tracker_->IsAdScriptInStack();
} }
...@@ -161,6 +163,39 @@ TEST_F(AdTrackerTest, TopOfStackIncluded) { ...@@ -161,6 +163,39 @@ TEST_F(AdTrackerTest, TopOfStackIncluded) {
EXPECT_TRUE(AnyExecutingScriptsTaggedAsAdResource()); EXPECT_TRUE(AnyExecutingScriptsTaggedAsAdResource());
} }
TEST_F(AdTrackerTest, AdStackFrameCounting) {
AppendToKnownAdScripts("https://example.com/ad.js");
WillExecuteScript("https://example.com/vanilla.js");
WillExecuteScript("https://example.com/vanilla.js");
EXPECT_FALSE(AnyExecutingScriptsTaggedAsAdResource());
WillExecuteScript("https://example.com/ad.js");
EXPECT_TRUE(AnyExecutingScriptsTaggedAsAdResource());
DidExecuteScript();
EXPECT_FALSE(AnyExecutingScriptsTaggedAsAdResource());
WillExecuteScript("https://example.com/ad.js");
WillExecuteScript("https://example.com/ad.js");
WillExecuteScript("https://example.com/vanilla.js");
EXPECT_TRUE(AnyExecutingScriptsTaggedAsAdResource());
DidExecuteScript();
DidExecuteScript();
EXPECT_TRUE(AnyExecutingScriptsTaggedAsAdResource());
DidExecuteScript();
EXPECT_FALSE(AnyExecutingScriptsTaggedAsAdResource());
DidExecuteScript();
DidExecuteScript();
EXPECT_FALSE(AnyExecutingScriptsTaggedAsAdResource());
WillExecuteScript("https://example.com/ad.js");
EXPECT_TRUE(AnyExecutingScriptsTaggedAsAdResource());
}
class AdTrackerSimTest : public SimTest { class AdTrackerSimTest : public SimTest {
protected: protected:
void SetUp() override { void SetUp() override {
......
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