Extensions: Store disable reasons in Sync

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 54f4dee21782ad230c940a65073a1b34e5bc41d8..e638c2c2338bd6ab0d8c33633edeb06df632615a 100644
--- a/chrome/browser/extensions/extension_service.cc
+++ b/chrome/browser/extensions/extension_service.cc
@@ -539,6 +539,13 @@ bool ExtensionService::UpdateExtension(const extensions::CRXFileInfo& file,
     creation_flags = pending_extension_info->creation_flags();
     if (pending_extension_info->mark_acknowledged())
       external_install_manager_->AcknowledgeExternalExtension(id);
+
+    // If the extension came in disabled due to a permission increase, then

[not at google - send to devlin, 2015/05/12 18:04:17]

Is it reasonable to defer this change until after this patch has gone in? Less
to think about that way: first patch to migrate to the new model, second patch
to fix the bugs that it enables fixing?

[Marc Treib, 2015/05/19 12:12:03]

Yup, that makes sense. Reverted this change.

+    // don't grant it all the permissions! crbug.com/484214
+    if (extensions::ExtensionPrefs::Get(profile_)->HasDisableReason(
+            id, Extension::DISABLE_PERMISSIONS_INCREASE)) {
+      installer->set_grant_permissions(false);
+    }
   } else if (extension) {
     installer->set_install_source(extension->location());
   }
@@ -869,12 +876,12 @@ void ExtensionService::EnableExtension(const std::string& extension_id) {
 
 void ExtensionService::DisableExtension(
     const std::string& extension_id,
-    Extension::DisableReason disable_reason) {
+    Extension::DisableReason disable_reasons) {
   CHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
 
   // The extension may have been disabled already. Just add a disable reason.
   if (!IsExtensionEnabled(extension_id)) {
-    extension_prefs_->AddDisableReason(extension_id, disable_reason);
+    extension_prefs_->AddDisableReason(extension_id, disable_reasons);

[not at google - send to devlin, 2015/05/12 18:04:17]

If AddDisableReason() does support multiple reasons, it would be nice to rename
it AddDisableReasons() (and take an int). Or, leave AddDisableReason() which
takes an enum, and add a second function AddDisableReasons() which takes an int
bitmask.

[Marc Treib, 2015/05/19 12:12:03]

I've added ExtensionPrefs::AddDisableReasons(int). I think there's a few other
places where Extensions::DisableReason is used as a bitmask, though. I'll clean
them up if I come across any...

     return;
   }
 
@@ -885,15 +892,15 @@ void ExtensionService::DisableExtension(
   // can be uninstalled by the browser if the user sets extension-specific
   // preferences.
   if (extension &&
-      disable_reason != Extension::DISABLE_RELOAD &&
-      disable_reason != Extension::DISABLE_UPDATE_REQUIRED_BY_POLICY &&
+      !(disable_reasons & Extension::DISABLE_RELOAD) &&
+      !(disable_reasons & Extension::DISABLE_UPDATE_REQUIRED_BY_POLICY) &&
       !system_->management_policy()->UserMayModifySettings(extension, NULL) &&
       extension->location() != Manifest::EXTERNAL_COMPONENT) {
     return;
   }
 
   extension_prefs_->SetExtensionState(extension_id, Extension::DISABLED);
-  extension_prefs_->AddDisableReason(extension_id, disable_reason);
+  extension_prefs_->AddDisableReason(extension_id, disable_reasons);
 
   int include_mask =
       ExtensionRegistry::EVERYTHING & ~ExtensionRegistry::DISABLED;