Temporarily disable extensions and sync while a profile is locked - Profiles Approach

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 252 matching lines @@
       pending_extension_manager_(profile),
       install_directory_(install_directory),
       extensions_enabled_(extensions_enabled),
       show_extensions_prompts_(true),
       install_updates_when_idle_(true),
       ready_(ready),
       update_once_all_providers_are_ready_(false),
       browser_terminating_(false),
       installs_delayed_for_gc_(false),
       is_first_run_(false),
+      block_extensions_(false),
       shared_module_service_(new extensions::SharedModuleService(profile_)) {
   CHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
 
   // Figure out if extension installation should be enabled.
   if (extensions::ExtensionsBrowserClient::Get()->AreExtensionsDisabled(
           *command_line, profile))
     extensions_enabled_ = false;
 
   registrar_.Add(this, chrome::NOTIFICATION_APP_TERMINATING,
                  content::NotificationService::AllBrowserContextsAndSources());
@@ skipping 84 matching lines @@
       ->GetForBrowserContext(profile())
       ->RemoveObserver(this);
   system_->management_policy()->UnregisterProvider(
       shared_module_policy_provider_.get());
 }
 
 const Extension* ExtensionService::GetExtensionById(
     const std::string& id, bool include_disabled) const {
   int include_mask = ExtensionRegistry::ENABLED;
   if (include_disabled) {
-    // Include blacklisted extensions here because there are hundreds of
-    // callers of this function, and many might assume that this includes those
-    // that have been disabled due to blacklisting.
+    // Include blacklisted and blocked extensions here because there are
+    // hundreds of callers of this function, and many might assume that this
+    // includes those that have been disabled due to blacklisting or blocking.
     include_mask |= ExtensionRegistry::DISABLED |
-                    ExtensionRegistry::BLACKLISTED;
+                    ExtensionRegistry::BLACKLISTED |
+                    ExtensionRegistry::BLOCKED;
   }
   return registry_->GetExtensionById(id, include_mask);
 }
 
 void ExtensionService::Init() {
   CHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
 
   base::Time begin_time = base::Time::Now();
 
   DCHECK(!is_ready());  // Can't redo init.
@@ skipping 180 matching lines @@
     const std::string& transient_extension_id,
     bool be_noisy) {
   CHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
 
   // If the extension is already reloading, don't reload again.
   if (extension_prefs_->GetDisableReasons(transient_extension_id) &
       Extension::DISABLE_RELOAD) {
     return;
   }
 
-  // Ignore attempts to reload a blacklisted extension. Sometimes this can
-  // happen in a convoluted reload sequence triggered by the termination of a
-  // blacklisted extension and a naive attempt to reload it. For an example see
-  // http://crbug.com/373842.
-  if (registry_->blacklisted_extensions().Contains(transient_extension_id))
+  // Ignore attempts to reload a blacklisted or blocked extension. Sometimes
+  // this can happen in a convoluted reload sequence triggered by the
+  // termination of a blacklisted or blocked extension and a naive attempt to
+  // reload it. For an example see http://crbug.com/373842.
+  if (registry_->blacklisted_extensions().Contains(transient_extension_id) ||
+      registry_->blocked_extensions().Contains(transient_extension_id)) {
     return;
+  }
 
   base::FilePath path;
 
   std::string extension_id = transient_extension_id;
   const Extension* transient_current_extension =
       GetExtensionById(extension_id, false);
 
   // Disable the extension if it's loaded. It might not be loaded if it crashed.
   if (transient_current_extension) {
     // If the extension has an inspector open for its background page, detach
@@ skipping 184 matching lines @@
 }
 
 bool ExtensionService::IsExtensionEnabled(
     const std::string& extension_id) const {
   if (registry_->enabled_extensions().Contains(extension_id) ||
       registry_->terminated_extensions().Contains(extension_id)) {
     return true;
   }
 
   if (registry_->disabled_extensions().Contains(extension_id) ||
-      registry_->blacklisted_extensions().Contains(extension_id)) {
+      registry_->blacklisted_extensions().Contains(extension_id) ||
+      registry_->blocked_extensions().Contains(extension_id)) {
     return false;
   }
 
   // If the extension hasn't been loaded yet, check the prefs for it. Assume
   // enabled unless otherwise noted.
   return !extension_prefs_->IsExtensionDisabled(extension_id) &&
          !extension_prefs_->IsExtensionBlacklisted(extension_id) &&
+         !extension_prefs_->IsExtensionBlocked(extension_id) &&
          !extension_prefs_->IsExternalExtensionUninstalled(extension_id);
 }
 
 void ExtensionService::EnableExtension(const std::string& extension_id) {
   CHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
 
   if (IsExtensionEnabled(extension_id))
     return;
   const Extension* extension =
       registry_->disabled_extensions().GetByID(extension_id);
@@ skipping 103 matching lines @@
     if ((*extension)->was_installed_by_default() &&
         extension_urls::IsWebstoreUpdateUrl(
             extensions::ManifestURL::GetUpdateURL(extension->get())))
       continue;
     const std::string& id = (*extension)->id();
     if (except_ids.end() == std::find(except_ids.begin(), except_ids.end(), id))
       DisableExtension(id, extensions::Extension::DISABLE_USER_ACTION);
   }
 }
 
+// Extensions that are not locked, components or forced by policy should be
+// locked. Extensions are no longer considered enabled or disabled. Blacklisted
+// extensions are now considered both blacklisted and locked.
+void ExtensionService::BlockAllExtensions() {

[not at google - send to devlin, 2014/11/12 23:41:43]

Oh, you might want to check that it's not already blocked and/or there are no
extensions in the current block list?

[Mike Lerman, 2014/11/13 15:30:05]

There's no reason this method couldn't be called multiple times. It logically
won't the way the profiles code is implemented to call this, and shouldn't have
any effect with the safeguard that's added to AddExtension(), but I see no harm
in leaving it possible to be called multiple times.

If we add a DCHECK or if()-return , I see some future developer looking over
this code, needing to call it multiple times and saying, "But WHY was this
prevented? What edge-case was the author preventing?"

+  block_extensions_ = true;
+  scoped_ptr<ExtensionSet> extensions =
+      registry_->GenerateInstalledExtensionsSet();
+
+  ExtensionIdSet to_lock;

[not at google - send to devlin, 2014/11/12 23:41:43]

You can delete this now.

[Mike Lerman, 2014/11/13 15:30:05]

Bye bye!

+  for (const scoped_refptr<const Extension> extension : *extensions.get()) {
+    const std::string& id = extension.get()->id();

[not at google - send to devlin, 2014/11/12 23:41:43]

just extension->id();

[Mike Lerman, 2014/11/13 15:30:04]

Done.

+
+    if (!CanBlockExtension(extension.get()))
+      continue;
+
+    // Blacklisted extensions remain in the blacklisted list.
+    registry_->RemoveEnabled(id);
+    registry_->RemoveDisabled(id);
+    registry_->RemoveTerminated(id);
+

[not at google - send to devlin, 2014/11/12 23:41:43]

Come to think of it it's probably also wise/safer not to have an Extension in
both the blacklisted and blocked list, just to be safe.

Hey here's an idea: how about adding a 2nd version of
GenerateInstallExtensionsSet which takes an IncludeMask, then only include
non-blacklisted extensions in here in the first place. Then you don't have to
worry about that at all.

[Mike Lerman, 2014/11/13 15:30:05]

I'm concerned about removing things from the blacklist - it seems very important
that blacklisted things stay that way, and I'd hate for a bug here to somehow
result in something escaping the blacklist.

Also, if I remove it from the blacklist, I need a foolproof way to put it back
there in UnblockAllBlockedExtensions(), and I can't find a way to determine
that.

[not at google - send to devlin, 2014/11/13 18:09:47]

I'm not suggesting removing it from the blacklist. I'm suggesting that you
exclude the blacklisted extensions from ever actually entering the installed
extensions list by the method I mentioned above.

[Mike Lerman, 2014/11/13 21:42:32]

Done.

+    registry_->AddBlocked(extension.get());
+    extension_prefs_->SetExtensionState(id, Extension::BLOCKED);

[not at google - send to devlin, 2014/11/12 23:41:43]

Actually - why do you need to change prefs at all? Blocking is a global state,
you shouldn't need to change the extension *prefs*.

[Mike Lerman, 2014/11/13 15:30:04]

The goal was the keep the ExtensionState sync-ed with the ExtensionRegistry -
having a state of DISABLED but not being in the disabled_extensions() seems like
a problem waiting to happen. So I need to set the state to something, and made a
new one.

[not at google - send to devlin, 2014/11/13 18:09:47]

I don't think so. For example we keep terminated extensions in their
enabled/disabled prefs. It will simplify things considerably if you don't make
any changes to the prefs.

[Mike Lerman, 2014/11/13 21:42:32]

Done.

+    UnloadExtension(
+        id, extensions::UnloadedExtensionInfo::REASON_LOCK_ALL);
+  }
+}
+
+// All locked extensions should revert to being either enabled or disabled
+// as appropriate.
+void ExtensionService::UnblockAllBlockedExtensions() {
+  ExtensionIdSet to_unblock = registry_->blocked_extensions().GetIDs();

[not at google - send to devlin, 2014/11/12 23:41:43]

(see previous point about just copying the whole extension set)

[Mike Lerman, 2014/11/13 15:30:04]

Which one?

Is making a copy of the set better than just referring to the IDs? Also, using
the IDs is consistent with the implementations of similar loops in
UpdateBlockedExtensions and UpdateGreylistedExtensions.

[not at google - send to devlin, 2014/11/13 18:09:47]

Maybe they should copy the set as well. Needing 7 lines to deal with fetching
the extension back out of the list again is avoidable work (my point being
confusing to read rather than computationally efficient, though it probably is
also more efficient).

[Mike Lerman, 2014/11/13 21:42:32]

Copied as well. Done.

+
+  block_extensions_ = false;
+  for (ExtensionIdSet::iterator it = to_unblock.begin();

[not at google - send to devlin, 2014/11/12 23:41:43]

(and use : syntax here too)

[Mike Lerman, 2014/11/13 15:30:04]

Done.

+       it != to_unblock.end(); ++it) {
+    scoped_refptr<const Extension> extension = GetInstalledExtension(*it);
+    if (!extension.get()) {
+      NOTREACHED() << "Extension " << *it << " needs to be "
+                   << "unblocked, but it's not installed.";
+      continue;
+    }
+
+    registry_->RemoveBlocked(*it);
+    if (extensions_being_terminated_.find(extension->id()) !=

[not at google - send to devlin, 2014/11/12 23:41:43]

Mm, maybe this isn't actually useful. I doubt this set actually includes
anything in it anymore. Perhaps just re-enabling terminated extensions would be
fine, the important part is more just blocking them in the first place.

[Mike Lerman, 2014/11/13 15:30:05]

Done.

+               extensions_being_terminated_.end()) {
+      registry_->AddTerminated(extension.get());
+    } else {
+      if (extension_prefs_->GetDisableReasons(*it)) {

[not at google - send to devlin, 2014/11/12 23:41:43]

GetDisableReasons returns a bitmask, so it's a little confusing doing if
(GetDisableReasons). prefs_->IsExtensionDisabled(..) might be more appropriate.

However, see comment above.

[Mike Lerman, 2014/11/13 15:30:04]

I agree about using IsExtensionDisabled, if we have the state. Pending comment
above resolution :)

+        extension_prefs_->SetExtensionState(*it, Extension::DISABLED);
+      } else {
+        extension_prefs_->SetExtensionState(*it, Extension::ENABLED);
+      }
+      AddExtension(extension.get());
+    }
+  }
+}
+
 void ExtensionService::GrantPermissionsAndEnableExtension(
     const Extension* extension) {
   GrantPermissions(extension);
   RecordPermissionMessagesHistogram(extension,
                                     "Extensions.Permissions_ReEnable2");
   extension_prefs_->SetDidExtensionEscalatePermissions(extension, false);
   EnableExtension(extension->id());
 }
 
 void ExtensionService::GrantPermissions(const Extension* extension) {
@@ skipping 433 matching lines @@
     // new one.  ReloadExtension disables the extension, which is sufficient.
     UnloadExtension(extension->id(), UnloadedExtensionInfo::REASON_UPDATE);
   }
 
   if (extension_prefs_->IsExtensionBlacklisted(extension->id())) {
     // Only prefs is checked for the blacklist. We rely on callers to check the
     // blacklist before calling into here, e.g. CrxInstaller checks before
     // installation then threads through the install and pending install flow
     // of this class, and we check when loading installed extensions.
     registry_->AddBlacklisted(extension);
+  } else if (extension_prefs_->IsExtensionBlocked(extension->id()) ||
+             (block_extensions_ && CanBlockExtension(extension))) {
+    registry_->AddBlocked(extension);
   } else if (!reloading &&
              extension_prefs_->IsExtensionDisabled(extension->id())) {
     registry_->AddDisabled(extension);
     if (extension_sync_service_)
       extension_sync_service_->SyncExtensionChangeIfNeeded(*extension);
     content::NotificationService::current()->Notify(
         extensions::NOTIFICATION_EXTENSION_UPDATE_DISABLED,
         content::Source<Profile>(profile_),
         content::Details<const Extension>(extension));
 
@@ skipping 782 matching lines @@
     if (extension->GetType() != Manifest::TYPE_HOSTED_APP &&
         Manifest::IsExternalLocation(extension->location()) &&
         !extension_prefs_->IsExternalExtensionAcknowledged(extension->id())) {
       return false;
     }
   }
 
   return true;
 }
 
+// Helper method to determine if an extension can be blocked.
+bool ExtensionService::CanBlockExtension(const Extension* extension) {
+  return (extension->location() != Manifest::COMPONENT &&

[not at google - send to devlin, 2014/11/12 23:41:43]

Outer parens are unnecessary.

[Mike Lerman, 2014/11/13 15:30:04]

Done.

+      extension->location() != Manifest::EXTERNAL_COMPONENT &&
+      !system_->management_policy()->MustRemainEnabled(extension, NULL));
+}
+
 bool ExtensionService::ShouldDelayExtensionUpdate(
     const std::string& extension_id,
     bool install_immediately) const {
   const char kOnUpdateAvailableEvent[] = "runtime.onUpdateAvailable";
 
   // If delayed updates are globally disabled, or just for this extension,
   // don't delay.
   if (!install_updates_when_idle_ || install_immediately)
     return false;
 
@@ skipping 68 matching lines @@
       case extensions::BLACKLISTED_POTENTIALLY_UNWANTED:
         greylist.insert(it->first);
         break;
 
       case extensions::BLACKLISTED_UNKNOWN:
         unchanged.insert(it->first);
         break;
     }
   }
 
   UpdateBlockedExtensions(blocked, unchanged);

[not at google - send to devlin, 2014/11/12 23:41:43]

Looks like the "blocked" term is already used, but it actually means
blacklisting in this case, so could you rename this method to
UpdateBlacklistedExtensions?

[Mike Lerman, 2014/11/13 15:30:04]

Done.

   UpdateGreylistedExtensions(greylist, unchanged, state_map);
 
   error_controller_->ShowErrorIfNeeded();
 }
 
 namespace {
 void Partition(const ExtensionIdSet& before,
                const ExtensionIdSet& after,
                const ExtensionIdSet& unchanged,
                ExtensionIdSet* no_longer,
@@ skipping 111 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);
   }
 }