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/profile.h"
 #include "chrome/common/extensions/extension.h"
 #include "chrome/common/extensions/url_pattern.h"
 #include "chrome/common/notification_service.h"
 #include "chrome/common/pref_names.h"
 
 using base::Time;
 
 namespace {
 
 // Additional preferences keys
@@ skipping 64 matching lines @@
 const char kBrowserActionVisible[] = "browser_action_visible";
 
 // Preferences that hold which permissions the user has granted the extension.
 // We explicitly keep track of these so that extensions can contain unknown
 // permissions, for backwards compatibility reasons, and we can still prompt
 // the user to accept them once recognized.
 const char kPrefGrantedPermissionsAPI[] = "granted_permissions.api";
 const char kPrefGrantedPermissionsHost[] = "granted_permissions.host";
 const char kPrefGrantedPermissionsAll[] = "granted_permissions.full";
 
+// 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";
+
 }  // 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 22 matching lines @@
                               std::set<std::string>* result) {
   ExtensionExtent::PatternList patterns = host_extent.patterns();
   ExtensionExtent::PatternList::const_iterator i;
 
   for (i = patterns.begin(); i != patterns.end(); ++i)
     result->insert(i->GetAsString());
 }
 
 }  // namespace
 
-ExtensionPrefs::ExtensionPrefs(PrefService* prefs, const FilePath& root_dir)
-    : prefs_(prefs),
+ExtensionPrefs::ExtensionPrefs(Profile* profile,
+                               PrefService* prefs,
+                               const FilePath& root_dir)
+    : profile_(profile),
+      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 481 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 install_time = 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(install_time.ToInternalValue())));
+  UpdateExtensionPref(id, kPrefPreferences, new DictionaryValue());
+
   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 pref_keys;
+  GetExtensionControlledPrefKeys(extension_id, &pref_keys);
+
   // 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 = pref_keys.begin(); i != pref_keys.end(); ++i)
+    UpdateWinningPref(*i, false);

[Mattias Nissler (ping if slow), 2010/12/01 10:36:36]

I believe Aaron already asked to factor this loop into its own function, since
you have it 3 times in the code. Any reason not to do so?

[battre (please use the other), 2010/12/01 17:44:38]

Done.

 }
 
 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 pref_keys;
+  GetExtensionControlledPrefKeys(extension->id(), &pref_keys);
+  for (PrefKeySet::iterator i = pref_keys.begin(); i != pref_keys.end(); ++i)
+    UpdateWinningPref(*i, false);
+
   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 the extension doesn't have a pref, it's a --load-extension.
+  if (!extension)
+    return NULL;

[Mattias Nissler (ping if slow), 2010/12/01 10:36:36]

Add a newline here and above before the comment? Jochen wouldn't like it, but I
think the rest of this file has these newlines, so we should be consistent.

[battre (please use the other), 2010/12/01 17:44:38]

Done.

+  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 {
+  const DictionaryValue* extension = GetExtensionPref(extension_id);
+  // If the extension doesn't have a pref, it's a --load-extension.
+  if (!extension)
+    return base::Time::Time();

[Mattias Nissler (ping if slow), 2010/12/01 10:36:36]

Some whitespace here, too?

[battre (please use the other), 2010/12/01 17:44:38]

Done.

+  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);
+  }
+}
+
+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 persist_required = false;
+  for (ExtensionIdSet::const_iterator ext_id = extension_ids.begin();
+       ext_id != extension_ids.end(); ++ext_id) {
+    DictionaryValue* extension = GetExtensionPref(*ext_id);
+    // If the extension doesn't have a pref, it's a --load-extension.
+    if (extension == NULL)
+      continue;
+
+    if (GetInstallTime(*ext_id) == base::Time::Time()) {
+      const base::Time install_time = GetCurrentTime();
+      extension->Set(kPrefInstallTime,
+                     Value::CreateStringValue(
+                         base::Int64ToString(install_time.ToInternalValue())));
+      persist_required = true;
+    }
+  }
+  if (persist_required)
+    SavePrefsAndNotify();
+}
+
+void ExtensionPrefs::InstallPersistedExtensionControlledPrefs() {
+  // When this is called, the PrefService is initialized and provides access
+  // to the user preferences stored in a JSON file.
+  ExtensionIdSet extension_ids;
+  GetEnabledExtensions(&extension_ids);
+  FixMissingPrefs(extension_ids);
+
+  // Collect the unique extension controlled preference keys of all extensions.
+  std::set<std::string> ext_controlled_prefs;

[Mattias Nissler (ping if slow), 2010/12/01 10:36:36]

Isn't this a PrefKeySet?

[battre (please use the other), 2010/12/01 17:44:38]

No, we need to filter for *unique* entries. The PrefKeySet is a vector.

I have changed this to collect everything in a PrefKeySet, then sort and
unique-ify it. Then we have three invocations of the same loop of
UpdateWinningPref(...) calls, which I have factored out into a function.

Done.

+  for (ExtensionIdSet::iterator ext_id = extension_ids.begin();
+       ext_id != extension_ids.end(); ++ext_id) {
+    DictionaryValue* preferences = GetExtensionControlledPrefs(*ext_id);

[Mattias Nissler (ping if slow), 2010/12/01 10:36:36]

Why not just call GetExtensionControlledPrefKeys(&ext_controlled_prefs) here?

[battre (please use the other), 2010/12/01 17:44:38]

See above. Done.

+    if (!preferences)
+      continue;
+    for (DictionaryValue::key_iterator prefkey = preferences->begin_keys();
+         prefkey != preferences->end_keys(); ++prefkey) {
+      ext_controlled_prefs.insert(*prefkey);
+    }
+  }
+
+  // Store winning preference for each extension controlled preference.
+  // Note that we need to suppress notifications here as the preferences
+  // have not been registered with the PrefService, when this function
+  // is called.

[Mattias Nissler (ping if slow), 2010/12/01 10:36:36]

I don't understand why it would be necessary to suppress the notifications then?

[battre (please use the other), 2010/12/01 17:44:38]

The notification mechanism checked that the value for which we send
notifications has a governor. Usually the default pref store becomes the
governor for preferences when they are registered. However, this has not
happened at this time. Therefore, everything we insert here, has not default
preference, yet. Therefore, the notification triggered a DCHECK violation.

After discussion with mnissler, I have changed PrefValueStore to ignore
notifications of preferences that have not been registered, yet. Now
ExtensionPrefs verifies in SetExtensionControlledPref that preferences have been
registered before they are set.

Done.

+  for (std::set<std::string>::iterator i = ext_controlled_prefs.begin();
+       i != ext_controlled_prefs.end(); ++i) {
+    UpdateWinningPref(*i, true);
+  }
+}
+
+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,
+                                       bool suppressNotification) {
+  PrefStore* extension_pref_store = profile_->GetExtensionPrefStore();
+  const Value* winning_pref_value =
+      WinningExtensionControlledPrefValue(pref_key);
+  Value* old_value = NULL;
+  extension_pref_store->prefs()->Get(pref_key, &old_value);
+  bool changed = !Value::Equals(winning_pref_value, old_value);
+
+  if (winning_pref_value)
+    extension_pref_store->prefs()->Set(pref_key,
+                                       winning_pref_value->DeepCopy());

[Mattias Nissler (ping if slow), 2010/12/01 10:36:36]

Need curlies here.

[battre (please use the other), 2010/12/01 17:44:38]

Done.

+  else
+    extension_pref_store->prefs()->Remove(pref_key, NULL);
+
+  if (changed && !suppressNotification)

[Mattias Nissler (ping if slow), 2010/12/01 10:36:36]

FYI to other reviewers: Note that there is work underway that will change the
PrefStore interface to not allow to write to prefs(). This means that the whole
business of updating the value in ExtensionPrefStore and sending out
notifications will move to there.

+    pref_service()->pref_notifier()->OnPreferenceSet(
+        pref_key.c_str(), PrefNotifier::EXTENSION_STORE);

[Mattias Nissler (ping if slow), 2010/12/01 10:36:36]

curlies here as well.

[battre (please use the other), 2010/12/01 17:44:38]

Done.

+}
+
+void ExtensionPrefs::SetExtensionControlledPref(const std::string& extension_id,
+                                                const std::string& pref_key,
+                                                Value* value) {
+  DictionaryValue* extension_preferences =
+      GetExtensionControlledPrefs(extension_id);
+
+  if (extension_preferences == NULL) {  // May be pruned when writing to disk.
+    DictionaryValue* extension = GetExtensionPref(extension_id);
+    CHECK(extension);
+    // TODO(battre) what do we do with --load-extension extensions?
+    // extension might be null.

[Mattias Nissler (ping if slow), 2010/12/01 10:36:36]

They just can't override prefs? Or should we create the entry? If we had
separate pref paths for regular prefs and the override cache, we wouldn't run
into this problem, right? I don't want to start this discussion again though.
Aaron?

However, the CHECK() is the wrong answer, since that will make us crash
reproducibly. So please at least turn it into a LOG(WARNING) + return

[battre (please use the other), 2010/12/01 17:44:38]

Done.

+    extension_preferences = new DictionaryValue;
+    extension->Set(kPrefPreferences, extension_preferences);
+  }
+
+  Value* oldValue = NULL;
+  extension_preferences->GetWithoutPathExpansion(pref_key, &oldValue);
+  bool modified = !Value::Equals(oldValue, value);
+  if (!modified)
+    return;
+
+  if (value == NULL)
+    extension_preferences->RemoveWithoutPathExpansion(pref_key, NULL);
+  else
+    extension_preferences->SetWithoutPathExpansion(pref_key, value);
+  pref_service()->ScheduleSavePersistentPrefs();
+
+  UpdateWinningPref(pref_key, false);
+}
+
+void ExtensionPrefs::GetExtensionControlledPrefKeys(
+    const std::string& extension_id, PrefKeySet *out) const {
+  DCHECK(out != NULL);
+  DictionaryValue* ext_prefs = GetExtensionControlledPrefs(extension_id);
+  if (ext_prefs) {
+    for (DictionaryValue::key_iterator i = ext_prefs->begin_keys();
+         i != ext_prefs->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 */);
 }