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

chrome/browser/extensions/extension_prefs.cc

Index: chrome/browser/extensions/extension_prefs.cc
diff --git a/chrome/browser/extensions/extension_prefs.cc b/chrome/browser/extensions/extension_prefs.cc
index 83fbb596830c553f4484409b4fe0ea96fe13c33d..4f4388aa955ed5d53e9b7b8fe50da81079dc5045 100644
--- a/chrome/browser/extensions/extension_prefs.cc
+++ b/chrome/browser/extensions/extension_prefs.cc
@@ -748,58 +748,44 @@ void ExtensionPrefs::ClearDisableReasons(const std::string& extension_id) {
   UpdateExtensionPref(extension_id, kPrefDisableReasons, NULL);
 }
 
-void ExtensionPrefs::UpdateBlacklist(
-    const std::set<std::string>& blacklist_set) {
-  ExtensionIdList remove_pref_ids;
-  std::set<std::string> used_id_set;
+std::set<std::string> ExtensionPrefs::GetBlacklistedExtensions() {
+  std::set<std::string> ids;
+
   const DictionaryValue* extensions = prefs_->GetDictionary(kExtensionsPref);
+  if (!extensions)
+    return ids;
 
-  if (extensions) {
-    for (DictionaryValue::key_iterator extension_id = extensions->begin_keys();
-         extension_id != extensions->end_keys(); ++extension_id) {
-      const DictionaryValue* ext;
-      if (!extensions->GetDictionaryWithoutPathExpansion(*extension_id, &ext)) {
-        NOTREACHED() << "Invalid pref for extension " << *extension_id;
-        continue;
-      }
-      const std::string& id(*extension_id);
-      if (blacklist_set.find(id) == blacklist_set.end()) {
-        if (!IsBlacklistBitSet(ext)) {
-          // This extension is not in blacklist. And it was not blacklisted
-          // before.
-          continue;
-        } else {
-          if (ext->size() == 1) {
-            // We should remove the entry if the only flag here is blacklist.
-            remove_pref_ids.push_back(id);
-          } else {
-            // Remove the blacklist bit.
-            UpdateExtensionPref(id, kPrefBlacklist, NULL);
-          }
-        }
-      } else {
-        if (!IsBlacklistBitSet(ext)) {
-          // Only set the blacklist if it was not set.
-          UpdateExtensionPref(id, kPrefBlacklist,
-                              Value::CreateBooleanValue(true));
-        }
-        // Keep the record if this extension is already processed.
-        used_id_set.insert(id);
-      }
+  for (DictionaryValue::Iterator it(*extensions); it.HasNext(); it.Advance()) {
+    if (!it.value().IsType(Value::TYPE_DICTIONARY)) {
+      NOTREACHED() << "Invalid pref for extension " << it.key();
+      continue;
     }
+    if (IsBlacklistBitSet(static_cast<const DictionaryValue*>(&it.value())))
+      ids.insert(it.key());
   }
 
-  // Iterate the leftovers to set blacklist in pref
-  std::set<std::string>::const_iterator set_itr = blacklist_set.begin();
-  for (; set_itr != blacklist_set.end(); ++set_itr) {
-    if (used_id_set.find(*set_itr) == used_id_set.end()) {
-      UpdateExtensionPref(*set_itr, kPrefBlacklist,
-                          Value::CreateBooleanValue(true));
-    }
-  }
-  for (size_t i = 0; i < remove_pref_ids.size(); ++i) {
-    DeleteExtensionPrefs(remove_pref_ids[i]);
+  return ids;
+}
+
+void ExtensionPrefs::SetExtensionBlacklisted(const std::string& extension_id,
+                                             bool is_blacklisted) {
+  if (is_blacklisted) {
+    UpdateExtensionPref(extension_id,
+                        kPrefBlacklist,
+                        new base::FundamentalValue(true));

[asargent_no_longer_on_chrome, 2012/11/30 21:44:22]

optional suggestion: I personally prefer "base::CreateBooleanValue(true)"

[not at google - send to devlin, 2012/11/30 23:09:54]

I don't really have a preference though I'm used to using CreateBooleanValue -
however, it seems somebody has written "DEPRECATED" on them (git blame says 20th
november), which is why I did it this way.

[asargent_no_longer_on_chrome, 2012/11/30 23:30:25]

Ah, I missed that. Ok, probably better to go with "new FundamentalValue" then.

(I looked at https://chromiumcodereview.appspot.com/11418048 and don't really
agree with it, but only for bikeshedding reasons. Consistency is far more
important). 

+    UpdateExtensionPref(extension_id,
+                        kPrefBlacklistAcknowledged,
+                        new base::FundamentalValue(true));
+    return;
   }
+
+  // If we're clearing the blacklist bit, we also want to delete the entry for
+  // the extension in prefs altogether if the resulting entry is empty.
+  UpdateExtensionPref(extension_id, kPrefBlacklist, NULL);
+  UpdateExtensionPref(extension_id, kPrefBlacklistAcknowledged, NULL);
+  const DictionaryValue* dict = GetExtensionPref(extension_id);
+  if (dict && dict->empty())
+    DeleteExtensionPrefs(extension_id);
 }
 
 bool ExtensionPrefs::IsExtensionBlacklisted(const std::string& id) const {
@@ -1650,8 +1636,6 @@ scoped_ptr<ExtensionInfo> ExtensionPrefs::GetInstalledExtensionInfo(
   if (!extensions ||
       !extensions->GetDictionaryWithoutPathExpansion(extension_id, &ext))
     return scoped_ptr<ExtensionInfo>();
-  if (IsBlacklistBitSet(ext))
-    return scoped_ptr<ExtensionInfo>();
   int state_value;
   if (!ext->GetInteger(kPrefState, &state_value) ||
       state_value == Extension::ENABLED_COMPONENT) {
@@ -2011,8 +1995,6 @@ ExtensionIdList ExtensionPrefs::GetExtensionsFrom(
       NOTREACHED() << "Invalid pref for extension " << *it;
       continue;
     }
-    if (!IsBlacklistBitSet(ext))
-      result.push_back(*it);
   }
   return result;
 }