[Themes Service] Implement better default value for which Google logo to use.

chrome/browser/themes/theme_service.cc

 // Copyright (c) 2012 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_service.h"
 
 #include "base/bind.h"
 #include "base/memory/ref_counted_memory.h"
 #include "base/message_loop/message_loop.h"
 #include "base/prefs/pref_service.h"
@@ skipping 60 matching lines @@
   return color_utils::HSLToSkColor(result, SkColorGetA(color));
 }
 
 // Writes the theme pack to disk on a separate thread.
 void WritePackToDiskCallback(BrowserThemePack* pack,
                              const base::FilePath& path) {
   if (!pack->WriteToDisk(path))
     NOTREACHED() << "Could not write theme pack to disk";
 }
 
+// Heuristic to determine if color is grayscale. This is used to decide whether
+// to use the colorful or white logo, if a theme fails to specify which.
+bool IsColorGrayscale(SkColor color) {
+  const int kChannelTolerance = 9;
+  int r = SkColorGetR(color);
+  int g = SkColorGetG(color);
+  int b = SkColorGetB(color);
+  // range = max(r, g, b) - min(r, g, b).

[pkotwicz, 2014/09/16 03:57:18]

Nit: Use std::max() and std::min(). It makes the code more readable

[huangs, 2014/09/16 15:38:54]

Done.

+  int range = (r > g) ? (r > b ? r : b) - (g < b ? g : b) :
+      (g > b ? g : b) - (r < b ? r : b);
+  return range < kChannelTolerance;
+}
+
 }  // namespace
 
 ThemeService::ThemeService()
     : ready_(false),
       rb_(ResourceBundle::GetSharedInstance()),
       profile_(NULL),
       installed_pending_load_id_(kDefaultThemeID),
       number_of_infobars_(0),
       weak_ptr_factory_(this) {
 }
@@ skipping 95 matching lines @@
   return Properties::GetDefaultColor(id);
 }
 
 int ThemeService::GetDisplayProperty(int id) const {
   int result = 0;
   if (theme_supplier_.get() &&
       theme_supplier_->GetDisplayProperty(id, &result)) {
     return result;
   }
 
-  if (id == Properties::NTP_LOGO_ALTERNATE &&
-      !UsingDefaultTheme() &&
-      !UsingSystemTheme()) {
-    // Use the alternate logo for themes from the web store except for
-    // |kDefaultThemeGalleryID|.
-    return 1;
+  if (id == Properties::NTP_LOGO_ALTERNATE) {
+    // Use colorful logo for |kDefaultThemeGalleryID| themes.

[pkotwicz, 2014/09/16 03:57:18]

Remove the comment. It no longer makes sense.

[huangs, 2014/09/16 15:38:54]

Done; made the comments more succinct.

+    if (UsingDefaultTheme() || UsingSystemTheme())
+      return 0;
+
+    // Use white logo if background image is used.
+    if (HasCustomImage(IDR_THEME_NTP_BACKGROUND))
+      return 1;
+
+    SkColor background_color = GetColor(Properties::COLOR_NTP_BACKGROUND);
+    return IsColorGrayscale(background_color) ? 0 : 1;
   }
 
   return Properties::GetDefaultDisplayProperty(id);
 }
 
 bool ThemeService::ShouldUseNativeFrame() const {
   if (HasCustomImage(IDR_THEME_FRAME))
     return false;
 #if defined(OS_WIN)
   return ui::win::IsAeroGlassEnabled();
@@ skipping 391 matching lines @@
 void ThemeService::OnInfobarDestroyed() {
   number_of_infobars_--;
 
   if (number_of_infobars_ == 0)
     RemoveUnusedThemes(false);
 }
 
 ThemeSyncableService* ThemeService::GetThemeSyncableService() const {
   return theme_syncable_service_.get();
 }