De-suck the NTP a bit more:

* When there are no apps, don't show the apps section.
* Expand the MV section by default, like it was in M6.
* Fix bug where recently closed is incorrectly toggleable.
* Stop showing miniview items for MV section when there is no more room.
* Don't show filler MV items in miniview.
* MV section is 5px too high because we were only apply extra padding
  to the first section, even if that section was disabled.

TEST=Visual inspection.

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

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

8 files changed, 57 insertions, 18 deletions

diff --git a/chrome/browser/dom_ui/new_tab_ui.h b/chrome/browser/dom_ui/new_tab_ui.h
index 35e2a87..e091fcf 100644
--- a/chrome/browser/dom_ui/new_tab_ui.h
+++ b/chrome/browser/dom_ui/new_tab_ui.h
@@ -96,7 +96,7 @@ class NewTabUI : public DOMUI,
   NotificationRegistrar registrar_;
 
   // The preference version. This used for migrating prefs of the NTP.
-  static const int current_pref_version_ = 3;
+  static const int current_pref_version_ = 2;
 
   DISALLOW_COPY_AND_ASSIGN(NewTabUI);
 };
diff --git a/chrome/browser/dom_ui/shown_sections_handler.cc b/chrome/browser/dom_ui/shown_sections_handler.cc
index deb4d01..44cc8b2 100644
--- a/chrome/browser/dom_ui/shown_sections_handler.cc
+++ b/chrome/browser/dom_ui/shown_sections_handler.cc
@@ -110,10 +110,17 @@ void ShownSectionsHandler::MigrateUserPrefs(PrefService* pref_service,
   bool changed = false;
   int shown_sections = pref_service->GetInteger(prefs::kNTPShownSections);
 
-  if (old_pref_version < 3) {
-    // At version 3, we added apps. To bring attention to the feature, we start
-    // it off as the only expanded section.
-    shown_sections = APPS;
+  if (old_pref_version < 1) {
+    // 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;
+    changed = true;
+  }
+
+  if (old_pref_version < 2) {
+    // LIST is no longer used. Change to THUMB.
+    shown_sections &= ~LIST;
+    shown_sections |= THUMB;
     changed = true;
   }
 
diff --git a/chrome/browser/dom_ui/shown_sections_handler_unittest.cc b/chrome/browser/dom_ui/shown_sections_handler_unittest.cc
index 2f9e59b..48ca9a1 100644
--- a/chrome/browser/dom_ui/shown_sections_handler_unittest.cc
+++ b/chrome/browser/dom_ui/shown_sections_handler_unittest.cc
@@ -22,8 +22,28 @@ TEST_F(ShownSectionsHandlerTest, MigrateUserPrefs) {
   pref->RegisterIntegerPref(prefs::kNTPShownSections, 0);
   pref->SetInteger(prefs::kNTPShownSections, THUMB);
 
-  ShownSectionsHandler::MigrateUserPrefs(pref.get(), 0, 3);
+  ShownSectionsHandler::MigrateUserPrefs(pref.get(), 0, 1);
 
   int shown_sections = pref->GetInteger(prefs::kNTPShownSections);
-  EXPECT_EQ(APPS, shown_sections);
+
+  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);
+}
+
+TEST_F(ShownSectionsHandlerTest, MigrateUserPrefs1To2) {
+  scoped_ptr<PrefService> pref(new TestingPrefService);
+
+  // Set an *old* value
+  pref->RegisterIntegerPref(prefs::kNTPShownSections, 0);
+  pref->SetInteger(prefs::kNTPShownSections, LIST);
+
+  ShownSectionsHandler::MigrateUserPrefs(pref.get(), 1, 2);
+
+  int shown_sections = pref->GetInteger(prefs::kNTPShownSections);
+
+  EXPECT_TRUE(shown_sections & THUMB);
+  EXPECT_FALSE(shown_sections & LIST);
 }
diff --git a/chrome/browser/resources/new_new_tab.css b/chrome/browser/resources/new_new_tab.css
index ef82016..8987445 100644
--- a/chrome/browser/resources/new_new_tab.css
+++ b/chrome/browser/resources/new_new_tab.css
@@ -338,13 +338,6 @@ html[dir=rtl] #option-menu > [command=hide]:before {
   cursor: pointer;
 }
 
-/* The first one is special because we want a little extra space at the top of
-the layout. */
-.section:first-child > h2 {
-  padding-top: 20px;
-  background-size: 100% 44px, 100% 10px;
-}
-
 .section > h2 > .disclosure {
   margin-left: -13px;
   padding-right: 4px;
diff --git a/chrome/browser/resources/new_new_tab.html b/chrome/browser/resources/new_new_tab.html
index f14d019..ae85056 100644
--- a/chrome/browser/resources/new_new_tab.html
+++ b/chrome/browser/resources/new_new_tab.html
@@ -130,7 +130,8 @@ if ('mode' in hashParams) {
   </div>
 
   <div class="sections">
-    <div class="section" section="APPS" id="apps-section">
+    <!-- Start disabled. We only enable if we have at least one app. -->
+    <div class="section disabled" section="APPS" id="apps-section">
       <h2><img class="disclosure" img src="ntp/ntp_disclosure_triangle.png"
         ><span i18n-content="apps"></span><div class="back"></div></h2>
       <div class="maxiview" id="apps-section-content"></div>
diff --git a/chrome/browser/resources/new_new_tab.js b/chrome/browser/resources/new_new_tab.js
index f780837..89297c5c 100644
--- a/chrome/browser/resources/new_new_tab.js
+++ b/chrome/browser/resources/new_new_tab.js
@@ -6,6 +6,9 @@
 // than this many items in the miniview.
 var MAX_MINIVIEW_ITEMS = 15;
 
+// Extra spacing at the top of the layout.
+var LAYOUT_SPACING_TOP = 5;
+
 var loading = true;
 
 function updateSimpleSection(id, section) {
@@ -152,7 +155,7 @@ SectionLayoutInfo.getAll = function() {
 function layoutSections() {
   var sections = SectionLayoutInfo.getAll();
   var expandedSection = null;
-  var headerHeight = 0;
+  var headerHeight = LAYOUT_SPACING_TOP;
   var footerHeight = 0;
 
   // Calculate the height of the fixed elements above the expanded section. Also
@@ -204,7 +207,7 @@ function layoutSections() {
   }
 
   // Now position all the elements.
-  var y = 0;
+  var y = LAYOUT_SPACING_TOP;
   for (i = 0, section; section = sections[i]; i++) {
     section.header.style.top = y + 'px';
     y += section.header.offsetHeight;
@@ -737,7 +740,7 @@ $('main').addEventListener('click', function(e) {
   }
 
   p = p.parentNode;
-  if (p.noexpand) {
+  if (p.hasAttribute('noexpand')) {
     return;
   }
 
diff --git a/chrome/browser/resources/ntp/apps.js b/chrome/browser/resources/ntp/apps.js
index 5735b61..5bf1d1a 100644
--- a/chrome/browser/resources/ntp/apps.js
+++ b/chrome/browser/resources/ntp/apps.js
@@ -10,6 +10,11 @@ function getAppsCallback(data) {
   appsSectionContent.textContent = '';
   appsMiniview.textContent = '';
 
+  if (data.apps.length == 0) {
+    appsSection.classList.add('disabled');
+    return;
+  }
+
   data.apps.forEach(function(app) {
     appsSectionContent.appendChild(apps.createElement(app));
   });
@@ -20,6 +25,7 @@ function getAppsCallback(data) {
     appsMiniview.appendChild(apps.createMiniviewElement(app));
   });
 
+  appsSection.classList.remove('disabled');
   layoutSections();
 }
 
diff --git a/chrome/browser/resources/ntp/most_visited.js b/chrome/browser/resources/ntp/most_visited.js
index acb0200..ca01c1e 100644
--- a/chrome/browser/resources/ntp/most_visited.js
+++ b/chrome/browser/resources/ntp/most_visited.js
@@ -561,6 +561,10 @@ var MostVisited = (function() {
       this.miniview.textContent = '';
       var data = this.data.slice(0, MAX_MINIVIEW_ITEMS);
       for (var i = 0, item; item = data[i]; i++) {
+        if (item.filler) {
+          continue;
+        }
+
         var span = document.createElement('span');
         var a = span.appendChild(document.createElement('a'));
         a.href = item.url;
@@ -568,6 +572,11 @@ var MostVisited = (function() {
         a.style.backgroundImage = url('chrome://favicon/' + item.url);
         a.className = 'item';
         this.miniview.appendChild(span);
+
+        if ((a.offsetLeft + a.offsetWidth) > this.miniview.offsetWidth) {
+          this.miniview.removeChild(span);
+          return;
+        }
       }
     },