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

0 views
Skip to first unread message

Devlin Cronin (Gerrit)

unread,
Mar 11, 2026, 4:40:30 PM (20 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

Auto-Submit+1
Commit-Queue+1
Open in Gerrit

Related details

Attention is currently required from:
  • Tim
Submit Requirements:
  • requirement satisfiedCode-Coverage
  • requirement satisfiedCode-Owners
  • requirement is not satisfiedCode-Review
  • requirement is not satisfiedReview-Enforcement
Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. DiffyGerrit
Gerrit-MessageType: comment
Gerrit-Project: chromium/src
Gerrit-Branch: main
Gerrit-Change-Id: I9cc8cc0b779ed6d9da2e8dfa7b154abc160b2e30
Gerrit-Change-Number: 7657232
Gerrit-PatchSet: 2
Gerrit-Owner: Devlin Cronin <rdevlin...@chromium.org>
Gerrit-Reviewer: Devlin Cronin <rdevlin...@chromium.org>
Gerrit-Reviewer: Tim <tjud...@chromium.org>
Gerrit-Attention: Tim <tjud...@chromium.org>
Gerrit-Comment-Date: Wed, 11 Mar 2026 20:40:22 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: Yes
satisfied_requirement
unsatisfied_requirement
open
diffy

Tim (Gerrit)

unread,
Mar 11, 2026, 8:08:54 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 2 (Latest):
Tim . resolved

Thanks Devlin! This one was surprisingly smooth! Just a few nits, but LGTM other than those.

File chrome/browser/extensions/api/declarative_net_request/declarative_net_request_apitest.cc
Line 78, Patchset 2 (Latest):class DeclarativeNetRequestLazyApiTest : public DeclarativeNetRequestApiTest {
public:
DeclarativeNetRequestLazyApiTest() = default;
Tim . unresolved

nit: we can just delete this now and `:%s/DeclarativeNetRequestLazyApiTest/DeclarativeNetRequestApiTest/g`

Line 150, Patchset 2 (Latest):// This test explicitly exercises capabilities linked to MV2
// (webRequestBlocking). This can be updated in the future with e.g.
// a policy-installed extension.
Tim . unresolved

nit: We should preface this comment with the fact that this test still relies on an MV2 extension. At the moment it's just the justification for 'why' that is the case, without stating the actual 'what'.

Also to double check, does a policy-installed extension sound like the right path forward to still test the behavior with an MV3 extension?

Open in Gerrit

Related details

Attention is currently required from:
  • Devlin Cronin
Submit Requirements:
    • requirement satisfiedCode-Coverage
    • requirement satisfiedCode-Owners
    • requirement satisfiedCode-Review
    • requirement is not satisfiedNo-Unresolved-Comments
    • requirement satisfiedReview-Enforcement
    Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. DiffyGerrit
    Gerrit-MessageType: comment
    Gerrit-Project: chromium/src
    Gerrit-Branch: main
    Gerrit-Change-Id: I9cc8cc0b779ed6d9da2e8dfa7b154abc160b2e30
    Gerrit-Change-Number: 7657232
    Gerrit-PatchSet: 2
    Gerrit-Owner: Devlin Cronin <rdevlin...@chromium.org>
    Gerrit-Reviewer: Devlin Cronin <rdevlin...@chromium.org>
    Gerrit-Reviewer: Tim <tjud...@chromium.org>
    Gerrit-Attention: Devlin Cronin <rdevlin...@chromium.org>
    Gerrit-Comment-Date: Thu, 12 Mar 2026 00:08:45 +0000
    Gerrit-HasComments: Yes
    Gerrit-Has-Labels: Yes
    satisfied_requirement
    unsatisfied_requirement
    open
    diffy

    Devlin Cronin (Gerrit)

    unread,
    Mar 11, 2026, 8:52:41 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 3 comments

    Votes added by Devlin Cronin

    Auto-Submit+1
    Commit-Queue+2

    3 comments

    Patchset-level comments
    File-level comment, Patchset 3 (Latest):
    Devlin Cronin . resolved

    Thanks, Tim!

    File chrome/browser/extensions/api/declarative_net_request/declarative_net_request_apitest.cc
    Line 78, Patchset 2:class DeclarativeNetRequestLazyApiTest : public DeclarativeNetRequestApiTest {
    public:
    DeclarativeNetRequestLazyApiTest() = default;
    Tim . resolved

    nit: we can just delete this now and `:%s/DeclarativeNetRequestLazyApiTest/DeclarativeNetRequestApiTest/g`

    Devlin Cronin

    Done

    Line 150, Patchset 2:// This test explicitly exercises capabilities linked to MV2

    // (webRequestBlocking). This can be updated in the future with e.g.
    // a policy-installed extension.
    Tim . resolved

    nit: We should preface this comment with the fact that this test still relies on an MV2 extension. At the moment it's just the justification for 'why' that is the case, without stating the actual 'what'.

    Also to double check, does a policy-installed extension sound like the right path forward to still test the behavior with an MV3 extension?

    Devlin Cronin

    Updated (to also phrase it in the form of a TODO), and yep, using a policy-installed extension will work here.

    Open in Gerrit

    Related details

    Attention set is empty
    Submit Requirements:
      • requirement satisfiedCode-Coverage
      • requirement satisfiedCode-Owners
      • requirement satisfiedCode-Review
      • requirement satisfiedReview-Enforcement
      Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. DiffyGerrit
      Gerrit-MessageType: comment
      Gerrit-Project: chromium/src
      Gerrit-Branch: main
      Gerrit-Change-Id: I9cc8cc0b779ed6d9da2e8dfa7b154abc160b2e30
      Gerrit-Change-Number: 7657232
      Gerrit-PatchSet: 3
      Gerrit-Owner: Devlin Cronin <rdevlin...@chromium.org>
      Gerrit-Reviewer: Devlin Cronin <rdevlin...@chromium.org>
      Gerrit-Reviewer: Tim <tjud...@chromium.org>
      Gerrit-Comment-Date: Thu, 12 Mar 2026 00:52:32 +0000
      Gerrit-HasComments: Yes
      Gerrit-Has-Labels: Yes
      Comment-In-Reply-To: Tim <tjud...@chromium.org>
      satisfied_requirement
      open
      diffy

      Chromium LUCI CQ (Gerrit)

      unread,
      Mar 11, 2026, 11:05:13 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

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

      ```
      The name of the file: chrome/browser/extensions/api/declarative_net_request/declarative_net_request_apitest.cc
      Insertions: 13, Deletions: 18.

      @@ -75,16 +75,11 @@
         base::test::ScopedFeatureList feature_list_;
       };
       
      -class DeclarativeNetRequestLazyApiTest : public DeclarativeNetRequestApiTest {
      - public:
      -  DeclarativeNetRequestLazyApiTest() = default;
      -};
      -
      -IN_PROC_BROWSER_TEST_F(DeclarativeNetRequestLazyApiTest, DynamicRules) {
      +IN_PROC_BROWSER_TEST_F(DeclarativeNetRequestApiTest, DynamicRules) {
         ASSERT_TRUE(RunExtensionTest("dynamic_rules")) << message_;
       }
       
      -IN_PROC_BROWSER_TEST_F(DeclarativeNetRequestLazyApiTest, RegexRuleMessage) {
      +IN_PROC_BROWSER_TEST_F(DeclarativeNetRequestApiTest, RegexRuleMessage) {
         // Ensure the error message for large RegEx rules is updated with the
         // correct value for the memory limit.
         std::string expected_amount = base::StringPrintf(
      @@ -94,7 +89,7 @@
       }
       
       class DeclarativeNetRequestSafeRulesLazyApiTest
      -    : public DeclarativeNetRequestLazyApiTest {
      +    : public DeclarativeNetRequestApiTest {
        public:
         DeclarativeNetRequestSafeRulesLazyApiTest() {
           scoped_feature_list_.InitAndEnableFeature(
      @@ -143,29 +138,29 @@
       #if !BUILDFLAG(IS_ANDROID)
       // TODO(crbug.com/371432155): Port to desktop Android when chrome.tabs API is
       // available.
      -IN_PROC_BROWSER_TEST_F(DeclarativeNetRequestLazyApiTest, OnRulesMatchedDebug) {
      +IN_PROC_BROWSER_TEST_F(DeclarativeNetRequestApiTest, OnRulesMatchedDebug) {
         ASSERT_TRUE(RunExtensionTest("on_rules_matched_debug")) << message_;
       }
       
      -// This test explicitly exercises capabilities linked to MV2
      -// (webRequestBlocking). This can be updated in the future with e.g.
      -// a policy-installed extension.
      +// TODO(https://crbug.com/491516661): This test uses an MV2 extension because it
      +// explicitly exercises capabilities linked to MV2 (webRequestBlocking). This
      +// can be updated in the future with e.g.  a policy-installed extension.
       IN_PROC_BROWSER_TEST_F(DeclarativeNetRequestApiTest, ModifyHeaders) {
         ASSERT_TRUE(RunExtensionTest("modify_headers")) << message_;
       }
       
       // TODO(crbug.com/371432155): Port to desktop Android when chrome.tabs API is
       // available.
      -IN_PROC_BROWSER_TEST_F(DeclarativeNetRequestLazyApiTest, GetMatchedRules) {
      +IN_PROC_BROWSER_TEST_F(DeclarativeNetRequestApiTest, GetMatchedRules) {
         ASSERT_TRUE(RunExtensionTest("get_matched_rules")) << message_;
       }
       #endif  // !BUILDFLAG(IS_ANDROID)
       
      -IN_PROC_BROWSER_TEST_F(DeclarativeNetRequestLazyApiTest, IsRegexSupported) {
      +IN_PROC_BROWSER_TEST_F(DeclarativeNetRequestApiTest, IsRegexSupported) {
         ASSERT_TRUE(RunExtensionTest("is_regex_supported")) << message_;
       }
       
      -IN_PROC_BROWSER_TEST_F(DeclarativeNetRequestLazyApiTest, TestMatchOutcome) {
      +IN_PROC_BROWSER_TEST_F(DeclarativeNetRequestApiTest, TestMatchOutcome) {
         ASSERT_TRUE(RunExtensionTest("test_match_outcome")) << message_;
       }
       
      @@ -200,7 +195,7 @@
       
       #if !BUILDFLAG(IS_ANDROID)
       class DeclarativeNetRequestApiPrerenderingTest
      -    : public DeclarativeNetRequestLazyApiTest {
      +    : public DeclarativeNetRequestApiTest {
        public:
         DeclarativeNetRequestApiPrerenderingTest() = default;
         ~DeclarativeNetRequestApiPrerenderingTest() override = default;
      @@ -218,7 +213,7 @@
       #endif
       
       class DeclarativeNetRequestLazyApiResponseHeadersTest
      -    : public DeclarativeNetRequestLazyApiTest {
      +    : public DeclarativeNetRequestApiTest {
        public:
         DeclarativeNetRequestLazyApiResponseHeadersTest() {
           scoped_feature_list_.InitAndEnableFeature(
      @@ -228,7 +223,7 @@
        private:
         // TODO(crbug.com/40727004): Once feature is launched to stable and feature
         // flag can be removed, replace usages of this test class with just
      -  // DeclarativeNetRequestLazyApiTest.
      +  // DeclarativeNetRequestApiTest.
         base::test::ScopedFeatureList scoped_feature_list_;
         ScopedCurrentChannel current_channel_override_{version_info::Channel::DEV};
       };
      ```

      Change information

      Commit message:
      [Extensions] Update declarativeNetRequest 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 declarativeNetRequest 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.
      Bug: 491516661
      Change-Id: I9cc8cc0b779ed6d9da2e8dfa7b154abc160b2e30
      Commit-Queue: Devlin Cronin <rdevlin...@chromium.org>
      Reviewed-by: Tim <tjud...@chromium.org>
      Auto-Submit: Devlin Cronin <rdevlin...@chromium.org>
      Cr-Commit-Position: refs/heads/main@{#1598176}
      Files:
      • M chrome/browser/extensions/api/declarative_net_request/declarative_net_request_apitest.cc
      • M chrome/test/data/extensions/api_test/declarative_net_request/dynamic_rules/manifest.json
      • M chrome/test/data/extensions/api_test/declarative_net_request/dynamic_rules_limits/manifest.json
      • M chrome/test/data/extensions/api_test/declarative_net_request/fenced_frames/manifest.json
      • M chrome/test/data/extensions/api_test/declarative_net_request/get_matched_rules/manifest.json
      • M chrome/test/data/extensions/api_test/declarative_net_request/is_regex_supported/manifest.json
      • M chrome/test/data/extensions/api_test/declarative_net_request/on_rules_matched_debug/manifest.json
      • M chrome/test/data/extensions/api_test/declarative_net_request/prerendering/manifest.json
      • M chrome/test/data/extensions/api_test/declarative_net_request/test_match_outcome/manifest.json
      • M chrome/test/data/extensions/api_test/declarative_net_request/test_match_outcome_response_headers/manifest.json
      Change size: M
      Delta: 10 files changed, 48 insertions(+), 129 deletions(-)
      Branch: refs/heads/main
      Submit Requirements:
      • requirement satisfiedCode-Review: +1 by Tim
      Open in Gerrit
      Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. DiffyGerrit
      Gerrit-MessageType: merged
      Gerrit-Project: chromium/src
      Gerrit-Branch: main
      Gerrit-Change-Id: I9cc8cc0b779ed6d9da2e8dfa7b154abc160b2e30
      Gerrit-Change-Number: 7657232
      Gerrit-PatchSet: 4
      Gerrit-Owner: Devlin Cronin <rdevlin...@chromium.org>
      Gerrit-Reviewer: Chromium LUCI CQ <chromiu...@luci-project-accounts.iam.gserviceaccount.com>
      Gerrit-Reviewer: Devlin Cronin <rdevlin...@chromium.org>
      Gerrit-Reviewer: Tim <tjud...@chromium.org>
      open
      diffy
      satisfied_requirement
      Reply all
      Reply to author
      Forward
      0 new messages