[AdTagging] An ad script is considered an ad if its context is an ad

When determining if a particular script is an ad, the execution context should be considered and any script running in an ad context should be considered an ad script. This provides enhanced coverage when an ad script then reaches into another context to run a function. Bug: 894080 Change-Id: I69de3343b004b6ef7b7edfe24c7cbda67c925101 Reviewed-on: https://chromium-review.googlesource.com/c/1273823Reviewed-by: Daniel Cheng <dcheng@chromium.org> Commit-Queue: Josh Karlin <jkarlin@chromium.org> Cr-Commit-Position: refs/heads/master@{#598821}

[AdTagging] An ad script is considered an ad if its context is an ad
When determining if a particular script is an ad, the execution context should be considered and any script running in an ad context should be considered an ad script. This provides enhanced coverage when an ad script then reaches into another context to run a function. Bug: 894080 Change-Id: I69de3343b004b6ef7b7edfe24c7cbda67c925101 Reviewed-on: https://chromium-review.googlesource.com/c/1273823Reviewed-by: Daniel Cheng <dcheng@chromium.org> Commit-Queue: Josh Karlin <jkarlin@chromium.org> Cr-Commit-Position: refs/heads/master@{#598821}
db904e67 · Josh Karlin · Commit Bot · b0d114d7 · db904e67 · db904e67
Commit db904e67 authored Oct 11, 2018 by Josh Karlin Committed by Commit Bot Oct 11, 2018
2 changed files
--- a/third_party/blink/renderer/core/frame/ad_tracker.cc
+++ b/third_party/blink/renderer/core/frame/ad_tracker.cc
@@ -18,6 +18,20 @@

 namespace blink {

+namespace {
+
+bool IsKnownAdExecutionContext(ExecutionContext* execution_context) {
+  // TODO(jkarlin): Do the same check for worker contexts.
+  if (auto* document = DynamicTo<Document>(execution_context)) {
+    LocalFrame* frame = document->GetFrame();
+    if (frame && frame->IsAdSubframe())
+      return true;
+  }
+  return false;
+}
+
+}  // namespace
+
 AdTracker::AdTracker(LocalFrame* local_root) : local_root_(local_root) {
  local_root_->GetProbeSink()->addAdTracker(this);
 }
@@ -112,21 +126,10 @@ bool AdTracker::IsAdScriptInStack() {
  if (!execution_context)
    return false;

-  // If script is running in an ad context, then we consider the script to be ad
-  // script.
-  // TODO(jkarlin): Do the same check for worker contexts.
-  //
-  // TODO(jkarlin): Look at the execution context of every frame in the stack,
-  // not just the current frame. This requires some changes to v8.
-  //
-  // TODO(jkarlin): Minor memory optimization, stop tracking known ad scripts in
-  // ad contexts. This will reduce the size of executing_scripts_. Note that
-  // this is a minor win, as the strings are already ref-counted.
-  if (auto* document = DynamicTo<Document>(execution_context)) {
-    LocalFrame* frame = document->GetFrame();
-    if (frame && frame->IsAdSubframe())
-      return true;
-  }
+  // If we're in an ad context, then no matter what the executing script is it's
+  // considered an ad.
+  if (IsKnownAdExecutionContext(execution_context))
+    return true;

  // The pseudo-stack contains entry points into the stack (e.g., when v8 is
  // executed) but not the entire stack. It's cheap to retrieve the top of the
@@ -149,6 +152,12 @@ bool AdTracker::IsKnownAdScript(ExecutionContext* execution_context,
  if (!execution_context)
    return false;

+  // TODO(jkarlin): Minor memory optimization, stop tracking known ad scripts in
+  // ad contexts. This will reduce the size of executing_scripts_. Note that
+  // this is a minor win, as the strings are already ref-counted.
+  if (IsKnownAdExecutionContext(execution_context))
+    return true;
+
  auto it = known_ad_scripts_.find(execution_context);
  if (it == known_ad_scripts_.end())
    return false;

--- a/third_party/blink/renderer/core/frame/ad_tracker_test.cc
+++ b/third_party/blink/renderer/core/frame/ad_tracker_test.cc
@@ -237,6 +237,39 @@ TEST_F(AdTrackerSimTest, ScriptDetectedByContext) {
  EXPECT_TRUE(ad_tracker_->IsAdScriptInStack());
 }

+// When inline script in an ad frame inserts an iframe into a non-ad frame, the
+// new frame should be considered an ad.
+TEST_F(AdTrackerSimTest, InlineAdScriptRunningInNonAdContext) {
+  SimRequest ad_script("https://example.com/ad_script.js", "text/javascript");
+  SimRequest ad_iframe("https://example.com/ad_frame.html", "text/html");
+  ad_tracker_->SetAdSuffix("ad_script.js");
+
+  main_resource_->Complete("<body><script src='ad_script.js'></script></body>");
+  ad_script.Complete(R"SCRIPT(
+    frame = document.createElement("iframe");
+    frame.src = "ad_frame.html";
+    document.body.appendChild(frame);
+    )SCRIPT");
+
+  // Verify that the new frame is an ad frame.
+  EXPECT_TRUE(ToLocalFrame(GetDocument().GetFrame()->Tree().FirstChild())
+                  ->IsAdSubframe());
+
+  // Create a new sibling frame to the ad frame. The ad context calls the non-ad
+  // context's (top frame) appendChild.
+  ad_iframe.Complete(R"HTML(
+    <script>
+      frame = document.createElement("iframe");
+      frame.name = "ad_sibling";
+      parent.document.body.appendChild(frame);
+    </script>
+    )HTML");
+
+  // The new sibling frame should also be identified as an ad.
+  EXPECT_TRUE(ToLocalFrame(GetDocument().GetFrame()->Tree().Find("ad_sibling"))
+                  ->IsAdSubframe());
+}
+
 // Image loaded by ad script is tagged as ad.
 TEST_F(AdTrackerSimTest, ImageLoadedWhileExecutingAdScript) {
  const char kAdUrl[] = "https://example.com/ad_script.js";