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

chrome/browser/profiles/host_zoom_map_browsertest.cc

 // Copyright 2014 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 "content/public/browser/host_zoom_map.h"
 
 #include <algorithm>
 #include <string>
 #include <vector>
 
@@ skipping 57 matching lines @@
 };
 
 }  // namespace
 
 class HostZoomMapBrowserTest : public InProcessBrowserTest {
  public:
   HostZoomMapBrowserTest() {}
 
  protected:
   void SetDefaultZoomLevel(double level) {
-    browser()->profile()->GetPrefs()->SetDouble(
+    browser()->profile()->GetZoomLevelPrefs()->SetDouble(
         prefs::kDefaultZoomLevel, level);
   }
 
   double GetZoomLevel(const GURL& url) {
     content::HostZoomMap* host_zoom_map = static_cast<content::HostZoomMap*>(
         content::HostZoomMap::GetDefaultForBrowserContext(
             browser()->profile()));
     return host_zoom_map->GetZoomLevelForHostAndScheme(url.scheme(),
                                                        url.host());
   }
 
   std::vector<std::string> GetHostsWithZoomLevels() {
     typedef content::HostZoomMap::ZoomLevelVector ZoomLevelVector;
     content::HostZoomMap* host_zoom_map = static_cast<content::HostZoomMap*>(
         content::HostZoomMap::GetDefaultForBrowserContext(
             browser()->profile()));
     content::HostZoomMap::ZoomLevelVector zoom_levels =
         host_zoom_map->GetAllZoomLevels();
     std::vector<std::string> results;
     for (ZoomLevelVector::const_iterator it = zoom_levels.begin();
          it != zoom_levels.end(); ++it)
       results.push_back(it->host);
     return results;
   }
 
   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;
     if (values) {
       for (base::DictionaryValue::Iterator it(*values);
            !it.IsAtEnd(); it.Advance())
         results.push_back(it.key());
     }
     return results;
   }
@@ skipping 14 matching lines @@
   virtual void SetUpOnMainThread() OVERRIDE {
     ASSERT_TRUE(embedded_test_server()->InitializeAndWaitUntilReady());
     embedded_test_server()->RegisterRequestHandler(base::Bind(
         &HostZoomMapBrowserTest::HandleRequest, base::Unretained(this)));
     host_resolver()->AddRule("*", "127.0.0.1");
   }
 
   DISALLOW_COPY_AND_ASSIGN(HostZoomMapBrowserTest);
 };
 
-class HostZoomMapSanitizationBrowserTest : public HostZoomMapBrowserTest {
+class HostZoomMapBrowserTestWithPrefs : public HostZoomMapBrowserTest {
  public:
-  HostZoomMapSanitizationBrowserTest() {}
+  HostZoomMapBrowserTestWithPrefs(std::string prefs_data,

[Bernhard Bauer, 2014/09/12 10:28:49]

Pass the strings by (const)reference?

[wjmaclean, 2014/09/16 18:44:44]

Done.

+                                  std::string prefs_filename)
+      : prefs_data_(prefs_data), prefs_filename_(prefs_filename) {}
 
  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;
   }
 
+  std::string prefs_data_;
+  std::string prefs_filename_;
+
+  DISALLOW_COPY_AND_ASSIGN(HostZoomMapBrowserTestWithPrefs);
+};
+
+class HostZoomMapSanitizationBrowserTest
+    : public HostZoomMapBrowserTestWithPrefs {
+ public:
+  HostZoomMapSanitizationBrowserTest()
+      :  // Zoom-related preferences demonstrating the two problems that could

[Bernhard Bauer, 2014/09/12 10:28:49]

Indentation / line breaks are off here. Also, do you actually need to squeeze
the comment in between the class declaration and its superclass? You could put
it e.g. before the actual string.

[wjmaclean, 2014/09/16 18:44:44]

Done.

+         // 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.
+        HostZoomMapBrowserTestWithPrefs(
+            "{'partition': {"
+            "   'default_zoom_level': 1.2,"
+            "   'per_host_zoom_levels': {'': 1.1, 'host1': 1.20001, 'host2': "
+            "1.3}"
+            "}}",
+            chrome::kZoomLevelPreferencesFilename) {}
+
+ private:
   DISALLOW_COPY_AND_ASSIGN(HostZoomMapSanitizationBrowserTest);
 };
 
 // Regression test for crbug.com/364399.
 IN_PROC_BROWSER_TEST_F(HostZoomMapBrowserTest, ToggleDefaultZoomLevel) {
   const double default_zoom_level = content::ZoomFactorToZoomLevel(1.5);
 
   const char kTestURLTemplate1[] = "http://host1:%d/";
   const char kTestURLTemplate2[] = "http://host2:%d/";
 
@@ skipping 30 matching lines @@
   // any per-host values saved either to Pref, or internally in HostZoomMap.
   EXPECT_TRUE(GetHostsWithZoomLevels().empty());
   EXPECT_TRUE(GetHostsWithZoomLevelsFromPrefs().empty());
 }
 
 // Test that garbage data from crbug.com/364399 is cleared up on startup.
 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.
+        HostZoomMapBrowserTestWithPrefs(
+            "{'profile': {"
+            "   'default_zoom_level': 1.2,"
+            "   'per_host_zoom_levels': {'': 1.1, 'host1': 1.20001, 'host2': "
+            "1.3}"
+            "}}",
+            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));
+}
+