Get rid of PrefService::GetMutableDictionary/GetMutableList

BUG=77914
TEST=none, trybots remain green

Review URL: http://codereview.chromium.org/6706017

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@81075 0039d316-1c4b-4281-b951-d872f2087c98

6 files changed, 259 insertions, 167 deletions

diff --git a/chrome/browser/extensions/extension_management_browsertest.cc b/chrome/browser/extensions/extension_management_browsertest.cc
index 8666631..2bf05ba 100644
--- a/chrome/browser/extensions/extension_management_browsertest.cc
+++ b/chrome/browser/extensions/extension_management_browsertest.cc
@@ -423,9 +423,8 @@ IN_PROC_BROWSER_TEST_F(ExtensionManagementTest, ExternalPolicyRefresh) {
   PrefService* prefs = browser()->profile()->GetPrefs();
   {
     // Set the policy as a user preference and fire notification observers.
-    ScopedUserPrefUpdate pref_update(prefs, prefs::kExtensionInstallForceList);
-    ListValue* forcelist =
-        prefs->GetMutableList(prefs::kExtensionInstallForceList);
+    ListPrefUpdate pref_update(prefs, prefs::kExtensionInstallForceList);
+    ListValue* forcelist = pref_update.Get();
     ASSERT_TRUE(forcelist->empty());
     forcelist->Append(Value::CreateStringValue(
         std::string(kExtensionId) +
diff --git a/chrome/browser/extensions/extension_override_apitest.cc b/chrome/browser/extensions/extension_override_apitest.cc
index 1c419ec..9c87a7c 100644
--- a/chrome/browser/extensions/extension_override_apitest.cc
+++ b/chrome/browser/extensions/extension_override_apitest.cc
@@ -6,6 +6,7 @@
 #include "chrome/browser/extensions/extension_service.h"
 #include "chrome/browser/extensions/extension_web_ui.h"
 #include "chrome/browser/prefs/pref_service.h"
+#include "chrome/browser/prefs/scoped_user_pref_update.h"
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/ui/browser.h"
 #include "chrome/browser/ui/browser_list.h"
@@ -137,8 +138,11 @@ IN_PROC_BROWSER_TEST_F(ExtensionOverrideTest, ShouldCleanUpDuplicateEntries) {
   for (size_t i = 0; i < 3; ++i)
     list->Append(Value::CreateStringValue("http://www.google.com/"));
 
-  browser()->profile()->GetPrefs()->GetMutableDictionary(
-      ExtensionWebUI::kExtensionURLOverrides)->Set("history", list);
+  {
+    DictionaryPrefUpdate update(browser()->profile()->GetPrefs(),
+                                ExtensionWebUI::kExtensionURLOverrides);
+    update.Get()->Set("history", list);
+  }
 
   ASSERT_FALSE(CheckHistoryOverridesContainsNoDupes());
 
diff --git a/chrome/browser/extensions/extension_prefs.cc b/chrome/browser/extensions/extension_prefs.cc
index 38b68bc..bf79d14 100644
--- a/chrome/browser/extensions/extension_prefs.cc
+++ b/chrome/browser/extensions/extension_prefs.cc
@@ -113,11 +113,55 @@ const char kPrefInstallTime[] = "install_time";
 // A preference that contains any extension-controlled preferences.
 const char kPrefPreferences[] = "preferences";
 
-}  // namespace
+// Provider of write access to a dictionary storing extension prefs.
+class ScopedExtensionPrefUpdate : public DictionaryPrefUpdate {
+ public:
+  ScopedExtensionPrefUpdate(PrefService* service,
+                            const std::string& extension_id) :
+    DictionaryPrefUpdate(service, ExtensionPrefs::kExtensionsPref),
+    extension_id_(extension_id) {}
+  virtual ~ScopedExtensionPrefUpdate() {}
+  virtual DictionaryValue* Get() {
+    DictionaryValue* dict = DictionaryPrefUpdate::Get();
+    DictionaryValue* extension = NULL;
+    if (!dict->GetDictionary(extension_id_, &extension)) {
+      // Extension pref does not exist, create it.
+      extension = new DictionaryValue();
+      dict->Set(extension_id_, extension);
+    }
+    return extension;
+  }
+
+ private:
+  const std::string extension_id_;
+
+  DISALLOW_COPY_AND_ASSIGN(ScopedExtensionPrefUpdate);
+};
+
+// Provider of write access to a dictionary storing extension controlled prefs.
+class ScopedExtensionControlledPrefUpdate : public DictionaryPrefUpdate {
+ public:
+  ScopedExtensionControlledPrefUpdate(PrefService* service,
+                                      const std::string& extension_id) :
+    DictionaryPrefUpdate(service, ExtensionPrefs::kExtensionsPref),
+    extension_id_(extension_id) {}
+  virtual ~ScopedExtensionControlledPrefUpdate() {}
+  virtual DictionaryValue* Get() {
+    DictionaryValue* dict = DictionaryPrefUpdate::Get();
+    DictionaryValue* preferences = NULL;
+    std::string key = extension_id_ + std::string(".") + kPrefPreferences;
+    if (!dict->GetDictionary(key, &preferences)) {
+      preferences = new DictionaryValue;
+      dict->Set(key, preferences);
+    }
+    return preferences;
+  }
 
-////////////////////////////////////////////////////////////////////////////////
+ private:
+  const std::string extension_id_;
 
-namespace {
+  DISALLOW_COPY_AND_ASSIGN(ScopedExtensionControlledPrefUpdate);
+};
 
 // TODO(mihaip): This is cleanup code for keys for unpacked extensions (which
 // are derived from paths). As part of the wstring removal, we changed the way
@@ -126,8 +170,8 @@ namespace {
 // for more details).
 static void CleanupBadExtensionKeys(const FilePath& root_dir,
                                     PrefService* prefs) {
-  DictionaryValue* dictionary =
-      prefs->GetMutableDictionary(ExtensionPrefs::kExtensionsPref);
+  const DictionaryValue* dictionary =
+      prefs->GetDictionary(ExtensionPrefs::kExtensionsPref);
   std::map<std::string, std::string> remapped_keys;
   for (DictionaryValue::key_iterator i = dictionary->begin_keys();
        i != dictionary->end_keys(); ++i) {
@@ -162,19 +206,21 @@ static void CleanupBadExtensionKeys(const FilePath& root_dir,
   }
 
   if (!remapped_keys.empty()) {
+    DictionaryPrefUpdate update(prefs, ExtensionPrefs::kExtensionsPref);
+    DictionaryValue* update_dictionary = update.Get();
     for (std::map<std::string, std::string>::const_iterator i =
             remapped_keys.begin();
         i != remapped_keys.end();
         ++i) {
       // Don't clobber prefs under the correct ID if they already exist.
-      if (dictionary->HasKey(i->second)) {
-        CHECK(dictionary->RemoveWithoutPathExpansion(i->first, NULL));
+      if (update_dictionary->HasKey(i->second)) {
+        CHECK(update_dictionary->RemoveWithoutPathExpansion(i->first, NULL));
         continue;
       }
       Value* extension_prefs = NULL;
-      CHECK(dictionary->RemoveWithoutPathExpansion(
+      CHECK(update_dictionary->RemoveWithoutPathExpansion(
           i->first, &extension_prefs));
-      dictionary->SetWithoutPathExpansion(i->second, extension_prefs);
+      update_dictionary->SetWithoutPathExpansion(i->second, extension_prefs);
     }
 
     prefs->ScheduleSavePersistentPrefs();
@@ -213,13 +259,10 @@ ExtensionPrefs::~ExtensionPrefs() {}
 const char ExtensionPrefs::kExtensionsPref[] = "extensions.settings";
 
 static FilePath::StringType MakePathRelative(const FilePath& parent,
-                                             const FilePath& child,
-                                             bool *dirty) {
+                                             const FilePath& child) {
   if (!parent.IsParent(child))
     return child.value();
 
-  if (dirty)
-    *dirty = true;
   FilePath::StringType retval = child.value().substr(
       parent.value().length());
   if (FilePath::IsSeparator(retval[0]))
@@ -229,14 +272,15 @@ static FilePath::StringType MakePathRelative(const FilePath& parent,
 }
 
 void ExtensionPrefs::MakePathsRelative() {
-  bool dirty = false;
-  const DictionaryValue* dict = prefs_->GetMutableDictionary(kExtensionsPref);
+  const DictionaryValue* dict = prefs_->GetDictionary(kExtensionsPref);
   if (!dict || dict->empty())
     return;
 
+  // Collect all extensions ids with absolute paths in |absolute_keys|.
+  std::set<std::string> absolute_keys;
   for (DictionaryValue::key_iterator i = dict->begin_keys();
        i != dict->end_keys(); ++i) {
-    DictionaryValue* extension_dict;
+    DictionaryValue* extension_dict = NULL;
     if (!dict->GetDictionaryWithoutPathExpansion(*i, &extension_dict))
       continue;
     int location_value;
@@ -249,13 +293,26 @@ void ExtensionPrefs::MakePathsRelative() {
     if (!extension_dict->GetString(kPrefPath, &path_string))
       continue;
     FilePath path(path_string);
-    if (path.IsAbsolute()) {
-      extension_dict->SetString(kPrefPath,
-          MakePathRelative(install_directory_, path, &dirty));
-    }
+    if (path.IsAbsolute())
+      absolute_keys.insert(*i);
   }
-  if (dirty)
-    SavePrefsAndNotify();
+  if (absolute_keys.empty())
+    return;
+
+  // Fix these paths.
+  DictionaryPrefUpdate update(prefs_, kExtensionsPref);
+  const DictionaryValue* update_dict = update.Get();
+  for (std::set<std::string>::iterator i = absolute_keys.begin();
+       i != absolute_keys.end(); ++i) {
+    DictionaryValue* extension_dict = NULL;
+    update_dict->GetDictionaryWithoutPathExpansion(*i, &extension_dict);
+    FilePath::StringType path_string;
+    extension_dict->GetString(kPrefPath, &path_string);
+    FilePath path(path_string);
+    extension_dict->SetString(kPrefPath,
+        MakePathRelative(install_directory_, path));
+  }
+  SavePrefs();
 }
 
 void ExtensionPrefs::MakePathsAbsolute(DictionaryValue* dict) {
@@ -264,7 +321,7 @@ void ExtensionPrefs::MakePathsAbsolute(DictionaryValue* dict) {
 
   for (DictionaryValue::key_iterator i = dict->begin_keys();
        i != dict->end_keys(); ++i) {
-    DictionaryValue* extension_dict;
+    DictionaryValue* extension_dict = NULL;
     if (!dict->GetDictionaryWithoutPathExpansion(*i, &extension_dict)) {
       NOTREACHED();
       continue;
@@ -298,7 +355,7 @@ DictionaryValue* ExtensionPrefs::CopyCurrentExtensions() {
 }
 
 bool ExtensionPrefs::ReadBooleanFromPref(
-    DictionaryValue* ext, const std::string& pref_key) {
+    const DictionaryValue* ext, const std::string& pref_key) {
   bool bool_value = false;
   if (!ext->GetBoolean(pref_key, &bool_value))
     return false;
@@ -308,7 +365,7 @@ bool ExtensionPrefs::ReadBooleanFromPref(
 
 bool ExtensionPrefs::ReadExtensionPrefBoolean(
     const std::string& extension_id, const std::string& pref_key) {
-  DictionaryValue* ext = GetExtensionPref(extension_id);
+  const DictionaryValue* ext = GetExtensionPref(extension_id);
   if (!ext) {
     // No such extension yet.
     return false;
@@ -317,7 +374,7 @@ bool ExtensionPrefs::ReadExtensionPrefBoolean(
 }
 
 bool ExtensionPrefs::ReadIntegerFromPref(
-    DictionaryValue* ext, const std::string& pref_key, int* out_value) {
+    const DictionaryValue* ext, const std::string& pref_key, int* out_value) {
   if (!ext->GetInteger(pref_key, out_value))
     return false;
 
@@ -327,7 +384,7 @@ bool ExtensionPrefs::ReadIntegerFromPref(
 bool ExtensionPrefs::ReadExtensionPrefInteger(
     const std::string& extension_id, const std::string& pref_key,
     int* out_value) {
-  DictionaryValue* ext = GetExtensionPref(extension_id);
+  const DictionaryValue* ext = GetExtensionPref(extension_id);
   if (!ext) {
     // No such extension yet.
     return false;
@@ -337,10 +394,12 @@ bool ExtensionPrefs::ReadExtensionPrefInteger(
 
 bool ExtensionPrefs::ReadExtensionPrefList(
     const std::string& extension_id, const std::string& pref_key,
-    ListValue** out_value) {
-  DictionaryValue* ext = GetExtensionPref(extension_id);
-  if (!ext || !ext->GetList(pref_key, out_value))
+    const ListValue** out_value) {
+  const DictionaryValue* ext = GetExtensionPref(extension_id);
+  ListValue* out = NULL;
+  if (!ext || !ext->GetList(pref_key, &out))
     return false;
+  *out_value = out;
 
   return out_value != NULL;
 }
@@ -349,7 +408,7 @@ bool ExtensionPrefs::ReadExtensionPrefStringSet(
     const std::string& extension_id,
     const std::string& pref_key,
     std::set<std::string>* result) {
-  ListValue* value = NULL;
+  const ListValue* value = NULL;
   if (!ReadExtensionPrefList(extension_id, pref_key, &value))
     return false;
 
@@ -386,9 +445,8 @@ void ExtensionPrefs::AddToExtensionPrefStringSet(
   prefs_->ScheduleSavePersistentPrefs();
 }
 
-void ExtensionPrefs::SavePrefsAndNotify() {
+void ExtensionPrefs::SavePrefs() {
   prefs_->ScheduleSavePersistentPrefs();
-  ScopedUserPrefUpdate update(prefs_, kExtensionsPref);
 }
 
 bool ExtensionPrefs::IsBlacklistBitSet(DictionaryValue* ext) {
@@ -503,7 +561,7 @@ void ExtensionPrefs::UpdateBlacklist(
   for (unsigned int i = 0; i < remove_pref_ids.size(); ++i) {
     DeleteExtensionPrefs(remove_pref_ids[i]);
   }
-  SavePrefsAndNotify();
+  SavePrefs();
   return;
 }
 
@@ -542,7 +600,8 @@ Time ExtensionPrefs::LastPingDay(const std::string& extension_id) const {
 void ExtensionPrefs::SetLastPingDay(const std::string& extension_id,
                                     const Time& time) {
   DCHECK(Extension::IdIsValid(extension_id));
-  SaveTime(GetExtensionPref(extension_id), kLastPingDay, time);
+  ScopedExtensionPrefUpdate update(prefs_, extension_id);
+  SaveTime(update.Get(), kLastPingDay, time);
 }
 
 Time ExtensionPrefs::BlacklistLastPingDay() const {
@@ -551,9 +610,8 @@ Time ExtensionPrefs::BlacklistLastPingDay() const {
 }
 
 void ExtensionPrefs::SetBlacklistLastPingDay(const Time& time) {
-  SaveTime(prefs_->GetMutableDictionary(kExtensionsBlacklistUpdate),
-           kLastPingDay,
-           time);
+  DictionaryPrefUpdate update(prefs_, kExtensionsBlacklistUpdate);
+  SaveTime(update.Get(), kLastPingDay, time);
 }
 
 Time ExtensionPrefs::LastActivePingDay(const std::string& extension_id) {
@@ -564,11 +622,12 @@ Time ExtensionPrefs::LastActivePingDay(const std::string& extension_id) {
 void ExtensionPrefs::SetLastActivePingDay(const std::string& extension_id,
                                           const base::Time& time) {
   DCHECK(Extension::IdIsValid(extension_id));
-  SaveTime(GetExtensionPref(extension_id), kLastActivePingDay, time);
+  ScopedExtensionPrefUpdate update(prefs_, extension_id);
+  SaveTime(update.Get(), kLastActivePingDay, time);
 }
 
 bool ExtensionPrefs::GetActiveBit(const std::string& extension_id) {
-  DictionaryValue* dictionary = GetExtensionPref(extension_id);
+  const DictionaryValue* dictionary = GetExtensionPref(extension_id);
   bool result = false;
   if (dictionary && dictionary->GetBoolean(kActiveBit, &result))
     return result;
@@ -577,10 +636,8 @@ bool ExtensionPrefs::GetActiveBit(const std::string& extension_id) {
 
 void ExtensionPrefs::SetActiveBit(const std::string& extension_id,
                                   bool active) {
-  DictionaryValue* dictionary = GetExtensionPref(extension_id);
-  if (!dictionary)
-    return;
-  dictionary->SetBoolean(kActiveBit, active);
+  ScopedExtensionPrefUpdate update(prefs_, extension_id);
+  update.Get()->SetBoolean(kActiveBit, active);
 }
 
 bool ExtensionPrefs::GetGrantedPermissions(
@@ -590,7 +647,7 @@ bool ExtensionPrefs::GetGrantedPermissions(
     ExtensionExtent* host_extent) {
   CHECK(Extension::IdIsValid(extension_id));
 
-  DictionaryValue* ext = GetExtensionPref(extension_id);
+  const DictionaryValue* ext = GetExtensionPref(extension_id);
   if (!ext || !ext->GetBoolean(kPrefGrantedPermissionsAll, full_access))
     return false;
 
@@ -651,7 +708,7 @@ void ExtensionPrefs::AddGrantedPermissions(
         extension_id, kPrefGrantedPermissionsHost, host_permissions);
   }
 
-  SavePrefsAndNotify();
+  SavePrefs();
 }
 
 bool ExtensionPrefs::IsIncognitoEnabled(const std::string& extension_id) {
@@ -662,7 +719,7 @@ void ExtensionPrefs::SetIsIncognitoEnabled(const std::string& extension_id,
                                            bool enabled) {
   UpdateExtensionPref(extension_id, kPrefIncognitoEnabled,
                       Value::CreateBooleanValue(enabled));
-  SavePrefsAndNotify();
+  SavePrefs();
 }
 
 bool ExtensionPrefs::AllowFileAccess(const std::string& extension_id) {
@@ -673,12 +730,12 @@ void ExtensionPrefs::SetAllowFileAccess(const std::string& extension_id,
                                         bool allow) {
   UpdateExtensionPref(extension_id, kPrefAllowFileAccess,
                       Value::CreateBooleanValue(allow));
-  SavePrefsAndNotify();
+  SavePrefs();
 }
 
 bool ExtensionPrefs::HasAllowFileAccessSetting(
     const std::string& extension_id) const {
-  DictionaryValue* ext = GetExtensionPref(extension_id);
+  const DictionaryValue* ext = GetExtensionPref(extension_id);
   return ext && ext->HasKey(kPrefAllowFileAccess);
 }
 
@@ -762,12 +819,12 @@ void ExtensionPrefs::SetLaunchType(const std::string& extension_id,
                                    LaunchType launch_type) {
   UpdateExtensionPref(extension_id, kPrefLaunchType,
       Value::CreateIntegerValue(static_cast<int>(launch_type)));
-  SavePrefsAndNotify();
+  SavePrefs();
 }
 
 bool ExtensionPrefs::IsExternalExtensionUninstalled(
     const std::string& id) const {
-  DictionaryValue* extension = GetExtensionPref(id);
+  const DictionaryValue* extension = GetExtensionPref(id);
   if (!extension)
     return false;
   int state = 0;
@@ -790,45 +847,48 @@ std::vector<std::string> ExtensionPrefs::GetToolbarOrder() {
 
 void ExtensionPrefs::SetToolbarOrder(
     const std::vector<std::string>& extension_ids) {
-  ListValue* toolbar_order = prefs_->GetMutableList(kExtensionToolbar);
+  ListPrefUpdate update(prefs_, kExtensionToolbar);
+  ListValue* toolbar_order = update.Get();
   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();
+  SavePrefs();
 }
 
 void ExtensionPrefs::OnExtensionInstalled(
     const Extension* extension, Extension::State initial_state,
     bool initial_incognito_enabled) {
   const std::string& id = extension->id();
+  CHECK(Extension::IdIsValid(id));
+  ScopedExtensionPrefUpdate update(prefs_, id);
+  DictionaryValue* extension_dict = update.Get();
   const base::Time install_time = GetCurrentTime();
-  UpdateExtensionPref(id, kPrefState,
-                      Value::CreateIntegerValue(initial_state));
-  UpdateExtensionPref(id, kPrefIncognitoEnabled,
+  extension_dict->Set(kPrefState, Value::CreateIntegerValue(initial_state));
+  extension_dict->Set(kPrefIncognitoEnabled,
                       Value::CreateBooleanValue(initial_incognito_enabled));
-  UpdateExtensionPref(id, kPrefLocation,
+  extension_dict->Set(kPrefLocation,
                       Value::CreateIntegerValue(extension->location()));
-  UpdateExtensionPref(id, kPrefInstallTime,
+  extension_dict->Set(kPrefInstallTime,
                       Value::CreateStringValue(
                           base::Int64ToString(install_time.ToInternalValue())));
-  UpdateExtensionPref(id, kPrefPreferences, new DictionaryValue());
+  extension_dict->Set(kPrefPreferences, new DictionaryValue());
 
   FilePath::StringType path = MakePathRelative(install_directory_,
-      extension->path(), NULL);
-  UpdateExtensionPref(id, kPrefPath, Value::CreateStringValue(path));
+      extension->path());
+  extension_dict->Set(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_dict->Set(kPrefManifest,
                         extension->manifest_value()->DeepCopy());
   }
-  UpdateExtensionPref(id, kPrefAppLaunchIndex,
+  extension_dict->Set(kPrefAppLaunchIndex,
                       Value::CreateIntegerValue(GetNextAppLaunchIndex()));
   extension_pref_value_map_->RegisterExtension(
       id, install_time, initial_state == Extension::ENABLED);
-  SavePrefsAndNotify();
+  SavePrefs();
 }
 
 void ExtensionPrefs::OnExtensionUninstalled(const std::string& extension_id,
@@ -842,7 +902,7 @@ void ExtensionPrefs::OnExtensionUninstalled(const std::string& extension_id,
     UpdateExtensionPref(extension_id, kPrefState,
                         Value::CreateIntegerValue(
                             Extension::EXTERNAL_EXTENSION_UNINSTALLED));
-    SavePrefsAndNotify();
+    SavePrefs();
     extension_pref_value_map_->SetExtensionState(extension_id, false);
   } else {
     DeleteExtensionPrefs(extension_id);
@@ -851,7 +911,7 @@ void ExtensionPrefs::OnExtensionUninstalled(const std::string& extension_id,
 
 Extension::State ExtensionPrefs::GetExtensionState(
     const std::string& extension_id) const {
-  DictionaryValue* extension = GetExtensionPref(extension_id);
+  const DictionaryValue* extension = GetExtensionPref(extension_id);
 
   // If the extension doesn't have a pref, it's a --load-extension.
   if (!extension)
@@ -871,14 +931,14 @@ void ExtensionPrefs::SetExtensionState(const Extension* extension,
                                        Extension::State state) {
   UpdateExtensionPref(extension->id(), kPrefState,
                       Value::CreateIntegerValue(state));
-  SavePrefsAndNotify();
+  SavePrefs();
 
   bool enabled = (state == Extension::ENABLED);
   extension_pref_value_map_->SetExtensionState(extension->id(), enabled);
 }
 
 bool ExtensionPrefs::GetBrowserActionVisibility(const Extension* extension) {
-  DictionaryValue* extension_prefs = GetExtensionPref(extension->id());
+  const DictionaryValue* extension_prefs = GetExtensionPref(extension->id());
   if (!extension_prefs)
     return true;
   bool visible = false;
@@ -895,7 +955,7 @@ void ExtensionPrefs::SetBrowserActionVisibility(const Extension* extension,
 
   UpdateExtensionPref(extension->id(), kBrowserActionVisible,
                       Value::CreateBooleanValue(visible));
-  SavePrefsAndNotify();
+  SavePrefs();
 
   NotificationService::current()->Notify(
       NotificationType::EXTENSION_BROWSER_ACTION_VISIBILITY_CHANGED,
@@ -904,7 +964,7 @@ void ExtensionPrefs::SetBrowserActionVisibility(const Extension* extension,
 }
 
 std::string ExtensionPrefs::GetVersionString(const std::string& extension_id) {
-  DictionaryValue* extension = GetExtensionPref(extension_id);
+  const DictionaryValue* extension = GetExtensionPref(extension_id);
   if (!extension)
     return std::string();
 
@@ -919,9 +979,18 @@ std::string ExtensionPrefs::GetVersionString(const std::string& extension_id) {
 
 void ExtensionPrefs::UpdateManifest(const Extension* extension) {
   if (extension->location() != Extension::LOAD) {
-    UpdateExtensionPref(extension->id(), kPrefManifest,
-                        extension->manifest_value()->DeepCopy());
-    SavePrefsAndNotify();
+    const DictionaryValue* extension_dict = GetExtensionPref(extension->id());
+    if (!extension_dict)
+      return;
+    DictionaryValue* old_manifest = NULL;
+    bool update_required =
+        !extension_dict->GetDictionary(kPrefManifest, &old_manifest) ||
+        !extension->manifest_value()->Equals(old_manifest);
+    if (update_required) {
+      UpdateExtensionPref(extension->id(), kPrefManifest,
+                          extension->manifest_value()->DeepCopy());
+    }
+    SavePrefs();
   }
 }
 
@@ -944,33 +1013,22 @@ void ExtensionPrefs::UpdateExtensionPref(const std::string& extension_id,
     NOTREACHED() << "Invalid extension_id " << extension_id;
     return;
   }
-  DictionaryValue* extension = GetOrCreateExtensionPref(extension_id);
+  ScopedExtensionPrefUpdate update(prefs_, extension_id);
+  DictionaryValue* extension = update.Get();
   extension->Set(key, data_value);
 }
 
 void ExtensionPrefs::DeleteExtensionPrefs(const std::string& extension_id) {
-  DictionaryValue* dict = prefs_->GetMutableDictionary(kExtensionsPref);
+  DictionaryPrefUpdate update(prefs_, kExtensionsPref);
+  DictionaryValue* dict = update.Get();
   if (dict->HasKey(extension_id)) {
     dict->Remove(extension_id, NULL);
-    SavePrefsAndNotify();
+    SavePrefs();
   }
   extension_pref_value_map_->UnregisterExtension(extension_id);
 }
 
-DictionaryValue* ExtensionPrefs::GetOrCreateExtensionPref(
-    const std::string& extension_id) {
-  DCHECK(Extension::IdIsValid(extension_id));
-  DictionaryValue* dict = prefs_->GetMutableDictionary(kExtensionsPref);
-  DictionaryValue* extension = NULL;
-  if (!dict->GetDictionary(extension_id, &extension)) {
-    // Extension pref does not exist, create it.
-    extension = new DictionaryValue();
-    dict->Set(extension_id, extension);
-  }
-  return extension;
-}
-
-DictionaryValue* ExtensionPrefs::GetExtensionPref(
+const DictionaryValue* ExtensionPrefs::GetExtensionPref(
     const std::string& extension_id) const {
   const DictionaryValue* dict = prefs_->GetDictionary(kExtensionsPref);
   if (!dict)
@@ -1081,7 +1139,8 @@ void ExtensionPrefs::SetIdleInstallInfo(const std::string& extension_id,
                                         const FilePath& crx_path,
                                         const std::string& version,
                                         const base::Time& fetch_time) {
-  DictionaryValue* extension_prefs = GetExtensionPref(extension_id);
+  ScopedExtensionPrefUpdate update(prefs_, extension_id);
+  DictionaryValue* extension_prefs = update.Get();
   if (!extension_prefs) {
     NOTREACHED();
     return;
@@ -1093,15 +1152,16 @@ void ExtensionPrefs::SetIdleInstallInfo(const std::string& extension_id,
   info->SetString(kIdleInstallInfoFetchTime,
                   base::Int64ToString(fetch_time.ToInternalValue()));
   extension_prefs->Set(kIdleInstallInfo, info);
-  SavePrefsAndNotify();
+  SavePrefs();
 }
 
 bool ExtensionPrefs::RemoveIdleInstallInfo(const std::string& extension_id) {
-  DictionaryValue* extension_prefs = GetExtensionPref(extension_id);
-  if (!extension_prefs)
+  if (!GetExtensionPref(extension_id))
     return false;
+  ScopedExtensionPrefUpdate update(prefs_, extension_id);
+  DictionaryValue* extension_prefs = update.Get();
   bool result = extension_prefs->Remove(kIdleInstallInfo, NULL);
-  SavePrefsAndNotify();
+  SavePrefs();
   return result;
 }
 
@@ -1109,7 +1169,7 @@ bool ExtensionPrefs::GetIdleInstallInfo(const std::string& extension_id,
                                         FilePath* crx_path,
                                         std::string* version,
                                         base::Time* fetch_time) {
-  DictionaryValue* extension_prefs = GetExtensionPref(extension_id);
+  const DictionaryValue* extension_prefs = GetExtensionPref(extension_id);
   if (!extension_prefs)
     return false;
 
@@ -1162,12 +1222,11 @@ std::set<std::string> ExtensionPrefs::GetIdleInstallInfoIds() {
       continue;
     }
 
-    DictionaryValue* extension_prefs = GetExtensionPref(id);
+    const DictionaryValue* extension_prefs = GetExtensionPref(id);
     if (!extension_prefs)
       continue;
 
-    DictionaryValue* info = NULL;
-    if (extension_prefs->GetDictionary(kIdleInstallInfo, &info))
+    if (extension_prefs->GetDictionary(kIdleInstallInfo, NULL))
       result.insert(id);
   }
   return result;
@@ -1183,7 +1242,7 @@ bool ExtensionPrefs::GetWebStoreLogin(std::string* result) {
 
 void ExtensionPrefs::SetWebStoreLogin(const std::string& login) {
   prefs_->SetString(kWebStoreLogin, login);
-  SavePrefsAndNotify();
+  SavePrefs();
 }
 
 int ExtensionPrefs::GetAppLaunchIndex(const std::string& extension_id) {
@@ -1199,7 +1258,7 @@ void ExtensionPrefs::SetAppLaunchIndex(const std::string& extension_id,
   DCHECK_GE(index, 0);
   UpdateExtensionPref(extension_id, kPrefAppLaunchIndex,
                       Value::CreateIntegerValue(index));
-  SavePrefsAndNotify();
+  SavePrefs();
 }
 
 int ExtensionPrefs::GetNextAppLaunchIndex() {
@@ -1240,11 +1299,11 @@ void ExtensionPrefs::SetPageIndex(const std::string& extension_id, int index) {
   CHECK_GE(index, 0);
   UpdateExtensionPref(extension_id, kPrefPageIndex,
                       Value::CreateIntegerValue(index));
-  SavePrefsAndNotify();
+  SavePrefs();
 }
 
 bool ExtensionPrefs::WasAppDraggedByUser(const std::string& extension_id) {
-  DictionaryValue* dictionary = GetExtensionPref(extension_id);
+  const DictionaryValue* dictionary = GetExtensionPref(extension_id);
   if (!dictionary) {
     NOTREACHED();
     return false;
@@ -1254,30 +1313,32 @@ bool ExtensionPrefs::WasAppDraggedByUser(const std::string& extension_id) {
 }
 
 void ExtensionPrefs::SetAppDraggedByUser(const std::string& extension_id) {
-  DictionaryValue* dictionary = GetExtensionPref(extension_id);
-  if (!dictionary) {
+  if (!GetExtensionPref(extension_id)) {
     NOTREACHED();
     return;
   }
 
+  ScopedExtensionPrefUpdate update(prefs_, extension_id);
+  DictionaryValue* dictionary = update.Get();
   dictionary->SetBoolean(kPrefUserDraggedApp, true);
-  SavePrefsAndNotify();
+  SavePrefs();
 }
 
 void ExtensionPrefs::SetUpdateUrlData(const std::string& extension_id,
                                       const std::string& data) {
-  DictionaryValue* dictionary = GetExtensionPref(extension_id);
-  if (!dictionary) {
+  if (!GetExtensionPref(extension_id)) {
     NOTREACHED();
     return;
   }
 
+  ScopedExtensionPrefUpdate update(prefs_, extension_id);
+  DictionaryValue* dictionary = update.Get();
   dictionary->SetString(kUpdateUrlData, data);
-  SavePrefsAndNotify();
+  SavePrefs();
 }
 
 std::string ExtensionPrefs::GetUpdateUrlData(const std::string& extension_id) {
-  DictionaryValue* dictionary = GetExtensionPref(extension_id);
+  const DictionaryValue* dictionary = GetExtensionPref(extension_id);
   if (!dictionary)
     return std::string();
 
@@ -1323,15 +1384,14 @@ void ExtensionPrefs::FixMissingPrefs(const ExtensionIdSet& extension_ids) {
   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);
-    CHECK(extension);
-
     if (GetInstallTime(*ext_id) == base::Time()) {
       LOG(INFO) << "Could not parse installation time of extension "
                 << *ext_id << ". It was probably installed before setting "
                 << kPrefInstallTime << " was introduced. Updating "
                 << kPrefInstallTime << " to the current time.";
       const base::Time install_time = GetCurrentTime();
+      ScopedExtensionPrefUpdate update(prefs_, *ext_id);
+      DictionaryValue* extension = update.Get();
       extension->Set(kPrefInstallTime,
                      Value::CreateStringValue(
                          base::Int64ToString(install_time.ToInternalValue())));
@@ -1339,19 +1399,26 @@ void ExtensionPrefs::FixMissingPrefs(const ExtensionIdSet& extension_ids) {
     }
   }
   if (persist_required)
-    SavePrefsAndNotify();
+    SavePrefs();
 }
 
-DictionaryValue* ExtensionPrefs::GetExtensionControlledPrefs(
+const DictionaryValue* ExtensionPrefs::GetExtensionControlledPrefs(
     const std::string& extension_id) const {
-  DictionaryValue* source_dict = prefs_->GetMutableDictionary(kExtensionsPref);
-  DictionaryValue* preferences = NULL;
   std::string key = extension_id + std::string(".") + kPrefPreferences;
-  if (!source_dict->GetDictionary(key, &preferences)) {
-    source_dict->Set(key, new DictionaryValue);
-    bool success = source_dict->GetDictionary(key, &preferences);
-    DCHECK(success);
+  DictionaryValue* preferences = NULL;
+  // First try the regular lookup.
+  {
+    const DictionaryValue* source_dict = prefs_->GetDictionary(kExtensionsPref);
+    if (source_dict->GetDictionary(key, &preferences))
+      return preferences;
+  }
+  // And then create a dictionary if it did not exist before.
+  {
+    DictionaryPrefUpdate update(prefs_, kExtensionsPref);
+    update.Get()->Set(key, new DictionaryValue);
   }
+  const DictionaryValue* source_dict = prefs_->GetDictionary(kExtensionsPref);
+  source_dict->GetDictionary(key, &preferences);
   return preferences;
 }
 
@@ -1360,8 +1427,20 @@ void ExtensionPrefs::InitPrefStore() {
   // to the user preferences stored in a JSON file.
   ExtensionIdSet extension_ids;
   GetExtensions(&extension_ids);
-  FixMissingPrefs(extension_ids);
+  // Create empty preferences dictionary for each extension (these dictionaries
+  // are pruned when persisting the preferneces to disk).
+  {
+    DictionaryPrefUpdate update(prefs_, kExtensionsPref);
+    const DictionaryValue* source_dict = prefs_->GetDictionary(kExtensionsPref);
+    for (ExtensionIdSet::iterator ext_id = extension_ids.begin();
+         ext_id != extension_ids.end(); ++ext_id) {
+      std::string key = *ext_id + std::string(".") + kPrefPreferences;
+      if (!source_dict->GetDictionary(key, NULL))
+        update.Get()->Set(key, new DictionaryValue);
+    }
+  }
 
+  FixMissingPrefs(extension_ids);
   // Store extension controlled preference values in the
   // |extension_pref_value_map_|, which then informs the subscribers
   // (ExtensionPrefStores) about the winning values.
@@ -1372,7 +1451,7 @@ void ExtensionPrefs::InitPrefStore() {
         GetInstallTime(*ext_id),
         GetExtensionState(*ext_id) == Extension::ENABLED);
 
-    DictionaryValue* prefs = GetExtensionControlledPrefs(*ext_id);
+    const DictionaryValue* prefs = GetExtensionControlledPrefs(*ext_id);
     for (DictionaryValue::key_iterator i = prefs->begin_keys();
          i != prefs->end_keys(); ++i) {
       Value* value;
@@ -1403,7 +1482,8 @@ void ExtensionPrefs::SetExtensionControlledPref(const std::string& extension_id,
   if (!incognito) {
     // Also store in persisted Preferences file to recover after a
     // browser restart.
-    DictionaryValue* dict = GetExtensionControlledPrefs(extension_id);
+    ScopedExtensionControlledPrefUpdate update(prefs_, extension_id);
+    DictionaryValue* dict = update.Get();
     dict->SetWithoutPathExpansion(pref_key, value->DeepCopy());
     pref_service()->ScheduleSavePersistentPrefs();
   }
@@ -1423,7 +1503,8 @@ void ExtensionPrefs::RemoveExtensionControlledPref(
   if (!incognito) {
     // Also store in persisted Preferences file to recover after a
     // browser restart.
-    DictionaryValue* dict = GetExtensionControlledPrefs(extension_id);
+    ScopedExtensionControlledPrefUpdate update(prefs_, extension_id);
+    DictionaryValue* dict = update.Get();
     dict->RemoveWithoutPathExpansion(pref_key, NULL);
     pref_service()->ScheduleSavePersistentPrefs();
   }
diff --git a/chrome/browser/extensions/extension_prefs.h b/chrome/browser/extensions/extension_prefs.h
index 980ef70..c2c5ff8 100644
--- a/chrome/browser/extensions/extension_prefs.h
+++ b/chrome/browser/extensions/extension_prefs.h
@@ -354,7 +354,8 @@ class ExtensionPrefs {
 
   // Reads a boolean pref from |ext| with key |pref_key|.
   // Return false if the value is false or |pref_key| does not exist.
-  bool ReadBooleanFromPref(DictionaryValue* ext, const std::string& pref_key);
+  bool ReadBooleanFromPref(const DictionaryValue* ext,
+                           const std::string& pref_key);
 
   // Reads a boolean pref |pref_key| from extension with id |extension_id|.
   bool ReadExtensionPrefBoolean(const std::string& extension_id,
@@ -362,7 +363,8 @@ class ExtensionPrefs {
 
   // Reads an integer pref from |ext| with key |pref_key|.
   // Return false if the value does not exist.
-  bool ReadIntegerFromPref(DictionaryValue* ext, const std::string& pref_key,
+  bool ReadIntegerFromPref(const DictionaryValue* ext,
+                           const std::string& pref_key,
                            int* out_value);
 
   // Reads an integer pref |pref_key| from extension with id |extension_id|.
@@ -373,7 +375,7 @@ class ExtensionPrefs {
   // Reads a list pref |pref_key| from extension with id | extension_id|.
   bool ReadExtensionPrefList(const std::string& extension_id,
                              const std::string& pref_key,
-                             ListValue** out_value);
+                             const ListValue** out_value);
 
   // Reads a list pref |pref_key| as a string set from the extension with
   // id |extension_id|.
@@ -388,16 +390,15 @@ class ExtensionPrefs {
                                    const std::string& pref_key,
                                    const std::set<std::string>& added_values);
 
-  // Ensures and returns a mutable dictionary for extension |id|'s prefs.
-  DictionaryValue* GetOrCreateExtensionPref(const std::string& id);
-
-  // Same as above, but returns NULL if it doesn't exist.
-  DictionaryValue* GetExtensionPref(const std::string& id) const;
+  // Returns a dictionary for extension |id|'s prefs or NULL if it doesn't
+  // exist.
+  const DictionaryValue* GetExtensionPref(const std::string& id) const;
 
   // Returns the dictionary of preferences controlled by the specified extension
   // or creates a new one. All entries in the dictionary contain non-expanded
   // paths.
-  DictionaryValue* GetExtensionControlledPrefs(const std::string& id) const;
+  const DictionaryValue* GetExtensionControlledPrefs(
+      const std::string& id) const;
 
   // Serializes the data and schedules a persistent save via the |PrefService|.
   // Additionally fires a PREF_CHANGED notification with the top-level
@@ -405,7 +406,7 @@ class ExtensionPrefs {
   // TODO(andybons): Switch this to EXTENSION_PREF_CHANGED to be more granular.
   // TODO(andybons): Use a ScopedUserPrefUpdate to update observers on changes
   // to the mutable extension dictionary.
-  void SavePrefsAndNotify();
+  void SavePrefs();
 
   // Checks if kPrefBlacklist is set to true in the DictionaryValue.
   // Return false if the value is false or kPrefBlacklist does not exist.
diff --git a/chrome/browser/extensions/extension_service_unittest.cc b/chrome/browser/extensions/extension_service_unittest.cc
index d28eef5..ec73155 100644
--- a/chrome/browser/extensions/extension_service_unittest.cc
+++ b/chrome/browser/extensions/extension_service_unittest.cc
@@ -651,8 +651,8 @@ class ExtensionServiceTest
   }
 
   void ValidatePrefKeyCount(size_t count) {
-    DictionaryValue* dict =
-        profile_->GetPrefs()->GetMutableDictionary("extensions.settings");
+    const DictionaryValue* dict =
+        profile_->GetPrefs()->GetDictionary("extensions.settings");
     ASSERT_TRUE(dict != NULL);
     EXPECT_EQ(count, dict->size());
   }
@@ -746,8 +746,8 @@ class ExtensionServiceTest
                const std::string& pref_path,
                Value* value,
                const std::string& msg) {
-    const DictionaryValue* dict =
-        profile_->GetPrefs()->GetMutableDictionary("extensions.settings");
+    DictionaryPrefUpdate update(profile_->GetPrefs(), "extensions.settings");
+    DictionaryValue* dict = update.Get();
     ASSERT_TRUE(dict != NULL) << msg;
     DictionaryValue* pref = NULL;
     ASSERT_TRUE(dict->GetDictionary(extension_id, &pref)) << msg;
@@ -784,8 +784,8 @@ class ExtensionServiceTest
     std::string msg = " while clearing: ";
     msg += extension_id + " " + pref_path;
 
-    const DictionaryValue* dict =
-        profile_->GetPrefs()->GetMutableDictionary("extensions.settings");
+    DictionaryPrefUpdate update(profile_->GetPrefs(), "extensions.settings");
+    DictionaryValue* dict = update.Get();
     ASSERT_TRUE(dict != NULL) << msg;
     DictionaryValue* pref = NULL;
     ASSERT_TRUE(dict->GetDictionary(extension_id, &pref)) << msg;
@@ -1024,10 +1024,12 @@ TEST_F(ExtensionServiceTest, CleanupOnStartup) {
   InitializeInstalledExtensionService(pref_path, source_install_dir);
 
   // Simulate that one of them got partially deleted by clearing its pref.
-  DictionaryValue* dict =
-      profile_->GetPrefs()->GetMutableDictionary("extensions.settings");
-  ASSERT_TRUE(dict != NULL);
-  dict->Remove("behllobkkfkfnphdnhnkndlbkcpglgmj", NULL);
+  {
+    DictionaryPrefUpdate update(profile_->GetPrefs(), "extensions.settings");
+    DictionaryValue* dict = update.Get();
+    ASSERT_TRUE(dict != NULL);
+    dict->Remove("behllobkkfkfnphdnhnkndlbkcpglgmj", NULL);
+  }
 
   service_->Init();
   loop_.RunAllPending();
@@ -2522,14 +2524,13 @@ TEST_F(ExtensionServiceTest, WillNotLoadPluginExtensionsFromDirectory) {
 TEST_F(ExtensionServiceTest, BlacklistedByPolicyWillNotInstall) {
   InitializeEmptyExtensionService();
 
-  ListValue* whitelist =
-      profile_->GetPrefs()->GetMutableList(prefs::kExtensionInstallAllowList);
-  ListValue* blacklist =
-      profile_->GetPrefs()->GetMutableList(prefs::kExtensionInstallDenyList);
-  ASSERT_TRUE(whitelist != NULL && blacklist != NULL);
-
   // Blacklist everything.
-  blacklist->Append(Value::CreateStringValue("*"));
+  {
+    ListPrefUpdate update(profile_->GetPrefs(),
+                          prefs::kExtensionInstallDenyList);
+    ListValue* blacklist = update.Get();
+    blacklist->Append(Value::CreateStringValue("*"));
+  }
 
   // Blacklist prevents us from installing good_crx.
   FilePath extensions_path;
@@ -2541,7 +2542,13 @@ TEST_F(ExtensionServiceTest, BlacklistedByPolicyWillNotInstall) {
   EXPECT_EQ(0u, service_->extensions()->size());
 
   // Now whitelist this particular extension.
-  whitelist->Append(Value::CreateStringValue(good_crx));
+  {
+    ListPrefUpdate update(profile_->GetPrefs(),
+                          prefs::kExtensionInstallAllowList);
+    ListValue* whitelist = update.Get();
+    whitelist->Append(Value::CreateStringValue(good_crx));
+  }
+
 
   // Ensure we can now install good_crx.
   StartCrxInstall(path);
@@ -2564,9 +2571,8 @@ TEST_F(ExtensionServiceTest, BlacklistedByPolicyRemovedIfRunning) {
 
   { // Scope for pref update notification.
     PrefService* prefs = profile_->GetPrefs();
-    ScopedUserPrefUpdate pref_update(prefs, prefs::kExtensionInstallDenyList);
-    ListValue* blacklist =
-        prefs->GetMutableList(prefs::kExtensionInstallDenyList);
+    ListPrefUpdate update(prefs, prefs::kExtensionInstallDenyList);
+    ListValue* blacklist = update.Get();
     ASSERT_TRUE(blacklist != NULL);
 
     // Blacklist this extension.
diff --git a/chrome/browser/extensions/extension_web_ui.cc b/chrome/browser/extensions/extension_web_ui.cc
index b7a7caf..c6c9611 100644
--- a/chrome/browser/extensions/extension_web_ui.cc
+++ b/chrome/browser/extensions/extension_web_ui.cc
@@ -13,6 +13,7 @@
 #include "chrome/browser/extensions/extension_service.h"
 #include "chrome/browser/extensions/image_loading_tracker.h"
 #include "chrome/browser/prefs/pref_service.h"
+#include "chrome/browser/prefs/scoped_user_pref_update.h"
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/ui/browser.h"
 #include "chrome/browser/ui/browser_list.h"
@@ -303,8 +304,8 @@ void ExtensionWebUI::RegisterChromeURLOverrides(
     return;
 
   PrefService* prefs = profile->GetPrefs();
-  DictionaryValue* all_overrides =
-      prefs->GetMutableDictionary(kExtensionURLOverrides);
+  DictionaryPrefUpdate update(prefs, kExtensionURLOverrides);
+  DictionaryValue* all_overrides = update.Get();
 
   // For each override provided by the extension, add it to the front of
   // the override list if it's not already in the list.
@@ -368,8 +369,8 @@ void ExtensionWebUI::UnregisterChromeURLOverride(const std::string& page,
   if (!override)
     return;
   PrefService* prefs = profile->GetPrefs();
-  DictionaryValue* all_overrides =
-      prefs->GetMutableDictionary(kExtensionURLOverrides);
+  DictionaryPrefUpdate update(prefs, kExtensionURLOverrides);
+  DictionaryValue* all_overrides = update.Get();
   ListValue* page_overrides;
   if (!all_overrides->GetList(page, &page_overrides)) {
     // If it's being unregistered, it should already be in the list.
@@ -386,8 +387,8 @@ void ExtensionWebUI::UnregisterChromeURLOverrides(
   if (overrides.empty())
     return;
   PrefService* prefs = profile->GetPrefs();
-  DictionaryValue* all_overrides =
-      prefs->GetMutableDictionary(kExtensionURLOverrides);
+  DictionaryPrefUpdate update(prefs, kExtensionURLOverrides);
+  DictionaryValue* all_overrides = update.Get();
   Extension::URLOverrideMap::const_iterator iter = overrides.begin();
   for (; iter != overrides.end(); ++iter) {
     const std::string& page = iter->first;