[Extensions] De-class MV2DeprecationImpactChecker [chromium/src : main]

0 views
Skip to first unread message

Devlin Cronin (Gerrit)

unread,
Jul 7, 2026, 2:42:21 PM (yesterday) Jul 7
to Devlin Cronin, Tim, Chromium LUCI CQ, android-bu...@system.gserviceaccount.com, chromium...@chromium.org, chromium-a...@chromium.org, extension...@chromium.org
Attention needed from Tim

Devlin Cronin voted Auto-Submit+1

Auto-Submit+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: If09594f2638f7d9c3c4a61728a2c81d14f5ebfda
Gerrit-Change-Number: 8049680
Gerrit-PatchSet: 1
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: Tue, 07 Jul 2026 18:42:08 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: Yes
satisfied_requirement
unsatisfied_requirement
open
diffy

Tim (Gerrit)

unread,
2:16 PM (4 hours ago) 2:16 PM
to Devlin Cronin, Chromium LUCI CQ, android-bu...@system.gserviceaccount.com, 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 1 (Latest):
Tim . resolved

Thanks Devlin, LGTM with a couple tiny nits.

File chrome/browser/extensions/manifest_v2_util_unittest.cc
Line 16, Patchset 1 (Latest):class MV2DeprecationImpactCheckerUnitTest : public ExtensionServiceTestBase {
Tim . unresolved

Nitty nit: To fit the new name, this should really be `ManifestV2UtilUnitTest` now.

File extensions/browser/manifest_v2_util.h
Line 17, Patchset 1 (Latest):// A helper class to determine if an extension is affected by the MV2
Tim . unresolved

Even nittier nit: swap `A helper class` to `Helper methods`

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: If09594f2638f7d9c3c4a61728a2c81d14f5ebfda
    Gerrit-Change-Number: 8049680
    Gerrit-PatchSet: 1
    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: Wed, 08 Jul 2026 18:16:12 +0000
    Gerrit-HasComments: Yes
    Gerrit-Has-Labels: Yes
    satisfied_requirement
    unsatisfied_requirement
    open
    diffy

    Devlin Cronin (Gerrit)

    unread,
    2:22 PM (3 hours ago) 2:22 PM
    to Devlin Cronin, Tim, Chromium LUCI CQ, android-bu...@system.gserviceaccount.com, 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 2 (Latest):
    Devlin Cronin . resolved

    Thanks, Tim!

    File chrome/browser/extensions/manifest_v2_util_unittest.cc
    Line 16, Patchset 1:class MV2DeprecationImpactCheckerUnitTest : public ExtensionServiceTestBase {
    Tim . resolved

    Nitty nit: To fit the new name, this should really be `ManifestV2UtilUnitTest` now.

    Devlin Cronin

    Good catch; done

    File extensions/browser/manifest_v2_util.h
    Line 17, Patchset 1:// A helper class to determine if an extension is affected by the MV2
    Tim . resolved

    Even nittier nit: swap `A helper class` to `Helper methods`

    Devlin Cronin

    Done

    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: If09594f2638f7d9c3c4a61728a2c81d14f5ebfda
      Gerrit-Change-Number: 8049680
      Gerrit-PatchSet: 2
      Gerrit-Comment-Date: Wed, 08 Jul 2026 18:22:10 +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,
      4:11 PM (2 hours ago) 4:11 PM
      to Devlin Cronin, Tim, android-bu...@system.gserviceaccount.com, chromium...@chromium.org, chromium-a...@chromium.org, extension...@chromium.org

      Chromium LUCI CQ submitted the change with unreviewed changes

      Unreviewed changes

      1 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/manifest_v2_util_unittest.cc
      Insertions: 12, Deletions: 17.

      @@ -13,10 +13,10 @@
       
       namespace extensions::manifest_v2_util {
       
      -class MV2DeprecationImpactCheckerUnitTest : public ExtensionServiceTestBase {
      +class ManifestV2UtilUnitTest : public ExtensionServiceTestBase {
        public:
      -  MV2DeprecationImpactCheckerUnitTest() = default;
      -  ~MV2DeprecationImpactCheckerUnitTest() override = default;
      +  ManifestV2UtilUnitTest() = default;
      +  ~ManifestV2UtilUnitTest() override = default;
       
         void SetUp() override {
           ExtensionServiceTestBase::SetUp();
      @@ -94,8 +94,7 @@
       
       // Tests that user-visible MV2 extensions are properly considered affected by
       // the MV2 deprecation.
      -TEST_F(MV2DeprecationImpactCheckerUnitTest,
      -       UserVisibleMV2ExtensionsAreAffected) {
      +TEST_F(ManifestV2UtilUnitTest, UserVisibleMV2ExtensionsAreAffected) {
         scoped_refptr<const Extension> user_installed =
             ExtensionBuilder("user installed")
                 .SetLocation(mojom::ManifestLocation::kInternal)
      @@ -140,8 +139,7 @@
       
       // Checks that certain special cases of extensions, such as default-installed
       // and installed by OEM, are also affected by the MV2 deprecation.
      -TEST_F(MV2DeprecationImpactCheckerUnitTest,
      -       DefaultInstalledMV2ExtensionsAreAffected) {
      +TEST_F(ManifestV2UtilUnitTest, DefaultInstalledMV2ExtensionsAreAffected) {
         scoped_refptr<const Extension> default_installed =
             ExtensionBuilder("default installed")
                 .SetLocation(mojom::ManifestLocation::kInternal)
      @@ -162,7 +160,7 @@
       
       // Tests that component extensions are not included in the MV2 deprecation
       // (they're implementation details of the browser).
      -TEST_F(MV2DeprecationImpactCheckerUnitTest, ComponentExtensionsAreNotAffected) {
      +TEST_F(ManifestV2UtilUnitTest, ComponentExtensionsAreNotAffected) {
         scoped_refptr<const Extension> component =
             ExtensionBuilder("component")
                 .SetLocation(mojom::ManifestLocation::kComponent)
      @@ -181,7 +179,7 @@
       
       // Tests that MV3 extensions, of any location, are not affected by the MV2
       // deprecation.
      -TEST_F(MV2DeprecationImpactCheckerUnitTest, NoMV3ExtensionsAreAffected) {
      +TEST_F(ManifestV2UtilUnitTest, NoMV3ExtensionsAreAffected) {
         struct {
           mojom::ManifestLocation manifest_location;
           const char* name;
      @@ -213,8 +211,7 @@
       
       // Tests that MV2 policy-installed extensions are affected if and only if the
       // policy does not exempt them.
      -TEST_F(MV2DeprecationImpactCheckerUnitTest,
      -       MV2PolicyInstalledExtensionsAreAffected) {
      +TEST_F(ManifestV2UtilUnitTest, MV2PolicyInstalledExtensionsAreAffected) {
         scoped_refptr<const Extension> forced_policy = AddForceInstalledExtension(
             "forced policy", mojom::ManifestLocation::kExternalPolicy, 2);
         scoped_refptr<const Extension> forced_policy_download =
      @@ -238,8 +235,7 @@
       
       // Tests that MV2 extensions that are allowed by policy, but not policy-
       // installed, are treated as other MV2 extensions.
      -TEST_F(MV2DeprecationImpactCheckerUnitTest,
      -       MV2PolicyAllowedExtensionsMayBeAffected) {
      +TEST_F(ManifestV2UtilUnitTest, MV2PolicyAllowedExtensionsMayBeAffected) {
         scoped_refptr<const Extension> allowed_policy = AddAllowedPolicyExtension(
             "allowed policy", mojom::ManifestLocation::kExternalPolicy, 2);
         scoped_refptr<const Extension> allowed_policy_download =
      @@ -253,8 +249,7 @@
       
       // Tests that any MV3 extension installed by policy is never affected by
       // MV2 deprecation.
      -TEST_F(MV2DeprecationImpactCheckerUnitTest,
      -       MV3PolicyInstalledExtensionsNeverAffected) {
      +TEST_F(ManifestV2UtilUnitTest, MV3PolicyInstalledExtensionsNeverAffected) {
         scoped_refptr<const Extension> forced_policy = AddForceInstalledExtension(
             "forced policy", mojom::ManifestLocation::kExternalPolicy, 3);
         scoped_refptr<const Extension> forced_policy_download =
      @@ -285,7 +280,7 @@
       
       // Tests that non-extension "extension-like" things (such as platform apps and
       // hosted apps) are not affected by the MV2 deprecation.
      -TEST_F(MV2DeprecationImpactCheckerUnitTest, NonExtensionsAreNotAffected) {
      +TEST_F(ManifestV2UtilUnitTest, NonExtensionsAreNotAffected) {
         scoped_refptr<const Extension> platform_app =
             ExtensionBuilder("app", ExtensionBuilder::Type::PLATFORM_APP)
                 .SetManifestVersion(2)
      @@ -311,7 +306,7 @@
       
       // Tests that user script MV2 extensions are properly considered affected by
       // the MV2 deprecation.
      -TEST_F(MV2DeprecationImpactCheckerUnitTest, UserScriptsAreAffected) {
      +TEST_F(ManifestV2UtilUnitTest, UserScriptsAreAffected) {
         scoped_refptr<const Extension> user_script =
             ExtensionBuilder("user script")
                 .SetLocation(mojom::ManifestLocation::kInternal)
      ```
      ```
      The name of the file: extensions/browser/manifest_v2_util.h
      Insertions: 1, Deletions: 1.

      @@ -14,7 +14,7 @@
       namespace extensions {
       class Extension;
       
      -// A helper class to determine if an extension is affected by the MV2
      +// Helper methods to determine if an extension is affected by the MV2
       // deprecation.
       // NOTE: Instead of using this method directly, callers should go through the
       // ManifestV2Handler.
      ```

      Change information

      Commit message:
      [Extensions] De-class MV2DeprecationImpactChecker

      MV2DeprecationImpactChecker no longer contains any relevant state.
      Remove the class and replace it with a util method in a new
      namespace, extensions::manifest_v2_util. Update all callers
      appropriately.

      This CL has no behavior change.
      Bypass-Check-License: Moved source files
      Bug: 431097630
      Change-Id: If09594f2638f7d9c3c4a61728a2c81d14f5ebfda
      Reviewed-by: Tim <tjud...@chromium.org>
      Commit-Queue: Devlin Cronin <rdevlin...@chromium.org>
      Auto-Submit: Devlin Cronin <rdevlin...@chromium.org>
      Cr-Commit-Position: refs/heads/main@{#1659017}
      Files:
      • M chrome/browser/extensions/BUILD.gn
      • R chrome/browser/extensions/manifest_v2_util_unittest.cc
      • M extensions/browser/BUILD.gn
      • M extensions/browser/manifest_v2_handler.cc
      • M extensions/browser/manifest_v2_handler.h
      • R extensions/browser/manifest_v2_util.cc
      • A extensions/browser/manifest_v2_util.h
      • D extensions/browser/mv2_deprecation_impact_checker.h
      Change size: M
      Delta: 8 files changed, 95 insertions(+), 128 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: If09594f2638f7d9c3c4a61728a2c81d14f5ebfda
      Gerrit-Change-Number: 8049680
      Gerrit-PatchSet: 3
      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