Remove user-related data from local_state and add to user_preferences, in preparation for multi-profile.

devtools
  kDevToolsSplitLocation

browser
  kBrowserWindowPlacement

Also add a method to browser_prefs to delete obsolete preferences from local state, and fix all related tests.

BUG=66717
TEST=all browser, interactive, and ui tests work properly

Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=72153
Review URL: http://codereview.chromium.org/5915006

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

3 files changed, 127 insertions, 13 deletions

diff --git a/chrome/browser/prefs/browser_prefs.cc b/chrome/browser/prefs/browser_prefs.cc
index 452da3f..80d6477 100644
--- a/chrome/browser/prefs/browser_prefs.cc
+++ b/chrome/browser/prefs/browser_prefs.cc
@@ -53,6 +53,7 @@
 #include "chrome/browser/translate/translate_prefs.h"
 #include "chrome/browser/ui/browser.h"
 #include "chrome/browser/upgrade_detector.h"
+#include "chrome/common/pref_names.h"
 
 #if defined(TOOLKIT_VIEWS)  // TODO(port): whittle this down as we port
 #include "chrome/browser/ui/views/browser_actions_container.h"
@@ -155,4 +156,35 @@ void RegisterUserPrefs(PrefService* user_prefs) {
   policy::ProfilePolicyContext::RegisterUserPrefs(user_prefs);
 }
 
+void MigrateBrowserPrefs(PrefService* user_prefs, PrefService* local_state) {
+  // Copy pref values which have been migrated to user_prefs from local_state,
+  // or remove them from local_state outright, if copying is not required.
+  int current_version =
+      local_state->GetInteger(prefs::kMultipleProfilePrefMigration);
+
+  if ((current_version & WINDOWS_PREFS) == 0) {
+    // Migrate the devtools split location preference.
+    local_state->RegisterIntegerPref(prefs::kDevToolsSplitLocation, -1);
+    DCHECK(user_prefs->FindPreference(prefs::kDevToolsSplitLocation));
+    if (local_state->HasPrefPath(prefs::kDevToolsSplitLocation)) {
+      user_prefs->SetInteger(prefs::kDevToolsSplitLocation,
+          local_state->GetInteger(prefs::kDevToolsSplitLocation));
+    }
+    local_state->ClearPref(prefs::kDevToolsSplitLocation);
+
+    // Migrate the browser window placement preference.
+    local_state->RegisterDictionaryPref(prefs::kBrowserWindowPlacement);
+    DCHECK(user_prefs->FindPreference(prefs::kBrowserWindowPlacement));
+    if (local_state->HasPrefPath(prefs::kBrowserWindowPlacement)) {
+      user_prefs->Set(prefs::kBrowserWindowPlacement,
+          *(local_state->FindPreference(prefs::kBrowserWindowPlacement)->
+              GetValue()->DeepCopy()));
+    }
+    local_state->ClearPref(prefs::kBrowserWindowPlacement);
+
+    local_state->SetInteger(prefs::kMultipleProfilePrefMigration,
+                            current_version | WINDOWS_PREFS);
+  }
+}
+
 }  // namespace browser
diff --git a/chrome/browser/prefs/browser_prefs.h b/chrome/browser/prefs/browser_prefs.h
index ef109a4..4df26c1 100644
--- a/chrome/browser/prefs/browser_prefs.h
+++ b/chrome/browser/prefs/browser_prefs.h
@@ -1,4 +1,4 @@
-// Copyright (c) 2006-2008 The Chromium Authors. All rights reserved.
+// Copyright (c) 2011 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.
 
@@ -10,11 +10,19 @@ class PrefService;
 
 namespace browser {
 
+// Bitmask for kMultipleProfilePrefMigration.
+enum MigratedPreferences {
+  NO_PREFS = 0,
+  DNS_PREFS = 1 << 0,
+  WINDOWS_PREFS = 1 << 1,
+};
+
 // Makes the PrefService objects aware of all the prefs.
 void RegisterAllPrefs(PrefService* user_prefs, PrefService* local_state);
 void RegisterLocalState(PrefService* local_state);
 void RegisterUserPrefs(PrefService* user_prefs);
-
+// Migrate prefs from local_state to user_prefs.
+void MigrateBrowserPrefs(PrefService* user_prefs, PrefService* local_state);
 } // namespace browser
 
 #endif  // CHROME_BROWSER_PREFS_BROWSER_PREFS_H__
diff --git a/chrome/browser/prefs/pref_service_uitest.cc b/chrome/browser/prefs/pref_service_uitest.cc
index 9712481..c6540d74 100644
--- a/chrome/browser/prefs/pref_service_uitest.cc
+++ b/chrome/browser/prefs/pref_service_uitest.cc
@@ -1,4 +1,4 @@
-// Copyright (c) 2009 The Chromium Authors. All rights reserved.
+// Copyright (c) 2011 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.
 
@@ -29,17 +29,27 @@ class PreferenceServiceTest : public UITest {
     file_util::Delete(tmp_profile_, true);
     file_util::CreateDirectory(tmp_profile_);
 
-    FilePath reference_pref_file =
-        test_data_directory_
+    FilePath reference_pref_file;
+    if (new_profile_) {
+      reference_pref_file = test_data_directory_
+          .AppendASCII("profiles")
+          .AppendASCII("window_placement")
+          .AppendASCII("Default")
+          .Append(chrome::kPreferencesFilename);
+      tmp_pref_file_ = tmp_profile_.AppendASCII("Default");
+      ASSERT_TRUE(file_util::CreateDirectory(tmp_pref_file_));
+      tmp_pref_file_ = tmp_pref_file_.Append(chrome::kPreferencesFilename);
+    } else {
+      reference_pref_file = test_data_directory_
             .AppendASCII("profiles")
             .AppendASCII("window_placement")
             .Append(chrome::kLocalStateFilename);
-
-    tmp_pref_file_ = tmp_profile_.Append(chrome::kLocalStateFilename);
+      tmp_pref_file_ = tmp_profile_.Append(chrome::kLocalStateFilename);
+    }
 
     ASSERT_TRUE(file_util::PathExists(reference_pref_file));
-
-    // Copy only the Local State file, the rest will be automatically created
+    // Copy only the Preferences file if |new_profile_|, or Local State if not,
+    // and the rest will be automatically created.
     ASSERT_TRUE(file_util::CopyFile(reference_pref_file, tmp_pref_file_));
 
 #if defined(OS_WIN)
@@ -66,20 +76,21 @@ class PreferenceServiceTest : public UITest {
   }
 
  public:
+  bool new_profile_;
   FilePath tmp_pref_file_;
   FilePath tmp_profile_;
 };
 
-#if defined(OS_WIN)
+#if !defined(OS_LINUX)
 // This test verifies that the window position from the prefs file is restored
 // when the app restores.  This doesn't really make sense on Linux, where
 // the window manager might fight with you over positioning.  However, we
 // might be able to make this work on buildbots.
-// Also, not sure what should happen on the mac.  In any case, the code below
-// (minus the Windows bits) compiles fine on my Linux box now.
 // TODO(port): revisit this.
 TEST_F(PreferenceServiceTest, PreservedWindowPlacementIsLoaded) {
-  // The window should open with the reference profile
+  // The window should open with the new reference profile, with window
+  // placement values stored in the user data directory.
+  new_profile_ = true;
   ASSERT_TRUE(LaunchAppWithProfile());
 
   ASSERT_TRUE(file_util::PathExists(tmp_pref_file_));
@@ -132,3 +143,66 @@ TEST_F(PreferenceServiceTest, PreservedWindowPlacementIsLoaded) {
   EXPECT_EQ(is_maximized, is_window_maximized);
 }
 #endif
+
+#if !defined(OS_LINUX)
+TEST_F(PreferenceServiceTest, PreservedWindowPlacementIsMigrated) {
+  // The window should open with the old reference profile, with window
+  // placement values stored in Local State.
+  new_profile_ = false;
+  ASSERT_TRUE(LaunchAppWithProfile());
+
+  ASSERT_TRUE(file_util::PathExists(tmp_pref_file_));
+
+  JSONFileValueSerializer deserializer(tmp_pref_file_);
+  scoped_ptr<Value> root(deserializer.Deserialize(NULL, NULL));
+
+  ASSERT_TRUE(root.get());
+  ASSERT_TRUE(root->IsType(Value::TYPE_DICTIONARY));
+
+  // Retrieve the screen rect for the launched window
+  scoped_refptr<BrowserProxy> browser(automation()->GetBrowserWindow(0));
+  ASSERT_TRUE(browser.get());
+  scoped_refptr<WindowProxy> window(browser->GetWindow());
+  ASSERT_TRUE(window.get());
+
+  gfx::Rect bounds;
+  ASSERT_TRUE(window->GetBounds(&bounds));
+
+  // Values from old reference profile in Local State should have been
+  // correctly migrated to the user's Preferences -- if so, the window
+  // should be set to values taken from the user's Local State.
+  DictionaryValue* root_dict = static_cast<DictionaryValue*>(root.get());
+
+  // Retrieve the expected rect values from User Preferences, where they
+  // should have been migrated from Local State.
+  int bottom = 0;
+  std::string kBrowserWindowPlacement(prefs::kBrowserWindowPlacement);
+  EXPECT_TRUE(root_dict->GetInteger(kBrowserWindowPlacement + ".bottom",
+      &bottom));
+  EXPECT_EQ(bottom, bounds.y() + bounds.height());
+
+  int top = 0;
+  EXPECT_TRUE(root_dict->GetInteger(kBrowserWindowPlacement + ".top",
+      &top));
+  EXPECT_EQ(top, bounds.y());
+
+  int left = 0;
+  EXPECT_TRUE(root_dict->GetInteger(kBrowserWindowPlacement + ".left",
+      &left));
+  EXPECT_EQ(left, bounds.x());
+
+  int right = 0;
+  EXPECT_TRUE(root_dict->GetInteger(kBrowserWindowPlacement + ".right",
+      &right));
+  EXPECT_EQ(right, bounds.x() + bounds.width());
+
+  // Find if launched window is maximized.
+  bool is_window_maximized = false;
+  ASSERT_TRUE(window->IsMaximized(&is_window_maximized));
+  bool is_maximized = false;
+  EXPECT_TRUE(root_dict->GetBoolean(kBrowserWindowPlacement + ".maximized",
+      &is_maximized));
+  EXPECT_EQ(is_maximized, is_window_maximized);
+}
+#endif
+