Support multiple disable reasons in ExtensionPerfs

Use a bitmask to represent all the reasons an extension is disabled.
Switch to a new key in ExtensionPrefs for the bitmask.
When we init ExtensionPrefs, check for the old key, migrate the reason over
to the new key, and delete the old key.

BUG=142762


Review URL: https://chromiumcodereview.appspot.com/10828316

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@151785 0039d316-1c4b-4281-b951-d872f2087c98

chrome/browser/extensions/extension_disabled_ui_browsertest.cc

@@ -132,7 +132,7 @@ IN_PROC_BROWSER_TEST_F(ExtensionDisabledGlobalErrorTest,
   const Extension* extension = InstallIncreasingPermissionExtensionV1();
   DisableExtension(extension->id());
   // Clear disable reason to simulate legacy disables.
-  service_->extension_prefs()->RemoveDisableReason(extension->id());
+  service_->extension_prefs()->ClearDisableReasons(extension->id());
   // Upgrade to version 2. Infer from version 1 having the same permissions
   // granted by the user that it was disabled by the user.
   extension = UpdateIncreasingPermissionExtension(extension, path_v2_, 0);
@@ -146,7 +146,7 @@ IN_PROC_BROWSER_TEST_F(ExtensionDisabledGlobalErrorTest,
                        UnknownReasonHigherPermissions) {
   const Extension* extension = InstallAndUpdateIncreasingPermissionsExtension();
   // Clear disable reason to simulate legacy disables.
-  service_->extension_prefs()->RemoveDisableReason(extension->id());
+  service_->extension_prefs()->ClearDisableReasons(extension->id());
   // We now have version 2 but only accepted permissions for version 1.
   GlobalError* error = GetExtensionDisabledGlobalError();
   ASSERT_TRUE(error);

chrome/browser/extensions/extension_prefs.cc

@@ -64,8 +64,12 @@ const char kPrefOrphanAcknowledged[] = "ack_orphan";
 // Indicates whether to show an install warning when the user enables.
 const char kExtensionDidEscalatePermissions[] = "install_warning_on_enable";
 
+// DO NOT USE, use kPrefDisableReasons instead.
 // Indicates whether the extension was updated while it was disabled.
-const char kPrefDisableReason[] = "disable_reason";
+const char kDeprecatedPrefDisableReason[] = "disable_reason";
+
+// A bitmask of all the reasons an extension is disabled.
+const char kPrefDisableReasons[] = "disable_reasons";
 
 // A preference that tracks browser action toolbar configuration. This is a list
 // object stored in the Preferences file. The extensions are stored by ID.
@@ -706,25 +710,38 @@ void ExtensionPrefs::SetDidExtensionEscalatePermissions(
                       Value::CreateBooleanValue(did_escalate));
 }
 
-Extension::DisableReason ExtensionPrefs::GetDisableReason(
-    const std::string& extension_id) {
+int ExtensionPrefs::GetDisableReasons(const std::string& extension_id) {
   int value = -1;
-  if (ReadExtensionPrefInteger(extension_id, kPrefDisableReason, &value) &&
-      value >= 0 && value < Extension::DISABLE_LAST) {
-    return static_cast<Extension::DisableReason>(value);
+  if (ReadExtensionPrefInteger(extension_id, kPrefDisableReasons, &value) &&
+      value >= 0) {
+    return value;
   }
-  return Extension::DISABLE_UNKNOWN;
+  return Extension::DISABLE_NONE;
 }
 
-void ExtensionPrefs::SetDisableReason(const std::string& extension_id,
+void ExtensionPrefs::AddDisableReason(const std::string& extension_id,
                                       Extension::DisableReason disable_reason) {
-  UpdateExtensionPref(
-      extension_id, kPrefDisableReason,
-      Value::CreateIntegerValue(static_cast<int>(disable_reason)));
+  int new_value = GetDisableReasons(extension_id) |
+      static_cast<int>(disable_reason);
+  UpdateExtensionPref(extension_id, kPrefDisableReasons,
+                      Value::CreateIntegerValue(new_value));
+}
+
+void ExtensionPrefs::RemoveDisableReason(
+    const std::string& extension_id,
+    Extension::DisableReason disable_reason) {
+  int new_value = GetDisableReasons(extension_id) &
+      ~static_cast<int>(disable_reason);
+  if (new_value == Extension::DISABLE_NONE) {
+    UpdateExtensionPref(extension_id, kPrefDisableReasons, NULL);
+  } else {
+    UpdateExtensionPref(extension_id, kPrefDisableReasons,
+                        Value::CreateIntegerValue(new_value));
+  }
 }
 
-void ExtensionPrefs::RemoveDisableReason(const std::string& extension_id) {
-  UpdateExtensionPref(extension_id, kPrefDisableReason, NULL);
+void ExtensionPrefs::ClearDisableReasons(const std::string& extension_id) {
+  UpdateExtensionPref(extension_id, kPrefDisableReasons, NULL);
 }
 
 void ExtensionPrefs::UpdateBlacklist(
@@ -912,6 +929,34 @@ void ExtensionPrefs::MigratePermissions(const ExtensionIdSet& extension_ids) {
   }
 }
 
+void ExtensionPrefs::MigrateDisableReasons(
+    const ExtensionIdSet& extension_ids) {
+  for (ExtensionIdSet::const_iterator ext_id =
+       extension_ids.begin(); ext_id != extension_ids.end(); ++ext_id) {
+    int value = -1;
+    if (ReadExtensionPrefInteger(*ext_id, kDeprecatedPrefDisableReason,
+                                 &value)) {
+      int new_value = Extension::DISABLE_NONE;
+      switch (value) {
+        case Extension::DEPRECATED_DISABLE_USER_ACTION:
+          new_value = Extension::DISABLE_USER_ACTION;
+          break;
+        case Extension::DEPRECATED_DISABLE_PERMISSIONS_INCREASE:
+          new_value = Extension::DISABLE_PERMISSIONS_INCREASE;
+          break;
+        case Extension::DEPRECATED_DISABLE_RELOAD:
+          new_value = Extension::DISABLE_RELOAD;
+          break;
+      }
+
+      UpdateExtensionPref(*ext_id, kPrefDisableReasons,
+                          Value::CreateIntegerValue(new_value));
+      // Remove the old disable reason.
+      UpdateExtensionPref(*ext_id, kDeprecatedPrefDisableReason, NULL);
+    }
+  }
+}
+
 PermissionSet* ExtensionPrefs::GetGrantedPermissions(
     const std::string& extension_id) {
   CHECK(Extension::IdIsValid(extension_id));
@@ -1892,6 +1937,7 @@ void ExtensionPrefs::InitPrefStore(bool extensions_disabled) {
 
   FixMissingPrefs(extension_ids);
   MigratePermissions(extension_ids);
+  MigrateDisableReasons(extension_ids);
   extension_sorting_->Initialize(extension_ids);
 
   // Store extension controlled preference values in the

chrome/browser/extensions/extension_prefs.h

@@ -152,11 +152,12 @@ class ExtensionPrefs : public ContentSettingsStore::Observer,
       bool did_escalate);
 
   // Getter and setters for disabled reason.
-  Extension::DisableReason GetDisableReason(
-      const std::string& extension_id);
-  void SetDisableReason(const std::string& extension_id,
+  int GetDisableReasons(const std::string& extension_id);
+  void AddDisableReason(const std::string& extension_id,
                         Extension::DisableReason disable_reason);
-  void RemoveDisableReason(const std::string& extension_id);
+  void RemoveDisableReason(const std::string& extension_id,
+                           Extension::DisableReason disable_reason);
+  void ClearDisableReasons(const std::string& extension_id);
 
   // Returns the version string for the currently installed extension, or
   // the empty string if not found.
@@ -554,6 +555,9 @@ class ExtensionPrefs : public ContentSettingsStore::Observer,
   // Migrates the permissions data in the pref store.
   void MigratePermissions(const ExtensionIdSet& extension_ids);
 
+  // Migrates the disable reasons from a single enum to a bit mask.
+  void MigrateDisableReasons(const ExtensionIdSet& extension_ids);
+
   // Checks whether there is a state pref for the extension and if so, whether
   // it matches |check_state|.
   bool DoesExtensionHaveState(const std::string& id,

chrome/browser/extensions/extension_service.cc

@@ -833,7 +833,7 @@ void ExtensionService::EnableExtension(const std::string& extension_id) {
     return;
 
   extension_prefs_->SetExtensionState(extension_id, Extension::ENABLED);
-  extension_prefs_->RemoveDisableReason(extension_id);
+  extension_prefs_->ClearDisableReasons(extension_id);
 
   const Extension* extension = GetExtensionByIdInternal(extension_id,
       INCLUDE_DISABLED);
@@ -878,7 +878,7 @@ void ExtensionService::DisableExtension(
   }
 
   extension_prefs_->SetExtensionState(extension_id, Extension::DISABLED);
-  extension_prefs_->SetDisableReason(extension_id, disable_reason);
+  extension_prefs_->AddDisableReason(extension_id, disable_reason);
 
   int include_mask = INCLUDE_ENABLED;
   include_mask |= INCLUDE_TERMINATED;
@@ -1955,7 +1955,7 @@ void ExtensionService::AddExtension(const Extension* extension) {
         content::Source<Profile>(profile_),
         content::Details<const Extension>(extension));
 
-    if (extension_prefs_->GetDisableReason(extension->id()) ==
+    if (extension_prefs_->GetDisableReasons(extension->id()) &
         Extension::DISABLE_PERMISSIONS_INCREASE) {
       extensions::AddExtensionDisabledError(this, extension);
     }
@@ -2030,8 +2030,7 @@ void ExtensionService::InitializePermissions(const Extension* extension) {
   bool is_extension_upgrade = old != NULL;
   bool is_privilege_increase = false;
   bool previously_disabled = false;
-  Extension::DisableReason disable_reason =
-      extension_prefs_->GetDisableReason(extension->id());
+  int disable_reasons = extension_prefs_->GetDisableReasons(extension->id());
 
   // We only need to compare the granted permissions to the current permissions
   // if the extension is not allowed to silently increase its permissions.
@@ -2068,18 +2067,17 @@ void ExtensionService::InitializePermissions(const Extension* extension) {
     // disabled on permissions increase.
     previously_disabled = extension_prefs_->IsExtensionDisabled(old->id());
     if (previously_disabled) {
-      Extension::DisableReason reason = extension_prefs_->GetDisableReason(
-          old->id());
-      if (reason == Extension::DISABLE_UNKNOWN) {
+      int reasons = extension_prefs_->GetDisableReasons(old->id());
+      if (reasons == Extension::DISABLE_NONE) {
         // Initialize the reason for legacy disabled extensions from whether the
         // extension already exceeded granted permissions.
         if (extension_prefs_->DidExtensionEscalatePermissions(old->id()))
-          disable_reason = Extension::DISABLE_PERMISSIONS_INCREASE;
+          disable_reasons = Extension::DISABLE_PERMISSIONS_INCREASE;
         else
-          disable_reason = Extension::DISABLE_USER_ACTION;
+          disable_reasons = Extension::DISABLE_USER_ACTION;
       }
     } else {
-      disable_reason = Extension::DISABLE_PERMISSIONS_INCREASE;
+      disable_reasons = Extension::DISABLE_PERMISSIONS_INCREASE;
     }
 
     // To upgrade an extension in place, unload the old one and
@@ -2097,7 +2095,9 @@ void ExtensionService::InitializePermissions(const Extension* extension) {
     }
     extension_prefs_->SetExtensionState(extension->id(), Extension::DISABLED);
     extension_prefs_->SetDidExtensionEscalatePermissions(extension, true);
-    extension_prefs_->SetDisableReason(extension->id(), disable_reason);
+    extension_prefs_->AddDisableReason(
+        extension->id(),
+        static_cast<Extension::DisableReason>(disable_reasons));
   }
 }
 

chrome/common/extensions/extension.h

@@ -98,12 +98,19 @@ class Extension : public base::RefCountedThreadSafe<Extension> {
   };
 
   // Used to record the reason an extension was disabled.
+  enum DeprecatedDisableReason {
+    DEPRECATED_DISABLE_UNKNOWN,
+    DEPRECATED_DISABLE_USER_ACTION,
+    DEPRECATED_DISABLE_PERMISSIONS_INCREASE,
+    DEPRECATED_DISABLE_RELOAD,
+    DEPRECATED_DISABLE_LAST,  // Not used.
+  };
+
   enum DisableReason {
-    DISABLE_UNKNOWN,
-    DISABLE_USER_ACTION,
-    DISABLE_PERMISSIONS_INCREASE,
-    DISABLE_RELOAD,
-    DISABLE_LAST,  // Not used.
+    DISABLE_NONE = 0,
+    DISABLE_USER_ACTION = 1 << 0,
+    DISABLE_PERMISSIONS_INCREASE = 1 << 1,
+    DISABLE_RELOAD = 1 << 2,
   };
 
   enum InstallType {