Introduce ChromeZoomLevelPref, make zoom level prefs independent of profile prefs.

chrome/browser/profiles/host_zoom_map_browsertest.cc

Index: chrome/browser/profiles/host_zoom_map_browsertest.cc
diff --git a/chrome/browser/profiles/host_zoom_map_browsertest.cc b/chrome/browser/profiles/host_zoom_map_browsertest.cc
index e14a579048bbc586f142f9d74482c5069cd71ffa..aa95d8f1785140ef49c5b0c04fb20ec753612620 100644
--- a/chrome/browser/profiles/host_zoom_map_browsertest.cc
+++ b/chrome/browser/profiles/host_zoom_map_browsertest.cc
@@ -75,7 +75,7 @@ class HostZoomMapBrowserTest : public InProcessBrowserTest {
 
  protected:
   void SetDefaultZoomLevel(double level) {
-    browser()->profile()->GetPrefs()->SetDouble(
+    browser()->profile()->GetZoomLevelPrefs()->SetDouble(
         prefs::kDefaultZoomLevel, level);
   }
 
@@ -102,7 +102,7 @@ class HostZoomMapBrowserTest : public InProcessBrowserTest {
   }
 
   std::vector<std::string> GetHostsWithZoomLevelsFromPrefs() {
-    PrefService* prefs = browser()->profile()->GetPrefs();
+    PrefService* prefs = browser()->profile()->GetZoomLevelPrefs();
     const base::DictionaryValue* values =
         prefs->GetDictionary(prefs::kPerHostZoomLevels);
     std::vector<std::string> results;
@@ -137,35 +137,50 @@ class HostZoomMapBrowserTest : public InProcessBrowserTest {
   DISALLOW_COPY_AND_ASSIGN(HostZoomMapBrowserTest);
 };
 
-class HostZoomMapSanitizationBrowserTest : public HostZoomMapBrowserTest {
+class HostZoomMapBrowserTestWithPrefs : public HostZoomMapBrowserTest {
  public:
-  HostZoomMapSanitizationBrowserTest() {}
+  HostZoomMapBrowserTestWithPrefs() {}
+
+ protected:
+  std::string prefs_data_;
+  std::string prefs_filename_;

[Peter Kasting, 2014/09/11 00:45:47]

Nit: Rather than making these protected, have the constructor set them.  (The
Google style guide discourages protected members, and you only need them
protected so the subclass constructors can write to them.)

[wjmaclean, 2014/09/11 21:00:51]

Done.

 
  private:
   // InProcessBrowserTest:
   virtual bool SetUpUserDataDirectory() OVERRIDE {
-    // Zoom-related preferences demonstrating the two problems that could be
-    // caused by the bug. They incorrectly contain a per-host zoom level for the
-    // empty host; and a value for 'host1' that only differs from the default by
-    // epsilon. Neither should have been persisted.
-    const char kBrokenPrefs[] =
-        "{'profile': {"
-        "   'default_zoom_level': 1.2,"
-        "   'per_host_zoom_levels': {'': 1.1, 'host1': 1.20001, 'host2': 1.3}"
-        "}}";
-    std::string broken_prefs(kBrokenPrefs);
-    std::replace(broken_prefs.begin(), broken_prefs.end(), '\'', '\"');
+    std::replace(prefs_data_.begin(), prefs_data_.end(), '\'', '\"');
 
     base::FilePath user_data_directory, path_to_prefs;
     PathService::Get(chrome::DIR_USER_DATA, &user_data_directory);
     path_to_prefs = user_data_directory
         .AppendASCII(TestingProfile::kTestUserProfileDir)
-        .Append(chrome::kPreferencesFilename);
+        .Append(prefs_filename_);
     base::CreateDirectory(path_to_prefs.DirName());
-    base::WriteFile(path_to_prefs, broken_prefs.c_str(), broken_prefs.size());
+    base::WriteFile(
+        path_to_prefs, prefs_data_.c_str(), prefs_data_.size());
     return true;
   }
 
+  DISALLOW_COPY_AND_ASSIGN(HostZoomMapBrowserTestWithPrefs);
+};
+
+class HostZoomMapSanitizationBrowserTest
+    : public HostZoomMapBrowserTestWithPrefs {
+ public:
+  HostZoomMapSanitizationBrowserTest() {
+    // Zoom-related preferences demonstrating the two problems that could be
+    // caused by the bug. They incorrectly contain a per-host zoom level for the
+    // empty host; and a value for 'host1' that only differs from the default by
+    // epsilon. Neither should have been persisted.
+    prefs_data_ =
+        "{'partition': {"
+        "   'default_zoom_level': 1.2,"
+        "   'per_host_zoom_levels': {'': 1.1, 'host1': 1.20001, 'host2': 1.3}"
+        "}}";
+    prefs_filename_ = chrome::kZoomLevelPreferencesFilename;
+  }
+
+ private:
   DISALLOW_COPY_AND_ASSIGN(HostZoomMapSanitizationBrowserTest);
 };
 
@@ -216,3 +231,89 @@ IN_PROC_BROWSER_TEST_F(HostZoomMapSanitizationBrowserTest, ClearOnStartup) {
   EXPECT_THAT(GetHostsWithZoomLevels(), testing::ElementsAre("host2"));
   EXPECT_THAT(GetHostsWithZoomLevelsFromPrefs(), testing::ElementsAre("host2"));
 }
+
+class HostZoomMapMigrationBrowserTest : public HostZoomMapBrowserTestWithPrefs {
+ public:
+  HostZoomMapMigrationBrowserTest() {
+    // In this case we migrate the zoom level data from the profile prefs.
+    prefs_data_ =
+        "{'profile': {"
+        "   'default_zoom_level': 1.2,"
+        "   'per_host_zoom_levels': {'': 1.1, 'host1': 1.20001, 'host2': 1.3}"
+        "}}";
+    prefs_filename_ = chrome::kPreferencesFilename;
+  }
+
+  static const double kOriginalDefaultZoomLevel;
+
+ private:
+  DISALLOW_COPY_AND_ASSIGN(HostZoomMapMigrationBrowserTest);
+};
+
+const double HostZoomMapMigrationBrowserTest::kOriginalDefaultZoomLevel = 1.2;
+
+// This test is the same as HostZoomMapSanitizationBrowserTest, except that the
+// zoom level data is loaded from the profile prefs, transfered to the
+// zoom-level prefs, and we verify that the profile zoom level prefs are
+// erased in the process. We also test that changes to the host zoom map and the
+// default zoom level don't propagate back to the profile prefs.
+IN_PROC_BROWSER_TEST_F(HostZoomMapMigrationBrowserTest,
+                       MigrateProfileZoomPreferences) {
+  EXPECT_THAT(GetHostsWithZoomLevels(), testing::ElementsAre("host2"));
+  EXPECT_THAT(GetHostsWithZoomLevelsFromPrefs(), testing::ElementsAre("host2"));
+
+  PrefService* profile_prefs =
+      browser()->profile()->GetPrefs();
+  PrefService* zoom_level_prefs = browser()->profile()->GetZoomLevelPrefs();
+  // Make sure that the profile pref for default zoom level has been set to
+  // its default value of 0.0.
+  EXPECT_EQ(0.0, profile_prefs->GetDouble(prefs::kProfileDefaultZoomLevel));
+  EXPECT_EQ(kOriginalDefaultZoomLevel,
+            zoom_level_prefs->GetDouble(prefs::kDefaultZoomLevel));
+
+  // Make sure that the profile prefs for per-host zoom levels are erased.
+  {
+    const base::DictionaryValue* profile_host_zoom_dictionary =
+        profile_prefs->GetDictionary(prefs::kProfilePerHostZoomLevels);
+    EXPECT_EQ(0UL, profile_host_zoom_dictionary->size());
+  }
+
+  ZoomLevelChangeObserver observer(browser()->profile());
+  content::HostZoomMap* host_zoom_map = static_cast<content::HostZoomMap*>(
+      content::HostZoomMap::GetDefaultForBrowserContext(
+          browser()->profile()));
+
+  // Make sure that a change to a host zoom level doesn't propagate to the
+  // profile prefs.
+  std::string host3("host3");
+  host_zoom_map->SetZoomLevelForHost(host3, 1.3);
+  observer.BlockUntilZoomLevelForHostHasChanged(host3);
+  EXPECT_THAT(GetHostsWithZoomLevelsFromPrefs(),
+              testing::ElementsAre("host2", host3));
+  {
+    const base::DictionaryValue* profile_host_zoom_dictionary =
+        profile_prefs->GetDictionary(prefs::kProfilePerHostZoomLevels);
+    EXPECT_EQ(0UL, profile_host_zoom_dictionary->size());
+  }
+
+  // Make sure a change to the default zoom level doesn't propagate to the
+  // profile prefs.
+
+  // First, we need a host at the default zoom level to respond when the
+  // default zoom level changes.
+  const double kNewDefaultZoomLevel = 1.5;
+  GURL test_url = ConstructTestServerURL("http://host4:%d/");
+  ui_test_utils::NavigateToURL(browser(), test_url);
+  EXPECT_TRUE(content::ZoomValuesEqual(kOriginalDefaultZoomLevel,
+                                       GetZoomLevel(test_url)));
+
+  // Change the default zoom level and observe.
+  zoom_level_prefs->SetDouble(prefs::kDefaultZoomLevel, kNewDefaultZoomLevel);
+  observer.BlockUntilZoomLevelForHostHasChanged(test_url.host());
+  EXPECT_TRUE(
+      content::ZoomValuesEqual(kNewDefaultZoomLevel, GetZoomLevel(test_url)));
+  EXPECT_EQ(kNewDefaultZoomLevel,
+            zoom_level_prefs->GetDouble(prefs::kDefaultZoomLevel));
+  EXPECT_EQ(0.0, profile_prefs->GetDouble(prefs::kProfileDefaultZoomLevel));
+}
+