Fix the MD Incognito NTP background color regression

chrome/browser/themes/theme_properties.cc

 // Copyright (c) 2013 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 "chrome/browser/themes/theme_properties.h"
 
 #include <memory>
 
 #include "base/macros.h"
 #include "base/strings/string_split.h"
 #include "base/strings/string_util.h"
 #include "build/build_config.h"
 #include "chrome/browser/themes/browser_theme_pack.h"
+#include "chrome/common/chrome_features.h"
 #include "chrome/grit/theme_resources.h"
 #include "ui/base/material_design/material_design_controller.h"
 #include "ui/gfx/color_palette.h"
 #include "ui/resources/grit/ui_resources.h"
 
 namespace {
 
 // ----------------------------------------------------------------------------
 // Defaults for properties which are stored in the browser theme pack. If you
 // change these defaults, you must increment the version number in
@@ skipping 36 matching lines @@
     color_utils::GetSysSkColor(COLOR_WINDOW);
 const SkColor kDefaultColorNTPText =
     color_utils::GetSysSkColor(COLOR_WINDOWTEXT);
 const SkColor kDefaultColorNTPLink = color_utils::GetSysSkColor(COLOR_HOTLIGHT);
 #else
 // TODO(beng): source from theme provider.
 constexpr SkColor kDefaultColorNTPBackground = SK_ColorWHITE;
 constexpr SkColor kDefaultColorNTPText = SK_ColorBLACK;
 const SkColor kDefaultColorNTPLink = SkColorSetRGB(0x06, 0x37, 0x74);
 #endif  // OS_WIN
-const SkColor kDefaultColorNTPBackgroundOtr = SkColorSetRGB(0x30, 0x30, 0x30);
+
+// Then new MD Incognito NTP uses a slightly different shade of black.
+// TODO(msramek): Remove the old entry when the new NTP fully launches.

[Evan Stade, 2017/05/24 17:30:26]

nit: I don't really mind if you leave this TODO here but it seems unnecessary
given that when you remove the feature flag and are no longer using this
constant the compiler will remind you to remove it.

[msramek, 2017/05/24 17:39:49]

I just imagined that having two background colors for the same thing, especially
with such a tiny difference, might be confusing to a random reader; and I wanted
to emphasize that it's just temporary.

I would then leave it here for that purpose.

+const SkColor kDefaultColorNTPBackgroundOtr = SkColorSetRGB(0x32, 0x32, 0x32);
+const SkColor kDefaultColorNTPBackgroundOtrMD = SkColorSetRGB(0x30, 0x30, 0x30);
 
 const SkColor kDefaultColorNTPHeader = SkColorSetRGB(0x96, 0x96, 0x96);
 constexpr SkColor kDefaultColorButtonBackground = SK_ColorTRANSPARENT;
 
 // Default tints.
 constexpr color_utils::HSL kDefaultTintButtons = {-1, -1, -1};
 constexpr color_utils::HSL kDefaultTintButtonsIncognito = {-1, -1, 0.85};
 constexpr color_utils::HSL kDefaultTintFrame = {-1, -1, -1};
 constexpr color_utils::HSL kDefaultTintFrameInactive = {-1, -1, 0.75};
 constexpr color_utils::HSL kDefaultTintFrameIncognito = {-1, 0.2, 0.35};
@@ skipping 148 matching lines @@
                  : kDefaultColorFrameInactive;
     case COLOR_TOOLBAR:
       return otr ? kDefaultColorToolbarIncognito : kDefaultColorToolbar;
     case COLOR_TAB_TEXT:
     case COLOR_BOOKMARK_TEXT:
       return otr ? kDefaultColorToolbarTextIncognito : kDefaultColorToolbarText;
     case COLOR_BACKGROUND_TAB_TEXT:
       return otr ? kDefaultColorBackgroundTabTextIncognito
                  : kDefaultColorBackgroundTabText;
     case COLOR_NTP_BACKGROUND:
-      return otr ? kDefaultColorNTPBackgroundOtr : kDefaultColorNTPBackground;
+      if (!otr)
+        return kDefaultColorNTPBackground;
+      return base::FeatureList::IsEnabled(features::kMaterialDesignIncognitoNTP)
+                 ? kDefaultColorNTPBackgroundOtrMD
+                 : kDefaultColorNTPBackgroundOtr;
     case COLOR_NTP_TEXT:
       return kDefaultColorNTPText;
     case COLOR_NTP_LINK:
       return kDefaultColorNTPLink;
     case COLOR_NTP_HEADER:
       return kDefaultColorNTPHeader;
     case COLOR_BUTTON_BACKGROUND:
       return kDefaultColorButtonBackground;
 
     // Properties not stored in theme pack.
@@ skipping 47 matching lines @@
     case COLOR_FRAME_INCOGNITO:
     case COLOR_FRAME_INCOGNITO_INACTIVE:
       NOTREACHED() << "These values should be queried via their respective "
                       "non-incognito equivalents and an appropriate |otr| "
                       "value.";
       return gfx::kPlaceholderColor;
   }
 
   return gfx::kPlaceholderColor;
 }