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

chrome/browser/extensions/extension_prefs.cc

 // Copyright (c) 2012 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_prefs.h"
 
 #include "base/command_line.h"
 #include "base/prefs/pref_notifier.h"
 #include "base/string_number_conversions.h"
 #include "base/string_util.h"
@@ skipping 730 matching lines @@
   } else {
     UpdateExtensionPref(extension_id, kPrefDisableReasons,
                         Value::CreateIntegerValue(new_value));
   }
 }
 
 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));
-    }
+  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;
   }
-  for (size_t i = 0; i < remove_pref_ids.size(); ++i) {
-    DeleteExtensionPrefs(remove_pref_ids[i]);
-  }
+
+  // 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 {
   const DictionaryValue* ext_prefs = GetExtensionPref(id);
   return ext_prefs && IsBlacklistBitSet(ext_prefs);
 }
 
 namespace {
 
 // Serializes |time| as a string value mapped to |key| in |dictionary|.
@@ skipping 830 matching lines @@
   return extension;
 }
 
 scoped_ptr<ExtensionInfo> ExtensionPrefs::GetInstalledExtensionInfo(
     const std::string& extension_id) const {
   const DictionaryValue* ext;
   const DictionaryValue* extensions = prefs_->GetDictionary(kExtensionsPref);
   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) {
     // Old preferences files may not have kPrefState for component extensions.
     return scoped_ptr<ExtensionInfo>();
   }
 
   if (state_value == Extension::EXTERNAL_EXTENSION_UNINSTALLED) {
     LOG(WARNING) << "External extension with id " << extension_id
                  << " has been uninstalled by the user";
@@ skipping 339 matching lines @@
     return result;  // Empty set
   }
 
   for (base::DictionaryValue::key_iterator it = extension_prefs->begin_keys();
        it != extension_prefs->end_keys(); ++it) {
     const DictionaryValue* ext;
     if (!extension_prefs->GetDictionaryWithoutPathExpansion(*it, &ext)) {
       NOTREACHED() << "Invalid pref for extension " << *it;
       continue;
     }
-    if (!IsBlacklistBitSet(ext))
-      result.push_back(*it);
   }
   return result;
 }
 
 void ExtensionPrefs::FixMissingPrefs(const ExtensionIdList& extension_ids) {
   // Fix old entries that did not get an installation time entry when they
   // were installed or don't have a preferences field.
   for (ExtensionIdList::const_iterator ext_id = extension_ids.begin();
        ext_id != extension_ids.end(); ++ext_id) {
     if (GetInstallTime(*ext_id) == base::Time()) {
@@ skipping 288 matching lines @@
     const ExtensionIdList& strings) {
   ListPrefUpdate update(prefs_, pref);
   ListValue* list_of_values = update.Get();
   list_of_values->Clear();
   for (ExtensionIdList::const_iterator iter = strings.begin();
        iter != strings.end(); ++iter)
     list_of_values->Append(new StringValue(*iter));
 }
 
 }  // namespace extensions