Introduce HasOnlyDisableReason in ExtensionPrefs

Many consumers of ExtensionPrefs check if a given reason is the only
disable reason for an extension. They currently do so by getting the
disable reason set, checking its size is 1 and reading the set content.

This change adds a new HasOnlyDisableReason method which can be used
for this purpose. All callers have been updated to use this method,
making them cleaner.

Bug: 372186532
Change-Id: Ic602e920090fe02ca17c9c8302cef84c8ee50a32
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/6267643
Commit-Queue: Sohail Rajdev <sorajdev@microsoft.com>
Reviewed-by: Devlin Cronin <rdevlin.cronin@chromium.org>
Reviewed-by: Paul Adedeji <pauladedeji@google.com>
Reviewed-by: Ankush Singh <ankushkush@google.com>
Cr-Commit-Position: refs/heads/main@{#1420966}

chrome/browser/extensions/content_verifier_hash_fetch_behavior_browsertest.cc

@@ -232,9 +232,7 @@ class ContentVerifierHashTest
     ExtensionPrefs* prefs = ExtensionPrefs::Get(profile());
     // Make sure the extension got disabled due to corruption (and only due to
     // corruption).
-    DisableReasonSet reasons = prefs->GetDisableReasons(id());
+    return prefs->HasOnlyDisableReason(id(), disable_reason::DISABLE_CORRUPTED);
-    return reasons.size() == 1 &&
-           reasons.contains(disable_reason::DISABLE_CORRUPTED);
   }
 
   bool ExtensionIsEnabled() {

chrome/browser/extensions/extension_prefs_unittest.cc

@@ -1516,4 +1516,35 @@ TEST_F(ExtensionPrefsSimpleTest, ExtensionSpecificPrefsMapTest) {
   EXPECT_EQ(time, prefs.prefs()->ReadPrefAsTime(extension_id, kTestTimePref));
 }
 
+TEST_F(ExtensionPrefsSimpleTest, HasOnlyDisableReasonTest) {
+  content::BrowserTaskEnvironment task_environment;
+  TestExtensionPrefs prefs(base::SingleThreadTaskRunner::GetCurrentDefault());
+  std::string extension_id = prefs.AddExtension("Test Extension")->id();
+  ExtensionPrefs* extension_prefs = prefs.prefs();
+
+  // No disable reasons to begin with.
+  EXPECT_FALSE(extension_prefs->HasOnlyDisableReason(
+      extension_id, disable_reason::DISABLE_USER_ACTION));
+
+  // Add a disable reason.
+  extension_prefs->SetExtensionDisabled(extension_id,
+                                        {disable_reason::DISABLE_USER_ACTION});
+  EXPECT_TRUE(extension_prefs->HasOnlyDisableReason(
+      extension_id, disable_reason::DISABLE_USER_ACTION));
+
+  // Add another disable reason.
+  extension_prefs->AddDisableReason(extension_id,
+                                    disable_reason::DISABLE_EXTERNAL_EXTENSION);
+  EXPECT_FALSE(extension_prefs->HasOnlyDisableReason(
+      extension_id, disable_reason::DISABLE_USER_ACTION));
+  EXPECT_FALSE(extension_prefs->HasOnlyDisableReason(
+      extension_id, disable_reason::DISABLE_EXTERNAL_EXTENSION));
+
+  // Remove the first disable reason.
+  extension_prefs->RemoveDisableReason(extension_id,
+                                       disable_reason::DISABLE_USER_ACTION);
+  EXPECT_TRUE(extension_prefs->HasOnlyDisableReason(
+      extension_id, disable_reason::DISABLE_EXTERNAL_EXTENSION));
+}
+
 }  // namespace extensions

chrome/browser/extensions/extension_service.cc

@@ -1074,10 +1074,8 @@ void ExtensionService::CheckManagementPolicy() {
     // for update.
     ExtensionUpdater::CheckParams to_recheck;
     for (const auto& extension : registry_->disabled_extensions()) {
-      DisableReasonSet disable_reasons =
+      if (extension_prefs_->HasOnlyDisableReason(
-          extension_prefs_->GetDisableReasons(extension->id());
+              extension->id(),
-      if (disable_reasons.size() == 1 &&
-          disable_reasons.contains(
               disable_reason::DISABLE_UPDATE_REQUIRED_BY_POLICY)) {
         // The minimum version check is the only thing holding this extension
         // back, so check if it can be updated to fix that.

chrome/browser/themes/theme_service.cc

@@ -474,12 +474,8 @@ void ThemeService::RemoveUnusedThemes() {
       // Only uninstall themes which are not disabled or are disabled with
       // reason DISABLE_USER_ACTION. We cannot blanket uninstall all disabled
       // themes because externally installed themes are initially disabled.
-      extensions::DisableReasonSet disable_reasons =
+      bool is_disabled_by_user = prefs->HasOnlyDisableReason(
-          prefs->GetDisableReasons(extension->id());
+          extension->id(), extensions::disable_reason::DISABLE_USER_ACTION);
-      bool is_disabled_by_user =
-          disable_reasons.size() == 1 &&
-          disable_reasons.contains(
-              extensions::disable_reason::DISABLE_USER_ACTION);
       if (!prefs->IsExtensionDisabled(extension->id()) || is_disabled_by_user) {
         remove_list.push_back(extension->id());
       }

chrome/browser/themes/theme_syncable_service.cc

@@ -550,12 +550,9 @@ ThemeSyncableService::ThemeSyncState ThemeSyncableService::MaybeSetTheme(
         theme_service_->SetTheme(extension);
         return ThemeSyncState::kApplied;
       }
-      const extensions::DisableReasonSet disable_reasons =
-          extensions::ExtensionPrefs::Get(profile_)->GetDisableReasons(id);
       bool is_disabled_by_user =
-          disable_reasons.size() == 1 &&
+          extensions::ExtensionPrefs::Get(profile_)->HasOnlyDisableReason(
-          disable_reasons.contains(
+              id, extensions::disable_reason::DISABLE_USER_ACTION);
-              extensions::disable_reason::DISABLE_USER_ACTION);
       if (is_disabled_by_user) {
         // The user had installed this theme but disabled it (by installing
         // another atop it); re-enable.

chrome/browser/ui/extensions/controlled_home_bubble_delegate_unittest.cc

@@ -70,11 +70,8 @@ class ControlledHomeBubbleDelegateTest : public BrowserWithTestWindowTest {
   bool IsExtensionDisabled(
       const extensions::ExtensionId& id,
       extensions::disable_reason::DisableReason disable_reason) {
-    extensions::DisableReasonSet disable_reasons =
-        extension_prefs_->GetDisableReasons(id);
     return extension_registry_->disabled_extensions().GetByID(id) &&
-           disable_reasons.size() == 1 &&
+           extension_prefs_->HasOnlyDisableReason(id, disable_reason);
-           disable_reasons.contains(disable_reason);
   }
 
   // Returns true if the extension has been acknowledged by the user.

extensions/browser/extension_prefs.cc

@@ -1031,6 +1031,14 @@ bool ExtensionPrefs::HasDisableReason(
   return GetDisableReasons(extension_id).contains(disable_reason);
 }
 
+bool ExtensionPrefs::HasOnlyDisableReason(
+    const ExtensionId& extension_id,
+    disable_reason::DisableReason disable_reason) const {
+  const DisableReasonSet disable_reasons = GetDisableReasons(extension_id);
+  return disable_reasons.size() == 1 &&
+         disable_reasons.contains(disable_reason);
+}
+
 void ExtensionPrefs::AddDisableReason(
     const ExtensionId& extension_id,
     disable_reason::DisableReason disable_reason) {

extensions/browser/extension_prefs.h

@@ -423,18 +423,18 @@ class ExtensionPrefs : public KeyedService {
   // an extension. In particular, AddDisableReason(s) is only legal when the
   // extension is not enabled.
   DisableReasonSet GetDisableReasons(const ExtensionId& extension_id) const;
+
+  // Returns true if the extension has `disable_reason` in its disable reasons.
   bool HasDisableReason(const ExtensionId& extension_id,
                         disable_reason::DisableReason disable_reason) const;
 
-  // TODO(crbug.com/372186532): Add a HasOnlyDisableReason() method which checks
+  // Returns true if the extension has only `disable_reason` in its disable
-  // if the given reason is the only reason in the extension's DisableReasonSet.
+  // reasons.
-  // A good number of callers need this (see crrev.com/c/6218840).
+  bool HasOnlyDisableReason(const ExtensionId& extension_id,
+                            disable_reason::DisableReason disable_reason) const;
 
   void AddDisableReason(const ExtensionId& extension_id,
                         disable_reason::DisableReason disable_reason);
-
-  // TODO(crbug.com/372186532): Remove this method as its usage is limited (only
-  // used in tests) and all its callers can use ReplaceDisableReasons() instead.
   void AddDisableReasons(const ExtensionId& extension_id,
                          const DisableReasonSet& disable_reasons);
 

extensions/browser/extension_registrar.cc

@@ -363,10 +363,8 @@ void ExtensionRegistrar::DisableExtensionWithSource(
 void ExtensionRegistrar::EnabledReloadableExtensions() {
   std::vector<std::string> extensions_to_enable;
   for (const auto& e : registry_->disabled_extensions()) {
-    DisableReasonSet disable_reasons =
+    if (extension_prefs_->HasOnlyDisableReason(
-        extension_prefs_->GetDisableReasons(e->id());
+            e->id(), disable_reason::DISABLE_RELOAD)) {
-    if (disable_reasons.size() == 1 &&
-        disable_reasons.contains(disable_reason::DISABLE_RELOAD)) {
       extensions_to_enable.push_back(e->id());
     }
   }