NTP: Allow hiding tips and bookmark sync.

This change adds 2 new menu items to the option menu.

There is pref migration code to make tips and sync visible by default.

BUG=24319

TEST=Hide and show the different sections and reload to make sure it
is persisted across instances of NTP.

Review URL: http://codereview.chromium.org/337011

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

13 files changed, 194 insertions, 37 deletions

diff --git a/chrome/app/generated_resources.grd b/chrome/app/generated_resources.grd
index d52f306..f086093 100644
--- a/chrome/app/generated_resources.grd
+++ b/chrome/app/generated_resources.grd
@@ -4486,8 +4486,8 @@ Keep your key file in a safe place. You will need it to create new versions of y
         Recent activities
       </message>
       <message name="IDS_NEW_TAB_TIPS"
-               desc="Heading text for the section containing tips and suggestions.">
-        Tips and Suggestions
+               desc="Menu item text for showing and hiding the tips section.">
+        Tips
       </message>
       <message name="IDS_NEW_TAB_DEFAULT_TIPS_TITLE"
                desc="Default content of tip box when no tips are available.  This is a short advertisement and explanation for the tips and suggestions to come.">
@@ -4606,6 +4606,10 @@ Keep your key file in a safe place. You will need it to create new versions of y
          desc="Title for the hard coded thumbnail that represents the Google Chrome Themes Gallery page. This is used on the NTP when there aren't enough thumbnails to show.">
         Google Chrome Themes Gallery
       </message>
+      <message name="IDS_NEW_TAB_SHOW_HIDE_BOOKMARK_SYNC"
+         desc="Menu item text used to show or hide the bookmark sync status.">
+        Bookmark sync
+      </message>
 
       <!-- New tab page web clips. These are not in use yet -->
       <message name="IDS_NEW_TAB_WEB_CLIPS_WEB_CLIPS"
diff --git a/chrome/browser/dom_ui/new_tab_ui.cc b/chrome/browser/dom_ui/new_tab_ui.cc
index 07b8b12..0e53325 100644
--- a/chrome/browser/dom_ui/new_tab_ui.cc
+++ b/chrome/browser/dom_ui/new_tab_ui.cc
@@ -633,10 +633,32 @@ void NewTabUI::InitializeCSSCaches() {
 
 // static
 void NewTabUI::RegisterUserPrefs(PrefService* prefs) {
+  prefs->RegisterIntegerPref(prefs::kNTPPrefVersion, 0);
+
   MostVisitedHandler::RegisterUserPrefs(prefs);
   ShownSectionsHandler::RegisterUserPrefs(prefs);
   if (NewTabUI::WebResourcesEnabled())
     TipsHandler::RegisterUserPrefs(prefs);
+
+  UpdateUserPrefsVersion(prefs);
+}
+
+// static
+bool NewTabUI::UpdateUserPrefsVersion(PrefService* prefs) {
+  const int old_pref_version = prefs->GetInteger(prefs::kNTPPrefVersion);
+  if (old_pref_version != current_pref_version()) {
+    MigrateUserPrefs(prefs, old_pref_version, current_pref_version());
+    prefs->SetInteger(prefs::kNTPPrefVersion, current_pref_version());
+    return true;
+  }
+  return false;
+}
+
+// static
+void NewTabUI::MigrateUserPrefs(PrefService* prefs, int old_pref_version,
+                                int new_pref_version) {
+  ShownSectionsHandler::MigrateUserPrefs(prefs, old_pref_version,
+                                         current_pref_version());
 }
 
 // static
@@ -859,6 +881,11 @@ void NewTabUI::NewTabHTMLSource::InitFullHTML() {
       l10n_util::GetString(IDS_NEW_TAB_MAKE_THIS_HOMEPAGE));
   localized_strings.SetString(L"themelink",
       l10n_util::GetString(IDS_THEMES_GALLERY_URL));
+  localized_strings.SetString(L"tips",
+      l10n_util::GetString(IDS_NEW_TAB_TIPS));
+  localized_strings.SetString(L"sync",
+      l10n_util::GetString(IDS_NEW_TAB_SHOW_HIDE_BOOKMARK_SYNC));
+
   // Don't initiate the sync related message passing with the page if the sync
   // code is not present.
   if (profile_->GetProfileSyncService())
diff --git a/chrome/browser/dom_ui/new_tab_ui.h b/chrome/browser/dom_ui/new_tab_ui.h
index 5aed7c5..e4b494c 100644
--- a/chrome/browser/dom_ui/new_tab_ui.h
+++ b/chrome/browser/dom_ui/new_tab_ui.h
@@ -10,6 +10,7 @@
 #include "chrome/browser/dom_ui/dom_ui.h"
 #include "chrome/browser/dom_ui/chrome_url_data_manager.h"
 #include "chrome/common/notification_registrar.h"
+#include "testing/gtest/include/gtest/gtest_prod.h"
 
 class GURL;
 class MessageLoop;
@@ -29,6 +30,8 @@ class NewTabUI : public DOMUI,
   virtual void RenderViewReused(RenderViewHost* render_view_host);
 
   static void RegisterUserPrefs(PrefService* prefs);
+  static void MigrateUserPrefs(PrefService* prefs, int old_pref_version,
+                               int new_pref_version);
 
   // Whether we should disable the web resources backend service
   static bool WebResourcesEnabled();
@@ -43,6 +46,9 @@ class NewTabUI : public DOMUI,
                                       const string16& title,
                                       const GURL& gurl);
 
+  // The current preference version.
+  static const int current_pref_version() { return current_pref_version_; }
+
   class NewTabHTMLSource : public ChromeURLDataManager::DataSource {
    public:
     explicit NewTabHTMLSource(Profile* profile);
@@ -102,6 +108,8 @@ class NewTabUI : public DOMUI,
   };
 
  private:
+  FRIEND_TEST(NewTabUITest, UpdateUserPrefsVersion);
+
   void Observe(NotificationType type,
                const NotificationSource& source,
                const NotificationDetails& details);
@@ -109,6 +117,10 @@ class NewTabUI : public DOMUI,
   // Reset the CSS caches.
   void InitializeCSSCaches();
 
+  // Updates the user prefs version and calls |MigrateUserPrefs| if needed.
+  // Returns true if the version was updated.
+  static bool UpdateUserPrefsVersion(PrefService* prefs);
+
   NotificationRegistrar registrar_;
 
   // The message id that should be displayed in this NewTabUIContents
@@ -119,6 +131,9 @@ class NewTabUI : public DOMUI,
   // what HTML to load.
   bool incognito_;
 
+  // The preference version. This used for migrating prefs of the NTP.
+  static const int current_pref_version_ = 1;
+
   DISALLOW_COPY_AND_ASSIGN(NewTabUI);
 };
 
diff --git a/chrome/browser/dom_ui/new_tab_ui_uitest.cc b/chrome/browser/dom_ui/new_tab_ui_uitest.cc
index 64d1e37..5779b96e 100644
--- a/chrome/browser/dom_ui/new_tab_ui_uitest.cc
+++ b/chrome/browser/dom_ui/new_tab_ui_uitest.cc
@@ -4,7 +4,11 @@
 
 #include "chrome/test/ui/ui_test.h"
 
+#include "base/file_path.h"
 #include "chrome/app/chrome_dll_resource.h"
+#include "chrome/browser/dom_ui/new_tab_ui.h"
+#include "chrome/common/pref_names.h"
+#include "chrome/common/pref_service.h"
 #include "chrome/test/automation/browser_proxy.h"
 #include "chrome/test/automation/tab_proxy.h"
 #include "chrome/test/automation/window_proxy.h"
@@ -54,3 +58,25 @@ TEST_F(NewTabUITest, NTPHasThumbnails) {
   }
   EXPECT_EQ(0, filler_thumbnails_count);
 }
+
+TEST_F(NewTabUITest, UpdateUserPrefsVersion) {
+  PrefService prefs(FilePath(), NULL);
+
+  // Does the migration
+  NewTabUI::RegisterUserPrefs(&prefs);
+
+  ASSERT_EQ(NewTabUI::current_pref_version(),
+            prefs.GetInteger(prefs::kNTPPrefVersion));
+
+  // Reset the version
+  prefs.ClearPref(prefs::kNTPPrefVersion);
+  ASSERT_EQ(0, prefs.GetInteger(prefs::kNTPPrefVersion));
+
+  bool migrated = NewTabUI::UpdateUserPrefsVersion(&prefs);
+  ASSERT_TRUE(migrated);
+  ASSERT_EQ(NewTabUI::current_pref_version(),
+            prefs.GetInteger(prefs::kNTPPrefVersion));
+
+  migrated = NewTabUI::UpdateUserPrefsVersion(&prefs);
+  ASSERT_FALSE(migrated);
+}
diff --git a/chrome/browser/dom_ui/shown_sections_handler.cc b/chrome/browser/dom_ui/shown_sections_handler.cc
index ad05112..d6769b3 100644
--- a/chrome/browser/dom_ui/shown_sections_handler.cc
+++ b/chrome/browser/dom_ui/shown_sections_handler.cc
@@ -47,6 +47,18 @@ void ShownSectionsHandler::HandleSetShownSections(const Value* value) {
 // static
 void ShownSectionsHandler::RegisterUserPrefs(PrefService* prefs) {
   prefs->RegisterIntegerPref(prefs::kNTPShownSections,
-      THUMB | RECENT | TIPS);
+      THUMB | RECENT | TIPS | SYNC);
 }
 
+// static
+void ShownSectionsHandler::MigrateUserPrefs(PrefService* prefs,
+                                            int old_pref_version,
+                                            int new_pref_version) {
+  if (old_pref_version < 1) {
+    int shown_sections = prefs->GetInteger(prefs::kNTPShownSections);
+    // TIPS was used in early builds of the NNTP but since it was removed before
+    // Chrome 3.0 we want to ensure that it is shown by default.
+    shown_sections |= TIPS | SYNC;
+    prefs->SetInteger(prefs::kNTPShownSections, shown_sections);
+  }
+}
diff --git a/chrome/browser/dom_ui/shown_sections_handler.h b/chrome/browser/dom_ui/shown_sections_handler.h
index cc1a300..132ecbf 100644
--- a/chrome/browser/dom_ui/shown_sections_handler.h
+++ b/chrome/browser/dom_ui/shown_sections_handler.h
@@ -16,7 +16,8 @@ enum Section {
   THUMB = 1,
   LIST = 2,
   RECENT = 4,
-  TIPS = 8
+  TIPS = 8,
+  SYNC = 16
 };
 
 class ShownSectionsHandler : public DOMMessageHandler {
@@ -35,6 +36,9 @@ class ShownSectionsHandler : public DOMMessageHandler {
 
   static void RegisterUserPrefs(PrefService* prefs);
 
+  static void MigrateUserPrefs(PrefService* prefs, int old_pref_version,
+                               int new_pref_version);
+
  private:
   DISALLOW_COPY_AND_ASSIGN(ShownSectionsHandler);
 };
diff --git a/chrome/browser/dom_ui/shown_sections_handler_unittest.cc b/chrome/browser/dom_ui/shown_sections_handler_unittest.cc
new file mode 100644
index 0000000..a3c4429
--- /dev/null
+++ b/chrome/browser/dom_ui/shown_sections_handler_unittest.cc
@@ -0,0 +1,32 @@
+// Copyright (c) 2009 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/dom_ui/shown_sections_handler.h"
+
+#include "base/file_path.h"
+#include "base/scoped_ptr.h"
+#include "chrome/common/pref_names.h"
+#include "chrome/common/pref_service.h"
+#include "testing/gtest/include/gtest/gtest.h"
+
+class ShownSectionsHandlerTest : public testing::Test {
+};
+
+TEST_F(ShownSectionsHandlerTest, MigrateUserPrefs) {
+  PrefService pref(FilePath(), NULL);
+
+  // Set an *old* value
+  pref.RegisterIntegerPref(prefs::kNTPShownSections, 0);
+  pref.SetInteger(prefs::kNTPShownSections, THUMB);
+
+  ShownSectionsHandler::MigrateUserPrefs(&pref, 0, 1);
+
+  int shown_sections = pref.GetInteger(prefs::kNTPShownSections);
+
+  EXPECT_TRUE(shown_sections & THUMB);
+  EXPECT_FALSE(shown_sections & LIST);
+  EXPECT_FALSE(shown_sections & RECENT);
+  EXPECT_TRUE(shown_sections & TIPS);
+  EXPECT_TRUE(shown_sections & SYNC);
+}
diff --git a/chrome/browser/resources/new_new_tab.css b/chrome/browser/resources/new_new_tab.css
index 6436802..8a6c190 100644
--- a/chrome/browser/resources/new_new_tab.css
+++ b/chrome/browser/resources/new_new_tab.css
@@ -12,6 +12,7 @@ body {
   position: relative;
   margin: 0 auto;
   width: 920px;
+  min-height: 50px;
   -webkit-transition: width .15s;
 }
 
@@ -468,7 +469,6 @@ html[dir='rtl'] .item {
 }
 
 #recently-closed {
-  -webkit-transition: opacity .15s;
   background-color: hsla(213, 60%, 92%, .4);
   border: 1px solid hsl(213, 60%, 92%);
   -webkit-border-radius: 5px;
@@ -683,6 +683,10 @@ html[dir='rtl'] #option-menu > [command='hide']:before {
   text-align: center;
 }
 
+#attribution {
+  margin: 10px 0;
+}
+
 #themes-promo {
   position: absolute;
   bottom: 0px;
diff --git a/chrome/browser/resources/new_new_tab.html b/chrome/browser/resources/new_new_tab.html
index f4c9783..0d6ecd2 100644
--- a/chrome/browser/resources/new_new_tab.html
+++ b/chrome/browser/resources/new_new_tab.html
@@ -66,7 +66,9 @@ registerCallback('tips');
 var Section = {
   THUMB: 1,
   LIST: 2,
-  RECENT: 4
+  RECENT: 4,
+  TIPS: 8,
+  SYNC: 16
 };
 
 var shownSections = templateData['shown_sections'];
@@ -160,6 +162,10 @@ function applyMostVisitedRects() {
   }
 }
 
+function updateSimpleSection(id, section) {
+  $(id).style.display = shownSections & section ? '' : 'none';
+}
+
 </script>
 </head>
 <body class="loading"
@@ -178,6 +184,9 @@ function applyMostVisitedRects() {
   <div id="option-menu" class="window-menu">
     <div command="hide" section="THUMB" i18n-content="mostvisited"></div>
     <div command="hide" section="RECENT" i18n-content="recentlyclosed"></div>
+    <div command="hide" section="TIPS" i18n-content="tips"></div>
+    <div command="hide" section="SYNC" i18n-content="sync"
+         id="sync-menu-item"></div>
     <hr>
     <div command="clear-all-blacklisted"
          i18n-content="restorethumbnails"></div>
@@ -186,6 +195,8 @@ function applyMostVisitedRects() {
   <script>
     $('thumb-checkbox').checked = shownSections & Section.THUMB;
     $('list-checkbox').checked = shownSections & Section.LIST;
+    $('sync-menu-item').style.display =
+        templateData['syncispresent'] == 'true' ? '' : 'none';
   </script>
 
   <div id="notification">
@@ -235,16 +246,13 @@ function applyMostVisitedRects() {
           i18n-content="viewfullhistory"></a>
     </span>
   </div>
-  <script>
-    if (!(shownSections & Section.RECENT)) {
-      $('recently-closed').className = 'collapsed';
-    }
-  </script>
+  <script>updateSimpleSection('recently-closed', Section.RECENT);</script>
 
   <div id="sync-status">
     <h2></h2>
     <span></span>
   </div>
+  <script>updateSimpleSection('sync-status', Section.SYNC);</script>
 
   <div id="set-as-homepage">
     <button class="link">
@@ -253,6 +261,7 @@ function applyMostVisitedRects() {
   </div>
 
   <div id="tip-line"></div>
+  <script>updateSimpleSection('tip-line', Section.TIPS);</script>
 
   <div id="attribution" class="attribution">
     <div i18n-content="attributionintro"></div>
diff --git a/chrome/browser/resources/new_new_tab.js b/chrome/browser/resources/new_new_tab.js
index 42619f4..cb7b44b 100644
--- a/chrome/browser/resources/new_new_tab.js
+++ b/chrome/browser/resources/new_new_tab.js
@@ -8,7 +8,10 @@ function hasClass(el, name) {
 }
 
 function addClass(el, name) {
-  el.className += ' ' + name;
+  var names = el.className.split(/\s+/);
+  if (names.indexOf(name) == -1) {
+    el.className += ' ' + name;
+  }
 }
 
 function removeClass(el, name) {
@@ -297,17 +300,27 @@ function showSection(section) {
     if (section == Section.THUMB) {
       // hide LIST
       shownSections &= ~Section.LIST;
-      mostVisited.invalidate();
     } else if (section == Section.LIST) {
       // hide THUMB
       shownSections &= ~Section.THUMB;
-      mostVisited.invalidate();
-    } else {
-      renderRecentlyClosed();
     }
-
-    mostVisited.updateDisplayMode();
-    mostVisited.layout();
+    switch (section) {
+      case Section.THUMB:
+      case Section.LIST:
+        mostVisited.invalidate();
+        mostVisited.updateDisplayMode();
+        mostVisited.layout();
+        break;
+      case Section.RECENT:
+        renderRecentlyClosed();
+        break;
+      case Section.TIPS:
+        $('tip-line').style.display = '';
+        break;
+      case Section.SYNC:
+        $('sync-status').style.display = '';
+        break;
+    }
   }
 }
 
@@ -315,16 +328,23 @@ function hideSection(section) {
   if (section & shownSections) {
     shownSections &= ~section;
 
-    if (section & Section.THUMB || section & Section.LIST) {
-      mostVisited.invalidate();
-    }
-
-    if (section & Section.RECENT) {
-      renderRecentlyClosed();
+    switch (section) {
+      case Section.THUMB:
+      case Section.LIST:
+        mostVisited.invalidate();
+        mostVisited.updateDisplayMode();
+        mostVisited.layout();
+        break;
+      case Section.RECENT:
+        renderRecentlyClosed();
+        break;
+      case Section.TIPS:
+        $('tip-line').style.display = 'none';
+        break;
+      case Section.SYNC:
+        $('sync-status').style.display = 'none';
+        break;
     }
-
-    mostVisited.updateDisplayMode();
-    mostVisited.layout();
   }
 }
 
@@ -545,15 +565,12 @@ var mostVisited = {
 // Recently closed
 
 function layoutRecentlyClosed() {
-  var recentElement = $('recently-closed');
   var recentShown = shownSections & Section.RECENT;
-  var style = recentElement.style;
-
-  if (!recentShown) {
-    addClass(recentElement, 'collapsed');
-  } else {
-    removeClass(recentElement, 'collapsed');
+  updateSimpleSection('recently-closed', Section.RECENT);
 
+  if (recentShown) {
+    var recentElement = $('recently-closed');
+    var style = recentElement.style;
     // We cannot use clientWidth here since the width has a transition.
     var spacing = 20;
     var headerEl = recentElement.firstElementChild;
@@ -602,9 +619,10 @@ function layoutRecentlyClosed() {
 function syncMessageChanged(newMessage) {
   var syncStatusElement = $('sync-status');
   var style = syncStatusElement.style;
+  $('sync-menu-item').style.display = 'block';
 
   // Hide the section if the message is emtpy.
-  if (!newMessage.syncsectionisvisible) {
+  if (!newMessage['syncsectionisvisible'] || !(shownSections & Section.SYNC)) {
     style.display = 'none';
     return;
   }
diff --git a/chrome/chrome.gyp b/chrome/chrome.gyp
index d97f584..278fd06 100755
--- a/chrome/chrome.gyp
+++ b/chrome/chrome.gyp
@@ -4517,6 +4517,7 @@
         'browser/debugger/devtools_manager_unittest.cc',
         'browser/dom_ui/dom_ui_theme_source_unittest.cc',
         'browser/dom_ui/dom_ui_unittest.cc',
+        'browser/dom_ui/shown_sections_handler_unittest.cc',
         'browser/download/download_manager_unittest.cc',
         'browser/download/download_request_infobar_delegate_unittest.cc',
         'browser/download/download_request_manager_unittest.cc',
diff --git a/chrome/common/pref_names.cc b/chrome/common/pref_names.cc
index ca6c17f..27e4407 100644
--- a/chrome/common/pref_names.cc
+++ b/chrome/common/pref_names.cc
@@ -581,9 +581,13 @@ const wchar_t kNTPTipsServer[] = L"ntp.tips_server";
 // 1 - Show the most visited sites in a grid
 // 2 - Show the most visited sites as a list
 // 4 - Show the recent section
-// 8 - Show recommendations
+// 8 - Show tips
+// 16 - show sync status
 const wchar_t kNTPShownSections[] = L"ntp.shown_sections";
 
+// This pref is used for migrating the prefs for the NTP
+const wchar_t kNTPPrefVersion[] = L"ntp.pref_version";
+
 // A boolean specifying whether dev tools window should be opened docked.
 const wchar_t kDevToolsOpenDocked[] = L"devtools.open_docked";
 
diff --git a/chrome/common/pref_names.h b/chrome/common/pref_names.h
index dfbf8dd1..60b6ef8 100644
--- a/chrome/common/pref_names.h
+++ b/chrome/common/pref_names.h
@@ -221,6 +221,7 @@ extern const wchar_t kNTPTipsCache[];
 extern const wchar_t kNTPTipsCacheUpdate[];
 extern const wchar_t kNTPTipsServer[];
 extern const wchar_t kNTPShownSections[];
+extern const wchar_t kNTPPrefVersion[];
 
 extern const wchar_t kDevToolsOpenDocked[];
 extern const wchar_t kDevToolsSplitLocation[];