From c93c5d88e4f60a6586e10ff1ce4a8feeaf3f4248 Mon Sep 17 00:00:00 2001
From: Chris Fredrickson <cfredric@chromium.org>
Date: Wed, 15 Apr 2026 12:37:06 -0700
Subject: [PATCH] Add LoadAndExtractContent's tabs to ActorTask's tab set

This ensures that LoadAndExtractContent does not bypass the enterprise
policy blocklist, sensitive site list, or static navigation blocklist.

This CL also adds the ability to add a tab to the controlled tabs set
without stopping the ActorTask when that tab is detached/closed. This is
needed for the ephemeral tabs created by the LoadAndExtractContent tool.

Fixed: 497342466
Change-Id: I355e2bc4ab9a27d52281de10ce6bdc98b50cc43a
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/7727485
Reviewed-by: Jeremy Roman <jbroman@chromium.org>
Auto-Submit: Chris Fredrickson <cfredric@chromium.org>
Reviewed-by: Avi Drissman <avi@chromium.org>
Commit-Queue: Chris Fredrickson <cfredric@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1615339}
---

diff --git a/chrome/browser/actor/actor_keyed_service.cc b/chrome/browser/actor/actor_keyed_service.cc
index c9ec080..b44037bd 100644
--- a/chrome/browser/actor/actor_keyed_service.cc
+++ b/chrome/browser/actor/actor_keyed_service.cc
@@ -64,6 +64,7 @@
   if (base::FeatureList::IsEnabled(actor::kActorBindCreatedTabToTask) && tab) {
     task.AddTab(
         tab->GetHandle(),
+        /*stop_task_on_detach=*/true,
         base::BindOnce(
             [](actor::ActorKeyedService::CreateActorTabCallback callback,
                tabs::TabHandle handle, actor::TaskId task_id,
diff --git a/chrome/browser/actor/actor_keyed_service_browsertest.cc b/chrome/browser/actor/actor_keyed_service_browsertest.cc
index a2271f0..7cd1e548 100644
--- a/chrome/browser/actor/actor_keyed_service_browsertest.cc
+++ b/chrome/browser/actor/actor_keyed_service_browsertest.cc
@@ -109,7 +109,8 @@
 
   void AddTabToTask(tabs::TabHandle tab_handle, ActorTask* actor_task) {
     TestFuture<mojom::ActionResultPtr> add_tab_future;
-    actor_task->AddTab(tab_handle, add_tab_future.GetCallback());
+    actor_task->AddTab(tab_handle, /*stop_task_on_detach=*/true,
+                       add_tab_future.GetCallback());
     auto add_tab_result = add_tab_future.Take();
     ASSERT_TRUE(add_tab_result);
   }
diff --git a/chrome/browser/actor/actor_keyed_service_unittest.cc b/chrome/browser/actor/actor_keyed_service_unittest.cc
index eb61de7..13832c8 100644
--- a/chrome/browser/actor/actor_keyed_service_unittest.cc
+++ b/chrome/browser/actor/actor_keyed_service_unittest.cc
@@ -96,6 +96,7 @@
   base::WeakPtr<ActorTask> task = actor_service->GetTask(id)->GetWeakPtr();
   base::RunLoop loop;
   task->AddTab(tabs::TabHandle(123),
+               /*stop_task_on_detach=*/true,
                base::BindLambdaForTesting([&](mojom::ActionResultPtr result) {
                  EXPECT_TRUE(IsOk(*result));
                  loop.Quit();
@@ -149,6 +150,7 @@
   {
     base::RunLoop loop;
     task->AddTab(tab_handle,
+                 /*stop_task_on_detach=*/true,
                  base::BindLambdaForTesting([&](mojom::ActionResultPtr result) {
                    EXPECT_EQ(result->code,
                              mojom::ActionResultCode::kTaskPaused);
@@ -177,7 +179,8 @@
 
   {
     base::test::TestFuture<mojom::ActionResultPtr> future;
-    task->AddTab(tab_handle, future.GetCallback());
+    task->AddTab(tab_handle, /*stop_task_on_detach=*/true,
+                 future.GetCallback());
     ASSERT_TRUE(future.Wait());
   }
 
diff --git a/chrome/browser/actor/actor_task.cc b/chrome/browser/actor/actor_task.cc
index b111e7c..2589634 100644
--- a/chrome/browser/actor/actor_task.cc
+++ b/chrome/browser/actor/actor_task.cc
@@ -10,6 +10,7 @@
 
 #include "base/barrier_callback.h"
 #include "base/cancelable_callback.h"
+#include "base/containers/map_util.h"
 #include "base/feature_list.h"
 #include "base/no_destructor.h"
 #include "base/state_transitions.h"
@@ -98,8 +99,10 @@
 
 }  // namespace
 
-ActorTask::ActorControlledTabState::ActorControlledTabState(ActorTask* task)
-    : task(task) {}
+ActorTask::ActorControlledTabState::ActorControlledTabState(
+    ActorTask* task,
+    bool stop_task_on_detach)
+    : task(task), stop_task_on_detach(stop_task_on_detach) {}
 ActorTask::ActorControlledTabState::~ActorControlledTabState() {
   // Stop observing the Webcontents immediately to prevent reentrant calls to
   // OnVisibilityChanged() when other members (e.g. `actuation_runner`) are
@@ -348,7 +351,7 @@
     auto add_tabs_barrier = base::BarrierCallback<mojom::ActionResultPtr>(
         tabs_to_add.size(), did_add_tabs_callback_.callback());
     for (const tabs::TabHandle& tab : tabs_to_add) {
-      AddTab(tab, add_tabs_barrier);
+      AddTab(tab, /*stop_task_on_detach=*/true, add_tabs_barrier);
     }
   } else {
     SetState(State::kActing);
@@ -546,7 +549,9 @@
   return end_time_;
 }
 
-void ActorTask::AddTab(tabs::TabHandle tab_handle, AddTabCallback callback) {
+void ActorTask::AddTab(tabs::TabHandle tab_handle,
+                       bool stop_task_on_detach,
+                       AddTabCallback callback) {
   if (!IsUnderActorControl()) {
     journal_->Log(
         GURL(), id(), "ActorTask::AddTab",
@@ -571,7 +576,8 @@
       JournalDetailsBuilder().Add("tab_id", tab_handle.raw_value()).Build());
 
   auto emplace_result = controlled_tabs_.emplace(
-      tab_handle, std::make_unique<ActorControlledTabState>(this));
+      tab_handle,
+      std::make_unique<ActorControlledTabState>(this, stop_task_on_detach));
   if (tabs::TabInterface* tab = tab_handle.Get()) {
     emplace_result.first->second->will_detach_subscription =
         tab->RegisterWillDetach(base::BindRepeating(
@@ -655,10 +661,10 @@
       GURL(), id(), "ObserveTabOnce",
       JournalDetailsBuilder().Add("tab_id", tab_handle.raw_value()).Build());
 
-  auto itr =
-      to_observe_tabs_
-          .emplace(tab_handle, std::make_unique<ActorControlledTabState>(this))
-          .first;
+  auto itr = to_observe_tabs_
+                 .emplace(tab_handle, std::make_unique<ActorControlledTabState>(
+                                          this, /*stop_task_on_detach=*/true))
+                 .first;
   ActorControlledTabState* state = itr->second.get();
 
   state->will_detach_subscription = tab->RegisterWillDetach(base::BindRepeating(
@@ -678,7 +684,9 @@
     // else.
     to_observe_tabs_.erase(tab->GetHandle());
   }
-  if (!HasTab(tab->GetHandle())) {
+  const auto* controlled_tab_state =
+      base::FindPtrOrNull(controlled_tabs_, tab->GetHandle());
+  if (!controlled_tab_state || !controlled_tab_state->stop_task_on_detach) {
     return;
   }
 
diff --git a/chrome/browser/actor/actor_task.h b/chrome/browser/actor/actor_task.h
index 581f7af..15a06ba 100644
--- a/chrome/browser/actor/actor_task.h
+++ b/chrome/browser/actor/actor_task.h
@@ -205,9 +205,12 @@
 
   // Add/remove the given TabHandle to the set of tabs this task is operating
   // over and notify the UI if this is a new tab for the task. Added tabs will
-  // enter actuation mode and be kept as visible.
+  // enter actuation mode and be kept as visible. If `stop_task_on_detach` is
+  // true, then the task will be stopped when the given tab is detached.
   using AddTabCallback = base::OnceCallback<void(mojom::ActionResultPtr)>;
-  void AddTab(tabs::TabHandle tab, AddTabCallback callback);
+  void AddTab(tabs::TabHandle tab,
+              bool stop_task_on_detach,
+              AddTabCallback callback);
   void RemoveTab(tabs::TabHandle tab);
 
   // Transient version of the above. The tab will enter the same
@@ -254,7 +257,7 @@
  private:
   class ActorControlledTabState : public content::WebContentsObserver {
    public:
-    explicit ActorControlledTabState(ActorTask* task);
+    ActorControlledTabState(ActorTask* task, bool stop_task_on_detach);
     ~ActorControlledTabState() override;
 
     void SetContents(content::WebContents* web_contents);
@@ -277,6 +280,9 @@
     base::CallbackListSubscription will_detach_subscription;
     // Subscription for TabInterface::WillDiscardContents.
     base::CallbackListSubscription content_discarded_subscription;
+
+    // Whether to stop the task when the tab is detached.
+    bool stop_task_on_detach = true;
   };
 
   // Transitions a tab/contents into a state where only the actor is responsible
diff --git a/chrome/browser/actor/execution_engine.cc b/chrome/browser/actor/execution_engine.cc
index c06513e4..e5b5a2ee 100644
--- a/chrome/browser/actor/execution_engine.cc

diff --git a/chrome/browser/actor/actor_keyed_service_browsertest.cc b/chrome/browser/actor/actor_keyed_service_browsertest.cc
index a2271f0..7cd1e548 100644
--- a/chrome/browser/actor/actor_keyed_service_browsertest.cc
+++ b/chrome/browser/actor/actor_keyed_service_browsertest.cc
@@ -109,7 +109,8 @@
 
   void AddTabToTask(tabs::TabHandle tab_handle, ActorTask* actor_task) {
     TestFuture<mojom::ActionResultPtr> add_tab_future;
-    actor_task->AddTab(tab_handle, add_tab_future.GetCallback());
+    actor_task->AddTab(tab_handle, /*stop_task_on_detach=*/true,
+                       add_tab_future.GetCallback());
     auto add_tab_result = add_tab_future.Take();
     ASSERT_TRUE(add_tab_result);
   }
diff --git a/chrome/browser/actor/actor_keyed_service_unittest.cc b/chrome/browser/actor/actor_keyed_service_unittest.cc
index eb61de7..13832c8 100644
--- a/chrome/browser/actor/actor_keyed_service_unittest.cc
+++ b/chrome/browser/actor/actor_keyed_service_unittest.cc
@@ -96,6 +96,7 @@
   base::WeakPtr<ActorTask> task = actor_service->GetTask(id)->GetWeakPtr();
   base::RunLoop loop;
   task->AddTab(tabs::TabHandle(123),
+               /*stop_task_on_detach=*/true,
                base::BindLambdaForTesting([&](mojom::ActionResultPtr result) {
                  EXPECT_TRUE(IsOk(*result));
                  loop.Quit();
@@ -149,6 +150,7 @@
   {
     base::RunLoop loop;
     task->AddTab(tab_handle,
+                 /*stop_task_on_detach=*/true,
                  base::BindLambdaForTesting([&](mojom::ActionResultPtr result) {
                    EXPECT_EQ(result->code,
                              mojom::ActionResultCode::kTaskPaused);
@@ -177,7 +179,8 @@
 
   {
     base::test::TestFuture<mojom::ActionResultPtr> future;
-    task->AddTab(tab_handle, future.GetCallback());
+    task->AddTab(tab_handle, /*stop_task_on_detach=*/true,
+                 future.GetCallback());
     ASSERT_TRUE(future.Wait());
   }
diff --git a/chrome/browser/actor/execution_engine_browsertest.cc b/chrome/browser/actor/execution_engine_browsertest.cc
index 1b619c1a8..cd76356 100644
--- a/chrome/browser/actor/execution_engine_browsertest.cc
+++ b/chrome/browser/actor/execution_engine_browsertest.cc
@@ -419,6 +419,7 @@
   base::RunLoop loop;
   actor_task().AddTab(
       active_tab()->GetHandle(),
+      /*stop_task_on_detach=*/true,
       base::BindLambdaForTesting([&](mojom::ActionResultPtr result) {
         EXPECT_TRUE(IsOk(*result));
         loop.Quit();
@@ -533,6 +534,7 @@
   base::RunLoop loop;
   actor_task().AddTab(
       active_tab()->GetHandle(),
+      /*stop_task_on_detach=*/true,
       base::BindLambdaForTesting([&](mojom::ActionResultPtr result) {
         EXPECT_TRUE(IsOk(*result));
         loop.Quit();
@@ -661,6 +663,7 @@
     base::RunLoop loop;
     actor_task().AddTab(
         active_tab()->GetHandle(),
+        /*stop_task_on_detach=*/true,
         base::BindLambdaForTesting([&](mojom::ActionResultPtr result) {
           EXPECT_TRUE(IsOk(*result));
           loop.Quit();
@@ -913,7 +916,8 @@
                        SkipBeforeUnloadDialogAndNavigate) {
   if (IsActorActive()) {
     base::test::TestFuture<mojom::ActionResultPtr> future;
-    actor_task().AddTab(active_tab()->GetHandle(), future.GetCallback());
+    actor_task().AddTab(active_tab()->GetHandle(), /*stop_task_on_detach=*/true,
+                        future.GetCallback());
     mojom::ActionResultPtr result = future.Take();
     ASSERT_TRUE(IsOk(*result));
   } else {
diff --git a/chrome/browser/actor/execution_engine_unittest.cc b/chrome/browser/actor/execution_engine_unittest.cc
index bb3e3cb0..dd9adfc 100644
--- a/chrome/browser/actor/execution_engine_unittest.cc
+++ b/chrome/browser/actor/execution_engine_unittest.cc
@@ -893,7 +893,8 @@
        MAYBE_VisibleNotVisibleActuationCompletedHistogram) {
   content::NavigationSimulator::NavigateAndCommitFromBrowser(
       web_contents(), GURL("http://localhost/"));
-  task_->AddTab(GetTab()->GetHandle(), base::DoNothing());
+  task_->AddTab(GetTab()->GetHandle(), /*stop_task_on_detach=*/true,
+                base::DoNothing());
   web_contents()->WasShown();
 
   // Simulate visible actuation.
@@ -928,7 +929,8 @@
        MAYBE_VisibleNotVisibleActuationStoppedHistogram) {
   content::NavigationSimulator::NavigateAndCommitFromBrowser(
       web_contents(), GURL("http://localhost/"));
-  task_->AddTab(GetTab()->GetHandle(), base::DoNothing());
+  task_->AddTab(GetTab()->GetHandle(), /*stop_task_on_detach=*/true,
+                base::DoNothing());
   web_contents()->WasShown();
 
   // Simulate visible actuation.
@@ -965,7 +967,8 @@
        MAYBE_VisibleNotVisibleActuationWithPauseHistogram) {
   content::NavigationSimulator::NavigateAndCommitFromBrowser(
       web_contents(), GURL("http://localhost/"));
-  task_->AddTab(GetTab()->GetHandle(), base::DoNothing());
+  task_->AddTab(GetTab()->GetHandle(), /*stop_task_on_detach=*/true,
+                base::DoNothing());
   web_contents()->WasShown();
 
   // Simulate visible actuation.
@@ -1005,7 +1008,8 @@
        MAYBE_VisibleNotVisibleActuationWithWaitingHistogram) {
   content::NavigationSimulator::NavigateAndCommitFromBrowser(
       web_contents(), GURL("http://localhost/"));
-  task_->AddTab(GetTab()->GetHandle(), base::DoNothing());
+  task_->AddTab(GetTab()->GetHandle(), /*stop_task_on_detach=*/true,
+                base::DoNothing());
   web_contents()->WasShown();
   task_->SetState(ActorTask::State::kReflecting);
diff --git a/chrome/browser/actor/tools/load_and_extract_content_tool_browsertest.cc b/chrome/browser/actor/tools/load_and_extract_content_tool_browsertest.cc
index dbc6970..abf67f5 100644
--- a/chrome/browser/actor/tools/load_and_extract_content_tool_browsertest.cc
+++ b/chrome/browser/actor/tools/load_and_extract_content_tool_browsertest.cc
@@ -2,12 +2,16 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
+#include <algorithm>
+#include <iterator>
 #include <memory>
 #include <optional>
 #include <string>
 #include <string_view>
+#include <utility>
 #include <vector>
 
+#include "base/containers/span.h"
 #include "base/functional/bind.h"
 #include "base/memory/raw_ptr.h"
 #include "base/run_loop.h"
@@ -26,7 +30,9 @@
 #include "chrome/browser/ui/tabs/tab_enums.h"
 #include "chrome/browser/ui/tabs/tab_strip_model.h"
 #include "chrome/browser/ui/tabs/tab_strip_model_observer.h"
+#include "chrome/common/actor.mojom.h"
 #include "components/actor/core/actor_features.h"
+#include "components/actor/core/safety_list_manager.h"
 #include "components/optimization_guide/proto/features/actions_data.pb.h"
 #include "components/optimization_guide/proto/features/common_quality_data.pb.h"
 #include "content/public/browser/browser_context.h"
@@ -40,6 +46,7 @@
 #include "testing/gmock/include/gmock/gmock.h"
 #include "testing/gtest/include/gtest/gtest.h"
 #include "url/gurl.h"
+#include "url/url_constants.h"
 
 namespace actor {
 
@@ -49,6 +56,15 @@
 using TabObservationResult = TabObservation::TabObservationResult;
 using ActionsResult = optimization_guide::proto::ActionsResult;
 
+// Returns a vector of URLs with all copies of `about:blank` removed.
+std::vector<GURL> IgnoreAboutBlank(base::span<const GURL> urls) {
+  std::vector<GURL> result;
+  result.reserve(urls.size());
+  std::ranges::remove_copy(urls, std::back_inserter(result),
+                           GURL(url::kAboutBlankURL));
+  return result;
+}
+
 class ActorLoadAndExtractContentToolBrowserTest : public ActorToolsTest {
  public:
   ActorLoadAndExtractContentToolBrowserTest() = default;
@@ -62,6 +78,18 @@
         &ActorLoadAndExtractContentToolBrowserTest::HandleStallRequest,
         base::Unretained(this)));
     ASSERT_TRUE(embedded_test_server()->Start());
+
+    std::string json = content::JsReplace(
+        R"json({
+      "navigation_allowed": [
+        { "from": "*", "to": $1 }
+      ],
+      "navigation_blocked": [
+        { "from": "*", "to": "blocked.example.com" }
+      ]
+    })json",
+        net::GetHostAndPort(embedded_test_server()->base_url()));
+    SafetyListManager::GetInstance()->ParseSafetyLists(json);
   }
 
  protected:
@@ -89,7 +117,7 @@
   void VerifyActionsResult(
       mojom::ActionResultCode expected_code,
       ActResultFuture& result_future,
-      std::vector<ExpectedTabObservation> expected_tab_observations) {
+      base::span<const ExpectedTabObservation> expected_tab_observations) {
     base::test::TestFuture<
         base::TimeTicks, std::vector<actor::ActionResultWithLatencyInfo>,
         actor::TaskId, bool,
@@ -169,6 +197,14 @@
                       .text = "This is a simple test page"}}}}};
   }
 
+  ExpectedNode GetCookiePageExpectedNode(std::string_view expected_text) {
+    return {
+        .type = optimization_guide::proto::CONTENT_ATTRIBUTE_ROOT,
+        .text = std::nullopt,
+        .children = {{.type = optimization_guide::proto::CONTENT_ATTRIBUTE_TEXT,
+                      .text = std::string(expected_text)}}};
+  }
+
  private:
   base::test::ScopedFeatureList scoped_feature_list_{
       kGlicActorLoadAndExtractContentTool};
@@ -292,13 +328,42 @@
   EXPECT_EQ(observer.tabs_added_count(), expected_urls.size());
 
   // The tool should have successfully navigated in the opened tabs.
-  std::vector<GURL> actual_urls = observer.navigated_urls();
+  std::vector<GURL> actual_urls = IgnoreAboutBlank(observer.navigated_urls());
   EXPECT_THAT(actual_urls, testing::UnorderedElementsAreArray(expected_urls));
 
   // The tool should have closed the tabs it opened.
   observer.VerifyTabCountRestored();
 }
 
+IN_PROC_BROWSER_TEST_F(ActorLoadAndExtractContentToolBrowserTest,
+                       SameSiteStrictCookieNotSent) {
+  const GURL url = embedded_test_server()->GetURL("/echoheader?cookie");
+
+  ASSERT_TRUE(content::SetCookie(browser()->profile(), url,
+                                 "strict_cookie=1; SameSite=Strict"));
+  ASSERT_TRUE(content::SetCookie(browser()->profile(), url,
+                                 "lax_cookie=1; SameSite=Lax"));
+
+  std::unique_ptr<ToolRequest> request =
+      std::make_unique<LoadAndExtractContentToolRequest>(std::vector{url});
+
+  TabNavigationObserver observer(browser());
+
+  ActResultFuture result;
+  actor_task().Act(ToRequestList(std::move(request)), result.GetCallback());
+  ExpectOkResult(result);
+
+  VerifyActionsResult(
+      mojom::ActionResultCode::kOk, result,
+      {{TabObservation::TAB_OBSERVATION_OK, url.spec(),
+        /*expected_title=*/"", GetCookiePageExpectedNode("lax_cookie=1")}});
+
+  EXPECT_EQ(observer.tabs_added_count(), 1);
+  EXPECT_THAT(IgnoreAboutBlank(observer.navigated_urls()),
+              testing::ElementsAre(url));
+  observer.VerifyTabCountRestored();
+}
+
 // Verifies that the tool works with a single valid URL.
 IN_PROC_BROWSER_TEST_F(ActorLoadAndExtractContentToolBrowserTest, SingleURL) {
   const GURL url = embedded_test_server()->GetURL("/actor/simple.html");
@@ -321,6 +386,38 @@
                       expected_tab_observations);
 
   EXPECT_EQ(observer.tabs_added_count(), 1);
+  EXPECT_THAT(IgnoreAboutBlank(observer.navigated_urls()),
+              testing::ElementsAre(url));
+  observer.VerifyTabCountRestored();
+}
+
+// Verifies that the tool works with a navigation to about:blank.
+IN_PROC_BROWSER_TEST_F(ActorLoadAndExtractContentToolBrowserTest, AboutBlank) {
+  const GURL url(url::kAboutBlankURL);
+
+  std::unique_ptr<ToolRequest> request =
+      std::make_unique<LoadAndExtractContentToolRequest>(std::vector{url});
+
+  TabNavigationObserver observer(browser());
+
+  ActResultFuture result;
+  actor_task().Act(ToRequestList(std::move(request)), result.GetCallback());
+  ExpectOkResult(result);
+
+  VerifyActionsResult(
+      mojom::ActionResultCode::kOk, result,
+      {
+          {
+              TabObservation::TAB_OBSERVATION_OK,
+              url::kAboutBlankURL,
+              /*expected_title=*/"",
+              ExpectedNode{
+                  .type = optimization_guide::proto::CONTENT_ATTRIBUTE_ROOT,
+              },
+          },
+      });
... (truncated)