Revert "Speed up IsAdScriptInStack by passing ExecutionContext when known"

This reverts commit bda6c898.

Reason for revert: GetExecutionContext() isn't as slow as I initially though. Reverting due to added complexity this change added for marginal performance win.

Original change's description:
> Speed up IsAdScriptInStack by passing ExecutionContext when known
> 
> What: Some callers to IsAdScriptInStack know the ExecutionContext that
> triggered the event. Those callers should pass the context in to
> IsAdScriptInStack as there is a small cost to looking it up.
> 
> Why: Some callers (such as core_probes) call frequently and looking up
> the ExecutionContext when it's already known only slows things down.
> 
> Bug: 851531
> Change-Id: Id8eadfc9855cdda0d18df34a4829dd47e31ac64f
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2090232
> Commit-Queue: Josh Karlin <jkarlin@chromium.org>
> Reviewed-by: Daniel Cheng <dcheng@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#747899}

TBR=dcheng@chromium.org,jkarlin@chromium.org

# Not skipping CQ checks because original CL landed > 1 day ago.

Bug: 851531
Change-Id: Idc280fb6acaf90a948a6363a8c7b3c23aad12834
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2127902Reviewed-by: Josh Karlin <jkarlin@chromium.org>
Reviewed-by: Daniel Cheng <dcheng@chromium.org>
Commit-Queue: Josh Karlin <jkarlin@chromium.org>
Cr-Commit-Position: refs/heads/master@{#754909}

third_party/blink/renderer/core/dom/events/event_dispatcher.cc

@@ -157,9 +157,9 @@ DispatchEventResult EventDispatcher::Dispatch() {
     Document& document = node_->GetDocument();
     if (frame) {
       // A genuine mouse click cannot be triggered by script so we don't expect
-      // there to be any script in the stack.
+      // there are any script in the stack.
       DCHECK(!frame->GetAdTracker() ||
-             !frame->GetAdTracker()->IsAdScriptInStackSlow(
+             !frame->GetAdTracker()->IsAdScriptInStack(
                  AdTracker::StackType::kBottomAndTop));
       if (frame->IsAdSubframe()) {
         UseCounter::Count(document, WebFeature::kAdClick);

third_party/blink/renderer/core/frame/ad_tracker.cc

@@ -11,7 +11,6 @@
 #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/execution_context/execution_context.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"
@@ -156,9 +155,8 @@ bool AdTracker::CalculateIfAdSubresource(ExecutionContext* execution_context,
                                          bool known_ad) {
   // Check if the document loading the resource is an ad or if any executing
   // script is an ad.
-  known_ad =
-      known_ad || IsKnownAdExecutionContext(execution_context) ||
-      IsAdScriptInStackForContext(StackType::kBottomAndTop, execution_context);
+  known_ad = known_ad || IsKnownAdExecutionContext(execution_context) ||
+             IsAdScriptInStack(StackType::kBottomAndTop);
 
   // If it is a script marked as an ad and it's not in an ad context, append it
   // to the known ad script set. We don't need to keep track of ad scripts in ad
@@ -172,13 +170,12 @@ bool AdTracker::CalculateIfAdSubresource(ExecutionContext* execution_context,
   return known_ad;
 }
 
-void AdTracker::DidCreateAsyncTask(probe::AsyncTaskId* task,
-                                   ExecutionContext* execution_context) {
+void AdTracker::DidCreateAsyncTask(probe::AsyncTaskId* task) {
   DCHECK(task);
   if (!async_stack_enabled_)
     return;
 
-  if (IsAdScriptInStackForContext(StackType::kBottomOnly, execution_context))
+  if (IsAdScriptInStack(StackType::kBottomOnly))
     task->SetAdTask();
 }
 
@@ -200,12 +197,11 @@ void AdTracker::DidFinishAsyncTask(probe::AsyncTaskId* task) {
     running_ad_async_tasks_ -= 1;
 }
 
-bool AdTracker::IsAdScriptInStackForContext(
-    StackType stack_type,
-    ExecutionContext* execution_context) {
+bool AdTracker::IsAdScriptInStack(StackType stack_type) {
   if (num_ads_in_stack_ > 0 || running_ad_async_tasks_ > 0)
     return true;
 
+  ExecutionContext* execution_context = GetCurrentExecutionContext();
   if (!execution_context)
     return false;
 
@@ -229,10 +225,6 @@ bool AdTracker::IsAdScriptInStackForContext(
   return false;
 }
 
-bool AdTracker::IsAdScriptInStackSlow(StackType stack_type) {
-  return IsAdScriptInStackForContext(stack_type, GetCurrentExecutionContext());
-}
-
 bool AdTracker::IsKnownAdScript(ExecutionContext* execution_context,
                                 const String& url) {
   if (!execution_context)

third_party/blink/renderer/core/frame/ad_tracker.h

@@ -61,8 +61,7 @@ class CORE_EXPORT AdTracker : public GarbageCollected<AdTracker> {
 
   // Called when an async task is created. Check at this point for ad script on
   // the stack and annotate the task if so.
-  void DidCreateAsyncTask(probe::AsyncTaskId* task,
-                          ExecutionContext* execution_context);
+  void DidCreateAsyncTask(probe::AsyncTaskId* task);
 
   // Called when an async task is eventually run.
   void DidStartAsyncTask(probe::AsyncTaskId* task);
@@ -71,19 +70,13 @@ class CORE_EXPORT AdTracker : public GarbageCollected<AdTracker> {
   void DidFinishAsyncTask(probe::AsyncTaskId* task);
 
   // Returns true if any script in the pseudo call stack has previously been
-  // identified as an ad resource, if |execution_context| is a known ad
-  // execution context, or if the script at the top of |execution_context|'s
+  // identified as an ad resource, if the current ExecutionContext is a known ad
+  // execution context, or if the script at the top of isolate's
   // stack is ad script. Whether to look at just the bottom of the
   // stack or the top and bottom is indicated by |stack_type|. kBottomAndTop is
   // generally best as it catches more ads, but if you're calling very
   // frequently then consider just the bottom of the stack for performance sake.
-  bool IsAdScriptInStackForContext(StackType stack_type,
-                                   ExecutionContext* execution_context);
-
-  // Determines the current ExecutionContext and then calls
-  // IsAdScriptInStackForContext with it. If you know the ExecutionContext*,
-  // call IsAdScriptInStackForContext, as it's faster than looking it up.
-  bool IsAdScriptInStackSlow(StackType stack_type);
+  bool IsAdScriptInStack(StackType stack_type);
 
   virtual void Trace(Visitor*);
 

third_party/blink/renderer/core/frame/ad_tracker_test.cc

@@ -103,15 +103,13 @@ class AdTrackerTest : public testing::Test {
     if (ad_tracker_)
       ad_tracker_->Shutdown();
     ad_tracker_ = MakeGarbageCollected<TestAdTracker>(GetFrame());
-    ad_tracker_->SetExecutionContext(ExecutionContext());
+    ad_tracker_->SetExecutionContext(
+        page_holder_->GetDocument().ToExecutionContext());
   }
 
   void WillExecuteScript(const String& script_url) {
-    ad_tracker_->WillExecuteScript(ExecutionContext(), String(script_url));
-  }
-
-  ExecutionContext* ExecutionContext() {
-    return page_holder_->GetDocument().ToExecutionContext();
+    ad_tracker_->WillExecuteScript(
+        page_holder_->GetDocument().ToExecutionContext(), String(script_url));
   }
 
   void DidExecuteScript() { ad_tracker_->DidExecuteScript(); }
@@ -123,11 +121,12 @@ class AdTrackerTest : public testing::Test {
 
   bool AnyExecutingScriptsTaggedAsAdResourceWithStackType(
       AdTracker::StackType stack_type) {
-    return ad_tracker_->IsAdScriptInStackSlow(stack_type);
+    return ad_tracker_->IsAdScriptInStack(stack_type);
   }
 
   void AppendToKnownAdScripts(const String& url) {
-    ad_tracker_->AppendToKnownAdScripts(*ExecutionContext(), url);
+    ad_tracker_->AppendToKnownAdScripts(
+        *page_holder_->GetDocument().ToExecutionContext(), url);
   }
 
   Persistent<TestAdTracker> ad_tracker_;
@@ -290,7 +289,7 @@ TEST_F(AdTrackerTest, AsyncTagging) {
   probe::AsyncTaskId async_task;
 
   // Create an async task while ad script is running.
-  ad_tracker_->DidCreateAsyncTask(&async_task, ExecutionContext());
+  ad_tracker_->DidCreateAsyncTask(&async_task);
 
   // Finish executing the ad script.
   DidExecuteScript();
@@ -394,18 +393,13 @@ TEST_F(AdTrackerSimTest, ScriptDetectedByContext) {
       To<LocalFrame>(GetDocument().GetFrame()->Tree().FirstChild());
   EXPECT_TRUE(child_frame->IsAdSubframe());
 
-  // Run vanilla script in the main frame context. It shouldn't be an ad.
+  // 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()->ToExecutionContext());
   ad_tracker_->SetScriptAtTopOfStack("foo.js");
-  EXPECT_FALSE(ad_tracker_->IsAdScriptInStackForContext(
-      AdTracker::StackType::kBottomAndTop, GetDocument().ToExecutionContext()));
-
-  // Run the same vanilla script in the child's context. It should be considered
-  // an ad based on context alone. This also tests IsAdScriptInStack's
-  // |execution_context| param.
-  ExecutionContext* execution_context =
-      child_frame->GetDocument()->ToExecutionContext();
-  EXPECT_TRUE(ad_tracker_->IsAdScriptInStackForContext(
-      AdTracker::StackType::kBottomAndTop, execution_context));
+  EXPECT_TRUE(
+      ad_tracker_->IsAdScriptInStack(AdTracker::StackType::kBottomAndTop));
 }
 
 TEST_F(AdTrackerSimTest, RedirectToAdUrl) {

third_party/blink/renderer/core/frame/local_frame.cc

@@ -1278,7 +1278,7 @@ void LocalFrame::SetIsAdSubframeIfNecessary() {
   bool parent_is_ad = parent->IsAdSubframe();
 
   if (parent_is_ad ||
-      ad_tracker_->IsAdScriptInStackSlow(AdTracker::StackType::kBottomAndTop)) {
+      ad_tracker_->IsAdScriptInStack(AdTracker::StackType::kBottomAndTop)) {
     SetIsAdSubframe(parent_is_ad ? blink::mojom::AdFrameType::kChildAd
                                  : blink::mojom::AdFrameType::kRootAd);
   }

third_party/blink/renderer/core/page/create_window.cc

@@ -204,7 +204,7 @@ static void MaybeLogWindowOpen(LocalFrame& opener_frame) {
 
   bool is_ad_subframe = opener_frame.IsAdSubframe();
   bool is_ad_script_in_stack =
-      ad_tracker->IsAdScriptInStackSlow(AdTracker::StackType::kBottomAndTop);
+      ad_tracker->IsAdScriptInStack(AdTracker::StackType::kBottomAndTop);
   FromAdState state =
       blink::GetFromAdState(is_ad_subframe, is_ad_script_in_stack);
 

third_party/blink/renderer/core/probe/core_probes.cc

@@ -124,7 +124,7 @@ void AsyncTaskScheduled(ExecutionContext* context,
 
   blink::AdTracker* ad_tracker = AdTracker::FromExecutionContext(context);
   if (ad_tracker)
-    ad_tracker->DidCreateAsyncTask(task, context);
+    ad_tracker->DidCreateAsyncTask(task);
 }
 
 void AsyncTaskScheduledBreakable(ExecutionContext* context,