This CL makes managed users use the normal Google logo on the --enable-instant-extended-api NTP.

I also changed the signature of ui::ThemeProvider::GetDisplayProperty() because most of the callers were ignoring the return value. In some cases we were using uninitialized variables because of this.

BUG=279978
TEST=Manual, see bug
R=erg
TBR=sky (For trivial refactor)

Review URL: https://chromiumcodereview.appspot.com/23614007

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@220537 0039d316-1c4b-4281-b951-d872f2087c98

chrome/browser/search/instant_service.cc

@@ -332,21 +332,17 @@ void InstantService::OnThemeChanged(ThemeService* theme_service) {
   theme_info_->header_color = SkColorToRGBAColor(header_color);
   theme_info_->section_border_color = SkColorToRGBAColor(section_border_color);
 
-  // Set logo for the theme. By default, use alternate logo.
-  theme_info_->logo_alternate = true;
-  int logo_alternate = 0;
-  if (theme_service->GetDisplayProperty(
-      ThemeProperties::NTP_LOGO_ALTERNATE, &logo_alternate))
-    theme_info_->logo_alternate = logo_alternate == 1;
+  int logo_alternate = theme_service->GetDisplayProperty(
+      ThemeProperties::NTP_LOGO_ALTERNATE);
+  theme_info_->logo_alternate = logo_alternate == 1;
 
   if (theme_service->HasCustomImage(IDR_THEME_NTP_BACKGROUND)) {
     // Set theme id for theme background image url.
     theme_info_->theme_id = theme_service->GetThemeID();
 
     // Set theme background image horizontal alignment.
-    int alignment = 0;
-    theme_service->GetDisplayProperty(
-        ThemeProperties::NTP_BACKGROUND_ALIGNMENT, &alignment);
+    int alignment = theme_service->GetDisplayProperty(
+        ThemeProperties::NTP_BACKGROUND_ALIGNMENT);
     if (alignment & ThemeProperties::ALIGN_LEFT)
       theme_info_->image_horizontal_alignment = THEME_BKGRND_IMAGE_ALIGN_LEFT;
     else if (alignment & ThemeProperties::ALIGN_RIGHT)
@@ -363,9 +359,8 @@ void InstantService::OnThemeChanged(ThemeService* theme_service) {
       theme_info_->image_vertical_alignment = THEME_BKGRND_IMAGE_ALIGN_CENTER;
 
     // Set theme backgorund image tiling.
-    int tiling = 0;
-    theme_service->GetDisplayProperty(ThemeProperties::NTP_BACKGROUND_TILING,
-                                      &tiling);
+    int tiling = theme_service->GetDisplayProperty(
+        ThemeProperties::NTP_BACKGROUND_TILING);
     switch (tiling) {
       case ThemeProperties::NO_REPEAT:
         theme_info_->image_tiling = THEME_BKGRND_IMAGE_NO_REPEAT;

chrome/browser/themes/theme_properties.cc

@@ -71,10 +71,11 @@ const color_utils::HSL kDefaultTintBackgroundTab = { -1, 0.5, 0.75 };
 
 // Default display properties.
 const int kDefaultDisplayPropertyNTPAlignment =
-    ThemeProperties::ALIGN_BOTTOM;
+    ThemeProperties::ALIGN_CENTER;
 const int kDefaultDisplayPropertyNTPTiling =
     ThemeProperties::NO_REPEAT;
-const int kDefaultDisplayPropertyNTPInverseLogo = 0;
+// By default, we do not use the ntp alternate logo.
+const int kDefaultDisplayPropertyNTPAlternateLogo = 0;
 
 // ----------------------------------------------------------------------------
 // Defaults for properties which are not stored in the browser theme pack.
@@ -302,18 +303,15 @@ SkColor ThemeProperties::GetDefaultColor(int id) {
 }
 
 // static
-bool ThemeProperties::GetDefaultDisplayProperty(int id, int* result) {
+int ThemeProperties::GetDefaultDisplayProperty(int id) {
   switch (id) {
     case NTP_BACKGROUND_ALIGNMENT:
-      *result = kDefaultDisplayPropertyNTPAlignment;
-      return true;
+      return kDefaultDisplayPropertyNTPAlignment;
     case NTP_BACKGROUND_TILING:
-      *result = kDefaultDisplayPropertyNTPTiling;
-      return true;
+      return kDefaultDisplayPropertyNTPTiling;
     case NTP_LOGO_ALTERNATE:
-      *result = kDefaultDisplayPropertyNTPInverseLogo;
-      return true;
+      return kDefaultDisplayPropertyNTPAlternateLogo;
   }
 
-  return false;
+  return -1;
 }

chrome/browser/themes/theme_properties.h

@@ -140,9 +140,9 @@ class ThemeProperties {
   // Returns SK_ColorRED if |id| is invalid.
   static SkColor GetDefaultColor(int id);
 
-  // Returns true and sets |result| to the requested default property, if |id|
-  // is valid.
-  static bool GetDefaultDisplayProperty(int id, int* result);
+  // Returns the default value for the given property |id|. Returns -1 if |id|
+  // is invalid.
+  static int GetDefaultDisplayProperty(int id);
 
  private:
   DISALLOW_IMPLICIT_CONSTRUCTORS(ThemeProperties);

chrome/browser/themes/theme_service.cc

@@ -169,11 +169,22 @@ SkColor ThemeService::GetColor(int id) const {
   return Properties::GetDefaultColor(id);
 }
 
-bool ThemeService::GetDisplayProperty(int id, int* result) const {
-  if (theme_supplier_.get())
-    return theme_supplier_->GetDisplayProperty(id, result);
+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() &&
+      !UsingNativeTheme()) {
+    // Use the alternate logo for themes from the web store except for
+    // |kDefaultThemeGalleryID|.
+    return 1;
+  }
 
-  return Properties::GetDefaultDisplayProperty(id, result);
+  return Properties::GetDefaultDisplayProperty(id);
 }
 
 bool ThemeService::ShouldUseNativeFrame() const {
@@ -200,8 +211,7 @@ base::RefCountedMemory* ThemeService::GetRawData(
     int id,
     ui::ScaleFactor scale_factor) const {
   // Check to see whether we should substitute some images.
-  int ntp_alternate;
-  GetDisplayProperty(Properties::NTP_LOGO_ALTERNATE, &ntp_alternate);
+  int ntp_alternate = GetDisplayProperty(Properties::NTP_LOGO_ALTERNATE);
   if (id == IDR_PRODUCT_LOGO && ntp_alternate != 0)
     id = IDR_PRODUCT_LOGO_WHITE;
 
@@ -364,7 +374,7 @@ void ThemeService::SetNativeTheme() {
 bool ThemeService::UsingDefaultTheme() const {
   std::string id = GetThemeID();
   return id == ThemeService::kDefaultThemeID ||
-      (id == kDefaultThemeGalleryID && !IsManagedUser());
+      id == kDefaultThemeGalleryID;
 }
 
 bool ThemeService::UsingNativeTheme() const {

chrome/browser/themes/theme_service.h

@@ -78,7 +78,7 @@ class ThemeService : public base::NonThreadSafe,
   // Overridden from ui::ThemeProvider:
   virtual gfx::ImageSkia* GetImageSkiaNamed(int id) const OVERRIDE;
   virtual SkColor GetColor(int id) const OVERRIDE;
-  virtual bool GetDisplayProperty(int id, int* result) const OVERRIDE;
+  virtual int GetDisplayProperty(int id) const OVERRIDE;
   virtual bool ShouldUseNativeFrame() const OVERRIDE;
   virtual bool HasCustomImage(int id) const OVERRIDE;
   virtual base::RefCountedMemory* GetRawData(

chrome/browser/ui/cocoa/bookmarks/bookmark_bar_controller_unittest.mm

@@ -210,8 +210,8 @@ class FakeTheme : public ui::ThemeProvider {
     return NULL;
   }
   virtual SkColor GetColor(int id) const OVERRIDE { return SkColor(); }
-  virtual bool GetDisplayProperty(int id, int* result) const OVERRIDE {
-    return false;
+  virtual int GetDisplayProperty(int id) const OVERRIDE {
+    return -1;
   }
   virtual bool ShouldUseNativeFrame() const OVERRIDE { return false; }
   virtual bool HasCustomImage(int id) const OVERRIDE { return false; }

chrome/browser/ui/cocoa/download/background_theme.h

@@ -18,7 +18,7 @@ class BackgroundTheme : public ui::ThemeProvider {
   // Overridden from ui::ThemeProvider:
   virtual gfx::ImageSkia* GetImageSkiaNamed(int id) const OVERRIDE;
   virtual SkColor GetColor(int id) const OVERRIDE;
-  virtual bool GetDisplayProperty(int id, int* result) const OVERRIDE;
+  virtual int GetDisplayProperty(int id) const OVERRIDE;
   virtual bool ShouldUseNativeFrame() const OVERRIDE;
   virtual bool HasCustomImage(int id) const OVERRIDE;
   virtual base::RefCountedMemory* GetRawData(

chrome/browser/ui/cocoa/download/background_theme.mm

@@ -35,8 +35,8 @@ SkColor BackgroundTheme::GetColor(int id) const {
   return SkColor();
 }
 
-bool BackgroundTheme::GetDisplayProperty(int id, int* result) const {
-  return false;
+int BackgroundTheme::GetDisplayProperty(int id) const {
+  return -1;
 }
 
 bool BackgroundTheme::ShouldUseNativeFrame() const {

chrome/browser/ui/ntp_background_util.cc

@@ -74,16 +74,13 @@ void NtpBackgroundUtil::PaintBackgroundDetachedMode(ui::ThemeProvider* tp,
   canvas->FillRect(area, tp->GetColor(ThemeProperties::COLOR_NTP_BACKGROUND));
 
   if (tp->HasCustomImage(IDR_THEME_NTP_BACKGROUND)) {
-    int tiling = ThemeProperties::NO_REPEAT;
-    tp->GetDisplayProperty(ThemeProperties::NTP_BACKGROUND_TILING, &tiling);
-    int alignment;
-    if (tp->GetDisplayProperty(ThemeProperties::NTP_BACKGROUND_ALIGNMENT,
-                               &alignment)) {
-      gfx::ImageSkia* ntp_background =
-          tp->GetImageSkiaNamed(IDR_THEME_NTP_BACKGROUND);
+    int tiling = tp->GetDisplayProperty(ThemeProperties::NTP_BACKGROUND_TILING);
+    int alignment = tp->GetDisplayProperty(
+        ThemeProperties::NTP_BACKGROUND_ALIGNMENT);
+    gfx::ImageSkia* ntp_background =
+        tp->GetImageSkiaNamed(IDR_THEME_NTP_BACKGROUND);
 
-      PaintThemeBackground(
-          canvas, ntp_background, tiling, alignment, area, tab_contents_height);
-    }
+    PaintThemeBackground(
+        canvas, ntp_background, tiling, alignment, area, tab_contents_height);
   }
 }

chrome/browser/ui/views/frame/browser_desktop_root_window_host_win.cc

@@ -45,8 +45,8 @@ class DesktopThemeProvider : public ui::ThemeProvider {
   virtual SkColor GetColor(int id) const OVERRIDE {
     return delegate_->GetColor(id);
   }
-  virtual bool GetDisplayProperty(int id, int* result) const OVERRIDE {
-    return delegate_->GetDisplayProperty(id, result);
+  virtual int GetDisplayProperty(int id) const OVERRIDE {
+    return delegate_->GetDisplayProperty(id);
   }
   virtual bool ShouldUseNativeFrame() const OVERRIDE {
     return delegate_->ShouldUseNativeFrame();

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

@@ -127,10 +127,6 @@ SkColor GetThemeColor(ui::ThemeProvider* tp, int id) {
 // states when the bar is attached or detached.
 std::string GetNewTabBackgroundCSS(const ui::ThemeProvider* theme_provider,
                                    bool bar_attached) {
-  int alignment;
-  theme_provider->GetDisplayProperty(
-      ThemeProperties::NTP_BACKGROUND_ALIGNMENT, &alignment);
-
   // TODO(glen): This is a quick workaround to hide the notused.png image when
   // no image is provided - we don't have time right now to figure out why
   // this is painting as white.
@@ -139,6 +135,9 @@ std::string GetNewTabBackgroundCSS(const ui::ThemeProvider* theme_provider,
     return "-64px";
   }
 
+  int alignment = theme_provider->GetDisplayProperty(
+      ThemeProperties::NTP_BACKGROUND_ALIGNMENT);
+
   if (bar_attached)
     return ThemeProperties::AlignmentToString(alignment);
 
@@ -161,9 +160,8 @@ std::string GetNewTabBackgroundCSS(const ui::ThemeProvider* theme_provider,
 // masks in theme_service.h).
 std::string GetNewTabBackgroundTilingCSS(
     const ui::ThemeProvider* theme_provider) {
-  int repeat_mode;
-  theme_provider->GetDisplayProperty(
-      ThemeProperties::NTP_BACKGROUND_TILING, &repeat_mode);
+  int repeat_mode = theme_provider->GetDisplayProperty(
+      ThemeProperties::NTP_BACKGROUND_TILING);
   return ThemeProperties::TilingToString(repeat_mode);
 }
 
@@ -506,10 +504,9 @@ void NTPResourceCache::CreateNewTabHTML() {
   // Control fade and resize animations.
   load_time_data.SetBoolean("anim", ui::Animation::ShouldRenderRichAnimation());
 
-  int alignment;
   ui::ThemeProvider* tp = ThemeServiceFactory::GetForProfile(profile_);
-  tp->GetDisplayProperty(ThemeProperties::NTP_BACKGROUND_ALIGNMENT,
-                         &alignment);
+  int alignment = tp->GetDisplayProperty(
+      ThemeProperties::NTP_BACKGROUND_ALIGNMENT);
   load_time_data.SetString("themegravity",
       (alignment & ThemeProperties::ALIGN_RIGHT) ? "right" : "");
 

ui/base/default_theme_provider.cc

@@ -26,8 +26,8 @@ SkColor DefaultThemeProvider::GetColor(int id) const {
   return 0xff0000ff;
 }
 
-bool DefaultThemeProvider::GetDisplayProperty(int id, int* result) const {
-  return false;
+int DefaultThemeProvider::GetDisplayProperty(int id) const {
+  return -1;
 }
 
 bool DefaultThemeProvider::ShouldUseNativeFrame() const {

ui/base/default_theme_provider.h

@@ -26,7 +26,7 @@ class UI_EXPORT DefaultThemeProvider : public ThemeProvider {
   // Overridden from ui::ThemeProvider:
   virtual gfx::ImageSkia* GetImageSkiaNamed(int id) const OVERRIDE;
   virtual SkColor GetColor(int id) const OVERRIDE;
-  virtual bool GetDisplayProperty(int id, int* result) const OVERRIDE;
+  virtual int GetDisplayProperty(int id) const OVERRIDE;
   virtual bool ShouldUseNativeFrame() const OVERRIDE;
   virtual bool HasCustomImage(int id) const OVERRIDE;
   virtual base::RefCountedMemory* GetRawData(

ui/base/theme_provider.h

@@ -59,7 +59,7 @@ class UI_EXPORT ThemeProvider {
 
   // Get the property (e.g. an alignment expressed in an enum, or a width or
   // height) specified by |id|.
-  virtual bool GetDisplayProperty(int id, int* result) const = 0;
+  virtual int GetDisplayProperty(int id) const = 0;
 
   // Whether we should use the native system frame (typically Aero glass) or
   // a custom frame.