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

chrome/browser/ui/browser_ui_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/ui/browser_ui_prefs.h"
 
 #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"
 #include "components/pref_registry/pref_registry_syncable.h"
 #include "components/translate/core/common/translate_pref_names.h"
 
 namespace chrome {
 
+void MigrateBrowserUIUserPrefs(PrefService* prefs) {
+  scoped_ptr<base::DictionaryValue> browser_dict =
+      prefs->GetBrowserDictionary();

[gab, 2014/09/05 00:53:10]

I'm torn here, you could do this without requiring a DeepCopy of the "browser"
dictionary and could also avoid DeepCopy of the individual dicts below, but it
would involve PrefService exposing the true DictionaryValue* which is perhaps
even more intrusive (but encapsulation is already broken with the current
approach so wtv..?)... 
The idea would be to use DictionaryValue::Remove which allows you to grab the
deleted data and put it somewhere else rather than DeepCopy+ClearPref below.

The DeepCopy is particularly painful as you will do it every launch despite it
being a no-op for every launch but the first one... (you should probably exit
fast if you can tell the migration has already occurred).

[dgrogan, 2014/09/06 00:47:06]

I moved the migration to work off a PrefStoreObserver which does a Remove like
you suggest here. So there are no DeepCopy calls in the new code. Yay. Also
added a fast-exit check.

+  if (!browser_dict)
+    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 */)) {
+      if (full_key == prefs::kBrowserWindowPlacementPopup)
+        continue;
+      std::string new_key(full_key);
+      ReplaceFirstSubstringAfterOffset(&new_key, 0, search_for, "");

[gab, 2014/09/05 00:53:10]

|new_key| (which is equal to |full_key| at this point) always begins with
"browser.window_placement_" (as per the condition this code is wrapped in)

So you can replace the above two lines by:

std::string new_key(full_key.substr(search_for.length()));

[dgrogan, 2014/09/06 00:47:06]

Done.

+      update->Set(new_key, it.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);
   registry->RegisterIntegerPref(prefs::kShowFirstRunBubbleOption,
                              first_run::FIRST_RUN_BUBBLE_DONT_SHOW);
 }
 
 void RegisterBrowserUserPrefs(user_prefs::PrefRegistrySyncable* registry) {
   registry->RegisterBooleanPref(
       prefs::kHomePageIsNewTabPage,
@@ skipping 91 matching lines @@
   registry->RegisterBooleanPref(
       prefs::kDevToolsDisabled,
       false,
       user_prefs::PrefRegistrySyncable::UNSYNCABLE_PREF);
   registry->RegisterDictionaryPref(
       prefs::kBrowserWindowPlacement,
       user_prefs::PrefRegistrySyncable::UNSYNCABLE_PREF);
   registry->RegisterDictionaryPref(
       prefs::kBrowserWindowPlacementPopup,
       user_prefs::PrefRegistrySyncable::UNSYNCABLE_PREF);
+  registry->RegisterDictionaryPref(
+      prefs::kAppWindowPlacement,
+      user_prefs::PrefRegistrySyncable::UNSYNCABLE_PREF);
   registry->RegisterBooleanPref(
       prefs::kImportBookmarks,
       true,
       user_prefs::PrefRegistrySyncable::UNSYNCABLE_PREF);
   registry->RegisterBooleanPref(
       prefs::kImportHistory,
       true,
       user_prefs::PrefRegistrySyncable::UNSYNCABLE_PREF);
   registry->RegisterBooleanPref(
       prefs::kImportHomepage,
@@ skipping 39 matching lines @@
       false,
       user_prefs::PrefRegistrySyncable::UNSYNCABLE_PREF);
 #if !defined(OS_MACOSX)
   registry->RegisterBooleanPref(
       prefs::kFullscreenAllowed,
       true,
       user_prefs::PrefRegistrySyncable::UNSYNCABLE_PREF);
 #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