Extensions: Store disable reasons in Sync

chrome/browser/extensions/extension_service.cc

 // Copyright (c) 2013 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "chrome/browser/extensions/extension_service.h"
 
 #include <algorithm>
 #include <iterator>
 #include <set>
 
@@ skipping 521 matching lines @@
   installer->set_expected_hash(file.expected_hash);
   int creation_flags = Extension::NO_FLAGS;
   if (pending_extension_info) {
     installer->set_install_source(pending_extension_info->install_source());
     installer->set_allow_silent_install(true);
     if (pending_extension_info->remote_install())
       installer->set_grant_permissions(false);
     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());
   }
   // If the extension was installed from or has migrated to the webstore, or
   // its auto-update URL is from the webstore, treat it as a webstore install.
   // Note that we ignore some older extensions with blank auto-update URLs
   // because we are mostly concerned with restrictions on NaCl extensions,
   // which are newer.
   if ((extension && extension->from_webstore()) ||
       (extension && extensions::ManifestURL::UpdatesFromGallery(extension)) ||
@@ skipping 310 matching lines @@
       extensions::NOTIFICATION_EXTENSION_ENABLED,
       content::Source<Profile>(profile_),
       content::Details<const Extension>(extension));
 
   if (extension_sync_service_)
     extension_sync_service_->SyncEnableExtension(*extension);
 }
 
 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;
   }
 
   const Extension* extension = GetInstalledExtension(extension_id);
   // |extension| can be NULL if sync disables an extension that is not
   // installed yet.
   // EXTERNAL_COMPONENT extensions are not generally modifiable by users, but
   // 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;
   extension = registry_->GetExtensionById(extension_id, include_mask);
   if (!extension)
     return;
 
   // The extension is either enabled or terminated.
   DCHECK(registry_->enabled_extensions().Contains(extension->id()) ||
          registry_->terminated_extensions().Contains(extension->id()));
@@ skipping 1691 matching lines @@
 }
 
 void ExtensionService::OnProfileDestructionStarted() {
   ExtensionIdSet ids_to_unload = registry_->enabled_extensions().GetIDs();
   for (ExtensionIdSet::iterator it = ids_to_unload.begin();
        it != ids_to_unload.end();
        ++it) {
     UnloadExtension(*it, UnloadedExtensionInfo::REASON_PROFILE_SHUTDOWN);
   }
 }