Fix the MD Incognito NTP background color regression

chrome/browser/ui/webui/ntp/ntp_resource_cache.cc

Index: chrome/browser/ui/webui/ntp/ntp_resource_cache.cc
diff --git a/chrome/browser/ui/webui/ntp/ntp_resource_cache.cc b/chrome/browser/ui/webui/ntp/ntp_resource_cache.cc
index 761fd55ac6faeb03a2e5623d347aa5b5377f0d42..0821dfabea5542b3029e22f78905a34847468f27 100644
--- a/chrome/browser/ui/webui/ntp/ntp_resource_cache.cc
+++ b/chrome/browser/ui/webui/ntp/ntp_resource_cache.cc
@@ -496,10 +496,15 @@ void NTPResourceCache::CreateNewTabIncognitoCSS() {
       ThemeService::GetThemeProviderForProfile(profile_);
 
   // Get our theme colors
+  SkColor color_background_default =
+      base::FeatureList::IsEnabled(features::kMaterialDesignIncognitoNTP)
+          ? SkColorSetRGB(0x30, 0x30, 0x30)
+          : SkColorSetRGB(0x32, 0x32, 0x32);
+
   SkColor color_background =
       tp.HasCustomImage(IDR_THEME_NTP_BACKGROUND)
           ? GetThemeColor(tp, ThemeProperties::COLOR_NTP_BACKGROUND)
-          : SkColorSetRGB(0x32, 0x32, 0x32);

[Evan Stade, 2017/05/24 00:23:43]

I'm confused. Can you just remove this ternary and always use GetThemeColor(tp,
ThemeProperties::COLOR_NTP_BACKGROUND)?

[msramek, 2017/05/24 01:05:21]

Nope.

If ThemeService routes that to ThemeProperties, which  was patched in
https://codereview.chromium.org/2891983003/ to return 0x30, it's fine.

But if it routes to a CustomThemeSupplier subclass (e.g. when you select the GTK
theme on Linux), it will incorrectly return *white*. That's because
CustomThemeSupplier doesn't have a concept of incognito variants, and the
non-incognito variant of COLOR_NTP_BACKGROUND is white.

This seems to be the origin of the white flash bug (crbug.com/21798).

I could move this override one level lower into ThemeService, but I don't think
it's that much cleaner solution.

The cleanest solution is probably to define a new constant
COLOR_NTP_BACKGROUND_INCOGNITO and set it to 0x32 in all CustomThemeSupplier
subclasses. I can do that if you think it makes sense, although I'm not familiar
with the themes code.

In this CL, I'm simply keeping the override of the background color which was
always here, just change it to 0x30, since we have slightly changed the color
for the |features::kMaterialDesignIncognitoNTP| redesign.

[Evan Stade, 2017/05/24 02:00:25]

it seems like the new ternary (the feature flag one) should go in
theme_properties.cc.

The last branch of this HasCustomImage ternary can change to
ThemeProperties::GetDefaultColor(), I think.

[msramek, 2017/05/24 09:32:18]

Done. Thanks for the suggestion, I agree that that's a bit cleaner.

+          : color_background_default;
 
   // Generate the replacements.
   ui::TemplateReplacements substitutions;