[NTP4] Auto-deletion of empty apps panes.

chrome/browser/resources/ntp4/page_list_view.js

 // Copyright (c) 2011 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.
 
 /**
  * @fileoverview PageListView implementation.
  * PageListView manages page list, dot list, switcher buttons and handles apps
  * pages callbacks from backend.
  *
  * Note that you need to have AppLauncherHandler in your WebUI to use this code.
@@ skipping 17 matching lines @@
   }
 
   PageListView.prototype = {
     /**
      * The CardSlider object to use for changing app pages.
      * @type {CardSlider|undefined}
      */
     cardSlider: undefined,
 
     /**
-     * The frame div for cardSlider.
+     * The frame div for this.cardSlider.
      * @type {!Element|undefined}
      */
     sliderFrame: undefined,
 
     /**
      * The 'page-list' element.
      * @type {!Element|undefined}
      */
     pageList: undefined,
 
@@ skipping 88 matching lines @@
         new ntp4.Trash(this.trash);
 
       this.pageSwitcherStart = opt_pageSwitcherStart;
       if (this.pageSwitcherStart)
         ntp4.initializePageSwitcher(this.pageSwitcherStart);
 
       this.pageSwitcherEnd = opt_pageSwitcherEnd;
       if (this.pageSwitcherEnd)
         ntp4.initializePageSwitcher(this.pageSwitcherEnd);
 
-      this.shownPage = templateData['shown_page_type'];
-      this.shownPageIndex = templateData['shown_page_index'];
+      this.shownPage = templateData.shown_page_type;
+      this.shownPageIndex = templateData.shown_page_index;
 
       // Request data on the apps so we can fill them in.
       // Note that this is kicked off asynchronously.  'getAppsCallback' will be
       // invoked at some point after this function returns.
       chrome.send('getApps');
 
       document.addEventListener('keydown', this.onDocKeyDown_.bind(this));
       // Prevent touch events from triggering any sort of native scrolling
       document.addEventListener('touchmove', function(e) {
         e.preventDefault();
       }, true);
 
       this.tilePages = this.pageList.getElementsByClassName('tile-page');
       this.appsPages = this.pageList.getElementsByClassName('apps-page');
 
       // Initialize the cardSlider without any cards at the moment
       this.sliderFrame = cardSliderFrame;
       this.cardSlider = new cr.ui.CardSlider(this.sliderFrame, this.pageList,
           this.sliderFrame.offsetWidth);
       this.cardSlider.initialize();
 
       // Handle the page being changed
       this.pageList.addEventListener(
           cr.ui.CardSlider.EventType.CARD_CHANGED,
           this.cardChangedHandler_.bind(this));
 
+      // Handle the end of cards being changed (useful if animated).
+      this.pageList.addEventListener(
+          cr.ui.CardSlider.EventType.CARD_CHANGE_ENDED,
+          this.cardChangeEndedHandler_.bind(this));
+
+      // Handle tiles being removed from tile pages.
+      this.pageList.addEventListener(
+          ntp4.TilePage.EventType.TILE_REMOVED,
+          this.tileRemovedHandler_.bind(this));
+
       // Ensure the slider is resized appropriately with the window
       window.addEventListener('resize', this.onWindowResize_.bind(this));
 
       // Update apps when online state changes.
       window.addEventListener('online',
           this.updateOfflineEnabledApps_.bind(this));
       window.addEventListener('offline',
           this.updateOfflineEnabledApps_.bind(this));
     },
 
     /**
      * Appends a tile page.
      *
      * @param {TilePage} page The page element.
      * @param {string} title The title of the tile page.
      * @param {bool} titleIsEditable If true, the title can be changed.
      * @param {TilePage} opt_refNode Optional reference node to insert in front
-     * of.
+     *     of.
      * When opt_refNode is falsey, |page| will just be appended to the end of
      * the page list.
      */
     appendTilePage: function(page, title, titleIsEditable, opt_refNode) {
       // When opt_refNode is falsey, insertBefore acts just like appendChild.
       this.pageList.insertBefore(page, opt_refNode);
 
+      if (!document.documentElement.classList.contains('starting-up')) {
+        // If we're inserting a card in front of the current card, add 1 to the
+        // current index to adjust for the new card in the order.
+        var index = Array.prototype.indexOf.call(this.tilePages, page);
+        if (typeof this.cardSlider.currentCard != 'undefined' &&
+            index <= this.cardSlider.currentCard) {
+          this.cardSlider.currentCard += 1;
+        }
+        if (page instanceof ntp4.AppsPage &&
+            this.shownPage == templateData.apps_page_id &&
+            this.getAppsPageIndex(page) <= this.shownPageIndex) {
+          this.shownPageIndex += 1;
+        }
+      }
+
       // Remember special MostVisitedPage.
       if (typeof ntp4.MostVisitedPage != 'undefined' &&
           page instanceof ntp4.MostVisitedPage) {
         assert(this.tilePages.length == 1,
                'MostVisitedPage should be added as first tile page');
         this.mostVisitedPage = page;
       }
 
       // If we're appending an AppsPage and it's a temporary page, animate it.
       var animate = page instanceof ntp4.AppsPage &&
@@ skipping 10 matching lines @@
       this.eventTracker.add(page, 'pagelayout', this.onPageLayout_.bind(this));
     },
 
     /**
      * Called by chrome when an existing app has been disabled or
      * removed/uninstalled from chrome.
      * @param {Object} appData A data structure full of relevant information for
      *     the app.
      * @param {boolean} isUninstall True if the app is being uninstalled;
      *     false if the app is being disabled.
+     * @param {boolean} fromPage If the removal was from the current page.
      */
-    appRemoved: function(appData, isUninstall) {
+    appRemoved: function(appData, isUninstall, fromPage) {
       var app = $(appData.id);
       assert(app, 'trying to remove an app that doesn\'t exist');
 
-      if (!isUninstall)
+      if (!isUninstall) {
         app.replaceAppData(appData);
-      else
-        app.remove();
+      } else {
+        // Unset the ID immediately, because the app is already gone. But leave
+        // the tile on the page as it animates out.
+        app.id = '';

[Evan Stade, 2011/12/08 02:34:56]

i think this can go inside the if (fromPage) case?

[Dan Beam, 2011/12/08 04:46:18]

Done.

+        // If the uninstall was from this page, run the blipout animation and
+        // the tile will be removed in TilePage#onContentsAnimationEnd_.
+        // Otherwise delete the tile without auto-deleting the page to avoid
+        // re-deleting the same page (or the page that slid in to take its
+        // place).
+        if (fromPage) {
+          app.classList.add('removing-tile-contents');
+        } else {
+          var tilePage = app.tile.tilePage;
+          tilePage.removeTile(app.tile, false, true);
+          this.removeAppsPageIfEmpty_(tilePage, false, true);
+        }
+      }
     },
 
     /**
      * Callback invoked by chrome with the apps available.
      *
      * Note that calls to this function can occur at any time, not just in
      * response to a getApps request. For example, when a user
      * installs/uninstalls an app on another synchronized devices.
      * @param {Object} data An object with all the data on available
      *        applications.
      */
     getAppsCallback: function(data) {
       var startTime = Date.now();
 
       // Clear any existing apps pages and dots.
       // TODO(rbyers): It might be nice to preserve animation of dots after an
       // uninstall. Could we re-use the existing page and dot elements?  It
       // seems unfortunate to have Chrome send us the entire apps list after an
       // uninstall.
       while (this.appsPages.length > 0) {
         var page = this.appsPages[0];
-        var dot = page.navigationDot;
 
         this.eventTracker.remove(page);
-        page.tearDown();
-        page.parentNode.removeChild(page);
-        dot.parentNode.removeChild(dot);
+        this.removeTilePageAndDot_(page);
       }
 
       // Get the array of apps and add any special synthesized entries
       var apps = data.apps;
 
       // Get a list of page names
-      var pageNames = data.appPageNames;
+      var pageNames = data.appsPageNames;
 
       function stringListIsEmpty(list) {
         for (var i = 0; i < list.length; i++) {
           if (list[i])
             return false;
         }
         return true;
       }
 
       // Sort by launch index
@@ skipping 48 matching lines @@
      * @param {Object} appData A data structure full of relevant information for
      *     the app.
      */
     appAdded: function(appData, opt_highlight) {
       if (appData.id == this.highlightAppId) {
         opt_highlight = true;
         this.highlightAppId = null;
       }
 
       var pageIndex = appData.page_index || 0;
-
-      if (pageIndex >= this.appsPages.length) {
-        while (pageIndex >= this.appsPages.length) {
-          this.appendTilePage(new ntp4.AppsPage(),
-                              localStrings.getString('appDefaultPageName'),
-                              true);
-        }
-        this.updateSliderCards();
-      }
+      this.ensureAppsPageAtIndex_(pageIndex);
 
       var page = this.appsPages[pageIndex];
       var app = $(appData.id);
       if (app)
         app.replaceAppData(appData);
       else
         page.appendApp(appData, opt_highlight);
     },
 
     /**
      * Callback invoked by chrome whenever an app preference changes.
      * @param {Object} data An object with all the data on available
      *     applications.
      */
     appsPrefChangedCallback: function(data) {
-      for (var i = 0; i < data.apps.length; ++i) {
+      for (var i = 0; i < data.apps.length; ++i)
         $(data.apps[i].id).appData = data.apps[i];
-      }
 
-      // Set the App dot names. Skip the first dot (Most Visited).
+      // Set the App dot names. Skip dots that don't belong to apps pages.
       var dots = this.dotList.getElementsByClassName('dot');
-      var start = this.mostVisitedPage ? 1 : 0;
-      for (var i = start; i < dots.length; ++i) {
-        dots[i].displayTitle = data.appPageNames[i - start] || '';
+      var firstAppDot = this.getTilePageIndex(this.appsPages[0]);
+      for (var i = 0; i < data.appsPageNames.length; ++i) {
+        // When we drop an app on a navigation dot or new page we first move the
+        // tile (which calls this), *then* save the page name so sometimes we'll
+        // be one short here.
+        var dot = dots[firstAppDot + i];
+        if (dot)
+          dot.displayTitle = data.appsPageNames[i];
       }
     },
 
     /**
      * Invoked whenever the pages in apps-page-list have changed so that
      * the Slider knows about the new elements.
      */
     updateSliderCards: function() {
-      var pageNo = Math.min(this.cardSlider.currentCard,
-                            this.tilePages.length - 1);
-      this.cardSlider.setCards(Array.prototype.slice.call(this.tilePages),
-                                                          pageNo);
+      var index = -1;
+      var prevPage = this.shownPage;
+      var prevIndex = this.shownPageIndex;
+      var tiles = Array.prototype.slice.call(this.tilePages);
+      // It doesn't hurt to clamp value this every time, even if it's only
+      // applicable to apps. It helps self-heal strange / un-expected data, i.e.

[Evan Stade, 2011/12/08 02:34:56]

strange/unexepected data most likely comes from prefs files. If you mention that
fact it will be more motivating than "if something does something somehow"

[Dan Beam, 2011/12/08 04:46:18]

Done. (changed comment)

+      // going from an apps page to the most visited page directly without
+      // setting this.shownPageIndex = 0 somehow.
+      this.shownPageIndex = Math.max(0, Math.min(this.shownPageIndex,
+                                                 this.appsPages.length - 1));
       switch (this.shownPage) {
-        case templateData['apps_page_id']:
-          this.cardSlider.selectCardByValue(
-              this.appsPages[Math.min(this.shownPageIndex,
-                                      this.appsPages.length - 1)]);
+        case templateData.apps_page_id:
+          index = tiles.indexOf(this.appsPages[this.shownPageIndex]);
           break;
-        case templateData['most_visited_page_id']:
-          if (this.mostVisitedPage)
-            this.cardSlider.selectCardByValue(this.mostVisitedPage);
+        case templateData.most_visited_page_id:
+          index = tiles.indexOf(this.mostVisitedPage);
           break;
       }
+      // If shownPage was saved as a page that's now disabled or the shownPage
+      // doesn't exist any more, index will be -1. Change to the preferred
+      // default page (first apps pane) in this case.
+      if (index < 0) {
+        this.shownPage = templateData.apps_page_id;
+        index = tiles.indexOf(this.appsPages[0]);
+      }
+      // Set the new cards and index.
+      this.cardSlider.setCards(tiles, index);
+      // If we got new page or index, update the pref.
+      if (prevIndex != this.shownPageIndex || prevPage != this.shownPage)

[Evan Stade, 2011/12/08 02:34:56]

you should just always do this. this.shownPageIndex can be out of sync with the
browser's idea of shown page index (e.g. if you click around on a different NTP
after opening this one)

[Dan Beam, 2011/12/08 04:46:18]

Done.  OK, I just noticed it was often doing this more than necessary.

+        chrome.send('pageSelected', [this.shownPage, this.shownPageIndex]);
     },
 
     /**
      * Called whenever tiles should be re-arranging themselves out of the way
      * of a moving or insert tile.
      */
     enterRearrangeMode: function() {
       var tempPage = new ntp4.AppsPage();
       tempPage.classList.add('temporary');
-      this.appendTilePage(tempPage,
-                          localStrings.getString('appDefaultPageName'),
-                          true);
-      var tempIndex = Array.prototype.indexOf.call(this.tilePages, tempPage);
-      if (this.cardSlider.currentCard >= tempIndex)
-        this.cardSlider.currentCard += 1;
+      var pageName = localStrings.getString('appDefaultPageName');
+      this.appendTilePage(tempPage, pageName, true);
       this.updateSliderCards();
+      this.updatePageSwitchers();
 
       if (ntp4.getCurrentlyDraggingTile().firstChild.canBeRemoved())
         $('footer').classList.add('showing-trash-mode');
     },
 
     /**
      * Invoked whenever some app is released
      */
     leaveRearrangeMode: function() {
       var tempPage = document.querySelector('.tile-page.temporary');
-      var dot = tempPage.navigationDot;
-      if (!tempPage.tileCount && tempPage != this.cardSlider.currentCardValue) {
-        dot.animateRemove();
-        var tempIndex = Array.prototype.indexOf.call(this.tilePages, tempPage);
-        if (this.cardSlider.currentCard > tempIndex)
-          this.cardSlider.currentCard -= 1;
-        tempPage.parentNode.removeChild(tempPage);
-        this.updateSliderCards();
-      } else {
+      // Either remove a temp page if it's empty or save the page name (as an
+      // app has just been dropped on it or created somehow.

[Evan Stade, 2011/12/08 02:34:56]

close paren

[Dan Beam, 2011/12/08 04:46:18]

Done.  http://xkcd.com/859/

+      if (tempPage && !this.removeAppsPageIfEmpty_(tempPage, true, true)) {
+        this.saveAppsPageName(tempPage, tempPage.navigationDot.displayTitle);
         tempPage.classList.remove('temporary');
-        this.saveAppPageName(tempPage,
-                             localStrings.getString('appDefaultPageName'));
       }
 
       $('footer').classList.remove('showing-trash-mode');
     },
 
     /**
      * Callback for the 'pagelayout' event.
      * @param {Event} e The event.
      */
     onPageLayout_: function(e) {
@@ skipping 36 matching lines @@
       pageSwitcherRight.style.right = scrollbarWidth + 'px';
 
       var offsetTop = page.querySelector('.tile-page-content').offsetTop + 'px';
       pageSwitcherLeft.style.top = offsetTop;
       pageSwitcherRight.style.top = offsetTop;
       pageSwitcherLeft.style.paddingBottom = offsetTop;
       pageSwitcherRight.style.paddingBottom = offsetTop;
     },
 
     /**
-     * Returns the index of the given page.
-     * @param {AppsPage} page The AppsPage for we wish to find.
-     * @return {number} The index of |page|, or -1 if it is not here.
+     * Returns the index of the given apps page.
+     * @param {AppsPage} page The AppsPage we wish to find.
+     * @return {number} The index of |page| or -1 if it is not in the
+     *    collection.
      */
     getAppsPageIndex: function(page) {
       return Array.prototype.indexOf.call(this.appsPages, page);
     },
 
     /**
      * Handler for CARD_CHANGED on cardSlider.
      * @param {Event} e The CARD_CHANGED event.
      * @private
      */
     cardChangedHandler_: function(e) {
       var page = e.cardSlider.currentCardValue;
 
       // Don't change shownPage until startup is done (and page changes actually
       // reflect user actions).
       if (!document.documentElement.classList.contains('starting-up')) {
+        var prevPage = this.shownPage;
+        var prevIndex = this.shownPageIndex;
         if (page.classList.contains('apps-page')) {
-          this.shownPage = templateData['apps_page_id'];
+          this.shownPage = templateData.apps_page_id;
           this.shownPageIndex = this.getAppsPageIndex(page);
         } else if (page.classList.contains('most-visited-page')) {
-          this.shownPage = templateData['most_visited_page_id'];
+          this.shownPage = templateData.most_visited_page_id;
           this.shownPageIndex = 0;
         } else {
           console.error('unknown page selected');
         }
-        chrome.send('pageSelected', [this.shownPage, this.shownPageIndex]);
+        if (prevPage != this.shownPage || prevIndex != this.shownPageIndex)

[Dan Beam, 2011/12/08 04:46:18]

estade: I'm assuming you'll want me to remove the conditional around here as
well?

+          chrome.send('pageSelected', [this.shownPage, this.shownPageIndex]);
       }
 
       // Update the active dot
       var curDot = this.dotList.getElementsByClassName('selected')[0];
       if (curDot)
         curDot.classList.remove('selected');
       page.navigationDot.classList.add('selected');
       this.updatePageSwitchers();
     },
 
-    /*
-     * Save the name of an app page.
-     * Store the app page name into the preferences store.
-     * @param {AppsPage} appPage The app page for which we wish to save.
+    /**
+     * Save the name of an apps page.
+     * Store the apps page name into the preferences store.
+     * @param {AppsPage} appsPage The app page for which we wish to save.
      * @param {string} name The name of the page.
      */
-    saveAppPageName: function(appPage, name) {
-      var index = this.getAppsPageIndex(appPage);
+    saveAppsPageName: function(appsPage, name) {
+      var index = this.getAppsPageIndex(appsPage);
       assert(index != -1);
-      chrome.send('saveAppPageName', [name, index]);
+      chrome.send('saveAppsPageName', [name, index]);
     },
 
     /**
+     * An Array of callbacks to be called on the next CARD_CHANGE_ENDED event
+     * handled from the cardSlider.
+     * @private
+     */
+    cardChangeEndedCallbacks_: [],
+
+    /**
+     * Handler for CARD_CHANGE_ENDED on cardSlider.
+     * @param {Event} e The CARD_CHANGE_ENDED event.
+     * @private
+     */
+    cardChangeEndedHandler_: function(e) {
+      if (!document.documentElement.classList.contains('starting-up')) {
+        while (this.cardChangeEndedCallbacks_.length > 0)
+          this.cardChangeEndedCallbacks_.shift().call(this, e);
+      }
+    },
+
+    /**
+     * Happens when a tile is removed from a tile page.
+     * @param {Event} e An event dispatched from a tile when it is removed.
+     */
+    tileRemovedHandler_: function(e) {
+      if (e.tilePage instanceof ntp4.AppsPage)
+        this.removeAppsPageIfEmpty_(e.tilePage, e.wasAnimated);
+    },
+
+    /**
+     * Remove an apps page if it now has no tiles (is empty).
+     * @param {AppsPage} appsPage A page to check for emptiness.
+     * @param {boolean=} opt_animate Whether the prospective removal should be
+     *     animated.
+     * @param {boolean=} opt_dontNotify Whether this NTP's AppLauncherHandler
+     *     shouldn't be notified of this pages' prospective removal (default is
+     *     to notify).
+     * @return {boolean} If |appsPage| was removed or not.
+     */
+    removeAppsPageIfEmpty_: function(appsPage, opt_animate, opt_dontNotify) {
+      assert(appsPage instanceof ntp4.AppsPage,
+             '|appsPage| is really an AppsPage');

[Evan Stade, 2011/12/08 02:34:56]

isn't the string an error message? Hence you would say "...is not really..."

[Dan Beam, 2011/12/08 04:46:18]

Done.

+      if (appsPage.tileCount == 0) {
+        var whenDone = function() {

[Evan Stade, 2011/12/08 02:34:56]

some comment about whenDone is necessary

[Dan Beam, 2011/12/08 04:46:18]

Done.

+          if (!opt_dontNotify) {

[Evan Stade, 2011/12/08 02:34:56]

no curlies

[Dan Beam, 2011/12/08 04:46:18]

Done.

+            chrome.send('deleteAppsPage', [this.getAppsPageIndex(appsPage)]);
+          }
+          this.removeTilePageAndDot_(appsPage, opt_animate);
+          this.updateSliderCards();
+          this.updatePageSwitchers();
+        };

[Evan Stade, 2011/12/08 02:34:56]

newline

[Dan Beam, 2011/12/08 04:46:18]

Done.

+        // If currently on the empty apps pane, move off of it before deleting.
+        if (appsPage === this.cardSlider.currentCardValue) {
+          this.cardChangeEndedCallbacks_.push(whenDone);
+          this.moveOffEmptyAppsPage(appsPage, opt_animate);
+        } else {
+          whenDone.call(this);
+        }
+        return true;
+      }
+      return false;

[Evan Stade, 2011/12/08 02:34:56]

if (appsPage.tileCount != 0)
  return false;

...

this way the entire function body isn't indented

[Dan Beam, 2011/12/08 04:46:18]

Done.

+    },
+
+    /**
      * Window resize handler.
      * @private
      */
     onWindowResize_: function(e) {
       this.cardSlider.resize(this.sliderFrame.offsetWidth);
       this.updatePageSwitchers();
     },
 
     /**
      * Listener for offline status change events. Updates apps that are
@@ skipping 27 matching lines @@
         direction = 1;
       else
         return;
 
       var cardIndex =
           (this.cardSlider.currentCard + direction +
            this.cardSlider.cardCount) % this.cardSlider.cardCount;
       this.cardSlider.selectCard(cardIndex, true);
 
       e.stopPropagation();
-    }
+    },
+
+    /**
+     * Re-order apps on this inactive page when an active NTP gets re-ordered.
+     * @param {Object} data Position hashmap ordered by app id with indices:
+     *    {id => {page_index: ##, app_launch_index: ##}}
+     */
+    appsReordered: function(data) {
+      var pages = Object.keys(data);
+      for (var i = 0; i < pages.length; ++i) {
+        var page = data[pages[i]];
+        var indices = Object.keys(page);
+        for (var j = 0; j < indices.length; ++j) {
+          var app = $(page[indices[j]]);
+          var originalPage = app.tile.tilePage;
+          if (i != this.getAppsPageIndex(originalPage) || j != app.tile.index) {
+            originalPage.removeTile(app.tile, false, true);
+            this.ensureAppsPageAtIndex_(i);
+            this.appsPages[i].addTileAt(app, j);
+          }
+        }
+      }
+      for (var i = 0; i < this.appsPages.length; ++i) {
+        // If a page is now empty, remove it and move backward one in our

[Evan Stade, 2011/12/08 02:34:56]

comments are nice but this code is self-documenting/self-evident enough that I
don't think it needs a comment. The other thing you can do is

forEach removeAppsPageIfEmpty
forEach cleanupDrag

which is a flow that's even a little easier to follow imho

[Dan Beam, 2011/12/08 04:46:18]

Done.
I originally did this but it was annoying as I had to subtract one from i every
time as it is a live list so I just combined the two.  That cool?

+        // array-like object as it is a live NodeList and the length/indice

[Evan Stade, 2011/12/08 02:34:56]

INDICE

[Dan Beam, 2011/12/08 04:46:18]

DONE. (well, removed whole comment...)

+        // change when you update it. Otherwise, make sure the tiles are in the
+        // right positions (just like after a user is done dragging).
+        if (this.removeAppsPageIfEmpty_(this.appsPages[i], false, true))
+          --i;
+        else
+          this.appsPages[i].cleanupDrag();
+      }
+    },
+
+    /**
+     * Ensure there is an apps page at the given |index|. If index is smaller

[Evan Stade, 2011/12/08 02:34:56]

smaller??

[Dan Beam, 2011/12/08 04:46:18]

Done.

+     * than the current number of apps pages, additional apps pages will be
+     * created.
+     * @param {number} index An apps page index to ensure exists.
+     */
+    ensureAppsPageAtIndex_: function(index) {
+      if (index >= this.appsPages.length) {
+        var pageName = localStrings.getString('appDefaultPageName');
+        while (index >= this.appsPages.length)
+          this.appendTilePage(new ntp4.AppsPage(), pageName, true);
+        this.updateSliderCards();
+      }
+    },
+
+    /**
+     * Returns the index of a given tile page.
+     * @param {TilePage} page The TilePage we wish to find.
+     * @return {number} The index of |page| or -1 if it is not in the
+     *    collection.
+     */
+    getTilePageIndex: function(page) {
+      return Array.prototype.indexOf.call(this.tilePages, page);
+    },
+
+    /**
+     * Move to a non-empty page before/while we delete an empty one.
+     * @param {AppsPage} appsPage An empty apps page that we want to move off of
+     *     (probably because it's empty).
+     * @param {boolean=} opt_animate Whether we should animate or not.
+     */
+    moveOffEmptyAppsPage: function(appsPage, opt_animate) {
+      assert(appsPage instanceof ntp4.AppsPage,
+             '|appsPage| is really an AppsPage');
+      // Select the page further toward the end than the empty |appsPage|. If
+      // the empty |appsPage| is at the end, the apps page before it will be
+      // selected.
+      var index = this.getAppsPageIndex(appsPage);
+      assert(index >= 0, '|appsPage| is in this.appsPages');
+      var direction = (index == this.appsPages.length - 1 ? -1 : 1);

[Evan Stade, 2011/12/08 02:34:56]

no ()

[Dan Beam, 2011/12/08 04:46:18]

Done. (but I still don't agree that this is easier to read or safe)

+      this.cardSlider.selectCard(this.cardSlider.currentCard + direction,
+                                 opt_animate);
+    },
+
+    /**
+     * Removes a page and navigation dot (if the navdot exists).
+     * @param {TilePage} page The page to be removed.
+     * @param {boolean=} opt_animate If the removal should be animated.
+     */
+    removeTilePageAndDot_: function(page, opt_animate) {
+      if (!document.documentElement.classList.contains('starting-up')) {
+        // If the card being deleted is before the current card, keep the same
+        // spot in the list by removing 1 from the current card index.
+        var index = Array.prototype.indexOf.call(this.tilePages, page);
+        if (index < this.cardSlider.currentCard) {

[Evan Stade, 2011/12/08 02:34:56]

no curlies

[Dan Beam, 2011/12/08 04:46:18]

Done.

+          this.cardSlider.currentCard -= 1;
+        }
+        // If the page we're about to delete is in front of the current page,
+        // remove 1 from the shown page index.
+        if (page instanceof ntp4.AppsPage &&
+            this.shownPage == templateData.apps_page_id &&
+            this.getAppsPageIndex(page) < this.shownPageIndex) {
+          this.shownPageIndex -= 1;

[Evan Stade, 2011/12/08 02:34:56]

this doesn't seem necessary. Wont the shownPageIndex be updated by
cardSlider.currentCard assignment?

[Dan Beam, 2011/12/08 04:46:18]

cardSlider.currentCard is an ES5 getter but all it's doing it just setting the
private member and not automagically switching anything.  Is this what you were
thinking?  Why do you think this *isn't* necessary?  I'm hesitant to change it
now as everything works, gar.  Let me try.

[Dan Beam, 2011/12/08 07:27:52]

Working on fixing this to be in cardSlider instead like we talked about.

+        }
+      }
+      if (page.navigationDot)
+        page.navigationDot.remove(opt_animate);
+      page.remove();
+    },
   };
 
   return {
     PageListView: PageListView
   };
 });