Fix for Bug 50726 "Save extension list and "winning" prefs from extensions"

chrome/browser/extensions/extension_prefs.cc

 // Copyright (c) 2010 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/string_util.h"
 #include "base/string_number_conversions.h"
 #include "base/utf_string_conversions.h"
+#include "chrome/browser/prefs/in_memory_pref_store.h"
 #include "chrome/common/extensions/extension.h"
 #include "chrome/common/notification_service.h"
 #include "chrome/common/pref_names.h"
 
 using base::Time;
 
 namespace {
 
 // Additional preferences keys
 
@@ skipping 55 matching lines @@
 
 // A preference determining the order of which the apps appear on the NTP.
 const char kPrefAppLaunchIndex[] = "app_launcher_index";
 
 // A preference for storing extra data sent in update checks for an extension.
 const char kUpdateUrlData[] = "update_url_data";
 
 // Whether the browser action is visible in the toolbar.
 const char kBrowserActionVisible[] = "browser_action_visible";
 
+// A preference that indicates when an extension was installed.
+const char kPrefInstallTime[] = "install_time";
+
+// A preference that indicates the last effective preference values of an
+// extension. The value is a dictionary mapping (non-expanded) preference keys
+// to the values configured by the extension.
+const char kPrefPreferences[] = "preferences";

[Aaron Boodman, 2010/11/23 20:35:13]

I'm split on having this subtree and the corresponding distinction between
extension-controlled preferences, and other per-extension preferences.

On the one hand it's a namespace separation, which seems like it could be handy,
and it makes the current design cleaner because we don't need a whitelist
anywhere of extension-controlled preferences.

But on the other hand, it adds some complexity, and the whitelist issue would be
fixed automatically with bug 64232.

I guess I lean toward combining the extension-controlled preferences with the
existing tree, and having a whitelist of known ECP in the code that builds up
the tree to hand back to PrefValueStore.

I don't have any suggestion for how to prevent the whitelist from going stale
though, so that is a downside.

Jochen, Dominic, what are your thoughts?

[Mattias Nissler (ping if slow), 2010/11/23 22:02:44]

My vote is for the namespace separation. This also makes it clearer that these
values don't apply to how the extension system handles the extensions at all,
but only represent a cache for the pref system. To that end, I propose changing
the name from preferences to extension_prefs_cache or something similar.

[battre (please use the other), 2010/11/30 17:46:53]

We have discussed this with several people in MUC (mnissler, danno, battre,
bauerb) and prefer to keep it as a preferences subtree. How is a fix for 64232
going to fix this automatically?

+
 }  // namespace
 
 ////////////////////////////////////////////////////////////////////////////////
 
 namespace {
 
 // TODO(asargent) - This is cleanup code for a key that was introduced into
 // the extensions.settings sub-dictionary which wasn't a valid extension
 // id. We can remove this in a couple of months. (See http://crbug.com/40017
 // and http://crbug.com/39745 for more details).
@@ skipping 21 matching lines @@
 }  // namespace
 
 ExtensionPrefs::ExtensionPrefs(PrefService* prefs, const FilePath& root_dir)
     : prefs_(prefs),
       install_directory_(root_dir) {
   // TODO(asargent) - Remove this in a couple of months. (See comment above
   // CleanupBadExtensionKeys).
   CleanupBadExtensionKeys(prefs);
 
   MakePathsRelative();
+
+  InstallPersistedExtensionControlledPrefs();
 }
 
 ExtensionPrefs::~ExtensionPrefs() {}
 
 // static
 const char ExtensionPrefs::kExtensionsPref[] = "extensions.settings";
 
 static FilePath::StringType MakePathRelative(const FilePath& parent,
                                              const FilePath& child,
                                              bool *dirty) {
@@ skipping 358 matching lines @@
     // Check to see if the extension has been killed.
     int state;
     if (extension->GetInteger(kPrefState, &state) &&
         state == static_cast<int>(Extension::KILLBIT)) {
       killed_ids->insert(StringToLowerASCII(key_name));
     }
   }
 }
 
 std::vector<std::string> ExtensionPrefs::GetToolbarOrder() {
-  std::vector<std::string> extension_ids;
+  ExtensionPrefs::ExtensionIdSet extension_ids;
   const ListValue* toolbar_order = prefs_->GetList(kExtensionToolbar);
   if (toolbar_order) {
     for (size_t i = 0; i < toolbar_order->GetSize(); ++i) {
       std::string extension_id;
       if (toolbar_order->GetString(i, &extension_id))
         extension_ids.push_back(extension_id);
     }
   }
   return extension_ids;
 }
 
 void ExtensionPrefs::SetToolbarOrder(
     const std::vector<std::string>& extension_ids) {
   ListValue* toolbar_order = prefs_->GetMutableList(kExtensionToolbar);
   toolbar_order->Clear();
   for (std::vector<std::string>::const_iterator iter = extension_ids.begin();
        iter != extension_ids.end(); ++iter) {
     toolbar_order->Append(new StringValue(*iter));
   }
   SavePrefsAndNotify();
 }
 
 void ExtensionPrefs::OnExtensionInstalled(
     const Extension* extension, Extension::State initial_state,
     bool initial_incognito_enabled) {
   const std::string& id = extension->id();
+  const base::Time installTime = GetCurrentTime();
   UpdateExtensionPref(id, kPrefState,
                       Value::CreateIntegerValue(initial_state));
   UpdateExtensionPref(id, kPrefIncognitoEnabled,
                       Value::CreateBooleanValue(initial_incognito_enabled));
   UpdateExtensionPref(id, kPrefLocation,
                       Value::CreateIntegerValue(extension->location()));
+  UpdateExtensionPref(id, kPrefInstallTime,
+                      Value::CreateStringValue(
+                          base::Int64ToString(installTime.ToInternalValue())));
+  UpdateExtensionPref(id, kPrefPreferences, new DictionaryValue());

[Aaron Boodman, 2010/11/23 20:35:13]

Nit: since you know that this will be removed if empty when serialized, and you
create it lazily in a different place, there is no need for this line.

[battre (please use the other), 2010/11/30 17:46:53]

That is true, but on the other hand, I like having one place that shows me which
preferences to expect. I think it is unfortunate that we need the lazy creation
but having this here, makes stuff cleaner.

+
   FilePath::StringType path = MakePathRelative(install_directory_,
       extension->path(), NULL);
   UpdateExtensionPref(id, kPrefPath, Value::CreateStringValue(path));
   // We store prefs about LOAD extensions, but don't cache their manifest
   // since it may change on disk.
   if (extension->location() != Extension::LOAD) {
     UpdateExtensionPref(id, kPrefManifest,
                         extension->manifest_value()->DeepCopy());
   }
   UpdateExtensionPref(id, kPrefAppLaunchIndex,
                       Value::CreateIntegerValue(GetNextAppLaunchIndex()));
   SavePrefsAndNotify();
 }
 
 void ExtensionPrefs::OnExtensionUninstalled(const std::string& extension_id,
                                             const Extension::Location& location,
                                             bool external_uninstall) {
+  PrefKeySet prefKeys;
+  GetExtensionControlledPrefKeys(extension_id, &prefKeys);
+
   // For external extensions, we save a preference reminding ourself not to try
   // and install the extension anymore (except when |external_uninstall| is
   // true, which signifies that the registry key was deleted or the pref file
   // no longer lists the extension).
   if (!external_uninstall && Extension::IsExternalLocation(location)) {
     UpdateExtensionPref(extension_id, kPrefState,
                         Value::CreateIntegerValue(Extension::KILLBIT));
     SavePrefsAndNotify();
   } else {
     DeleteExtensionPrefs(extension_id);
   }
+
+  for (PrefKeySet::iterator i = prefKeys.begin(); i != prefKeys.end(); ++i)
+    UpdateWinningPref(*i);
 }
 
 Extension::State ExtensionPrefs::GetExtensionState(
-    const std::string& extension_id) {
+    const std::string& extension_id) const {
   DictionaryValue* extension = GetExtensionPref(extension_id);
 
   // If the extension doesn't have a pref, it's a --load-extension.
   if (!extension)
     return Extension::ENABLED;
 
   int state = -1;
   if (!extension->GetInteger(kPrefState, &state) ||
       state < 0 || state >= Extension::NUM_STATES) {
     LOG(ERROR) << "Bad or missing pref 'state' for extension '"
                << extension_id << "'";
     return Extension::ENABLED;
   }
   return static_cast<Extension::State>(state);
 }
 
 void ExtensionPrefs::SetExtensionState(const Extension* extension,
                                        Extension::State state) {
   UpdateExtensionPref(extension->id(), kPrefState,
                       Value::CreateIntegerValue(state));
+
+  PrefKeySet prefKeys;
+  GetExtensionControlledPrefKeys(extension->id(), &prefKeys);
+  for (PrefKeySet::iterator i = prefKeys.begin(); i != prefKeys.end(); ++i)
+    UpdateWinningPref(*i);

[Aaron Boodman, 2010/11/23 20:35:13]

This pattern is repeated a few times. Factor out a UpdateExtensionPrefStore()
method?

[battre (please use the other), 2010/11/30 17:46:53]

I think the pattern is repeated only twice (UpdateWinningPref is called only
three times, twice in such a pattern). The two occurrences of the pattern are
not identical however. In OnExtensionUninstalled we first store the prefKeys,
then remove ECPs and then do the update. The intermission makes a significant
difference.

+
   SavePrefsAndNotify();
 }
 
 bool ExtensionPrefs::GetBrowserActionVisibility(const Extension* extension) {
   DictionaryValue* extension_prefs = GetExtensionPref(extension->id());
   bool visible = false;
   if (!extension_prefs->GetBoolean(kBrowserActionVisible, &visible) || visible)
     return true;
 
   return false;
@@ skipping 83 matching lines @@
 DictionaryValue* ExtensionPrefs::GetExtensionPref(
     const std::string& extension_id) const {
   const DictionaryValue* dict = prefs_->GetDictionary(kExtensionsPref);
   if (!dict)
     return NULL;
   DictionaryValue* extension = NULL;
   dict->GetDictionary(extension_id, &extension);
   return extension;
 }
 
+DictionaryValue* ExtensionPrefs::GetExtensionControlledPrefs(
+    const std::string& extension_id) const {
+  DictionaryValue* extension = GetExtensionPref(extension_id);
+  if (!extension)
+    return NULL;
+  DictionaryValue* preferences = NULL;
+  extension->GetDictionary(kPrefPreferences, &preferences);
+  return preferences;
+}
+
 // Helper function for GetInstalledExtensionsInfo.
 static ExtensionInfo* GetInstalledExtensionInfoImpl(
     DictionaryValue* extension_data,
     DictionaryValue::key_iterator extension_id) {
   DictionaryValue* ext;
   if (!extension_data->GetDictionaryWithoutPathExpansion(*extension_id, &ext)) {
     LOG(WARNING) << "Invalid pref for extension " << *extension_id;
     NOTREACHED();
     return NULL;
   }
@@ skipping 239 matching lines @@
 std::string ExtensionPrefs::GetUpdateUrlData(const std::string& extension_id) {
   DictionaryValue* dictionary = GetExtensionPref(extension_id);
   if (!dictionary)
     return std::string();
 
   std::string data;
   dictionary->GetString(kUpdateUrlData, &data);
   return data;
 }
 
+base::Time ExtensionPrefs::GetCurrentTime() const {
+  return base::Time::Now();
+}
+
+base::Time ExtensionPrefs::GetInstallTime(const std::string& extension_id)
+    const {

[Aaron Boodman, 2010/11/23 20:35:13]

Is the const indented on its own correct style? It looks weird to me. I think
maybe the new line should begin with const std::string...

[battre (please use the other), 2010/11/30 17:46:53]

Done.

+  const DictionaryValue* extension = GetExtensionPref(extension_id);
+  if (!extension)

[Aaron Boodman, 2010/11/23 20:35:13]

Do we intend for this to happen? If not, we should at least LOG that it was
unexpected.

[battre (please use the other), 2010/11/30 17:46:53]

Done. Added comment: // If the extension doesn't have a pref, it's a
--load-extension.

+    return base::Time::Time();
+  std::string install_time_str("0");
+  extension->GetString(kPrefInstallTime, &install_time_str);
+  int64 install_time_i64 = 0;
+  base::StringToInt64(install_time_str, &install_time_i64);
+  LOG_IF(ERROR, install_time_i64 == 0)
+      << "Error parsing installation time of an extension.";
+  return base::Time::FromInternalValue(install_time_i64);
+}
+
+void ExtensionPrefs::GetEnabledExtensions(ExtensionIdSet* out) const {
+  DCHECK(out);
+  const DictionaryValue* extensions =
+      pref_service()->GetDictionary(kExtensionsPref);
+
+  for (DictionaryValue::key_iterator ext_id = extensions->begin_keys();
+         ext_id != extensions->end_keys(); ++ext_id) {
+      if (GetExtensionState(*ext_id) != Extension::ENABLED)
+        continue;
+      out->push_back(*ext_id);
+  }
+}
+
+// Comparator that sorts extensions by their increasing installation time
+// or uses the extension id as a fall back for equal installation times.
+struct InstallTimeComparator {
+  explicit InstallTimeComparator(ExtensionPrefs* ext_prefs)
+      : ext_prefs_(ext_prefs) {}
+
+  bool operator()(std::string extid_a, std::string extid_b) {
+    base::Time install_time_a = ext_prefs_->GetInstallTime(extid_a);
+    base::Time install_time_b = ext_prefs_->GetInstallTime(extid_b);
+    if (install_time_a == install_time_b)
+      return extid_a < extid_b;
+    return install_time_a < install_time_b;
+  }
+
+  ExtensionPrefs* ext_prefs_;  // Weak pointer.
+};
+
+void ExtensionPrefs::FixMissingPrefs(const ExtensionIdSet& extension_ids) {
+  // Fix old entries that did not get an installation time entry when they
+  // were installed or don't have a preferences field.
+  bool persistRequired = false;
+  for (ExtensionIdSet::const_iterator ext_id = extension_ids.begin();
+      ext_id != extension_ids.end(); ++ext_id) {
+    DictionaryValue* extension = GetExtensionPref(*ext_id);
+    CHECK(extension != NULL);
+
+    if (GetInstallTime(*ext_id) == base::Time::Time()) {
+      const base::Time installTime = GetCurrentTime();
+      extension->Set(kPrefInstallTime,
+                     Value::CreateStringValue(
+                         base::Int64ToString(installTime.ToInternalValue())));
+      persistRequired = true;
+    }
+  }
+  if (persistRequired) {
+    SavePrefsAndNotify();
+  }
+}
+
+void ExtensionPrefs::InstallPersistedExtensionControlledPrefs() {
+  // When this is called, the PrefService is initialized and provides access
+  // to the user preferences stored in a JSON file. We take the persisted
+  // preferences of all extensions, calculate the effective preferences
+  // (considering that one extension overrides preferences of other extensions)
+  // and store the effective preferences in the extension pref store.
+  ExtensionIdSet extension_ids;
+  GetEnabledExtensions(&extension_ids);
+  FixMissingPrefs(extension_ids);
+
+  // Sort such that latest installed extension appears last.
+  std::sort(extension_ids.begin(), extension_ids.end(),
+            InstallTimeComparator(this));
+
+  // Collect all effective preferences (later ones override newer ones).
+  DictionaryValue merged_non_expanded;
+  for (ExtensionIdSet::iterator ext_id = extension_ids.begin();
+      ext_id != extension_ids.end(); ++ext_id) {
+    if (DictionaryValue* preferences = GetExtensionControlledPrefs(*ext_id))
+      merged_non_expanded.MergeDictionary(preferences);
+  }
+
+  // Expand all keys.
+  PrefStore* extension_prefs =
+      pref_service()->pref_value_store()->GetExtensionPrefStore();
+  for (DictionaryValue::key_iterator prefkey = merged_non_expanded.begin_keys();
+       prefkey != merged_non_expanded.end_keys();
+       ++prefkey) {
+    Value* value;
+    CHECK(merged_non_expanded.GetWithoutPathExpansion(*prefkey, &value));
+    extension_prefs->prefs()->Set(*prefkey, value->DeepCopy());
+  }
+}
+
+const Value* ExtensionPrefs::WinningExtensionControlledPrefValue(
+    const std::string& key) const {
+  Value *winner = NULL;
+  base::Time winners_install_time = base::Time::Time();
+
+  ExtensionIdSet extension_ids;
+  GetEnabledExtensions(&extension_ids);
+  for (ExtensionIdSet::iterator ext_id = extension_ids.begin();
+       ext_id != extension_ids.end(); ++ext_id) {
+    base::Time extension_install_time = GetInstallTime(*ext_id);
+
+    // We do not need to consider extensions that were installed before the
+    // most recent extension found that provides the requested preference.
+    if (extension_install_time < winners_install_time)
+      continue;
+
+    DictionaryValue* preferences = GetExtensionControlledPrefs(*ext_id);
+    Value *value = NULL;
+    if (preferences && preferences->GetWithoutPathExpansion(key, &value)) {
+      // This extension is more recent than the last one providing this pref.
+      winner = value;
+      winners_install_time = extension_install_time;
+    }
+  }
+
+  return winner;
+}
+
+void ExtensionPrefs::UpdateWinningPref(const std::string& pref_key) {

[Aaron Boodman, 2010/11/23 20:35:13]

Trying to update the individual preference that has changed seems to cause lots
of complexity. Can we just rebuild the entire tree of extension-controlled
preferences and pass it back to PrefValueStore?

[battre (please use the other), 2010/11/30 17:46:53]

We could do this and DictionaryValue::GetDifferingPaths would nicely allow to
send notifications of changed preferences. However, bug 64232 (get rid of
prefs() function in PrefStore interface) will create problems. Therefore, I have
removed the code duplication by making
ExtensionPrefs::InstallPersistedExtensionControlledPrefs use UpdateWinningPref
as well.

Done.

+  PrefStore* extensionPrefStore =

[Aaron Boodman, 2010/11/23 20:35:13]

Here, and many places in this file: Chromium uses under_scores, not camelCase,
for local variables.

[battre (please use the other), 2010/11/30 17:46:53]

Done.

+      pref_service()->pref_value_store()->GetExtensionPrefStore();
+  const Value* winningPrefValue = WinningExtensionControlledPrefValue(pref_key);
+  Value* oldValue = NULL;
+  extensionPrefStore->prefs()->Get(pref_key, &oldValue);
+  bool changed = !Value::Equals(winningPrefValue, oldValue);
+
+  if (winningPrefValue)
+    extensionPrefStore->prefs()->Set(pref_key, winningPrefValue->DeepCopy());
+  else
+    extensionPrefStore->prefs()->Remove(pref_key, NULL);
+
+  if (changed)
+    pref_service()->pref_notifier()->OnPreferenceSet(
+        pref_key.c_str(), PrefNotifier::EXTENSION_STORE);
+}
+
+void ExtensionPrefs::SetExtensionControlledPref(const std::string& extension_id,
+                                                const std::string& pref_key,
+                                                Value* value) {
+  DictionaryValue* extensionPreferences =
+      GetExtensionControlledPrefs(extension_id);
+
+  if (extensionPreferences == NULL) {  // May be pruned when writing to disk.
+    DictionaryValue* extension = GetExtensionPref(extension_id);
+    extensionPreferences = new DictionaryValue;
+    extension->Set(kPrefPreferences, extensionPreferences);
+  }
+
+  Value* oldValue = NULL;
+  extensionPreferences->GetWithoutPathExpansion(pref_key, &oldValue);
+  bool modified = !Value::Equals(oldValue, value);
+  if (!modified)
+    return;
+
+  if (value == NULL)
+    extensionPreferences->RemoveWithoutPathExpansion(pref_key, NULL);
+  else
+    extensionPreferences->SetWithoutPathExpansion(pref_key, value);
+  pref_service()->ScheduleSavePersistentPrefs();
+
+  UpdateWinningPref(pref_key);
+}
+
+void ExtensionPrefs::GetExtensionControlledPrefKeys(
+    const std::string& extension_id, PrefKeySet *out) const {
+  DCHECK(out != NULL);
+  DictionaryValue* extPrefs = GetExtensionControlledPrefs(extension_id);
+  if (extPrefs) {
+    for (DictionaryValue::key_iterator i = extPrefs->begin_keys();
+         i != extPrefs->end_keys(); ++i) {
+      out->push_back(*i);
+    }
+  }
+}
+
 // static
 void ExtensionPrefs::RegisterUserPrefs(PrefService* prefs) {
   prefs->RegisterDictionaryPref(kExtensionsPref);
   prefs->RegisterListPref(kExtensionToolbar);
   prefs->RegisterIntegerPref(prefs::kExtensionToolbarSize, -1);
   prefs->RegisterDictionaryPref(kExtensionsBlacklistUpdate);
   prefs->RegisterListPref(prefs::kExtensionInstallAllowList);
   prefs->RegisterListPref(prefs::kExtensionInstallDenyList);
   prefs->RegisterListPref(prefs::kExtensionInstallForceList);
   prefs->RegisterStringPref(kWebStoreLogin, std::string() /* default_value */);
 }