Javier Fernandez (Gerrit)

unread,

Aug 14, 2026, 4:00:48 AM (2 days ago) Aug 14

to Solomon Kinard, Chromium LUCI CQ, Ted Choc, Antonio Sartori, android-bu...@system.gserviceaccount.com, chromium...@chromium.org, chromium-a...@chromium.org, extension...@chromium.org

Attention needed from Antonio Sartori, Solomon Kinard and Ted Choc

Javier Fernandez voted Commit-Queue+1

Commit-Queue

+1

Open in Gerrit

Related details

Attention is currently required from:

Antonio Sartori
Solomon Kinard
Ted Choc

Submit Requirements:

Code-Coverage
Code-Owners
Code-Review
Review-Enforcement

Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings.

Gerrit

satisfied_requirement

unsatisfied_requirement

open

diffy

Antonio Sartori (Gerrit)

unread,

Aug 14, 2026, 4:04:20 AM (2 days ago) Aug 14

to Javier Fernandez, Solomon Kinard, Chromium LUCI CQ, Ted Choc, android-bu...@system.gserviceaccount.com, chromium...@chromium.org, chromium-a...@chromium.org, extension...@chromium.org

Attention needed from Javier Fernandez, Solomon Kinard and Ted Choc

Antonio Sartori voted Code-Review+1

Code-Review

+1

Open in Gerrit

Related details

Attention is currently required from:

Javier Fernandez
Solomon Kinard
Ted Choc

Submit Requirements:

Code-Coverage
Code-Owners

Code-Review
Review-Enforcement

Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings.

Gerrit

satisfied_requirement

unsatisfied_requirement

open

diffy

Ted Choc (Gerrit)

unread,

Aug 14, 2026, 1:07:18 PM (2 days ago) Aug 14

to Javier Fernandez, Antonio Sartori, Solomon Kinard, Chromium LUCI CQ, android-bu...@system.gserviceaccount.com, chromium...@chromium.org, chromium-a...@chromium.org, extension...@chromium.org

Attention needed from Javier Fernandez and Solomon Kinard

Ted Choc voted Code-Review+1

Code-Review

+1

Open in Gerrit

Related details

Attention is currently required from:

Javier Fernandez
Solomon Kinard

Submit Requirements:

Code-Coverage
Code-Owners
Code-Review
Review-Enforcement

Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings.

Gerrit

satisfied_requirement

unsatisfied_requirement

open

diffy

Solomon Kinard (Gerrit)

unread,

Aug 14, 2026, 8:05:35 PM (2 days ago) Aug 14

to Javier Fernandez, Ted Choc, Antonio Sartori, Chromium LUCI CQ, android-bu...@system.gserviceaccount.com, chromium...@chromium.org, chromium-a...@chromium.org, extension...@chromium.org

Attention needed from Javier Fernandez

Solomon Kinard added 1 comment

Commit Message

Line 9, Patchset 3 (Latest):This is a reland of commit 6aaad9dc9d6d602d9415612ca5199e9fba068b8b

Solomon Kinard . unresolved

What the difference between this CL and that one?

Open in Gerrit

Related details

Attention is currently required from:

Javier Fernandez

Submit Requirements:

Code-Coverage
Code-Owners
Code-Review

No-Unresolved-Comments
Review-Enforcement

Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings.

Gerrit

satisfied_requirement

unsatisfied_requirement

open

diffy

Solomon Kinard (Gerrit)

unread,

Aug 14, 2026, 8:42:28 PM (2 days ago) Aug 14

to Javier Fernandez, Ted Choc, Antonio Sartori, Chromium LUCI CQ, android-bu...@system.gserviceaccount.com, chromium...@chromium.org, chromium-a...@chromium.org, extension...@chromium.org

Attention needed from Javier Fernandez

Solomon Kinard voted and added 1 comment

Votes added by Solomon Kinard

Code-Review

+1

1 comment

Commit Message

Line 9, Patchset 3 (Latest):This is a reland of commit 6aaad9dc9d6d602d9415612ca5199e9fba068b8b

Solomon Kinard . unresolved

What the difference between this CL and that one?

Solomon Kinard

I found it myself with the following command. One good practice is to make PS1 the original code, PS2+ are the changes on top, and add a review request comment stating that for clarity.

`$ git diff change-8103530 change-8259429` ``` diff --git a/chrome/browser/custom_handlers/protocol_handler_registry_browsertest.cc b/chrome/browser/custom_handlers/protocol_handler_registry_browsertest.cc index b0e75ba59936d..07bf17a8ee87e 100644

-- a/chrome/browser/custom_handlers/protocol_handler_registry_browsertest.cc
+++ b/chrome/browser/custom_handlers/protocol_handler_registry_browsertest.cc
@@ -289,2 +289,18 @@ IN_PROC_BROWSER_TEST_F(ChromeRegisterProtocolHandlerBrowserTest, FencedFrame) {
using RegisterProtocolHandlerExtensionBrowserTest =
extensions::ExtensionBrowserTest;
+class RegisterProtocolHandlerExtensionBrowserTest
+ : public extensions::ExtensionBrowserTest {
+ public:
+ void SetUpOnMainThread() override {
+ extensions::ExtensionBrowserTest::SetUpOnMainThread();
+
+ // Disable OS-level registration, as
+ // ChromeRegisterProtocolHandlerBrowserTest does; see
+ // TestProtocolHandlerRegistryDelegate above. Without it these tests do a
+ // real OS round trip for every default they register, and its asynchronous
+ // reply clears the default handler again on the platforms where
+ // ShouldRemoveHandlersNotInOS() is true.
+ ProtocolHandlerRegistryFactory::GetForBrowserContext(
+ browser()->GetProfile())
+ ->SetDelegateForTesting(
+ std::make_unique<TestProtocolHandlerRegistryDelegate>());
+ }
+};
@@ -314 +330,5 @@ IN_PROC_BROWSER_TEST_F(RegisterProtocolHandlerExtensionBrowserTest,
// Register a handler from the extension page via the JS API.
+ // Register a handler from the extension page via the JS API. The waiter must
+ // only cover the registration; see the comment in
+ // JsHandlerDoesNotOverrideNonExtensionDefault.
+ ASSERT_TRUE(ui_test_utils::NavigateToURL(
+ browser(), GURL("chrome-extension://" + extension_id + "/test.html")));
@@ -317,2 +336,0 @@ IN_PROC_BROWSER_TEST_F(RegisterProtocolHandlerExtensionBrowserTest,
ASSERT_TRUE(ui_test_utils::NavigateToURL(
browser(), GURL("chrome-extension://" + extension_id + "/test.html")));
@@ -355 +373,3 @@ IN_PROC_BROWSER_TEST_F(RegisterProtocolHandlerExtensionBrowserTest, Basic) {
// Register the handler.
+ // Register the handler. The waiter must only cover the registration; see the
+ // comment in JsHandlerDoesNotOverrideNonExtensionDefault.
+ ASSERT_TRUE(ui_test_utils::NavigateToURL(browser(), GURL(handler_url)));
@@ -361 +380,0 @@ IN_PROC_BROWSER_TEST_F(RegisterProtocolHandlerExtensionBrowserTest, Basic) {
ASSERT_TRUE(ui_test_utils::NavigateToURL(browser(), GURL(handler_url)));
@@ -405 +424,6 @@ IN_PROC_BROWSER_TEST_F(RegisterProtocolHandlerExtensionBrowserTest,
// Register a 'geo' handler from the extension page via the JS API.
+ // Register a 'geo' handler from the extension page via the JS API. The waiter
+ // only covers the registration itself: it quits on the first registry change,
+ // so arming it before the navigation would let an unrelated change satisfy it
+ // and return before the handler is stored.
+ ASSERT_TRUE(ui_test_utils::NavigateToURL(
+ browser(), GURL("chrome-extension://" + extension->id() + "/test.html")));
@@ -408,3 +431,0 @@ IN_PROC_BROWSER_TEST_F(RegisterProtocolHandlerExtensionBrowserTest,
ASSERT_TRUE(ui_test_utils::NavigateToURL(
browser(),
GURL("chrome-extension://" + extension->id() + "/test.html")));
@@ -455 +476,5 @@ IN_PROC_BROWSER_TEST_F(RegisterProtocolHandlerExtensionBrowserTest,
// Register a 'geo' handler from the extension page via the JS API.
+ // Register a 'geo' handler from the extension page via the JS API. The waiter
+ // must only cover the registration; see the comment in
+ // JsHandlerDoesNotOverrideNonExtensionDefault.
+ ASSERT_TRUE(ui_test_utils::NavigateToURL(
+ browser(), GURL("chrome-extension://" + extension->id() + "/test.html")));
@@ -458,3 +482,0 @@ IN_PROC_BROWSER_TEST_F(RegisterProtocolHandlerExtensionBrowserTest,
ASSERT_TRUE(ui_test_utils::NavigateToURL(
browser(),
GURL("chrome-extension://" + extension->id() + "/test.html")));
```

Open in Gerrit

Related details

Attention is currently required from:

Javier Fernandez

Submit Requirements:

Code-Coverage
Code-Owners

Code-Review
No-Unresolved-Comments
Review-Enforcement

Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings.

Gerrit

satisfied_requirement

unsatisfied_requirement

open

diffy

Javier Fernandez (Gerrit)

unread,

Aug 15, 2026, 11:06:05 AM (yesterday) Aug 15

to Solomon Kinard, Ted Choc, Antonio Sartori, Chromium LUCI CQ, android-bu...@system.gserviceaccount.com, chromium...@chromium.org, chromium-a...@chromium.org, extension...@chromium.org

Javier Fernandez added 1 comment

Commit Message

Line 9, Patchset 3 (Latest):This is a reland of commit 6aaad9dc9d6d602d9415612ca5199e9fba068b8b

Solomon Kinard . resolved

Javier Fernandez

Oh, it's exactly like that, PS2 are the changes that fixed the regression. The PS3 is just an edit in the description.

Thanks anyway.

Open in Gerrit

Related details

Attention set is empty

Submit Requirements:

Code-Coverage
Code-Owners
Code-Review

Review-Enforcement

Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings.

Gerrit

satisfied_requirement

open

diffy

Javier Fernandez (Gerrit)

unread,

Aug 15, 2026, 11:06:08 AM (yesterday) Aug 15

to Solomon Kinard, Ted Choc, Antonio Sartori, Chromium LUCI CQ, android-bu...@system.gserviceaccount.com, chromium...@chromium.org, chromium-a...@chromium.org, extension...@chromium.org

Javier Fernandez voted Commit-Queue+2

Commit-Queue

+2

Open in Gerrit

Related details

Attention set is empty

Submit Requirements:

Code-Coverage
Code-Owners
Code-Review
Review-Enforcement

Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings.

Gerrit

satisfied_requirement

open

diffy

Chromium LUCI CQ (Gerrit)

unread,

Aug 15, 2026, 12:27:20 PM (yesterday) Aug 15

to Javier Fernandez, Solomon Kinard, Ted Choc, Antonio Sartori, android-bu...@system.gserviceaccount.com, chromium...@chromium.org, chromium-a...@chromium.org, extension...@chromium.org

Chromium LUCI CQ submitted the change

Change information

Commit message:

Reland "Custom handlers: tie extension-registered handlers to their extension"



This is a reland of commit 6aaad9dc9d6d602d9415612ca5199e9fba068b8b

Original change's description:
> Custom handlers: tie extension-registered handlers to their extension
>
> A protocol handler at the kExtensionFeatures security level relaxes the
> HTML-spec restrictions (allowing cross-origin target URLs and the
> ext+/chrome-extension schemes). Extension cleanup -- uninstall/disable
> handling and ProtocolHandlersSanityCheck -- keys off the handler's
> extension_id, so a handler with the elevated level but no extension_id
> can never be removed: its privileges survive the registering extension's
> removal, and a build that predates recording the extension_id at
> registration time could have persisted such an orphan to prefs.
>
> In this CL we associate such handlers with the registering extension:
>
> - In Browser, centralize handler construction in a new
>   CreateProtocolHandlerForFrame() helper. For a privileged extension
>   frame it builds the handler with the owning extension's id.
>
> - In ProtocolHandlersManager::OnExtensionUnloaded, remove every handler
>   returned by GetExtensionProtocolHandlers(extension->id()) instead of
>   only the manifest-declared ones.
>
> Additionally, a new method ProtocolHandler::IsAllowedExtensionHandler()
> adds an extra security check which drops orphaned handlers on reload
> rather than re-registering them with their elevated privileges intact.
>
> Add a browser test, HandlerRemovedOnExtensionUninstall, that registers a
> handler from an extension page through the JS API and verifies it is
> associated with the extension and removed on uninstall.
>
> Add also a regression test covering the orphan rejection and the pref
> round-trip, and update the two scheme-validation tests that previously
> registered untagged kExtensionFeatures handlers to use realistic
> extension-tagged handlers.
>
> Bug: 511754574
> Change-Id: I404c00a7c69a06443c8ece8f75ebe9fbef065a48
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/8103530
> Reviewed-by: Ted Choc <ted...@chromium.org>
> Reviewed-by: Antonio Sartori <antonio...@chromium.org>
> Reviewed-by: Solomon Kinard <solomo...@chromium.org>
> Commit-Queue: Javier Fernandez <jfern...@igalia.com>
> Cr-Commit-Position: refs/heads/main@{#1678351}

Bug: 511754574

Change-Id: Ia77132d66c1b98e26fe986f9e574cd04a37e61df

Include-Ci-Only-Tests: chromium.mac:mac26-arm64-rel-tests|browser_tests

Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/8259429

Reviewed-by: Solomon Kinard <solomo...@chromium.org>

Commit-Queue: Javier Fernandez <jfern...@igalia.com>

Reviewed-by: Antonio Sartori <antonio...@chromium.org>

Reviewed-by: Ted Choc <ted...@chromium.org>

Cr-Commit-Position: refs/heads/main@{#1680194}

Files:

M chrome/browser/custom_handlers/chrome_protocol_handler_registry_unittest.cc
M chrome/browser/custom_handlers/protocol_handler_registry_browsertest.cc
M chrome/browser/ui/browser_web_contents_delegate/browser_web_contents_delegate.cc
M chrome/test/data/extensions/api_test/protocol_handler/test_registration.js
M components/custom_handlers/protocol_handler.cc
M components/custom_handlers/protocol_handler.h
M components/custom_handlers/protocol_handler_registry.cc
M components/custom_handlers/protocol_handler_registry_unittest.cc
M extensions/browser/api/protocol_handlers/protocol_handlers_manager.cc

Change size: L

Delta: 9 files changed, 361 insertions(+), 42 deletions(-)

Branch: refs/heads/main

Submit Requirements:

Code-Review: +1 by Solomon Kinard, +1 by Antonio Sartori, +1 by Ted Choc

Open in Gerrit

Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings.

Gerrit

open

diffy

satisfied_requirement

Reply all

Reply to author

Forward

Reland "Custom handlers: tie extension-registered handlers to their extension" [chromium/src : main]

Javier Fernandez (Gerrit)

Javier Fernandez voted Commit-Queue+1

Related details

Antonio Sartori (Gerrit)

Antonio Sartori voted Code-Review+1

Related details

Ted Choc (Gerrit)

Ted Choc voted Code-Review+1

Related details

Solomon Kinard (Gerrit)

Solomon Kinard added 1 comment

Related details

Solomon Kinard (Gerrit)

Solomon Kinard voted and added 1 comment

Votes added by Solomon Kinard

1 comment

Related details

Javier Fernandez (Gerrit)

Javier Fernandez added 1 comment

Related details

Javier Fernandez (Gerrit)

Javier Fernandez voted Commit-Queue+2

Related details

Chromium LUCI CQ (Gerrit)

Chromium LUCI CQ submitted the change

Change information