Put "make this my home page" link into the tip section.

BUG= 28196
TEST= "make this my home page" should show up as a tip.


Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=33527
Review URL: http://codereview.chromium.org/449073

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

6 files changed, 65 insertions, 51 deletions

diff --git a/chrome/browser/dom_ui/new_tab_ui_uitest.cc b/chrome/browser/dom_ui/new_tab_ui_uitest.cc
index 1520b6e..46996d1 100644
--- a/chrome/browser/dom_ui/new_tab_ui_uitest.cc
+++ b/chrome/browser/dom_ui/new_tab_ui_uitest.cc
@@ -130,16 +130,19 @@ TEST_F(NewTabUITest, HomePageLink) {
 
   // TODO(arv): Extract common patterns for doing js testing.
 
-  // Fire click
+  // Fire click. Because tip service is turned off for testing, we first
+  // force the "make this my home page" tip to appear.
   // TODO(arv): Find screen position of element and use a lower level click
   // emulation.
   bool result;
   ASSERT_TRUE(tab->ExecuteAndExtractBool(L"",
     L"window.domAutomationController.send("
     L"(function() {"
+    L"  tipCache = [{\"set_homepage_tip\":\"Make this the home page\"}];"
+    L"  renderTip();"
     L"  var e = document.createEvent('Event');"
     L"  e.initEvent('click', true, true);"
-    L"  var el = document.querySelector('#set-as-home-page > *');"
+    L"  var el = document.querySelector('#tip-line > button');"
     L"  el.dispatchEvent(e);"
     L"  return true;"
     L"})()"
@@ -147,17 +150,17 @@ TEST_F(NewTabUITest, HomePageLink) {
     &result));
   ASSERT_TRUE(result);
 
-  // Make sure set as home page element is hidden.
-  std::wstring style_display;
+  // Make sure text of "set as home page" tip has been removed.
+  std::wstring tip_text_content;
   ASSERT_TRUE(tab->ExecuteAndExtractString(L"",
     L"window.domAutomationController.send("
     L"(function() {"
-    L"  var el = document.querySelector('#set-as-home-page');"
-    L"  return el.style.display;"
+    L"  var el = document.querySelector('#tip-line');"
+    L"  return el.textContent;"
     L"})()"
     L")",
-    &style_display));
-  ASSERT_EQ(L"none", style_display);
+    &tip_text_content));
+  ASSERT_EQ(L"", tip_text_content);
 
   // Make sure that the notification is visible
   bool has_class;
diff --git a/chrome/browser/dom_ui/ntp_resource_cache.cc b/chrome/browser/dom_ui/ntp_resource_cache.cc
index 42a2683..8f10ffe 100644
--- a/chrome/browser/dom_ui/ntp_resource_cache.cc
+++ b/chrome/browser/dom_ui/ntp_resource_cache.cc
@@ -143,7 +143,6 @@ NTPResourceCache::NTPResourceCache(Profile* profile) : profile_(profile) {
   // Watch for pref changes that cause us to need to invalidate the HTML cache.
   PrefService* pref_service = profile_->GetPrefs();
   pref_service->AddPrefObserver(prefs::kShowBookmarkBar, this);
-  pref_service->AddPrefObserver(prefs::kHomePageIsNewTabPage, this);
   pref_service->AddPrefObserver(prefs::kNTPShownSections, this);
 
   // Watch for pref changes that cause us to need to invalidate the CSS cache.
@@ -153,7 +152,6 @@ NTPResourceCache::NTPResourceCache(Profile* profile) : profile_(profile) {
 NTPResourceCache::~NTPResourceCache() {
   PrefService* pref_service = profile_->GetPrefs();
   pref_service->RemovePrefObserver(prefs::kShowBookmarkBar, this);
-  pref_service->RemovePrefObserver(prefs::kHomePageIsNewTabPage, this);
   pref_service->RemovePrefObserver(prefs::kNTPShownSections, this);
 
   pref_service->RemovePrefObserver(prefs::kNTPPromoLineRemaining, this);
@@ -359,9 +357,6 @@ void NTPResourceCache::CreateNewTabHTML() {
   else
     localized_strings.SetString(L"syncispresent", "false");
 
-  if (!profile_->GetPrefs()->GetBoolean(prefs::kHomePageIsNewTabPage))
-    localized_strings.SetString(L"showsetashomepage", "true");
-
   ChromeURLDataManager::DataSource::SetFontAndTextDirection(&localized_strings);
 
   // Control fade and resize animations.
diff --git a/chrome/browser/dom_ui/tips_handler.cc b/chrome/browser/dom_ui/tips_handler.cc
index ee429f7..7fd5ac1 100644
--- a/chrome/browser/dom_ui/tips_handler.cc
+++ b/chrome/browser/dom_ui/tips_handler.cc
@@ -4,6 +4,7 @@
 
 #include <string>
 
+#include "app/l10n_util.h"
 #include "base/string_util.h"
 #include "base/values.h"
 #include "chrome/browser/browser_process.h"
@@ -14,6 +15,7 @@
 #include "chrome/common/web_resource/web_resource_unpacker.h"
 #include "chrome/common/url_constants.h"
 #include "googleurl/src/gurl.h"
+#include "grit/generated_resources.h"
 
 DOMMessageHandler* TipsHandler::Attach(DOMUI* dom_ui) {
   dom_ui_ = dom_ui;
@@ -28,7 +30,7 @@ void TipsHandler::RegisterMessages() {
 }
 
 void TipsHandler::HandleGetTips(const Value* content) {
-  // List containing the tips to be displayed.
+  // List containing the tips to be displayed.
   ListValue list_value;
 
   // Holds the web resource data found in the preferences cache.
@@ -58,18 +60,34 @@ void TipsHandler::HandleGetTips(const Value* content) {
         tips_cache_->GetList(
         WebResourceService::kTipCachePrefName, &wr_list) &&
         wr_list && wr_list->GetSize() > 0) {
-      if (wr_list->GetSize() <= static_cast<size_t>(current_tip_index))
+      if (wr_list->GetSize() <= static_cast<size_t>(current_tip_index)) {
+        // Check to see whether the home page is set to NTP; if not, add tip
+        // to set home page before resetting tip index to 0.
         current_tip_index = 0;
+        if (!dom_ui_->GetProfile()->GetPrefs()->GetBoolean(
+            prefs::kHomePageIsNewTabPage)) {
+          SendTip(WideToASCII(l10n_util::GetString(
+              IDS_NEW_TAB_MAKE_THIS_HOMEPAGE)), L"set_homepage_tip",
+              current_tip_index);
+          return;
+        }
+      }
       if (wr_list->GetString(current_tip_index, &current_tip)) {
-        DictionaryValue* tip_dict = new DictionaryValue();
-        tip_dict->SetString(L"tip_html_text", current_tip);
-        list_value.Append(tip_dict);
-        tips_cache_->SetInteger(WebResourceService::kCurrentTipPrefName,
-                                current_tip_index + 1);
+        SendTip(current_tip, L"tip_html_text", current_tip_index + 1);
       }
     }
   }
+}
 
+void TipsHandler::SendTip(std::string tip, std::wstring tip_type,
+                          int tip_index) {
+  // List containing the tips to be displayed.
+  ListValue list_value;
+  DictionaryValue* tip_dict = new DictionaryValue();
+  tip_dict->SetString(tip_type, tip);
+  list_value.Append(tip_dict);
+  tips_cache_->SetInteger(WebResourceService::kCurrentTipPrefName,
+                          tip_index);
   // Send list of web resource items back out to the DOM.
   dom_ui_->CallJavascriptFunction(L"tips", list_value);
 }
diff --git a/chrome/browser/dom_ui/tips_handler.h b/chrome/browser/dom_ui/tips_handler.h
index a5915ec..d85d4d8 100644
--- a/chrome/browser/dom_ui/tips_handler.h
+++ b/chrome/browser/dom_ui/tips_handler.h
@@ -9,6 +9,8 @@
 #ifndef CHROME_BROWSER_DOM_UI_TIPS_HANDLER_H_
 #define CHROME_BROWSER_DOM_UI_TIPS_HANDLER_H_
 
+#include <string>
+
 #include "chrome/browser/dom_ui/dom_ui.h"
 
 class DictionaryValue;
@@ -35,6 +37,11 @@ class TipsHandler : public DOMMessageHandler {
   // Make sure the string we are pushing to the NTP is a valid URL.
   bool IsValidURL(const std::wstring& url_string);
 
+  // Send a tip to the NTP.  tip_type is "tip_html_text" if the tip is from
+  // the tip server, and "set_homepage_tip" if it's the tip to set the NTP
+  // as home page.
+  void SendTip(std::string tip, std::wstring tip_type, int tip_index);
+
   // So we can push data out to the page that has called this handler.
   DOMUI* dom_ui_;
 
diff --git a/chrome/browser/resources/new_new_tab.html b/chrome/browser/resources/new_new_tab.html
index a3ff823..831d3b1 100644
--- a/chrome/browser/resources/new_new_tab.html
+++ b/chrome/browser/resources/new_new_tab.html
@@ -4,8 +4,7 @@
   bookmarkbarattached:bookmarkbarattached;
   hasattribution:hasattribution;
   anim:anim;
-  syncispresent:syncispresent;
-  showsetashomepage:showsetashomepage">
+  syncispresent:syncispresent">
 
 <meta charset="utf-8">
 <title i18n-content="title"></title>
@@ -255,12 +254,6 @@ function updateSimpleSection(id, section) {
   </div>
   <script>updateSimpleSection('sync-status', Section.SYNC);</script>
 
-  <div id="set-as-home-page">
-    <button class="link">
-      <span class="link-color" i18n-content="makethishomepage"></span>
-    </button>
-  </div>
-
   <div id="tip-line"></div>
   <script>updateSimpleSection('tip-line', Section.TIPS);</script>
 
diff --git a/chrome/browser/resources/new_new_tab.js b/chrome/browser/resources/new_new_tab.js
index aecc89a..7b60721 100644
--- a/chrome/browser/resources/new_new_tab.js
+++ b/chrome/browser/resources/new_new_tab.js
@@ -102,20 +102,34 @@ function tips(data) {
 
 function createTip(data) {
   if (data.length) {
-    try {
-      return parseHtmlSubset(data[0].tip_html_text);
-    } catch (parseErr) {
-      console.error('Error parsing tips: ' + parseErr.message);
+    if (data[0].set_homepage_tip) {
+      var homepageButton = document.createElement('button');
+      homepageButton.className = 'link';
+      homepageButton.textContent = data[0].set_homepage_tip;
+      homepageButton.addEventListener('click', setAsHomePageLinkClicked);
+      return homepageButton;
+    } else {
+      try {
+        return parseHtmlSubset(data[0].tip_html_text);
+      } catch (parseErr) {
+        console.error('Error parsing tips: ' + parseErr.message);
+      }
     }
   }
   // Return an empty DF in case of failure.
   return document.createDocumentFragment();
 }
 
-function renderTip() {
+function clearTipLine() {
   var tipElement = $('tip-line');
   // There should always be only one tip.
   tipElement.textContent = '';
+  tipElement.removeEventListener('click', setAsHomePageLinkClicked);
+}
+
+function renderTip() {
+  clearTipLine();
+  var tipElement = $('tip-line');
   tipElement.appendChild(createTip(tipCache));
   fixLinkUnderlines(tipElement);
 }
@@ -1257,10 +1271,6 @@ document.addEventListener('DOMContentLoaded',
 document.addEventListener('DOMContentLoaded',
                           callGetSyncMessageIfSyncIsPresent);
 
-// This link allows user to make new tab page as homepage from the new tab
-// page itself (without going to Options dialog box).
-document.addEventListener('DOMContentLoaded', showSetAsHomePageLink);
-
 // Set up links and text-decoration for promotional message.
 document.addEventListener('DOMContentLoaded', setUpPromoMessage);
 
@@ -1280,22 +1290,10 @@ function setAsHomePageLinkClicked(e) {
   e.preventDefault();
 }
 
-function showSetAsHomePageLink() {
-  var setAsHomePageElement = $('set-as-home-page');
-  var style = setAsHomePageElement.style;
-  if (document.documentElement.getAttribute('showsetashomepage') != 'true') {
-    // Hide the section (if new tab page is already homepage).
-    return;
-  }
-
-  style.display = 'block';
-  var buttonElement = setAsHomePageElement.firstElementChild;
-  buttonElement.addEventListener('click', setAsHomePageLinkClicked);
-}
-
 function onHomePageSet(data) {
-  $('set-as-home-page').style.display = 'none';
   showNotification(data[0], data[1]);
+  // Removes the "make this my home page" tip.
+  clearTipLine();
 }
 
 function hideAllMenus() {