content: make theme_color more resilient to casting issues

content/renderer/manifest/manifest_parser_unittest.cc

Index: content/renderer/manifest/manifest_parser_unittest.cc
diff --git a/content/renderer/manifest/manifest_parser_unittest.cc b/content/renderer/manifest/manifest_parser_unittest.cc
index 1d6793ca3312eeca9b2fda3dbe11c86991429bea..f8ebf3ef391a078ac5b8aea047e0fb376828739a 100644
--- a/content/renderer/manifest/manifest_parser_unittest.cc
+++ b/content/renderer/manifest/manifest_parser_unittest.cc
@@ -10,6 +10,14 @@
 
 namespace content {
 
+namespace {
+
+uint32_t ExtractColor(int64_t color) {
+  return reinterpret_cast<uint32_t&>(color);
+}
+
+} // anonymous namespace
+
 class ManifestParserTest : public testing::Test  {
  protected:
   ManifestParserTest() {}
@@ -988,7 +996,7 @@ TEST_F(ManifestParserTest, ThemeColorParserRules) {
   // Smoke test.
   {
     Manifest manifest = ParseManifest("{ \"theme_color\": \"#FF0000\" }");
-    EXPECT_EQ(manifest.theme_color, 0xFFFF0000);
+    EXPECT_EQ(ExtractColor(manifest.theme_color), 0xFFFF0000);
     EXPECT_FALSE(manifest.IsEmpty());
     EXPECT_EQ(0u, GetErrorCount());
   }
@@ -996,7 +1004,7 @@ TEST_F(ManifestParserTest, ThemeColorParserRules) {
   // Trim whitespaces.
   {
     Manifest manifest = ParseManifest("{ \"theme_color\": \"  blue   \" }");
-    EXPECT_EQ(manifest.theme_color, 0xFF0000FF);
+    EXPECT_EQ(ExtractColor(manifest.theme_color), 0xFF0000FF);
     EXPECT_EQ(0u, GetErrorCount());
   }
 
@@ -1104,35 +1112,35 @@ TEST_F(ManifestParserTest, ThemeColorParserRules) {
   // Accept CSS color keyword format.
   {
     Manifest manifest = ParseManifest("{ \"theme_color\": \"blue\" }");
-    EXPECT_EQ(manifest.theme_color, 0xFF0000FF);
+    EXPECT_EQ(ExtractColor(manifest.theme_color), 0xFF0000FF);
     EXPECT_EQ(0u, GetErrorCount());
   }
 
   // Accept CSS color keyword format.
   {
     Manifest manifest = ParseManifest("{ \"theme_color\": \"chartreuse\" }");
-    EXPECT_EQ(manifest.theme_color, 0xFF7FFF00);
+    EXPECT_EQ(ExtractColor(manifest.theme_color), 0xFF7FFF00);
     EXPECT_EQ(0u, GetErrorCount());
   }
 
   // Accept CSS RGB format.
   {
     Manifest manifest = ParseManifest("{ \"theme_color\": \"#FFF\" }");
-    EXPECT_EQ(manifest.theme_color, 0xFFFFFFFF);
+    EXPECT_EQ(ExtractColor(manifest.theme_color), 0xFFFFFFFF);
     EXPECT_EQ(0u, GetErrorCount());
   }
 
   // Accept CSS RGB format.
   {
     Manifest manifest = ParseManifest("{ \"theme_color\": \"#ABC\" }");
-    EXPECT_EQ(manifest.theme_color, 0xFFAABBCC);
+    EXPECT_EQ(ExtractColor(manifest.theme_color), 0xFFAABBCC);
     EXPECT_EQ(0u, GetErrorCount());
   }
 
   // Accept CSS RRGGBB format.
   {
     Manifest manifest = ParseManifest("{ \"theme_color\": \"#FF0000\" }");
-    EXPECT_EQ(manifest.theme_color, 0xFFFF0000);
+    EXPECT_EQ(ExtractColor(manifest.theme_color), 0xFFFF0000);
     EXPECT_EQ(0u, GetErrorCount());
   }

[Bernhard Bauer, 2015/07/31 10:07:45]

Out of curiosity, do we allow transparent colors here?

[Lalit Maganti, 2015/07/31 10:13:17]

As far as I know there is no check against it here because we simply want to
store whatever is in the manifest. Higher up, we may well disallow transparent
colors as it doesn't make any sense to theme the UI transparent.

[Bernhard Bauer, 2015/07/31 10:26:55]

OK, in that case would it make sense to add tests for transparent values (can be
in a followup CL)? That would exercise the code paths for a) parsing argb color
values, and b) converting values with the MSB _not_ set (which probably is the
less problematic case, but it's still good to have coverage).

[mlamouri (slow - plz ping), 2015/08/05 13:35:33]

We could add a test with AA=00 in there, right?

 }