No longer register app window placement preference keys on the fly.

chrome/browser/ui/browser_ui_prefs.cc

Index: chrome/browser/ui/browser_ui_prefs.cc
diff --git a/chrome/browser/ui/browser_ui_prefs.cc b/chrome/browser/ui/browser_ui_prefs.cc
index 68215e479040a57a669fa2aa32a73b8d65eb87a7..94ebc14f29e8f9dbfce8030deb016e796515edcd 100644
--- a/chrome/browser/ui/browser_ui_prefs.cc
+++ b/chrome/browser/ui/browser_ui_prefs.cc
@@ -6,6 +6,8 @@
 
 #include "base/prefs/pref_registry_simple.h"
 #include "base/prefs/pref_service.h"
+#include "base/prefs/scoped_user_pref_update.h"
+#include "base/strings/string_util.h"
 #include "chrome/browser/first_run/first_run.h"
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/common/pref_names.h"
@@ -14,6 +16,40 @@
 
 namespace chrome {
 
+void MigrateBrowserUIUserPrefs(PrefService* prefs) {
+  scoped_ptr<base::DictionaryValue> browser_dict =
+      prefs->GetBrowserDictionary();

[dgrogan, 2014/09/04 00:50:51]

An alternative to the GetBrowserDictionary method is to plumb the
user_pref_store in here somehow. That didn't seem like a clear win.

+  if (!browser_dict.get())

[Bernhard Bauer, 2014/09/04 11:06:02]

scoped_ptr has a bool conversion operator.

[dgrogan, 2014/09/04 20:26:33]

Done.

+    return;
+
+  DictionaryPrefUpdate update(prefs, prefs::kAppWindowPlacement);
+  const std::string search_for =
+      std::string(prefs::kBrowserWindowPlacement) + "_";
+  for (base::DictionaryValue::Iterator it(*browser_dict); !it.IsAtEnd();
+       it.Advance()) {
+    std::string full_key("browser." + it.key());
+    if (StartsWithASCII(full_key, search_for, true /*case_sensitive*/)) {

[Bernhard Bauer, 2014/09/04 11:06:02]

Nit: space inside of a /* comment */.

[dgrogan, 2014/09/04 20:26:33]

Done.

+      if (full_key == prefs::kBrowserWindowPlacementPopup)
+        continue;
+      const base::DictionaryValue* old_value;
+      // Test this on mac with a local app.
+      bool found = browser_dict->GetDictionary(it.key(), &old_value);

[Bernhard Bauer, 2014/09/04 11:06:02]

You can use it.value() to get the value, then call GetAsDictionary(). That will
make it a bit clearer that we expect the value to be a dictionary (but we know
that it exists).

[dgrogan, 2014/09/04 20:26:33]

I got rid of the conversion to dictionary. The only thing I could do with it was
DCHECK that it was a dictionary, but as I brought up elsewhere it's possible
that someone has modified their Preferences such that the result isn't a
dictionary.

+      DCHECK(found) << it.key();
+      std::string new_key(full_key);
+      ReplaceFirstSubstringAfterOffset(&new_key, 0, search_for, "");
+      update->Set(new_key, old_value->DeepCopy());
+
+      user_prefs::PrefRegistrySyncable* registry =
+          static_cast<user_prefs::PrefRegistrySyncable*>(
+              prefs->DeprecatedGetPrefRegistry());
+      registry->RegisterDictionaryPref(
+                    full_key.c_str(),
+                    user_prefs::PrefRegistrySyncable::UNSYNCABLE_PREF);
+      prefs->ClearPref(full_key.c_str());
+    }
+  }
+}
+
 void RegisterBrowserPrefs(PrefRegistrySimple* registry) {
   registry->RegisterIntegerPref(prefs::kOptionsWindowLastTabIndex, 0);
   registry->RegisterBooleanPref(prefs::kAllowFileSelectionDialogs, true);
@@ -125,6 +161,9 @@ void RegisterBrowserUserPrefs(user_prefs::PrefRegistrySyncable* registry) {
   registry->RegisterDictionaryPref(
       prefs::kBrowserWindowPlacementPopup,
       user_prefs::PrefRegistrySyncable::UNSYNCABLE_PREF);
+  registry->RegisterDictionaryPref(
+      prefs::kAppWindowPlacement,
+      user_prefs::PrefRegistrySyncable::UNSYNCABLE_PREF);
   registry->RegisterBooleanPref(
       prefs::kImportBookmarks,
       true,
@@ -184,24 +223,4 @@ void RegisterBrowserUserPrefs(user_prefs::PrefRegistrySyncable* registry) {
 #endif
 }
 
-void RegisterAppPrefs(const std::string& app_name, Profile* profile) {
-  // We need to register the window position pref.
-  //
-  // TODO(mnissler): Use a separate pref name pointing to a single
-  // dictionary instead. Also tracked as http://crbug.com/167256
-  std::string window_pref(prefs::kBrowserWindowPlacement);
-  window_pref.append("_");
-  window_pref.append(app_name);
-  PrefService* prefs = profile->GetPrefs();
-  if (!prefs->FindPreference(window_pref.c_str())) {
-    // TODO(joi): Do all registration up front.
-    scoped_refptr<user_prefs::PrefRegistrySyncable> registry(
-        static_cast<user_prefs::PrefRegistrySyncable*>(
-            prefs->DeprecatedGetPrefRegistry()));
-    registry->RegisterDictionaryPref(
-        window_pref.c_str(), user_prefs::PrefRegistrySyncable::UNSYNCABLE_PREF);
-  }
-}
-
-
 }  // namespace chrome