retry r123804 - ntp theme fixes

1. fix where the theme background is displayed for default (0) alignment -- it should be center center. This matches what the bookmar
1a. fix tests for alignment conversions
2. Fix the bookmark bar for vertical center alignment.
3. update the ntp background when the bookmark bar is attached/detached.

TODO: transition (3) so that it looks good on windows and mac.
BUG=115594
TEST=unit tests

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

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

14 files changed, 79 insertions, 49 deletions

diff --git a/chrome/browser/ntp_background_util.cc b/chrome/browser/ntp_background_util.cc
index c6682d0..297a0f0 100644
--- a/chrome/browser/ntp_background_util.cc
+++ b/chrome/browser/ntp_background_util.cc
@@ -4,6 +4,8 @@
 
 #include "chrome/browser/ntp_background_util.h"
 
+#include <cmath>
+
 #include "base/logging.h"
 #include "chrome/browser/themes/theme_service.h"
 #include "grit/theme_resources.h"
@@ -22,15 +24,22 @@ void PaintThemeBackground(
   int width = area.width() + ntp_background->width();
   int height = area.height() + ntp_background->height();
 
-  if (alignment & ThemeService::ALIGN_BOTTOM)
+  if (alignment & ThemeService::ALIGN_BOTTOM) {
     y_pos += area.height() + tab_contents_height - ntp_background->height();
+  } else if (alignment & ThemeService::ALIGN_TOP) {
+    // no op
+  } else {  // ALIGN_CENTER
+    y_pos += std::floor(area.height() + tab_contents_height / 2.0 -
+        ntp_background->height() / 2.0 + 0.5);
+  }
 
   if (alignment & ThemeService::ALIGN_RIGHT) {
     x_pos += area.width() - ntp_background->width();
   } else if (alignment & ThemeService::ALIGN_LEFT) {
     // no op
   } else {  // ALIGN_CENTER
-    x_pos += area.width() / 2 - ntp_background->width() / 2;
+    x_pos +=
+        std::floor(area.width() / 2.0 - ntp_background->width() / 2.0 + 0.5);
   }
 
   if (tiling != ThemeService::REPEAT &&
@@ -68,7 +77,7 @@ void NtpBackgroundUtil::PaintBackgroundDetachedMode(ui::ThemeProvider* tp,
     tp->GetDisplayProperty(ThemeService::NTP_BACKGROUND_TILING, &tiling);
     int alignment;
     if (tp->GetDisplayProperty(ThemeService::NTP_BACKGROUND_ALIGNMENT,
-        &alignment)) {
+                               &alignment)) {
       SkBitmap* ntp_background = tp->GetBitmapNamed(IDR_THEME_NTP_BACKGROUND);
 
       PaintThemeBackground(
diff --git a/chrome/browser/resources/chromeos/guest_session_tab.html b/chrome/browser/resources/chromeos/guest_session_tab.html
index 5915b22..4eaa4f6 100644
--- a/chrome/browser/resources/chromeos/guest_session_tab.html
+++ b/chrome/browser/resources/chromeos/guest_session_tab.html
@@ -6,7 +6,7 @@
 <script>
 // Until themes can clear the cache, force-reload the theme stylesheet.
 document.write('<link id="incognitothemecss" rel="stylesheet" ' +
-               'href="chrome://theme/css/newincognitotab.css?' +
+               'href="chrome://theme/css/incognito_new_tab_theme.css?' +
                Date.now() + '">');
 </script>
 </head>
@@ -19,7 +19,7 @@ document.write('<link id="incognitothemecss" rel="stylesheet" ' +
 <script>
 function themeChanged() {
   document.getElementById('incognitothemecss').href =
-      'chrome://theme/css/newincognitotab.css?' + Date.now();
+      'chrome://theme/css/incognito_new_tab_theme.css?' + Date.now();
 }
 </script>
 </html>
diff --git a/chrome/browser/resources/incognito_tab.html b/chrome/browser/resources/incognito_tab.html
index 03dad1d..d2230e0 100644
--- a/chrome/browser/resources/incognito_tab.html
+++ b/chrome/browser/resources/incognito_tab.html
@@ -6,7 +6,7 @@
 <script>
 // Until themes can clear the cache, force-reload the theme stylesheet.
 document.write('<link id="incognitothemecss" rel="stylesheet" ' +
-               'href="chrome://theme/css/newincognitotab.css?' +
+               'href="chrome://theme/css/incognito_new_tab_theme.css?' +
                Date.now() + '">');
 </script>
 </head>
@@ -23,7 +23,7 @@ document.write('<link id="incognitothemecss" rel="stylesheet" ' +
 <script>
 function themeChanged() {
   document.getElementById('incognitothemecss').href =
-      'chrome://theme/css/newincognitotab.css?' + Date.now();
+      'chrome://theme/css/incognito_new_tab_theme.css?' + Date.now();
 }
 
 function bookmarkBarAttached() {
diff --git a/chrome/browser/resources/ntp4/new_tab.html b/chrome/browser/resources/ntp4/new_tab.html
index 5d3cf3c..af6777c 100644
--- a/chrome/browser/resources/ntp4/new_tab.html
+++ b/chrome/browser/resources/ntp4/new_tab.html
@@ -3,7 +3,8 @@
     dir:textdirection;
     hasattribution:hasattribution;
     customlogo:customlogo;
-    themegravity:themegravity;"
+    themegravity:themegravity;
+    bookmarkbarattached:bookmarkbarattached;"
   class="starting-up">
 <head>
 <meta charset="utf-8">
diff --git a/chrome/browser/resources/ntp4/new_tab.js b/chrome/browser/resources/ntp4/new_tab.js
index 621157e..4cdd44e 100644
--- a/chrome/browser/resources/ntp4/new_tab.js
+++ b/chrome/browser/resources/ntp4/new_tab.js
@@ -261,15 +261,22 @@ cr.define('ntp4', function() {
     document.querySelector('head').appendChild(styleElement);
   }
 
-  // TODO(estade): rename newtab.css to new_tab_theme.css
-  function themeChanged(hasAttribution) {
-    $('themecss').href = 'chrome://theme/css/newtab.css?' + Date.now();
-    if (typeof hasAttribution != 'undefined')
-      document.documentElement.setAttribute('hasattribution', hasAttribution);
+  function themeChanged(opt_hasAttribution) {
+    $('themecss').href = 'chrome://theme/css/new_tab_theme.css?' + Date.now();
+
+    if (typeof opt_hasAttribution != 'undefined') {
+      document.documentElement.setAttribute('hasattribution',
+                                            opt_hasAttribution);
+    }
+
     updateLogo();
     updateAttribution();
   }
 
+  function setBookmarkBarAttached(attached) {
+    document.documentElement.setAttribute('bookmarkbarattached', attached);
+  }
+
   /**
    * Sets the proper image for the logo at the bottom left.
    */
@@ -539,6 +546,7 @@ cr.define('ntp4', function() {
     leaveRearrangeMode: leaveRearrangeMode,
     saveAppPageName: saveAppPageName,
     setAppToBeHighlighted: setAppToBeHighlighted,
+    setBookmarkBarAttached: setBookmarkBarAttached,
     setMostVisitedPages: setMostVisitedPages,
     setRecentlyClosedTabs: setRecentlyClosedTabs,
     setStripeColor: setStripeColor,
diff --git a/chrome/browser/resources/ntp4/new_tab_theme.css b/chrome/browser/resources/ntp4/new_tab_theme.css
index d6039a3..dadae632 100644
--- a/chrome/browser/resources/ntp4/new_tab_theme.css
+++ b/chrome/browser/resources/ntp4/new_tab_theme.css
@@ -2,8 +2,7 @@
  * Use of this source code is governed by a BSD-style license that can be
  * found in the LICENSE file. */
 
-html,
-#most-visited-settings {
+html {
   background-attachment: fixed;
   background-color: $2;  /* COLOR_NTP_BACKGROUND */
   background-image: url(chrome://theme/IDR_THEME_NTP_BACKGROUND?$1);
@@ -11,6 +10,10 @@ html,
   background-repeat: $5;
 }
 
+html[bookmarkbarattached='true'] {
+  background-position: $4;
+}
+
 body {
   color: $8;  /* COLOR_NTP_TEXT */
   height: 100%;
diff --git a/chrome/browser/themes/theme_service.cc b/chrome/browser/themes/theme_service.cc
index e0f8f15..910d176 100644
--- a/chrome/browser/themes/theme_service.cc
+++ b/chrome/browser/themes/theme_service.cc
@@ -29,6 +29,7 @@ using content::BrowserThread;
 using content::UserMetricsAction;
 
 // Strings used in alignment properties.
+const char* ThemeService::kAlignmentCenter = "center";
 const char* ThemeService::kAlignmentTop = "top";
 const char* ThemeService::kAlignmentBottom = "bottom";
 const char* ThemeService::kAlignmentLeft = "left";
@@ -377,8 +378,8 @@ std::string ThemeService::GetThemeID() const {
 // static
 std::string ThemeService::AlignmentToString(int alignment) {
   // Convert from an AlignmentProperty back into a string.
-  std::string vertical_string;
-  std::string horizontal_string;
+  std::string vertical_string(kAlignmentCenter);
+  std::string horizontal_string(kAlignmentCenter);
 
   if (alignment & ThemeService::ALIGN_TOP)
     vertical_string = kAlignmentTop;
@@ -390,11 +391,7 @@ std::string ThemeService::AlignmentToString(int alignment) {
   else if (alignment & ThemeService::ALIGN_RIGHT)
     horizontal_string = kAlignmentRight;
 
-  if (vertical_string.empty())
-    return horizontal_string;
-  if (horizontal_string.empty())
-    return vertical_string;
-  return vertical_string + " " + horizontal_string;
+  return horizontal_string + " " + vertical_string;
 }
 
 // static
diff --git a/chrome/browser/themes/theme_service.h b/chrome/browser/themes/theme_service.h
index 4ecde66..12964cf 100644
--- a/chrome/browser/themes/theme_service.h
+++ b/chrome/browser/themes/theme_service.h
@@ -52,6 +52,7 @@ class ThemeService : public base::NonThreadSafe,
   // Public constants used in ThemeService and its subclasses:
 
   // Strings used in alignment properties.
+  static const char* kAlignmentCenter;
   static const char* kAlignmentTop;
   static const char* kAlignmentBottom;
   static const char* kAlignmentLeft;
diff --git a/chrome/browser/themes/theme_service_unittest.cc b/chrome/browser/themes/theme_service_unittest.cc
index 7b39c2e..a74c4b6 100644
--- a/chrome/browser/themes/theme_service_unittest.cc
+++ b/chrome/browser/themes/theme_service_unittest.cc
@@ -1,4 +1,4 @@
-// Copyright (c) 2011 The Chromium Authors. All rights reserved.
+// 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.
 
@@ -9,43 +9,44 @@
 
 TEST(ThemeServiceTest, AlignmentConversion) {
   // Verify that we get out what we put in.
-  std::string top_left = "top left";
+  std::string top_left = "left top";
   int alignment = ThemeService::StringToAlignment(top_left);
   EXPECT_EQ(ThemeService::ALIGN_TOP | ThemeService::ALIGN_LEFT,
             alignment);
   EXPECT_EQ(top_left, ThemeService::AlignmentToString(alignment));
 
+  // We get back a normalized version of what we put in.
   alignment = ThemeService::StringToAlignment("top");
   EXPECT_EQ(ThemeService::ALIGN_TOP, alignment);
-  EXPECT_EQ("top", ThemeService::AlignmentToString(alignment));
+  EXPECT_EQ("center top", ThemeService::AlignmentToString(alignment));
 
   alignment = ThemeService::StringToAlignment("left");
   EXPECT_EQ(ThemeService::ALIGN_LEFT, alignment);
-  EXPECT_EQ("left", ThemeService::AlignmentToString(alignment));
+  EXPECT_EQ("left center", ThemeService::AlignmentToString(alignment));
 
   alignment = ThemeService::StringToAlignment("right");
   EXPECT_EQ(ThemeService::ALIGN_RIGHT, alignment);
-  EXPECT_EQ("right", ThemeService::AlignmentToString(alignment));
+  EXPECT_EQ("right center", ThemeService::AlignmentToString(alignment));
 
   alignment = ThemeService::StringToAlignment("righttopbottom");
   EXPECT_EQ(ThemeService::ALIGN_CENTER, alignment);
-  EXPECT_EQ("", ThemeService::AlignmentToString(alignment));
+  EXPECT_EQ("center center", ThemeService::AlignmentToString(alignment));
 }
 
 TEST(ThemeServiceTest, AlignmentConversionInput) {
   // Verify that we output in an expected format.
-  int alignment = ThemeService::StringToAlignment("right bottom");
-  EXPECT_EQ("bottom right", ThemeService::AlignmentToString(alignment));
+  int alignment = ThemeService::StringToAlignment("bottom right");
+  EXPECT_EQ("right bottom", ThemeService::AlignmentToString(alignment));
 
   // Verify that bad strings don't cause explosions.
   alignment = ThemeService::StringToAlignment("new zealand");
-  EXPECT_EQ("", ThemeService::AlignmentToString(alignment));
+  EXPECT_EQ("center center", ThemeService::AlignmentToString(alignment));
 
   // Verify that bad strings don't cause explosions.
   alignment = ThemeService::StringToAlignment("new zealand top");
-  EXPECT_EQ("top", ThemeService::AlignmentToString(alignment));
+  EXPECT_EQ("center top", ThemeService::AlignmentToString(alignment));
 
   // Verify that bad strings don't cause explosions.
   alignment = ThemeService::StringToAlignment("new zealandtop");
-  EXPECT_EQ("", ThemeService::AlignmentToString(alignment));
+  EXPECT_EQ("center center", ThemeService::AlignmentToString(alignment));
 }
diff --git a/chrome/browser/ui/webui/ntp/new_tab_ui.cc b/chrome/browser/ui/webui/ntp/new_tab_ui.cc
index fe80f87..2a5e295 100644
--- a/chrome/browser/ui/webui/ntp/new_tab_ui.cc
+++ b/chrome/browser/ui/webui/ntp/new_tab_ui.cc
@@ -133,6 +133,8 @@ NewTabUI::NewTabUI(content::WebUI* web_ui)
       new NewTabHTMLSource(GetProfile()->GetOriginalProfile());
   GetProfile()->GetChromeURLDataManager()->AddDataSource(html_source);
 
+  pref_change_registrar_.Init(GetProfile()->GetPrefs());
+  pref_change_registrar_.Add(prefs::kShowBookmarkBar, this);
   // Listen for theme installation.
   registrar_.Add(this, chrome::NOTIFICATION_BROWSER_THEME_CHANGED,
                  content::Source<ThemeService>(
@@ -197,14 +199,19 @@ void NewTabUI::Observe(int type,
                        const content::NotificationSource& source,
                        const content::NotificationDetails& details) {
   switch (type) {
+    case chrome::NOTIFICATION_PREF_CHANGED: {  // kShowBookmarkBar
+      StringValue attached(
+          GetProfile()->GetPrefs()->GetBoolean(prefs::kShowBookmarkBar) ?
+              "true" : "false");
+      web_ui()->CallJavascriptFunction("ntp4.setBookmarkBarAttached", attached);
+      break;
+    }
     case chrome::NOTIFICATION_BROWSER_THEME_CHANGED: {
       InitializeCSSCaches();
-      ListValue args;
-      args.Append(Value::CreateStringValue(
+      StringValue attribution(
           ThemeServiceFactory::GetForProfile(GetProfile())->HasCustomImage(
-              IDR_THEME_NTP_ATTRIBUTION) ?
-          "true" : "false"));
-      web_ui()->CallJavascriptFunction("themeChanged", args);
+              IDR_THEME_NTP_ATTRIBUTION) ? "true" : "false");
+      web_ui()->CallJavascriptFunction("themeChanged", attribution);
       break;
     }
     case content::NOTIFICATION_RENDER_WIDGET_HOST_DID_PAINT: {
@@ -244,7 +251,6 @@ void NewTabUI::SetupFieldTrials() {
   g_hint_group = trial->AppendGroup("PlusIcon", 40);
 }
 
-
 // static
 bool NewTabUI::ShouldShowWebStoreFooterLink() {
   const CommandLine* cli = CommandLine::ForCurrentProcess();
diff --git a/chrome/browser/ui/webui/ntp/new_tab_ui.h b/chrome/browser/ui/webui/ntp/new_tab_ui.h
index 885e271..9cd11b5 100644
--- a/chrome/browser/ui/webui/ntp/new_tab_ui.h
+++ b/chrome/browser/ui/webui/ntp/new_tab_ui.h
@@ -11,6 +11,7 @@
 #include "base/gtest_prod_util.h"
 #include "base/time.h"
 #include "base/timer.h"
+#include "chrome/browser/prefs/pref_change_registrar.h"
 #include "chrome/browser/sessions/tab_restore_service.h"
 #include "chrome/browser/ui/webui/chrome_url_data_manager.h"
 #include "content/public/browser/notification_observer.h"
@@ -116,6 +117,8 @@ class NewTabUI : public content::WebUIController,
   // If the sync promo NTP bubble is being shown.
   bool showing_sync_bubble_;
 
+  PrefChangeRegistrar pref_change_registrar_;
+
   DISALLOW_COPY_AND_ASSIGN(NewTabUI);
 };
 
diff --git a/chrome/browser/ui/webui/ntp/ntp_resource_cache.cc b/chrome/browser/ui/webui/ntp/ntp_resource_cache.cc
index 724ed32..7d18d9c 100644
--- a/chrome/browser/ui/webui/ntp/ntp_resource_cache.cc
+++ b/chrome/browser/ui/webui/ntp/ntp_resource_cache.cc
@@ -141,9 +141,9 @@ std::string GetNewTabBackgroundCSS(const ui::ThemeProvider* theme_provider,
 
   if (alignment & ThemeService::ALIGN_TOP) {
     if (alignment & ThemeService::ALIGN_LEFT)
-      return "0% " + base::IntToString(-offset) + "px";
+      return "left " + base::IntToString(-offset) + "px";
     else if (alignment & ThemeService::ALIGN_RIGHT)
-      return "100% " + base::IntToString(-offset) + "px";
+      return "right " + base::IntToString(-offset) + "px";
     return "center " + base::IntToString(-offset) + "px";
   }
   return ThemeService::AlignmentToString(alignment);
diff --git a/chrome/browser/ui/webui/theme_source.cc b/chrome/browser/ui/webui/theme_source.cc
index bc07fff..b125bc7 100644
--- a/chrome/browser/ui/webui/theme_source.cc
+++ b/chrome/browser/ui/webui/theme_source.cc
@@ -1,4 +1,4 @@
-// Copyright (c) 2011 The Chromium Authors. All rights reserved.
+// 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.
 
@@ -21,8 +21,8 @@
 using content::BrowserThread;
 
 // use a resource map rather than hard-coded strings.
-static const char* kNewTabCSSPath = "css/newtab.css";
-static const char* kNewIncognitoTabCSSPath = "css/newincognitotab.css";
+static const char* kNewTabCSSPath = "css/new_tab_theme.css";
+static const char* kNewIncognitoTabCSSPath = "css/incognito_new_tab_theme.css";
 
 static std::string StripQueryParams(const std::string& path) {
   GURL path_url = GURL(std::string(chrome::kChromeUIScheme) + "://" +
diff --git a/chrome/browser/ui/webui/theme_source_unittest.cc b/chrome/browser/ui/webui/theme_source_unittest.cc
index 4cd427f..ef5f154 100644
--- a/chrome/browser/ui/webui/theme_source_unittest.cc
+++ b/chrome/browser/ui/webui/theme_source_unittest.cc
@@ -1,4 +1,4 @@
-// Copyright (c) 2011 The Chromium Authors. All rights reserved.
+// 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.
 
@@ -61,8 +61,9 @@ class WebUISourcesTest : public testing::Test {
 };
 
 TEST_F(WebUISourcesTest, ThemeSourceMimeTypes) {
-  EXPECT_EQ(theme_source()->GetMimeType("css/newtab.css"), "text/css");
-  EXPECT_EQ(theme_source()->GetMimeType("css/newtab.css?foo"), "text/css");
+  EXPECT_EQ(theme_source()->GetMimeType("css/new_tab_theme.css"), "text/css");
+  EXPECT_EQ(theme_source()->GetMimeType("css/new_tab_theme.css?foo"),
+                                        "text/css");
   EXPECT_EQ(theme_source()->GetMimeType("WRONGURL"), "image/png");
 }
 
@@ -88,11 +89,11 @@ TEST_F(WebUISourcesTest, ThemeSourceCSS) {
   // just check for a successful request and data that is non-null.
   size_t empty_size = 0;
 
-  theme_source()->StartDataRequest("css/newtab.css", false, 1);
+  theme_source()->StartDataRequest("css/new_tab_theme.css", false, 1);
   EXPECT_EQ(theme_source()->result_request_id_, 1);
   EXPECT_NE(theme_source()->result_data_size_, empty_size);
 
-  theme_source()->StartDataRequest("css/newtab.css?pie", false, 3);
+  theme_source()->StartDataRequest("css/new_tab_theme.css?pie", false, 3);
   EXPECT_EQ(theme_source()->result_request_id_, 3);
   EXPECT_NE(theme_source()->result_data_size_, empty_size);