Revert "Misc. cleanup to browser_theme_pack_unittest.cc; no functional change."

This reverts commit ec2cd434.

Reason for revert: Reverting in hopes of fixing unit_tests on chromeos bot. This one will hopefully take. Sorry for so many reverts here. The chain of patches made it difficult.

Original change's description:
> Misc. cleanup to browser_theme_pack_unittest.cc; no functional change.
> 
> * Add GetDefaultColor() mapping function to make it easy to fix DCHECKs due to
>   this code using raw _INCOGNITO color constants but
>   ThemeProperties::GetDefaultColor() not expecting them.  This is the change
>   that led me to get annoyed and do all the other stuff here.
> * Convert typedefs to type aliases.
> * Add TP = ThemeProperties type alias for brevity.
> * Use =default.
> * Mark functions that don't touch member variables "static".
> * Fix wrapping issues.
> * Because the class here is so long, split it into declaration and definition
>   pieces so it's easier to see what's the API and what's helper functions.
> * Make the API protected; make helpers private.
> * Make data members private.  Add a const accessor for the one member tests use.
> 
> Bug: none
> Change-Id: I4cc058bb29747cfa84835edbc5ce765401e14527
> Reviewed-on: https://chromium-review.googlesource.com/1152108
> Reviewed-by: Evan Stade <estade@chromium.org>
> Commit-Queue: Peter Kasting <pkasting@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#578775}

TBR=pkasting@chromium.org,estade@chromium.org

Change-Id: I2407b13ff9c69c240bb7479e21a515c17c1388b7
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: none
Reviewed-on: https://chromium-review.googlesource.com/1153991Reviewed-by: Scott Violet <sky@chromium.org>
Commit-Queue: Scott Violet <sky@chromium.org>
Cr-Commit-Position: refs/heads/master@{#578862}

chrome/browser/themes/browser_theme_pack_unittest.cc

@@ -23,178 +23,131 @@
 #include "ui/gfx/image/image_skia_rep.h"
 
 using extensions::Extension;
-using TP = ThemeProperties;
 
 // Maps scale factors (enum values) to file path.
 // A similar typedef in BrowserThemePack is private.
-using TestScaleFactorToFileMap = std::map<ui::ScaleFactor, base::FilePath>;
+typedef std::map<ui::ScaleFactor, base::FilePath> TestScaleFactorToFileMap;
 
 // Maps image ids to maps of scale factors to file paths.
 // A similar typedef in BrowserThemePack is private.
-using TestFilePathMap = std::map<int, TestScaleFactorToFileMap>;
-
-// BrowserThemePackTest --------------------------------------------------------
+typedef std::map<int, TestScaleFactorToFileMap> TestFilePathMap;
 
 class BrowserThemePackTest : public ::testing::Test {
  public:
-  BrowserThemePackTest();
-  ~BrowserThemePackTest() override = default;
-
- protected:
-  // Returns a mapping from each COLOR_* constant to the default value for this
-  // constant. Callers get this map, and then modify expected values and then
-  // run the resulting thing through VerifyColorMap().
-  static std::map<int, SkColor> GetDefaultColorMap();
-
-  void VerifyColorMap(const std::map<int, SkColor>& color_map);
-  void LoadColorJSON(const std::string& json);
-  void LoadColorDictionary(base::DictionaryValue* value);
-  void LoadTintJSON(const std::string& json);
-  void LoadTintDictionary(base::DictionaryValue* value);
-  void LoadDisplayPropertiesJSON(const std::string& json);
-  void LoadDisplayPropertiesDictionary(base::DictionaryValue* value);
-  void ParseImageNamesJSON(const std::string& json,
-                           TestFilePathMap* out_file_paths);
-  void ParseImageNamesDictionary(base::DictionaryValue* value,
-                                 TestFilePathMap* out_file_paths);
-  bool LoadRawBitmapsTo(const TestFilePathMap& out_file_paths);
-
-  // This function returns void in order to be able use ASSERT_...
-  // The BrowserThemePack is returned in |pack|.
-  static void BuildFromUnpackedExtension(const base::FilePath& extension_path,
-                                         scoped_refptr<BrowserThemePack>* pack);
-
-  static base::FilePath GetStarGazingPath();
-  static base::FilePath GetHiDpiThemePath();
-
-  // Verifies the data in star gazing. We do this multiple times for different
-  // BrowserThemePack objects to make sure it works in generated and mmapped
-  // mode correctly.
-  static void VerifyStarGazing(BrowserThemePack* pack);
-
-  static void VerifyHiDpiTheme(BrowserThemePack* pack);
-
-  const BrowserThemePack& theme_pack() const { return *theme_pack_; }
-
- private:
-  using ScopedSetSupportedScaleFactors =
-      std::unique_ptr<ui::test::ScopedSetSupportedScaleFactors>;
-
-  // Transformation for link underline colors.
-  static SkColor BuildThirdOpacity(SkColor color_link);
-
-  // Returns the appropriate default color for |id|.
-  static SkColor GetDefaultColor(int id);
-
-  static void GenerateDefaultFrameColor(std::map<int, SkColor>* colors,
-                                        int color,
-                                        int tint,
-                                        bool otr);
-
-  ScopedSetSupportedScaleFactors scoped_set_supported_scale_factors_;
-
-  content::TestBrowserThreadBundle thread_bundle_;
-  scoped_refptr<BrowserThemePack> theme_pack_;
-};
-
-BrowserThemePackTest::BrowserThemePackTest()
-    : theme_pack_(new BrowserThemePack()) {
+  BrowserThemePackTest() : theme_pack_(new BrowserThemePack()) {
     std::vector<ui::ScaleFactor> scale_factors;
     scale_factors.push_back(ui::SCALE_FACTOR_100P);
     scale_factors.push_back(ui::SCALE_FACTOR_200P);
     scoped_set_supported_scale_factors_.reset(
         new ui::test::ScopedSetSupportedScaleFactors(scale_factors));
-}
+  }
+  ~BrowserThemePackTest() override {}
+
+  // Transformation for link underline colors.
+  SkColor BuildThirdOpacity(SkColor color_link) {
+    return SkColorSetA(color_link, SkColorGetA(color_link) / 3);
+  }
+
+  void GenerateDefaultFrameColor(std::map<int, SkColor>* colors,
+                                 int color, int tint, bool otr) {
+    (*colors)[color] = HSLShift(
+        ThemeProperties::GetDefaultColor(ThemeProperties::COLOR_FRAME, false),
+        ThemeProperties::GetDefaultTint(tint, otr));
+  }
 
-// static
-std::map<int, SkColor> BrowserThemePackTest::GetDefaultColorMap() {
+  // Returns a mapping from each COLOR_* constant to the default value for this
+  // constant. Callers get this map, and then modify expected values and then
+  // run the resulting thing through VerifyColorMap().
+  std::map<int, SkColor> GetDefaultColorMap() {
     std::map<int, SkColor> colors;
-  GenerateDefaultFrameColor(&colors, TP::COLOR_FRAME, TP::TINT_FRAME, false);
-  GenerateDefaultFrameColor(&colors, TP::COLOR_FRAME_INACTIVE,
-                            TP::TINT_FRAME_INACTIVE, false);
-  GenerateDefaultFrameColor(&colors, TP::COLOR_FRAME_INCOGNITO, TP::TINT_FRAME,
-                            true);
-  GenerateDefaultFrameColor(&colors, TP::COLOR_FRAME_INCOGNITO_INACTIVE,
-                            TP::TINT_FRAME_INACTIVE, true);
+    GenerateDefaultFrameColor(&colors, ThemeProperties::COLOR_FRAME,
+                              ThemeProperties::TINT_FRAME, false);
+    GenerateDefaultFrameColor(&colors,
+                              ThemeProperties::COLOR_FRAME_INACTIVE,
+                              ThemeProperties::TINT_FRAME_INACTIVE, false);
+    GenerateDefaultFrameColor(&colors,
+                              ThemeProperties::COLOR_FRAME_INCOGNITO,
+                              ThemeProperties::TINT_FRAME, true);
+    GenerateDefaultFrameColor(
+        &colors,
+        ThemeProperties::COLOR_FRAME_INCOGNITO_INACTIVE,
+        ThemeProperties::TINT_FRAME_INACTIVE, true);
 
     // For the rest, use default colors.
-  for (int i = TP::COLOR_FRAME_INCOGNITO_INACTIVE + 1;
-       i <= TP::COLOR_BUTTON_BACKGROUND; ++i) {
-    colors[i] = GetDefaultColor(i);
+    for (int i = ThemeProperties::COLOR_FRAME_INCOGNITO_INACTIVE + 1;
+         i <= ThemeProperties::COLOR_BUTTON_BACKGROUND; ++i) {
+      colors[i] = ThemeProperties::GetDefaultColor(i, false);
     }
 
     return colors;
-}
+  }
 
-void BrowserThemePackTest::VerifyColorMap(
-    const std::map<int, SkColor>& color_map) {
+  void VerifyColorMap(const std::map<int, SkColor>& color_map) {
     for (std::map<int, SkColor>::const_iterator it = color_map.begin();
          it != color_map.end(); ++it) {
       SkColor color;
       if (!theme_pack_->GetColor(it->first, &color))
-      color = GetDefaultColor(it->first);
+        color = ThemeProperties::GetDefaultColor(it->first, false);
       EXPECT_EQ(it->second, color) << "Color id = " << it->first;
     }
-}
+  }
 
-void BrowserThemePackTest::LoadColorJSON(const std::string& json) {
+  void LoadColorJSON(const std::string& json) {
     std::unique_ptr<base::Value> value = base::JSONReader::Read(json);
     ASSERT_TRUE(value->is_dict());
     LoadColorDictionary(static_cast<base::DictionaryValue*>(value.get()));
-}
+  }
 
-void BrowserThemePackTest::LoadColorDictionary(base::DictionaryValue* value) {
+  void LoadColorDictionary(base::DictionaryValue* value) {
     theme_pack_->BuildColorsFromJSON(value);
-}
+  }
 
-void BrowserThemePackTest::LoadTintJSON(const std::string& json) {
+  void LoadTintJSON(const std::string& json) {
     std::unique_ptr<base::Value> value = base::JSONReader::Read(json);
     ASSERT_TRUE(value->is_dict());
     LoadTintDictionary(static_cast<base::DictionaryValue*>(value.get()));
-}
+  }
 
-void BrowserThemePackTest::LoadTintDictionary(base::DictionaryValue* value) {
+  void LoadTintDictionary(base::DictionaryValue* value) {
     theme_pack_->BuildTintsFromJSON(value);
-}
+  }
 
-void BrowserThemePackTest::LoadDisplayPropertiesJSON(const std::string& json) {
+  void LoadDisplayPropertiesJSON(const std::string& json) {
     std::unique_ptr<base::Value> value = base::JSONReader::Read(json);
     ASSERT_TRUE(value->is_dict());
     LoadDisplayPropertiesDictionary(
         static_cast<base::DictionaryValue*>(value.get()));
-}
+  }
 
-void BrowserThemePackTest::LoadDisplayPropertiesDictionary(
-    base::DictionaryValue* value) {
+  void LoadDisplayPropertiesDictionary(base::DictionaryValue* value) {
     theme_pack_->BuildDisplayPropertiesFromJSON(value);
-}
+  }
 
-void BrowserThemePackTest::ParseImageNamesJSON(
-    const std::string& json,
+  void ParseImageNamesJSON(const std::string& json,
                            TestFilePathMap* out_file_paths) {
     std::unique_ptr<base::Value> value = base::JSONReader::Read(json);
     ASSERT_TRUE(value->is_dict());
     ParseImageNamesDictionary(static_cast<base::DictionaryValue*>(value.get()),
                               out_file_paths);
-}
+  }
 
-void BrowserThemePackTest::ParseImageNamesDictionary(
+  void ParseImageNamesDictionary(
       base::DictionaryValue* value,
       TestFilePathMap* out_file_paths) {
-  theme_pack_->ParseImageNamesFromJSON(value, base::FilePath(), out_file_paths);
+    theme_pack_->ParseImageNamesFromJSON(value, base::FilePath(),
+                                         out_file_paths);
 
     // Build the source image list for HasCustomImage().
     theme_pack_->BuildSourceImagesArray(*out_file_paths);
-}
+  }
 
-bool BrowserThemePackTest::LoadRawBitmapsTo(
-    const TestFilePathMap& out_file_paths) {
+  bool LoadRawBitmapsTo(const TestFilePathMap& out_file_paths) {
     return theme_pack_->LoadRawBitmapsTo(out_file_paths, &theme_pack_->images_);
-}
+  }
 
-// static
-void BrowserThemePackTest::BuildFromUnpackedExtension(
-    const base::FilePath& extension_path,
+  // This function returns void in order to be able use ASSERT_...
+  // The BrowserThemePack is returned in |pack|.
+  void BuildFromUnpackedExtension(const base::FilePath& extension_path,
                                   scoped_refptr<BrowserThemePack>* pack) {
     base::FilePath manifest_path = extension_path.AppendASCII("manifest.json");
     std::string error;
@@ -203,20 +156,22 @@ void BrowserThemePackTest::BuildFromUnpackedExtension(
         base::DictionaryValue::From(deserializer.Deserialize(NULL, &error));
     EXPECT_EQ("", error);
     ASSERT_TRUE(valid_value.get());
-  scoped_refptr<Extension> extension(
-      Extension::Create(extension_path, extensions::Manifest::INVALID_LOCATION,
-                        *valid_value, Extension::REQUIRE_KEY, &error));
+    scoped_refptr<Extension> extension(Extension::Create(
+        extension_path, extensions::Manifest::INVALID_LOCATION, *valid_value,
+        Extension::REQUIRE_KEY, &error));
     ASSERT_TRUE(extension.get());
     ASSERT_EQ("", error);
     *pack = new BrowserThemePack;
     BrowserThemePack::BuildFromExtension(extension.get(), *pack);
     ASSERT_TRUE((*pack)->is_valid());
-}
+  }
 
-// static
-base::FilePath BrowserThemePackTest::GetStarGazingPath() {
+  base::FilePath GetStarGazingPath() {
     base::FilePath test_path;
-  DCHECK(base::PathService::Get(chrome::DIR_TEST_DATA, &test_path));
+    if (!base::PathService::Get(chrome::DIR_TEST_DATA, &test_path)) {
+      NOTREACHED();
+      return test_path;
+    }
 
     test_path = test_path.AppendASCII("profiles");
     test_path = test_path.AppendASCII("profile_with_complex_theme");
@@ -225,38 +180,44 @@ base::FilePath BrowserThemePackTest::GetStarGazingPath() {
     test_path = test_path.AppendASCII("mblmlcbknbnfebdfjnolmcapmdofhmme");
     test_path = test_path.AppendASCII("1.1");
     return base::FilePath(test_path);
-}
+  }
 
-// static
-base::FilePath BrowserThemePackTest::GetHiDpiThemePath() {
+  base::FilePath GetHiDpiThemePath() {
     base::FilePath test_path;
-  DCHECK(base::PathService::Get(chrome::DIR_TEST_DATA, &test_path));
-
+    if (!base::PathService::Get(chrome::DIR_TEST_DATA, &test_path)) {
+      NOTREACHED();
+      return test_path;
+    }
     test_path = test_path.AppendASCII("extensions");
     test_path = test_path.AppendASCII("theme_hidpi");
     return base::FilePath(test_path);
-}
+  }
 
-// static
-void BrowserThemePackTest::VerifyStarGazing(BrowserThemePack* pack) {
+  // Verifies the data in star gazing. We do this multiple times for different
+  // BrowserThemePack objects to make sure it works in generated and mmapped
+  // mode correctly.
+  void VerifyStarGazing(BrowserThemePack* pack) {
     // First check that values we know exist, exist.
     SkColor color;
-  EXPECT_TRUE(pack->GetColor(TP::COLOR_BOOKMARK_TEXT, &color));
+    EXPECT_TRUE(pack->GetColor(ThemeProperties::COLOR_BOOKMARK_TEXT,
+                               &color));
     EXPECT_EQ(SK_ColorBLACK, color);
 
-  EXPECT_TRUE(pack->GetColor(TP::COLOR_NTP_BACKGROUND, &color));
+    EXPECT_TRUE(pack->GetColor(ThemeProperties::COLOR_NTP_BACKGROUND,
+                               &color));
     EXPECT_EQ(SkColorSetRGB(57, 137, 194), color);
 
-  color_utils::HSL expected = {0.6, 0.553, 0.5};
+    color_utils::HSL expected = { 0.6, 0.553, 0.5 };
     color_utils::HSL actual;
-  EXPECT_TRUE(pack->GetTint(TP::TINT_BUTTONS, &actual));
+    EXPECT_TRUE(pack->GetTint(ThemeProperties::TINT_BUTTONS, &actual));
     EXPECT_DOUBLE_EQ(expected.h, actual.h);
     EXPECT_DOUBLE_EQ(expected.s, actual.s);
     EXPECT_DOUBLE_EQ(expected.l, actual.l);
 
     int val;
-  EXPECT_TRUE(pack->GetDisplayProperty(TP::NTP_BACKGROUND_ALIGNMENT, &val));
-  EXPECT_EQ(TP::ALIGN_TOP, val);
+    EXPECT_TRUE(pack->GetDisplayProperty(
+        ThemeProperties::NTP_BACKGROUND_ALIGNMENT, &val));
+    EXPECT_EQ(ThemeProperties::ALIGN_TOP, val);
 
     // The stargazing theme defines the following images:
     EXPECT_TRUE(pack->HasCustomImage(IDR_THEME_BUTTON_BACKGROUND));
@@ -283,7 +244,8 @@ void BrowserThemePackTest::VerifyStarGazing(BrowserThemePack* pack) {
     EXPECT_FALSE(pack->HasCustomImage(IDR_THEME_FRAME_OVERLAY));
     EXPECT_TRUE(pack->GetImageNamed(IDR_THEME_FRAME_OVERLAY).IsEmpty());
     EXPECT_FALSE(pack->HasCustomImage(IDR_THEME_FRAME_OVERLAY_INACTIVE));
-  EXPECT_TRUE(pack->GetImageNamed(IDR_THEME_FRAME_OVERLAY_INACTIVE).IsEmpty());
+    EXPECT_TRUE(
+        pack->GetImageNamed(IDR_THEME_FRAME_OVERLAY_INACTIVE).IsEmpty());
 
 #if !defined(OS_MACOSX)
     // The tab background image is missing, but will still be generated in
@@ -294,12 +256,12 @@ void BrowserThemePackTest::VerifyStarGazing(BrowserThemePack* pack) {
 #endif
 
     // Make sure we don't have phantom data.
-    EXPECT_FALSE(pack->GetColor(TP::COLOR_CONTROL_BACKGROUND, &color));
-    EXPECT_FALSE(pack->GetTint(TP::TINT_FRAME, &actual));
-}
+    EXPECT_FALSE(pack->GetColor(ThemeProperties::COLOR_CONTROL_BACKGROUND,
+                                &color));
+    EXPECT_FALSE(pack->GetTint(ThemeProperties::TINT_FRAME, &actual));
+  }
 
-// static
-void BrowserThemePackTest::VerifyHiDpiTheme(BrowserThemePack* pack) {
+  void VerifyHiDpiTheme(BrowserThemePack* pack) {
     // The high DPI theme defines the following images:
     EXPECT_TRUE(pack->HasCustomImage(IDR_THEME_FRAME));
     EXPECT_TRUE(pack->HasCustomImage(IDR_THEME_FRAME_INACTIVE));
@@ -372,7 +334,7 @@ void BrowserThemePackTest::VerifyHiDpiTheme(BrowserThemePack* pack) {
     // We take samples of colors and locations along the diagonal whenever
     // the color changes. Note these colors are slightly different from
     // the input PNG file due to input processing.
-  std::vector<std::pair<int, SkColor>> normal;
+    std::vector<std::pair<int, SkColor> > normal;
     int xy = 0;
     SkColor color = rep3.sk_bitmap().getColor(xy, xy);
     normal.push_back(std::make_pair(xy, color));
@@ -396,50 +358,29 @@ void BrowserThemePackTest::VerifyHiDpiTheme(BrowserThemePack* pack) {
       SkColor color = normal[i].second;
       EXPECT_EQ(color, rep4.sk_bitmap().getColor(xy, xy));
     }
-}
-
-// static
-SkColor BrowserThemePackTest::BuildThirdOpacity(SkColor color_link) {
-  return SkColorSetA(color_link, SkColorGetA(color_link) / 3);
-}
-
-// static
-SkColor BrowserThemePackTest::GetDefaultColor(int id) {
-  // Direct incognito IDs need to be mapped back to the non-incognito versions
-  // (plus passing "true" for |incognito|) to avoid DCHECK failures.
-  switch (id) {
-    case TP::COLOR_FRAME_INCOGNITO:
-      return TP::GetDefaultColor(TP::COLOR_FRAME, true);
-    case TP::COLOR_FRAME_INCOGNITO_INACTIVE:
-      return TP::GetDefaultColor(TP::COLOR_FRAME_INACTIVE, true);
-    default:
-      return TP::GetDefaultColor(id, false);
   }
-}
 
-// static
-void BrowserThemePackTest::GenerateDefaultFrameColor(
-    std::map<int, SkColor>* colors,
-    int color,
-    int tint,
-    bool otr) {
-  (*colors)[color] =
-      HSLShift(GetDefaultColor(TP::COLOR_FRAME), TP::GetDefaultTint(tint, otr));
-}
+ protected:
+  typedef std::unique_ptr<ui::test::ScopedSetSupportedScaleFactors>
+      ScopedSetSupportedScaleFactors;
+  ScopedSetSupportedScaleFactors scoped_set_supported_scale_factors_;
 
-// Actual tests ----------------------------------------------------------------
+  content::TestBrowserThreadBundle thread_bundle_;
+  scoped_refptr<BrowserThemePack> theme_pack_;
+};
 
-// 'ntp_section' used to correspond to COLOR_NTP_SECTION, but COLOR_NTP_SECTION
-// was since removed because it was never used.  While it was in use,
-// COLOR_NTP_HEADER used 'ntp_section' as a fallback when 'ntp_header' was
-// absent.  We still preserve this fallback for themes that relied on this.
+// 'ntp_section' used to correspond to ThemeProperties::COLOR_NTP_SECTION,
+// but COLOR_NTP_SECTION was since removed because it was never used.
+// While it was in use, COLOR_NTP_HEADER used 'ntp_section' as a fallback when
+// 'ntp_header' was absent.  We still preserve this fallback for themes that
+// relied on this.
 TEST_F(BrowserThemePackTest, UseSectionColorAsNTPHeader) {
   std::string color_json = "{ \"ntp_section\": [190, 190, 190] }";
   LoadColorJSON(color_json);
 
   std::map<int, SkColor> colors = GetDefaultColorMap();
   SkColor ntp_color = SkColorSetRGB(190, 190, 190);
-  colors[TP::COLOR_NTP_HEADER] = ntp_color;
+  colors[ThemeProperties::COLOR_NTP_HEADER] = ntp_color;
   VerifyColorMap(colors);
 }
 
@@ -449,7 +390,7 @@ TEST_F(BrowserThemePackTest, ProvideNtpHeaderColor) {
   LoadColorJSON(color_json);
 
   std::map<int, SkColor> colors = GetDefaultColorMap();
-  colors[TP::COLOR_NTP_HEADER] = SkColorSetRGB(120, 120, 120);
+  colors[ThemeProperties::COLOR_NTP_HEADER] = SkColorSetRGB(120, 120, 120);
   VerifyColorMap(colors);
 }
 
@@ -465,11 +406,13 @@ TEST_F(BrowserThemePackTest, SupportsAlpha) {
   std::map<int, SkColor> colors = GetDefaultColorMap();
   // Verify that valid alpha values are parsed correctly.
   // The toolbar color's alpha value is intentionally ignored by theme provider.
-  colors[TP::COLOR_TOOLBAR] = SkColorSetARGB(255, 0, 20, 40);
-  colors[TP::COLOR_TAB_TEXT] = SkColorSetARGB(255, 60, 80, 100);
-  colors[TP::COLOR_BACKGROUND_TAB_TEXT] = SkColorSetARGB(0, 120, 140, 160);
-  colors[TP::COLOR_BOOKMARK_TEXT] = SkColorSetARGB(255, 180, 200, 220);
-  colors[TP::COLOR_NTP_TEXT] = SkColorSetARGB(128, 240, 255, 0);
+  colors[ThemeProperties::COLOR_TOOLBAR] = SkColorSetARGB(255, 0, 20, 40);
+  colors[ThemeProperties::COLOR_TAB_TEXT] = SkColorSetARGB(255, 60, 80, 100);
+  colors[ThemeProperties::COLOR_BACKGROUND_TAB_TEXT] =
+      SkColorSetARGB(0, 120, 140, 160);
+  colors[ThemeProperties::COLOR_BOOKMARK_TEXT] =
+      SkColorSetARGB(255, 180, 200, 220);
+  colors[ThemeProperties::COLOR_NTP_TEXT] = SkColorSetARGB(128, 240, 255, 0);
   VerifyColorMap(colors);
 }
 
@@ -491,7 +434,8 @@ TEST_F(BrowserThemePackTest, CanReadTints) {
 
   color_utils::HSL expected = { 0.5, 0.5, 0.5 };
   color_utils::HSL actual = { -1, -1, -1 };
-  EXPECT_TRUE(theme_pack().GetTint(TP::TINT_BUTTONS, &actual));
+  EXPECT_TRUE(theme_pack_->GetTint(
+      ThemeProperties::TINT_BUTTONS, &actual));
   EXPECT_DOUBLE_EQ(expected.h, actual.h);
   EXPECT_DOUBLE_EQ(expected.s, actual.s);
   EXPECT_DOUBLE_EQ(expected.l, actual.l);
@@ -504,16 +448,16 @@ TEST_F(BrowserThemePackTest, CanReadDisplayProperties) {
   LoadDisplayPropertiesJSON(json);
 
   int out_val;
-  EXPECT_TRUE(
-      theme_pack().GetDisplayProperty(TP::NTP_BACKGROUND_ALIGNMENT, &out_val));
-  EXPECT_EQ(TP::ALIGN_BOTTOM, out_val);
+  EXPECT_TRUE(theme_pack_->GetDisplayProperty(
+      ThemeProperties::NTP_BACKGROUND_ALIGNMENT, &out_val));
+  EXPECT_EQ(ThemeProperties::ALIGN_BOTTOM, out_val);
 
-  EXPECT_TRUE(
-      theme_pack().GetDisplayProperty(TP::NTP_BACKGROUND_TILING, &out_val));
-  EXPECT_EQ(TP::REPEAT_X, out_val);
+  EXPECT_TRUE(theme_pack_->GetDisplayProperty(
+      ThemeProperties::NTP_BACKGROUND_TILING, &out_val));
+  EXPECT_EQ(ThemeProperties::REPEAT_X, out_val);
 
-  EXPECT_TRUE(
-      theme_pack().GetDisplayProperty(TP::NTP_LOGO_ALTERNATE, &out_val));
+  EXPECT_TRUE(theme_pack_->GetDisplayProperty(
+      ThemeProperties::NTP_LOGO_ALTERNATE, &out_val));
   EXPECT_EQ(0, out_val);
 }
 
@@ -563,17 +507,18 @@ TEST_F(BrowserThemePackTest, InvalidTints) {
 
   // We should ignore completely invalid (non-numeric) tints.
   color_utils::HSL actual = { -1, -1, -1 };
-  EXPECT_FALSE(theme_pack().GetTint(TP::TINT_BUTTONS, &actual));
+  EXPECT_FALSE(theme_pack_->GetTint(ThemeProperties::TINT_BUTTONS, &actual));
 
   // We should change invalid numeric HSL tint components to the special -1 "no
   // change" value.
-  EXPECT_TRUE(theme_pack().GetTint(TP::TINT_FRAME, &actual));
+  EXPECT_TRUE(theme_pack_->GetTint(ThemeProperties::TINT_FRAME, &actual));
   EXPECT_EQ(-1, actual.h);
   EXPECT_EQ(-1, actual.s);
   EXPECT_EQ(-1, actual.l);
 
   // We should correct partially incorrect inputs as well.
-  EXPECT_TRUE(theme_pack().GetTint(TP::TINT_FRAME_INCOGNITO_INACTIVE, &actual));
+  EXPECT_TRUE(theme_pack_->GetTint(
+      ThemeProperties::TINT_FRAME_INCOGNITO_INACTIVE, &actual));
   EXPECT_EQ(-1, actual.h);
   EXPECT_EQ(-1, actual.s);
   EXPECT_EQ(0.6, actual.l);
@@ -585,8 +530,8 @@ TEST_F(BrowserThemePackTest, InvalidDisplayProperties) {
   LoadDisplayPropertiesJSON(invalid_properties);
 
   int out_val;
-  EXPECT_FALSE(
-      theme_pack().GetDisplayProperty(TP::NTP_BACKGROUND_ALIGNMENT, &out_val));
+  EXPECT_FALSE(theme_pack_->GetDisplayProperty(
+      ThemeProperties::NTP_BACKGROUND_ALIGNMENT, &out_val));
 }
 
 // These three tests should just not cause a segmentation fault.
@@ -614,8 +559,8 @@ TEST_F(BrowserThemePackTest, TestHasCustomImage) {
   TestFilePathMap out_file_paths;
   ParseImageNamesJSON(images, &out_file_paths);
 
-  EXPECT_TRUE(theme_pack().HasCustomImage(IDR_THEME_FRAME));
-  EXPECT_FALSE(theme_pack().HasCustomImage(IDR_THEME_FRAME_INCOGNITO));
+  EXPECT_TRUE(theme_pack_->HasCustomImage(IDR_THEME_FRAME));
+  EXPECT_FALSE(theme_pack_->HasCustomImage(IDR_THEME_FRAME_INCOGNITO));
 }
 
 TEST_F(BrowserThemePackTest, TestNonExistantImages) {