[AdTagging] Any script in an ad frame's context is ad script

When calling IsAdScriptInStack, return true if the current context is an ad frame's context. This is a minor optimization in positive cases, and also covers more cases (e.g., inline script in the ad frame). BUG: 894080 Change-Id: I7b691c6746235bf4cc8be9860d81b7899c86e847 Reviewed-on: https://chromium-review.googlesource.com/c/1264955 Commit-Queue: Josh Karlin <jkarlin@chromium.org> Reviewed-by: Charlie Harrison <csharrison@chromium.org> Reviewed-by: Daniel Cheng <dcheng@chromium.org> Cr-Commit-Position: refs/heads/master@{#598765}

[AdTagging] Any script in an ad frame's context is ad script
When calling IsAdScriptInStack, return true if the current context is an ad frame's context. This is a minor optimization in positive cases, and also covers more cases (e.g., inline script in the ad frame). BUG: 894080 Change-Id: I7b691c6746235bf4cc8be9860d81b7899c86e847 Reviewed-on: https://chromium-review.googlesource.com/c/1264955 Commit-Queue: Josh Karlin <jkarlin@chromium.org> Reviewed-by: Charlie Harrison <csharrison@chromium.org> Reviewed-by: Daniel Cheng <dcheng@chromium.org> Cr-Commit-Position: refs/heads/master@{#598765}
62135e9f · Josh Karlin · Commit Bot · 2ca2969a · 62135e9f · 62135e9f
Commit 62135e9f authored Oct 11, 2018 by Josh Karlin Committed by Commit Bot Oct 11, 2018
3 changed files
--- a/chrome/browser/subresource_filter/ad_tagging_browsertest.cc
+++ b/chrome/browser/subresource_filter/ad_tagging_browsertest.cc
@@ -444,11 +444,11 @@ IN_PROC_BROWSER_TEST_F(AdTaggingBrowserTest, WindowOpenFromSubframe) {
      ExpectLatestWindowOpenUkmEntry(
          ukm_recorder, expected_num_entries, false /* from_main_frame */,
          main_frame_url, ad_frame /* from_ad_subframe */,
-          false /* from_ad_script */);
+          ad_frame /* from_ad_script */);
      ExpectWindowOpenUmaStatus(
-          histogram_tester, 0 /* adscript_adframe */,
-          expected_num_from_ad_subframe /* nonadscript_adframe */,
-          0 /* adscript_nonadframe */,
+          histogram_tester,
+          expected_num_from_ad_subframe /* adscript_adframe */,
+          0 /* nonadscript_adframe */, 0 /* adscript_nonadframe */,
          expected_num_entries -
              expected_num_from_ad_subframe /* nonadscript_nonadframe */);
    }

--- a/third_party/blink/renderer/core/frame/ad_tracker.cc
+++ b/third_party/blink/renderer/core/frame/ad_tracker.cc
@@ -9,10 +9,12 @@
 #include "third_party/blink/renderer/bindings/core/v8/source_location.h"
 #include "third_party/blink/renderer/bindings/core/v8/v8_binding_for_core.h"
 #include "third_party/blink/renderer/core/core_probe_sink.h"
+#include "third_party/blink/renderer/core/dom/document.h"
 #include "third_party/blink/renderer/core/frame/local_frame.h"
 #include "third_party/blink/renderer/core/probe/core_probes.h"
 #include "third_party/blink/renderer/platform/loader/fetch/resource_request.h"
 #include "third_party/blink/renderer/platform/weborigin/kurl.h"
+#include "third_party/blink/renderer/platform/wtf/casting.h"

 namespace blink {

@@ -110,6 +112,22 @@ 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;
+  }
+
  // 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
  // stack so scan that as well.

--- a/third_party/blink/renderer/core/frame/ad_tracker_test.cc
+++ b/third_party/blink/renderer/core/frame/ad_tracker_test.cc
@@ -211,6 +211,32 @@ TEST_F(AdTrackerSimTest, ScriptLoadedWhileExecutingAdScript) {
  EXPECT_TRUE(ad_tracker_->RequestWithUrlTaggedAsAd(kVanillaUrl));
 }

+// Unknown script running in an ad context should be labeled as ad script.
+TEST_F(AdTrackerSimTest, ScriptDetectedByContext) {
+  const char kAdScriptUrl[] = "https://example.com/ad_script.js";
+  SimRequest ad_script(kAdScriptUrl, "text/javascript");
+
+  ad_tracker_->SetAdSuffix("ad_script.js");
+
+  // Create an iframe that's considered an ad.
+  main_resource_->Complete("<body><script src='ad_script.js'></script></body>");
+  ad_script.Complete(R"SCRIPT(
+    frame = document.createElement("iframe");
+    document.body.appendChild(frame);
+    )SCRIPT");
+
+  // The child frame should be an ad subframe.
+  LocalFrame* child_frame =
+      ToLocalFrame(GetDocument().GetFrame()->Tree().FirstChild());
+  EXPECT_TRUE(child_frame->IsAdSubframe());
+
+  // Now run unknown script in the child's context. It should be considered an
+  // ad based on context alone.
+  ad_tracker_->SetExecutionContext(child_frame->GetDocument());
+  ad_tracker_->SetScriptAtTopOfStack("foo.js");
+  EXPECT_TRUE(ad_tracker_->IsAdScriptInStack());
+}
+
 // Image loaded by ad script is tagged as ad.
 TEST_F(AdTrackerSimTest, ImageLoadedWhileExecutingAdScript) {
  const char kAdUrl[] = "https://example.com/ad_script.js";