Make Blacklist::IsBlacklist asynchronous (it will need to be for safe

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 f49aaa819136327285f04351bcec21f20c32ad90..630cabe5f2787e5acdf8340633e5888e94c29a8b 100644
--- a/chrome/browser/extensions/extension_service.cc
+++ b/chrome/browser/extensions/extension_service.cc
@@ -5,6 +5,7 @@
 #include "chrome/browser/extensions/extension_service.h"
 
 #include <algorithm>
+#include <iterator>
 #include <set>
 
 #include "base/basictypes.h"
@@ -364,6 +365,7 @@ ExtensionService::ExtensionService(Profile* profile,
       profile_(profile),
       system_(extensions::ExtensionSystem::Get(profile)),
       extension_prefs_(extension_prefs),
+      blacklist_(blacklist),
       settings_frontend_(extensions::SettingsFrontend::Create(profile)),
       pending_extension_manager_(*ALLOW_THIS_IN_INITIALIZER_LIST(this)),
       install_directory_(install_directory),
@@ -469,23 +471,28 @@ const ExtensionSet* ExtensionService::terminated_extensions() const {
   return &terminated_extensions_;
 }
 
-const ExtensionSet* ExtensionService::GenerateInstalledExtensionsSet() const {
-  ExtensionSet* installed_extensions = new ExtensionSet();
+const ExtensionSet* ExtensionService::blacklisted_extensions() const {
+  return &blacklisted_extensions_;
+}
+
+scoped_ptr<const ExtensionSet>
+    ExtensionService::GenerateInstalledExtensionsSet() const {
+  scoped_ptr<ExtensionSet> installed_extensions(new ExtensionSet());
   installed_extensions->InsertAll(extensions_);
   installed_extensions->InsertAll(disabled_extensions_);
   installed_extensions->InsertAll(terminated_extensions_);
-  return installed_extensions;
+  return installed_extensions.PassAs<const ExtensionSet>();
 }
 
-const ExtensionSet* ExtensionService::GetWipedOutExtensions() const {
-  ExtensionSet* extension_set = new ExtensionSet();
+scoped_ptr<const ExtensionSet> ExtensionService::GetWipedOutExtensions() const {
+  scoped_ptr<ExtensionSet> extension_set(new ExtensionSet());
   for (ExtensionSet::const_iterator iter = disabled_extensions_.begin();
        iter != disabled_extensions_.end(); ++iter) {
     int disabled_reason = extension_prefs_->GetDisableReasons((*iter)->id());
     if ((disabled_reason & Extension::DISABLE_SIDELOAD_WIPEOUT) != 0)
       extension_set->Insert(*iter);
   }
-  return extension_set;
+  return extension_set.PassAs<const ExtensionSet>();
 }
 
 extensions::PendingExtensionManager*
@@ -890,8 +897,10 @@ bool ExtensionService::IsExtensionEnabled(
     return true;
   }
 
-  if (disabled_extensions_.Contains(extension_id))
+  if (disabled_extensions_.Contains(extension_id) ||
+      blacklisted_extensions_.Contains(extension_id)) {
     return false;
+  }
 
   // If the extension hasn't been loaded yet, check the prefs for it. Assume
   // enabled unless otherwise noted.
@@ -1267,6 +1276,7 @@ extensions::ExtensionUpdater* ExtensionService::updater() {
 
 void ExtensionService::CheckManagementPolicy() {
   std::vector<std::string> to_be_removed;
+
   // Loop through extensions list, unload installed extensions.
   for (ExtensionSet::const_iterator iter = extensions_.begin();
        iter != extensions_.end(); ++iter) {
@@ -1830,15 +1840,21 @@ void ExtensionService::IdentifyAlertableExtensions() {
 bool ExtensionService::PopulateExtensionErrorUI(
     ExtensionErrorUI* extension_error_ui) {
   bool needs_alert = false;
+
   for (ExtensionSet::const_iterator iter = extensions_.begin();
        iter != extensions_.end(); ++iter) {
     const Extension* e = *iter;
+
+    // Extensions disabled by policy. Note: this no longer includes blacklisted
+    // extensions, though we still show the same UI.
     if (!system_->management_policy()->UserMayLoad(e, NULL)) {
       if (!extension_prefs_->IsBlacklistedExtensionAcknowledged(e->id())) {
         extension_error_ui->AddBlacklistedExtension(e->id());
         needs_alert = true;
       }
     }
+
+    // Orphaned extensions.
     if (extension_prefs_->IsExtensionOrphaned(e->id())) {
       if (!extension_prefs_->IsOrphanedExtensionAcknowledged(e->id())) {
         extension_error_ui->AddOrphanedExtension(e->id());
@@ -1846,6 +1862,17 @@ bool ExtensionService::PopulateExtensionErrorUI(
       }
     }
   }
+
+  // Extensions that really are blacklisted.

[Yoyo Zhou, 2012/11/30 23:44:07]

I was confused reading the "disabled by policy" block before I got here, so
maybe this one should come first.

[not at google - send to devlin, 2012/12/01 02:51:30]

Done.

+  for (ExtensionSet::const_iterator it = blacklisted_extensions_.begin();
+       it != blacklisted_extensions_.end(); ++it) {
+    std::string id = (*it)->id();
+    if (!extension_prefs_->IsBlacklistedExtensionAcknowledged(id)) {
+      extension_error_ui->AddBlacklistedExtension(id);
+      needs_alert = true;
+    }
+  }
+
   return needs_alert;
 }
 
@@ -2085,7 +2112,17 @@ void ExtensionService::AddExtension(const Extension* extension) {
   // wipeout before, we might disable this extension here.
   MaybeWipeout(extension);
 
-  if (extension_prefs_->IsExtensionDisabled(extension->id())) {
+  // Communicated to the Blacklist.
+  std::set<std::string> already_in_blacklist;
+
+  if (extension_prefs_->IsExtensionBlacklisted(extension->id())) {
+    // Don't check the Blacklist here because it's asynchronous (we do it at

[Yoyo Zhou, 2012/11/30 23:44:07]

nit: s/here/yet/

[not at google - send to devlin, 2012/12/01 02:51:30]

Done.

+    // the end). This pre-emptive check is because we will always store the
+    // blacklisted state of *installed* extensions in prefs, and it's important
+    // not to re-enable blacklisted extensions.
+    blacklisted_extensions_.Insert(extension);
+    already_in_blacklist.insert(extension->id());
+  } else if (extension_prefs_->IsExtensionDisabled(extension->id())) {
     disabled_extensions_.Insert(extension);
     SyncExtensionChangeIfNeeded(*extension);
     content::NotificationService::current()->Notify(
@@ -2097,20 +2134,27 @@ void ExtensionService::AddExtension(const Extension* extension) {
         Extension::DISABLE_PERMISSIONS_INCREASE) {
       extensions::AddExtensionDisabledError(this, extension);
     }
-    return;
-  }
+  } else {
+    // All apps that are displayed in the launcher are ordered by their ordinals
+    // so we must ensure they have valid ordinals.
+    if (extension->RequiresSortOrdinal()) {
+      extension_prefs_->extension_sorting()->EnsureValidOrdinals(
+          extension->id(), syncer::StringOrdinal());
+    }
 
-  // All apps that are displayed in the launcher are ordered by their ordinals
-  // so we must ensure they have valid ordinals.
-  if (extension->RequiresSortOrdinal()) {
-    extension_prefs_->extension_sorting()->EnsureValidOrdinals(
-        extension->id(), syncer::StringOrdinal());
+    extensions_.Insert(extension);
+    SyncExtensionChangeIfNeeded(*extension);
+    NotifyExtensionLoaded(extension);
+    DoPostLoadTasks(extension);
   }
 
-  extensions_.Insert(extension);
-  SyncExtensionChangeIfNeeded(*extension);
-  NotifyExtensionLoaded(extension);
-  DoPostLoadTasks(extension);
+  // Lastly, begin the process for checking the blacklist status of extensions.
+  // This may need to go to other threads so is asynchronous.
+  blacklist_->IsBlacklisted(
+      extension->id(),
+      base::Bind(&ExtensionService::ManageBlacklist,
+                 AsWeakPtr(),
+                 already_in_blacklist));
 }
 
 void ExtensionService::AddComponentExtension(const Extension* extension) {
@@ -3031,5 +3075,47 @@ bool ExtensionService::ShouldDelayExtensionUpdate(
 }
 
 void ExtensionService::OnBlacklistUpdated() {
-  CheckManagementPolicy();
+  blacklist_->IsBlacklisted(
+      GenerateInstalledExtensionsSet()->GetIDs(),
+      base::Bind(&ExtensionService::ManageBlacklist,
+                 AsWeakPtr(),
+                 blacklisted_extensions_.GetIDs()));
+}
+
+void ExtensionService::ManageBlacklist(
+    const std::set<std::string>& old_blacklisted_ids,
+    const std::set<std::string>& new_blacklisted_ids) {
+  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
+
+  std::set<std::string> no_longer_blacklisted;
+  std::set_difference(old_blacklisted_ids.begin(), old_blacklisted_ids.end(),
+                      new_blacklisted_ids.begin(), new_blacklisted_ids.end(),
+                      std::inserter(no_longer_blacklisted,
+                                    no_longer_blacklisted.begin()));
+  std::set<std::string> not_yet_blacklisted;
+  std::set_difference(new_blacklisted_ids.begin(), new_blacklisted_ids.end(),
+                      old_blacklisted_ids.begin(), old_blacklisted_ids.end(),
+                      std::inserter(not_yet_blacklisted,
+                                    not_yet_blacklisted.begin()));
+
+  for (std::set<std::string>::iterator it = no_longer_blacklisted.begin();
+       it != no_longer_blacklisted.end(); ++it) {
+    scoped_refptr<const Extension> extension =
+        blacklisted_extensions_.GetByID(*it);
+    if (!extension)
+      continue;
+    blacklisted_extensions_.Remove(*it);
+    AddExtension(extension);
+  }
+
+  for (std::set<std::string>::iterator it = not_yet_blacklisted.begin();
+       it != not_yet_blacklisted.end(); ++it) {
+    scoped_refptr<const Extension> extension = GetInstalledExtension(*it);
+    if (!extension)
+      continue;
+    blacklisted_extensions_.Insert(extension);
+    UnloadExtension(*it, extension_misc::UNLOAD_REASON_BLACKLIST);
+  }
+
+  IdentifyAlertableExtensions();
 }