ToolbarView MaterialDesign in ThemeProvider

chrome/browser/themes/theme_properties.cc

Index: chrome/browser/themes/theme_properties.cc
diff --git a/chrome/browser/themes/theme_properties.cc b/chrome/browser/themes/theme_properties.cc
index f32530f28d53452f5f7b5f88a0fd00753d329b2b..d149329e4ed56c64319172206b7e17406cb15569 100644
--- a/chrome/browser/themes/theme_properties.cc
+++ b/chrome/browser/themes/theme_properties.cc
@@ -9,6 +9,7 @@
 #include "base/strings/string_util.h"
 #include "chrome/browser/themes/browser_theme_pack.h"
 #include "grit/theme_resources.h"
+#include "ui/base/resource/material_design/material_design_controller.h"
 #include "ui/resources/grit/ui_resources.h"
 
 namespace {
@@ -96,6 +97,21 @@ const SkColor kDefaultColorToolbarStrokeInactive = SkColorSetRGB(163, 163, 163);
 #endif
 
 // ----------------------------------------------------------------------------
+// Defaults for layout properties which are not stored in the browser theme
+// pack. The array indices map to ui::MaterialDesignController::Mode.

[pkotwicz, 2015/07/07 14:30:27]

Adding a comment with the mapping of the MaterialDesignController::Mode to index
here would be hugely helpful. Otherwise anyone editing this file will have to
also need to have material_design_controller.h open

[Peter Kasting, 2015/07/07 19:01:16]

OTOH if we do that we have copies of the same information in multiple places and
we now need to make sure we always edit both lists at once or things rapidly
become confusing.

I'd probably just say something like: "The array indexes here are the values of
ui::MaterialDesignController::Mode; see
ui/base/resource/material_design/material_design_controller.h."

This also reminds me: that enum should be updated to have at least "= 0" on the
first value (and maybe explicit initializers for all values?) and a comment that
the order can't be changed without changing the code over here.

[sadrul, 2015/07/07 21:53:04]

Maybe have COMPILE_ASSERT()s here to make sure if the values change, it gets
caught?

[jonross, 2015/07/08 15:23:39]

Updated the comments to point to the header.
Added COMPILE_ASSERTS to check the indices matching
Updated the enum to explicitly set the values

[jonross, 2015/07/08 15:23:39]

Done.

+
+// The edge graphics have some built-in spacing/shadowing, so we have to adjust
+// our spacing to make it match.
+const int kToolbarViewLeftEdgeSpacing[] = {3, 4, 8};
+const int kToolbarViewRightEdgeSpacing[] = {2, 4, 8};
+
+// Ash doesn't use a rounded content area and its top edge has an extra shadow.
+const int kToolbarViewContentShadowHeightAsh[] = {2, 0, 0};
+
+// Non-ash uses a rounded content area with no shadow in the assets.
+const int kToolbarViewContentShadowHeight[] = {0, 0, 0};
+
+// ----------------------------------------------------------------------------
 
 // Strings used in alignment properties.
 const char kAlignmentCenter[] = "center";
@@ -299,14 +315,23 @@ SkColor ThemeProperties::GetDefaultColor(int id) {
 
 // static
 int ThemeProperties::GetDefaultDisplayProperty(int id) {
+  int mode = static_cast<int>(ui::MaterialDesignController::GetMode());
   switch (id) {
-    case NTP_BACKGROUND_ALIGNMENT:
+    case ThemeProperties::NTP_BACKGROUND_ALIGNMENT:
       return kDefaultDisplayPropertyNTPAlignment;
-    case NTP_BACKGROUND_TILING:
+    case ThemeProperties::NTP_BACKGROUND_TILING:
       return kDefaultDisplayPropertyNTPTiling;
-    case NTP_LOGO_ALTERNATE:
+    case ThemeProperties::NTP_LOGO_ALTERNATE:
       return kDefaultDisplayPropertyNTPAlternateLogo;
+    case ThemeProperties::PROPERTY_TOOLBAR_VIEW_LEFT_EDGE_SPACING:
+      return kToolbarViewLeftEdgeSpacing[mode];
+    case ThemeProperties::PROPERTY_TOOLBAR_VIEW_RIGHT_EDGE_SPACING:
+      return kToolbarViewRightEdgeSpacing[mode];
+    case ThemeProperties::PROPERTY_TOOLBAR_VIEW_CONTENT_SHADOW_HEIGHT_ASH:
+      return kToolbarViewContentShadowHeightAsh[mode];
+    case ThemeProperties::PROPERTY_TOOLBAR_VIEW_CONTENT_SHADOW_HEIGHT:
+      return kToolbarViewContentShadowHeight[mode];
+    default:
+      return -1;
   }
-
-  return -1;
 }