Re-enable extensions disabled due to permission increase if they have all permissions

chrome/browser/extensions/extension_service.cc

Index: chrome/browser/extensions/extension_service.cc
diff --git a/chrome/browser/extensions/extension_service.cc b/chrome/browser/extensions/extension_service.cc
index 271ca2e13b1c0fdf6e0cda588b16397b36590e55..e4c4a36c90cbe3a128f14d4e745bdf15198b63b3 100644
--- a/chrome/browser/extensions/extension_service.cc
+++ b/chrome/browser/extensions/extension_service.cc
@@ -1439,10 +1439,10 @@ void ExtensionService::AddExtension(const Extension* extension) {
   }
 
   bool is_extension_upgrade = false;
-  bool is_extension_installed = false;
+  bool is_extension_loaded = false;
   const Extension* old = GetInstalledExtension(extension->id());
   if (old) {
-    is_extension_installed = true;
+    is_extension_loaded = true;
     int version_compare_result =
         extension->version()->CompareTo(*(old->version()));
     is_extension_upgrade = version_compare_result > 0;
@@ -1466,9 +1466,9 @@ void ExtensionService::AddExtension(const Extension* extension) {
 
   // Check if the extension's privileges have changed and mark the
   // extension disabled if necessary.
-  CheckPermissionsIncrease(extension, is_extension_installed);
+  CheckPermissionsIncrease(extension, is_extension_loaded);
 
-  if (is_extension_installed && !reloading) {
+  if (is_extension_loaded && !reloading) {
     // To upgrade an extension in place, unload the old one and then load the
     // new one.  ReloadExtension disables the extension, which is sufficient.
     UnloadExtension(extension->id(), UnloadedExtensionInfo::REASON_UPDATE);
@@ -1548,7 +1548,7 @@ void ExtensionService::AddComponentExtension(const Extension* extension) {
 }
 
 void ExtensionService::CheckPermissionsIncrease(const Extension* extension,
-                                                bool is_extension_installed) {
+                                                bool is_extension_loaded) {
   extensions::PermissionsUpdater(profile_).InitializePermissions(extension);
 
   // We keep track of all permissions the user has granted each extension.
@@ -1611,14 +1611,13 @@ void ExtensionService::CheckPermissionsIncrease(const Extension* extension,
       GrantPermissions(extension);
   }
 
-  if (is_extension_installed) {
-    // If the extension was already disabled, suppress any alerts for becoming
-    // disabled on permissions increase.
-    bool previously_disabled =
-        extension_prefs_->IsExtensionDisabled(extension->id());
+  bool previously_disabled =
+      extension_prefs_->IsExtensionDisabled(extension->id());
+  // TODO(treib): Is the |is_extension_loaded| check needed here?
+  if (is_extension_loaded && previously_disabled) {
     // Legacy disabled extensions do not have a disable reason. Infer that it
     // was likely disabled by the user.
-    if (previously_disabled && disable_reasons == Extension::DISABLE_NONE)
+    if (disable_reasons == Extension::DISABLE_NONE)
       disable_reasons |= Extension::DISABLE_USER_ACTION;
 
     // Extensions that came to us disabled from sync need a similar inference,
@@ -1626,8 +1625,7 @@ void ExtensionService::CheckPermissionsIncrease(const Extension* extension,
     // TODO(treib,devlin): Since M48, DISABLE_UNKNOWN_FROM_SYNC isn't used
     // anymore; this code is still here to migrate any existing old state.
     // Remove it after some grace period.
-    if (previously_disabled &&
-        (disable_reasons & Extension::DEPRECATED_DISABLE_UNKNOWN_FROM_SYNC)) {
+    if (disable_reasons & Extension::DEPRECATED_DISABLE_UNKNOWN_FROM_SYNC) {
       // Remove the DISABLE_UNKNOWN_FROM_SYNC reason.
       disable_reasons &= ~Extension::DEPRECATED_DISABLE_UNKNOWN_FROM_SYNC;
       extension_prefs_->RemoveDisableReason(
@@ -1638,6 +1636,22 @@ void ExtensionService::CheckPermissionsIncrease(const Extension* extension,
     }
   }
 
+  // If the extension is disabled due to a permissions increase, but does in
+  // fact have all permissions, remove that disable reason.
+  // TODO(devlin): This was added to fix crbug.com/616474, but it's unclear
+  // if this behavior should stay forever.
+  if (disable_reasons & Extension::DISABLE_PERMISSIONS_INCREASE) {
+    bool reset_permissions_increase = false;
+    if (!is_privilege_increase) {
+      reset_permissions_increase = true;
+      disable_reasons &= ~Extension::DISABLE_PERMISSIONS_INCREASE;
+      extension_prefs_->RemoveDisableReason(
+          extension->id(), Extension::DISABLE_PERMISSIONS_INCREASE);
+    }
+    UMA_HISTOGRAM_BOOLEAN("Extensions.ResetPermissionsIncrease",
+                          reset_permissions_increase);
+  }
+
   // Extension has changed permissions significantly. Disable it. A
   // notification should be sent by the caller. If the extension is already
   // disabled because it was installed remotely, don't add another disable
@@ -1664,7 +1678,10 @@ void ExtensionService::CheckPermissionsIncrease(const Extension* extension,
     }
 #endif
   }
-  if (disable_reasons != Extension::DISABLE_NONE)
+
+  if (disable_reasons == Extension::DISABLE_NONE)
+    extension_prefs_->SetExtensionEnabled(extension->id());
+  else
     extension_prefs_->SetExtensionDisabled(extension->id(), disable_reasons);
 }