Devlin Cronin (Gerrit)

unread,

Mar 11, 2026, 7:33:39 PM (17 hours ago) Mar 11

to Devlin Cronin, Tim, Chromium LUCI CQ, chromium...@chromium.org, chromium-a...@chromium.org, extension...@chromium.org

Attention needed from Tim

Devlin Cronin voted Commit-Queue+1

Commit-Queue

+1

Open in Gerrit

Related details

Attention is currently required from:

Tim

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

Tim (Gerrit)

unread,

Mar 11, 2026, 8:34:57 PM (16 hours ago) Mar 11

to Devlin Cronin, Chromium LUCI CQ, chromium...@chromium.org, chromium-a...@chromium.org, extension...@chromium.org

Attention needed from Devlin Cronin

Tim voted and added 3 comments

Votes added by Tim

Code-Review

+1

3 comments

Patchset-level comments

File-level comment, Patchset 3 (Latest):

Tim . resolved

Thanks Devlin! LGTM with just one optional addition.

File chrome/browser/extensions/api/permissions/permissions_apitest.cc

Line 180, Patchset 3 (Latest):

// TODO(https://crbug.com/491516661): This explicitly uses an MV2 extension
// because it "consumes" a user gesture from the background page via a
// window.open() call; this doesn't have an analogous version in service
// workers.

Tim . resolved

FYI: this is the exact style of comment I was looking to get on the other CL for the remaining MV2 test case, with the TODO pointing to the tracking bug.

File chrome/test/data/extensions/api_test/permissions/optional_deny/background.js

Line 21, Patchset 3 (Latest): return response.text();

Tim . unresolved

optional: might as well keep the 200 verification above this e.g.:
`assertEq(200, response.status);`
As I understand it, fetch doesn't reject on things like 404.

Unless you don't feel like that is actually important for this test.

Open in Gerrit

Related details

Attention is currently required from:

Devlin Cronin

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

Devlin Cronin (Gerrit)

unread,

Mar 11, 2026, 8:56:00 PM (15 hours ago) Mar 11

to Devlin Cronin, Tim, Chromium LUCI CQ, chromium...@chromium.org, chromium-a...@chromium.org, extension...@chromium.org

Devlin Cronin voted and added 2 comments

Votes added by Devlin Cronin

Commit-Queue

+2

2 comments

Patchset-level comments

File-level comment, Patchset 4 (Latest):

Devlin Cronin . resolved

Thanks!

File chrome/test/data/extensions/api_test/permissions/optional_deny/background.js

Line 21, Patchset 3: return response.text();

Tim . resolved

optional: might as well keep the 200 verification above this e.g.:
`assertEq(200, response.status);`
As I understand it, fetch doesn't reject on things like 404.
Unless you don't feel like that is actually important for this test.

Devlin Cronin

Good call; done

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,

Mar 11, 2026, 11:21:57 PM (13 hours ago) Mar 11

to Devlin Cronin, Tim, chromium...@chromium.org, chromium-a...@chromium.org, extension...@chromium.org

Chromium LUCI CQ submitted the change with unreviewed changes

Unreviewed changes

3 is the latest approved patch-set.
The change was submitted with unreviewed changes in the following files:

```
The name of the file: chrome/test/data/extensions/api_test/permissions/optional_deny/background.js
Insertions: 1, Deletions: 0.

@@ -18,6 +18,7 @@
 
     chrome.test.log("Requesting url: " + url);
     fetch(url).then(function(response) {
+      assertEq(200, response.status);
       return response.text();
     }).then(function(text) {
       assertEq("Hello!", text);
```

Change information

Commit message:

[Extensions] Update permissions API tests to MV3

MV2 has been unsupported for several months. We can remove testing
support for it. Existing tests should be updated to MV3.

Update API tests for the permissions API to use manifest version 3
instead of manifest version 2 and 3.

With the transition to MV3, many of these tests may also be able to be
updated to use new patterns, such as async / await, service worker
modules, and more. However, those changes are out of scope for these
CLs, and can be addressed in followups.

Additional Notes:
* PermissionsApiTestWithContextType is now no longer needed. Remove it.
* A few tests still use MV2 extensions because they rely on certain
  functionality (typically, access to a DOM or sharing a thread with
  other contexts). In this case, notes have been added above the
  associated test.
* XHR calls (which are unavailable in SWs) in tests have been updated
  to use fetch().

Bug: 491516661

Change-Id: If9daa1097f0c457acb95b8a8adbabb0b21cea7ca

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

Reviewed-by: Tim <tjud...@chromium.org>

Commit-Queue: Devlin Cronin <rdevlin...@chromium.org>

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

Files:

M chrome/browser/extensions/api/permissions/permissions_apitest.cc
M chrome/test/data/extensions/api_test/permissions/always_allowed/manifest.json
M chrome/test/data/extensions/api_test/permissions/disabled/manifest.json
M chrome/test/data/extensions/api_test/permissions/enabled/manifest.json
M chrome/test/data/extensions/api_test/permissions/experimental_disabled/manifest.json
M chrome/test/data/extensions/api_test/permissions/file_access_no/manifest.json
M chrome/test/data/extensions/api_test/permissions/file_access_yes/manifest.json
M chrome/test/data/extensions/api_test/permissions/file_load/background.js
M chrome/test/data/extensions/api_test/permissions/file_load/manifest.json
M chrome/test/data/extensions/api_test/permissions/host_subsets/manifest.json
M chrome/test/data/extensions/api_test/permissions/optional/manifest.json
M chrome/test/data/extensions/api_test/permissions/optional_deny/background.js
M chrome/test/data/extensions/api_test/permissions/optional_deny/manifest.json
M chrome/test/data/extensions/api_test/permissions/optional_gesture/manifest.json
M chrome/test/data/extensions/api_test/permissions/optional_policy_blocked/manifest.json

Change size: M

Delta: 15 files changed, 72 insertions(+), 103 deletions(-)

Branch: refs/heads/main

Submit Requirements:

Code-Review: +1 by Tim

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

[Extensions] Update permissions API tests to MV3 [chromium/src : main]

Devlin Cronin (Gerrit)

Devlin Cronin voted Commit-Queue+1

Related details

Tim (Gerrit)

Tim voted and added 3 comments

Votes added by Tim

3 comments

Related details

Devlin Cronin (Gerrit)

Devlin Cronin voted and added 2 comments

Votes added by Devlin Cronin

2 comments

Related details

Chromium LUCI CQ (Gerrit)

Chromium LUCI CQ submitted the change with unreviewed changes

Unreviewed changes

Change information