From a334d6ba386b854f599d33b77a6210d950a6838f Mon Sep 17 00:00:00 2001
From: Lukasz Anforowicz <lukasza@chromium.org>
Date: Mon, 13 Apr 2026 02:55:13 -0700
Subject: [PATCH] Don't unconditionally disable ORB in extensions with HTTP permission.

Disclaimer: This CL has been created with AI assistance.

The main change in this CL is in
`extensions/browser/url_loader_factory_manager.cc`.  Before this CL ORB
would be disabled altogether for extensions that may have HTTP
permissions (pre-declared via their  manifest, or granted by the user
via active tab).  After this CL, ORB is only disabled for content
scripts of platform apps that may have HTTP permissions.

The main change required changing `services/network/url_loader.cc` to
dynamically disable ORB when processing a specific request, but only if
the Network Service has been earlier notified that the particular
initiator origin has been granted access to the target of the http
request (this reuses `OriginAccessList` which has been put in place to
support OOR-CORS).

The change above means that the effects of calling
`ActiveTabPermissionGranter::GrantIfRequested` are no longer
synchronous, so this CL tweaks how `GrantIfRequested` calls
`SetCorsOriginAccessList` - before this CL it didn't wait for the new
permissions to propagate to the Network Service (it used to pass
`DoNothing` as the completion callback) but going forward
`GrantIfRequested` has to finish asynchronously.

This asynchronicity also had to be accounted for in a few other places
(mostly tests, but also in `extension_action_runner.h`).  To support
this kind of tests changes the CL changes
`extensions/test/permissions_manager_waiter.cc` to add an ability to
wait for a newly granted active tab permission.

Fixed: 497058611
Change-Id: Ie488b08643c28e49cb485b74ec09d481308f847d
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/7728286
Commit-Queue: Takashi Toyoshima <toyoshim@chromium.org>
Reviewed-by: Emilia Paz <emiliapaz@chromium.org>
Reviewed-by: Takashi Toyoshima <toyoshim@chromium.org>
Auto-Submit: Łukasz Anforowicz <lukasza@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1613591}
---

diff --git a/chrome/browser/extensions/extension_action_runner.cc b/chrome/browser/extensions/extension_action_runner.cc
index d2ecc3d8..0590b7e 100644
--- a/chrome/browser/extensions/extension_action_runner.cc
+++ b/chrome/browser/extensions/extension_action_runner.cc
@@ -157,12 +157,13 @@
 
   // If a refresh is required this prevents blocked actions (that wouldn't run
   // at the right time) from running until the user refreshes the page.
-  base::AutoReset<bool> ignore_active_tab(&ignore_active_tab_granted_,
-                                          refresh_required);
   // Immediately grant permissions to every extension.
+  auto* granter = ActiveTabPermissionGranter::FromWebContents(web_contents());
   for (auto* extension : extensions) {
-    ActiveTabPermissionGranter::FromWebContents(web_contents())
-        ->GrantIfRequested(extension);
+    if (refresh_required && !granter->IsGranted(extension)) {
+      extensions_to_ignore_active_tab_granted_.insert(extension->id());
+    }
+    granter->GrantIfRequested(extension);
   }
 
   if (!refresh_required) {
@@ -185,7 +186,7 @@
 
 void ExtensionActionRunner::OnActiveTabPermissionGranted(
     const Extension* extension) {
-  if (ignore_active_tab_granted_) {
+  if (extensions_to_ignore_active_tab_granted_.erase(extension->id())) {
     return;
   }
 
@@ -422,6 +423,7 @@
   }
   pending_scripts_.clear();
   web_request_blocked_.clear();
+  extensions_to_ignore_active_tab_granted_.clear();
 
   // Note: This needs to be called *after* the maps have been updated, so that
   // when the UI updates, this object returns the proper result for "wants to
diff --git a/chrome/browser/extensions/extension_action_runner.h b/chrome/browser/extensions/extension_action_runner.h
index bd905a52..a77ff25d 100644
--- a/chrome/browser/extensions/extension_action_runner.h
+++ b/chrome/browser/extensions/extension_action_runner.h
@@ -216,9 +216,9 @@
   // should incorporate more fully with ActiveTab.
   std::set<ExtensionId> permitted_extensions_;
 
-  // If true, ignore active tab being granted rather than running pending
-  // actions.
-  bool ignore_active_tab_granted_ = false;
+  // A set of ids for which the next active tab grant should be ignored (rather
+  // than running pending actions).
+  std::set<ExtensionId> extensions_to_ignore_active_tab_granted_;
 
   // TODO(crbug.com/424012380): reload page dialog ownership should be moved to
   // each caller.
diff --git a/chrome/browser/extensions/extension_action_runner_unittest.cc b/chrome/browser/extensions/extension_action_runner_unittest.cc
index ae9d9fe..6a2c12c 100644
--- a/chrome/browser/extensions/extension_action_runner_unittest.cc
+++ b/chrome/browser/extensions/extension_action_runner_unittest.cc
@@ -35,6 +35,7 @@
 #include "extensions/common/mojom/injection_type.mojom-shared.h"
 #include "extensions/common/mojom/run_location.mojom-shared.h"
 #include "extensions/common/user_script.h"
+#include "extensions/test/permissions_manager_waiter.h"
 
 static_assert(BUILDFLAG(ENABLE_EXTENSIONS_CORE));
 
@@ -246,7 +247,11 @@
   EXPECT_EQ(0u, GetExecutionCountForExtension(extension->id()));
 
   // Click to accept the extension executing.
-  runner()->RunForTesting(extension);
+  {
+    PermissionsManagerWaiter waiter(PermissionsManager::Get(profile()));
+    runner()->RunForTesting(extension);
+    waiter.WaitForActiveTabPermissionGranted(extension->id());
+  }
 
   // The extension should execute, and the extension shouldn't want to run.
   EXPECT_EQ(1u, GetExecutionCountForExtension(extension->id()));
@@ -271,7 +276,11 @@
 
   // Grant access.
   RequestInjection(extension);
-  runner()->RunForTesting(extension);
+  {
+    PermissionsManagerWaiter waiter(PermissionsManager::Get(profile()));
+    runner()->RunForTesting(extension);
+    waiter.WaitForActiveTabPermissionGranted(extension->id());
+  }
   EXPECT_EQ(2u, GetExecutionCountForExtension(extension->id()));
   EXPECT_FALSE(runner()->WantsToRun(extension));
 
@@ -303,7 +312,11 @@
 
   // Request and accept a new injection.
   RequestInjection(extension);
-  runner()->RunForTesting(extension);
+  {
+    PermissionsManagerWaiter waiter(PermissionsManager::Get(profile()));
+    runner()->RunForTesting(extension);
+    waiter.WaitForActiveTabPermissionGranted(extension->id());
+  }
 
   // The extension should only have executed once, even though a grand total
   // of two executions were requested.
@@ -328,7 +341,11 @@
 
   EXPECT_EQ(0u, GetExecutionCountForExtension(extension->id()));
 
-  runner()->RunForTesting(extension);
+  {
+    PermissionsManagerWaiter waiter(PermissionsManager::Get(profile()));
+    runner()->RunForTesting(extension);
+    waiter.WaitForActiveTabPermissionGranted(extension->id());
+  }
 
   // All pending injections should have executed.
   EXPECT_EQ(kNumInjections, GetExecutionCountForExtension(extension->id()));
@@ -339,12 +356,16 @@
   const Extension* extension = AddExtension();
   NavigateAndCommit(GURL("https://www.google.com"));
 
+  // Grant the extension active tab permissions. This normally happens, e.g.,
+  // if the user clicks on a browser action.
   ActiveTabPermissionGranter* active_tab_permission_granter =
       ActiveTabPermissionGranter::FromWebContents(web_contents());
   ASSERT_TRUE(active_tab_permission_granter);
-  // Grant the extension active tab permissions. This normally happens, e.g.,
-  // if the user clicks on a browser action.
-  active_tab_permission_granter->GrantIfRequested(extension);
+  {
+    PermissionsManagerWaiter waiter(PermissionsManager::Get(profile()));
+    active_tab_permission_granter->GrantIfRequested(extension);
+    waiter.WaitForActiveTabPermissionGranted(extension->id());
+  }
 
   // Since we have active tab permissions, we shouldn't need user consent
   // anymore.
@@ -372,7 +393,11 @@
   EXPECT_EQ(0u, GetExecutionCountForExtension(extension->id()));
 
   // Grant active tab.
-  active_tab_permission_granter->GrantIfRequested(extension);
+  {
+    PermissionsManagerWaiter waiter(PermissionsManager::Get(profile()));
+    active_tab_permission_granter->GrantIfRequested(extension);
+    waiter.WaitForActiveTabPermissionGranted(extension->id());
+  }
 
   // The pending injections should have run since active tab permission was
   // granted.
@@ -426,7 +451,11 @@
   // Allow the extension to always run on this origin.

diff --git a/chrome/browser/extensions/extension_action_runner_unittest.cc b/chrome/browser/extensions/extension_action_runner_unittest.cc
index ae9d9fe..6a2c12c 100644
--- a/chrome/browser/extensions/extension_action_runner_unittest.cc
+++ b/chrome/browser/extensions/extension_action_runner_unittest.cc
@@ -35,6 +35,7 @@
 #include "extensions/common/mojom/injection_type.mojom-shared.h"
 #include "extensions/common/mojom/run_location.mojom-shared.h"
 #include "extensions/common/user_script.h"
+#include "extensions/test/permissions_manager_waiter.h"
 
 static_assert(BUILDFLAG(ENABLE_EXTENSIONS_CORE));
 
@@ -246,7 +247,11 @@
   EXPECT_EQ(0u, GetExecutionCountForExtension(extension->id()));
 
   // Click to accept the extension executing.
-  runner()->RunForTesting(extension);
+  {
+    PermissionsManagerWaiter waiter(PermissionsManager::Get(profile()));
+    runner()->RunForTesting(extension);
+    waiter.WaitForActiveTabPermissionGranted(extension->id());
+  }
 
   // The extension should execute, and the extension shouldn't want to run.
   EXPECT_EQ(1u, GetExecutionCountForExtension(extension->id()));
@@ -271,7 +276,11 @@
 
   // Grant access.
   RequestInjection(extension);
-  runner()->RunForTesting(extension);
+  {
+    PermissionsManagerWaiter waiter(PermissionsManager::Get(profile()));
+    runner()->RunForTesting(extension);
+    waiter.WaitForActiveTabPermissionGranted(extension->id());
+  }
   EXPECT_EQ(2u, GetExecutionCountForExtension(extension->id()));
   EXPECT_FALSE(runner()->WantsToRun(extension));
 
@@ -303,7 +312,11 @@
 
   // Request and accept a new injection.
   RequestInjection(extension);
-  runner()->RunForTesting(extension);
+  {
+    PermissionsManagerWaiter waiter(PermissionsManager::Get(profile()));
+    runner()->RunForTesting(extension);
+    waiter.WaitForActiveTabPermissionGranted(extension->id());
+  }
 
   // The extension should only have executed once, even though a grand total
   // of two executions were requested.
@@ -328,7 +341,11 @@
 
   EXPECT_EQ(0u, GetExecutionCountForExtension(extension->id()));
 
-  runner()->RunForTesting(extension);
+  {
+    PermissionsManagerWaiter waiter(PermissionsManager::Get(profile()));
+    runner()->RunForTesting(extension);
+    waiter.WaitForActiveTabPermissionGranted(extension->id());
+  }
 
   // All pending injections should have executed.
   EXPECT_EQ(kNumInjections, GetExecutionCountForExtension(extension->id()));
@@ -339,12 +356,16 @@
   const Extension* extension = AddExtension();
   NavigateAndCommit(GURL("https://www.google.com"));
 
+  // Grant the extension active tab permissions. This normally happens, e.g.,
+  // if the user clicks on a browser action.
   ActiveTabPermissionGranter* active_tab_permission_granter =
       ActiveTabPermissionGranter::FromWebContents(web_contents());
   ASSERT_TRUE(active_tab_permission_granter);
-  // Grant the extension active tab permissions. This normally happens, e.g.,
-  // if the user clicks on a browser action.
-  active_tab_permission_granter->GrantIfRequested(extension);
+  {
+    PermissionsManagerWaiter waiter(PermissionsManager::Get(profile()));
+    active_tab_permission_granter->GrantIfRequested(extension);
+    waiter.WaitForActiveTabPermissionGranted(extension->id());
+  }
 
   // Since we have active tab permissions, we shouldn't need user consent
   // anymore.
@@ -372,7 +393,11 @@
   EXPECT_EQ(0u, GetExecutionCountForExtension(extension->id()));
 
   // Grant active tab.
-  active_tab_permission_granter->GrantIfRequested(extension);
+  {
+    PermissionsManagerWaiter waiter(PermissionsManager::Get(profile()));
+    active_tab_permission_granter->GrantIfRequested(extension);
+    waiter.WaitForActiveTabPermissionGranted(extension->id());
+  }
 
   // The pending injections should have run since active tab permission was
   // granted.
@@ -426,7 +451,11 @@
   // Allow the extension to always run on this origin.
   ScriptingPermissionsModifier modifier(profile(), extension);
   modifier.GrantHostPermission(web_contents()->GetLastCommittedURL());
-  runner()->RunForTesting(extension);
+  {
+    PermissionsManagerWaiter waiter(PermissionsManager::Get(profile()));
+    runner()->RunForTesting(extension);
+    waiter.WaitForActiveTabPermissionGranted(extension->id());
+  }
 
   // The extension should execute, and the extension shouldn't want to run.
   EXPECT_EQ(1u, GetExecutionCountForExtension(extension->id()));
@@ -482,7 +511,11 @@
   EXPECT_EQ(BLOCKED_ACTION_SCRIPT_AT_START | BLOCKED_ACTION_SCRIPT_OTHER,
             runner()->GetBlockedActions(extension->id()));
 
-  runner()->RunForTesting(extension);
+  {
+    PermissionsManagerWaiter waiter(PermissionsManager::Get(profile()));
+    runner()->RunForTesting(extension);
+    waiter.WaitForActiveTabPermissionGranted(extension->id());
+  }
   EXPECT_EQ(BLOCKED_ACTION_NONE, runner()->GetBlockedActions(extension->id()));
 }
diff --git a/chrome/browser/extensions/orb_and_cors_extension_browsertest.cc b/chrome/browser/extensions/orb_and_cors_extension_browsertest.cc
index e8a0b01..20562bb 100644
--- a/chrome/browser/extensions/orb_and_cors_extension_browsertest.cc
+++ b/chrome/browser/extensions/orb_and_cors_extension_browsertest.cc
@@ -61,6 +61,7 @@
 #include "extensions/browser/extension_host.h"
 #include "extensions/browser/extension_registrar.h"
 #include "extensions/browser/extension_util.h"
+#include "extensions/browser/permissions/active_tab_permission_granter.h"
 #include "extensions/browser/permissions_manager.h"
 #include "extensions/browser/service_worker/service_worker_test_utils.h"
 #include "extensions/browser/url_loader_factory_manager.h"
@@ -217,6 +218,7 @@
           "version": "1.0",
           "manifest_version": 2,
           "permissions": [
+              "activeTab",
               "tabs",
               "*://fetch-initiator.com/*",
               "*://127.0.0.1/*",
@@ -304,6 +306,7 @@
     // The test must setup resource_load_observer_ for the appropriate web
     // contents before calling this method.
     EXPECT_TRUE(resource_load_observer_);
+    resource_load_observer_->WaitForResourceCompletion(url);
     EXPECT_TRUE(resource_load_observer_->GetResource(url));
 
     // Non-cors requests may return an opaque response. The ResourceLoadObserver
@@ -3026,4 +3029,75 @@
   }
 }
 
+// Test that "active tab" permission allows bypassing ORB, but only for granted
+// origins.
+IN_PROC_BROWSER_TEST_F(OrbAndCorsExtensionBrowserTest,
+                       ActiveTabPersmissionVsOrb) {
+  ASSERT_TRUE(embedded_test_server()->Start());
+  ASSERT_TRUE(InstallExtension());
+
+  // 1. Navigate a tab to an http origin
+  GURL tab_url = embedded_test_server()->GetURL("bar.com", "/title1.html");
+  ASSERT_TRUE(ui_test_utils::NavigateToURL(browser(), tab_url));
+
+  // 2. Grant active tab permission to that tab.
+  // This is now asynchronous (it waits for Network Service to be updated).
+  PermissionsManagerWaiter waiter(
+      PermissionsManager::Get(browser()->profile()));
+  ActiveTabPermissionGranter* granter =
+      ActiveTabPermissionGranter::FromWebContents(active_web_contents());
+  ASSERT_TRUE(granter);
+  granter->GrantIfRequested(extension());
+  waiter.WaitForActiveTabPermissionGranted(extension()->id());
+
+  // 3. Navigate a separate, new tab to an extension origin.
+  GURL extension_resource = GetExtensionResource("page.html");
+  content::WebContents* extension_web_contents = nullptr;
+  {
+    NavigateParams nav_params(
+        browser(), extension_resource,
+        ui::PageTransitionFromInt(ui::PAGE_TRANSITION_GENERATED));
+    nav_params.disposition = WindowOpenDisposition::NEW_FOREGROUND_TAB;
+    content::WebContentsAddedObserver new_web_contents_observer;
+    Navigate(&nav_params);
+    extension_web_contents = new_web_contents_observer.GetWebContents();
+    content::TestNavigationObserver navigation_observer(extension_web_contents,
+                                                        1);
+    navigation_observer.Wait();
+  }
+
+  // 4. Monitor resource loads in the extension page.
+  ObserveResourceLoads(extension_web_contents);
+
+  // 5. In the extension page do a no-cors fetch (via `img` tag) of a resource
+  // for which the extension has gained access via ActiveTab permission.
+  const char kScript[] = R"(
+      var img = document.createElement('img');
+      img.src = $1;
+      new Promise(resolve => {
+        img.onload = () => resolve('LOADED');
+        img.onerror = e => resolve('ERROR: ' + e);
+      });
+  )";
+  GURL active_tab_origin_url =
+      embedded_test_server()->GetURL("bar.com", "/nosniff.xml");
+  std::ignore =
+      content::EvalJs(extension_web_contents,
+                      content::JsReplace(kScript, active_tab_origin_url));
+
+  // 5b. Verify that ORB didn't block the response.
+  VerifyFetchWasAllowedByOrb(active_tab_origin_url);
+
+  // 6. Same as step 5, but for an origin that hasn't been granted ActiveTab
+  // permission.  Using `<img>` element is even more important than in step 5,
+  // because CORS blocks `fetch`-based requests (in step 5 we could have passed
+  // `"mode" = "no-cors"` option to `fetch`, but it wouldn't work here).
+  GURL other_url = embedded_test_server()->GetURL("other.com", "/nosniff.xml");
+  std::ignore = content::EvalJs(extension_web_contents,
+                                content::JsReplace(kScript, other_url));
+
+  // 6b. Verify that ORB blocked the last response.
+  VerifyFetchWasBlockedByOrb(other_url);
+}
+
 }  // namespace extensions
diff --git a/chrome/browser/extensions/permissions/active_tab_apitest.cc b/chrome/browser/extensions/permissions/active_tab_apitest.cc
index a82d759c..f7e64d0 100644
--- a/chrome/browser/extensions/permissions/active_tab_apitest.cc
+++ b/chrome/browser/extensions/permissions/active_tab_apitest.cc
@@ -8,10 +8,10 @@
 #include "base/strings/stringprintf.h"
 #include "base/strings/utf_string_conversions.h"
 #include "build/chromeos_buildflags.h"
+#include "chrome/browser/extensions/extension_action_runner.h"
 #include "chrome/browser/extensions/extension_apitest.h"
 #include "chrome/browser/extensions/extension_service.h"
 #include "chrome/browser/extensions/extension_util.h"
-#include "chrome/browser/extensions/extension_action_runner.h"
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/ui/browser.h"
 #include "chrome/browser/ui/tabs/tab_strip_model.h"
@@ -25,6 +25,7 @@
 #include "extensions/common/constants.h"
 #include "extensions/common/extension.h"
 #include "extensions/test/extension_test_message_listener.h"
+#include "extensions/test/permissions_manager_waiter.h"
 #include "extensions/test/result_catcher.h"
 #include "net/base/filename_util.h"
 #include "net/dns/mock_host_resolver.h"
@@ -87,8 +88,12 @@
   // Granting to the extension should give it access to page.html.
   {
     ResultCatcher catcher;
-    ExtensionActionRunner::GetForWebContents(web_contents)
-        ->RunAction(extension, true);
+    {
+      PermissionsManagerWaiter waiter(PermissionsManager::Get(profile()));
+      ExtensionActionRunner::GetForWebContents(web_contents)
+          ->RunAction(extension, true);
+      waiter.WaitForActiveTabPermissionGranted(extension->id());
+    }
     EXPECT_TRUE(catcher.GetNextResult()) << message_;
   }
 
@@ -143,8 +148,12 @@
     // The injected content script has an access to page's origin without
     // explicit permissions other than "activeTab".
     ResultCatcher catcher;
-    ExtensionActionRunner::GetForWebContents(web_contents)
-        ->RunAction(extension, true);
+    {
+      PermissionsManagerWaiter waiter(PermissionsManager::Get(profile()));
+      ExtensionActionRunner::GetForWebContents(web_contents)
+          ->RunAction(extension, true);
+      waiter.WaitForActiveTabPermissionGranted(extension->id());
+    }
     EXPECT_TRUE(catcher.GetNextResult()) << message_;
   }
 }
@@ -307,8 +316,12 @@
   // Now grant the tab permission. Ensure the extension can now xhr file urls ,
   // script the active tab and embed file iframes. It should still not be able
   // to script the background tab.
-  ExtensionActionRunner::GetForWebContents(web_contents)
-      ->RunAction(extension.get(), true /*grant_tab_permissions*/);
+  {
+    PermissionsManagerWaiter waiter(PermissionsManager::Get(profile()));
+    ExtensionActionRunner::GetForWebContents(web_contents)
+        ->RunAction(extension.get(), true /*grant_tab_permissions*/);
+    waiter.WaitForActiveTabPermissionGranted(extension->id());
+  }
   EXPECT_TRUE(can_xhr_file_urls());
   EXPECT_TRUE(can_script_tab(active_tab_id));
   EXPECT_TRUE(can_load_file_iframe());
@@ -330,8 +343,12 @@
   // Grant the tab permission for the active url to the extension. Ensure it
   // still can't xhr file urls, script the active tab or embed file iframes
   // (since it does not have file access).
-  ExtensionActionRunner::GetForWebContents(web_contents)
... (truncated)