NTP5: Fix page blacklisting and remove recently closed tabs when they're clicked. Fix the styling …

chrome/browser/resources/ntp_search/most_visited_page.js

Index: chrome/browser/resources/ntp_search/most_visited_page.js
diff --git a/chrome/browser/resources/ntp_search/most_visited_page.js b/chrome/browser/resources/ntp_search/most_visited_page.js
index e3dae2bde32138bf869081f2f79006ab47e5b760..2ab8adfa10a40fcdae0d5183e0d8b1a3fb5bcbb8 100644
--- a/chrome/browser/resources/ntp_search/most_visited_page.js
+++ b/chrome/browser/resources/ntp_search/most_visited_page.js
@@ -210,114 +210,107 @@ cr.define('ntp', function() {
      * TODO(pedrosimonetti): Move data handling related code to TilePage. Make
      * sure the new logic works with Apps without requiring duplicating code.
      * @param {Array} data The array of data.
-     * @type (Array}
      */
-    set data(data) {
+    setData: function(data) {
       var startTime = Date.now();
-      var maxTileCount = this.config_.maxTileCount;
-
-      // The first time data is set, create the tiles.
-      if (!this.data_) {
-        this.data_ = data.slice(0, maxTileCount);
-        this.createTiles_(this.data_.length);
-      } else {
-        this.data_ = refreshData(this.data_, data, maxTileCount);
-      }
-
-      this.updateTiles_();
+      ThumbnailPage.prototype.setData.apply(this, arguments);
       logEvent('mostVisited.layout: ' + (Date.now() - startTime));
     },
-  };
 
-  /**
-   * Executed once the NTP has loaded. Checks if the Most Visited pane is
-   * shown or not. If it is shown, the 'mostVisitedSelected' message is sent
-   * to the C++ code, to record the fact that the user has seen this pane.
-   */
-  MostVisitedPage.onLoaded = function() {
-    if (ntp.getCardSlider() &&
-        ntp.getCardSlider().currentCardValue &&
-        ntp.getCardSlider().currentCardValue.classList
-        .contains('most-visited-page')) {
-      chrome.send('mostVisitedSelected');
-    }
-  };
+    /**
+     * Merges new Most Visited data into the old while minimizing re-ordering.

[Evan Stade, 2012/09/11 16:48:47]

would be easier to review this if you didn't move the function.

[jeremycho, 2012/09/12 04:11:50]

Deleted.

+     * @param {Array} oldData The current page list.
+     * @param {Array} newData The new page list.
+     * @param {number} maxTileCount The maximum number of tiles to render.
+     * @return {Array} The merged page list that should replace the current page
+     *     list.
+     * @private
+     */
+    refreshData_: function(oldData, newData, maxTileCount) {

[Evan Stade, 2012/09/11 16:48:47]

can you enable most_visited_browsertest for the search ntp

[jeremycho, 2012/09/12 04:11:50]

Now that refreshData_ is removed, that test isn't needed.

+      oldData = oldData.slice(0, maxTileCount);
+      newData = newData.slice(0, maxTileCount);
+
+      // Look through old pages; if they exist in the newData list, keep them
+      // where they are.
+      for (var i = 0; i < oldData.length; i++) {
+        if (!oldData[i] || oldData[i].updated)
+          continue;
 
-  /**
-   * We've gotten additional Most Visited data. Update our old data with the
-   * new data. The ordering of the new data is not important, except when a
-   * page is pinned. Thus we try to minimize re-ordering.
-   * @param {Array} oldData The current Most Visited page list.
-   * @param {Array} newData The new Most Visited page list.
-   * @return {Array} The merged page list that should replace the current page
-   *     list.
-   */
-  function refreshData(oldData, newData, maxTileCount) {
-    oldData = oldData.slice(0, maxTileCount);
-    newData = newData.slice(0, maxTileCount);
-
-    // Copy over pinned sites directly.
-    for (var j = 0; j < newData.length; j++) {
-      if (newData[j].pinned) {
-        oldData[j] = newData[j];
-        // Mark the entry as 'updated' so we don't try to update again.
-        oldData[j].updated = true;
-        // Mark the newData page as 'used' so we don't try to re-use it.
-        newData[j].used = true;
+        for (var j = 0; j < newData.length; j++) {
+          if (newData[j].used)
+            continue;
+
+          if (newData[j].url == oldData[i].url) {
+            // The background image and other data may have changed.
+            oldData[i] = newData[j];
+            oldData[i].updated = true;
+            newData[j].used = true;
+            break;
+          }
+        }
       }
-    }
-
-    // Look through old pages; if they exist in the newData list, keep them
-    // where they are.
-    for (var i = 0; i < oldData.length; i++) {
-      if (!oldData[i] || oldData[i].updated)
-        continue;
 
-      for (var j = 0; j < newData.length; j++) {
-        if (newData[j].used)
+      // Look through old pages that haven't been updated yet; replace them.
+      for (var i = 0; i < oldData.length; i++) {
+        if (oldData[i] && oldData[i].updated)
           continue;
 
-        if (newData[j].url == oldData[i].url) {
-          // The background image and other data may have changed.
+        for (var j = 0; j < newData.length; j++) {
+          if (newData[j].used)
+            continue;
+
           oldData[i] = newData[j];
           oldData[i].updated = true;
           newData[j].used = true;
           break;
         }
+
+        if (oldData[i] && !oldData[i].updated)
+          oldData[i] = null;
       }
-    }
 
-    // Look through old pages that haven't been updated yet; replace them.
-    for (var i = 0; i < oldData.length; i++) {
-      if (oldData[i] && oldData[i].updated)
-        continue;
+      // Clear 'updated' flags so this function will work next time it's called.
+      for (var i = 0; i < maxTileCount; i++) {
+        if (oldData[i])
+          oldData[i].updated = false;
+      }
 
-      for (var j = 0; j < newData.length; j++) {
-        if (newData[j].used)
-          continue;
+      return oldData;
+    },
 
-        oldData[i] = newData[j];
-        oldData[i].updated = true;
-        newData[j].used = true;
-        break;
+    /**
+     * Returns all non-null data entries.  Null is used by Most Visited Page as
+     * placeholders for blacklisted entries.
+     * @private
+     * @return {Array} Non-null data entries.
+     */
+    getValidData_: function() {
+      var validData = [];
+      var data = this.data_;
+      for (var i = 0, length = data.length; i < length; i++) {
+        if (data[i])
+          validData.push(data[i]);
       }
+      return validData;
+    },
+  };
 
-      if (oldData[i] && !oldData[i].updated)
-        oldData[i] = null;
-    }
-
-    // Clear 'updated' flags so this function will work next time it's called.
-    for (var i = 0; i < maxTileCount; i++) {
-      if (oldData[i])
-        oldData[i].updated = false;
+  /**
+   * Executed once the NTP has loaded. Checks if the Most Visited pane is
+   * shown or not. If it is shown, the 'mostVisitedSelected' message is sent
+   * to the C++ code, to record the fact that the user has seen this pane.
+   */
+  MostVisitedPage.onLoaded = function() {
+    if (ntp.getCardSlider() &&
+        ntp.getCardSlider().currentCardValue &&
+        ntp.getCardSlider().currentCardValue.classList
+        .contains('most-visited-page')) {
+      chrome.send('mostVisitedSelected');
     }
-
-    return oldData;
-  }
+  };
 
   return {
     MostVisitedPage: MostVisitedPage,
-    refreshData: refreshData,
   };
 });