NTP4/apps page: fix page stickiness

chrome/browser/resources/ntp4/page_list_view.js

 // Copyright (c) 2012 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 90 matching lines @@
 
     /**
      * The 'trash' element.  Note that technically this is unnecessary,
      * JavaScript creates the object for us based on the id.  But I don't want
      * to rely on the ID being the same, and JSCompiler doesn't know about it.
      * @type {!Element|undefined}
      */
     trash: undefined,
 
     /**
-     * The type of page that is currently shown. The value is a numerical ID.
-     * @type {number}
-     */
-    shownPage: 0,
-
-    /**
-     * The index of the page that is currently shown, within the page type.
-     * For example if the third Apps page is showing, this will be 2.
+     * The index of the page that is currently shown. For example if the third
+     * page is showing, this will be 2.
      * @type {number}
      */
     shownPageIndex: 0,
 
     /**
      * EventTracker for managing event listeners for page events.
      * @type {!EventTracker}
      */
     eventTracker: new EventTracker,
 
@@ skipping 30 matching lines @@
         new ntp.Trash(this.trash);
 
       this.pageSwitcherStart = opt_pageSwitcherStart;
       if (this.pageSwitcherStart)
         ntp.initializePageSwitcher(this.pageSwitcherStart);
 
       this.pageSwitcherEnd = opt_pageSwitcherEnd;
       if (this.pageSwitcherEnd)
         ntp.initializePageSwitcher(this.pageSwitcherEnd);
 
-      this.shownPage = loadTimeData.getInteger('shown_page_type');
       this.shownPageIndex = loadTimeData.getInteger('shown_page_index');
 
       // TODO(dbeam): remove showApps and everything that says if (apps).

[Dan Beam, 2016/03/03 21:09:41]

^

       assert(loadTimeData.getBoolean('showApps'));
 
       // 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));
 
       this.tilePages = this.pageList.getElementsByClassName('tile-page');
@@ skipping 260 matching lines @@
         }
         this.updateSliderCards();
       }
 
       var page = this.appsPages[pageIndex];
       var app = $(appData.id);
       if (app) {
         app.replaceAppData(appData);
       } else if (opt_highlight) {
         page.insertAndHighlightApp(appData);
-        this.setShownPage_(loadTimeData.getInteger('apps_page_id'),
-                           appData.page_index);
+        this.setShownPage_(appData.page_index);
       } else {
         page.insertApp(appData, false);
       }
     },
 
     /**
      * Callback invoked by chrome whenever an app preference changes.
      * @param {Object} data An object with all the data on available
      *     applications.
      */
@@ skipping 16 matching lines @@
      * @param {boolean} show Identifies if we should show or hide the promo.
      */
     appLauncherPromoPrefChangeCallback: function(show) {
       loadTimeData.overrideValues({showAppLauncherPromo: show});
       this.updateAppLauncherPromoHiddenState_();
     },
 
     /**
      * Updates the hidden state of the app launcher promo based on the page
      * shown and load data content.
+     * @private
      */
     updateAppLauncherPromoHiddenState_: function() {
       $('app-launcher-promo').hidden =
-          !loadTimeData.getBoolean('showAppLauncherPromo') ||
-          this.shownPage != loadTimeData.getInteger('apps_page_id');
+          !loadTimeData.getBoolean('showAppLauncherPromo');
     },
 
     /**
      * Invoked whenever the pages in apps-page-list have changed so that
      * the Slider knows about the new elements.
      */
     updateSliderCards: function() {
       var pageNo = Math.max(0, Math.min(this.cardSlider.currentCard,
                                         this.tilePages.length - 1));
       this.cardSlider.setCards(Array.prototype.slice.call(this.tilePages),
                                pageNo);
-      if (this.shownPage == loadTimeData.getInteger('apps_page_id') &&
-          loadTimeData.getBoolean('showApps')) {
+      if (loadTimeData.getBoolean('showApps')) {

[Dan Beam, 2016/03/02 00:02:53]

yeah, I know, this is probably never false...

[Evan Stade, 2016/03/03 19:59:53]

todo?

[Dan Beam, 2016/03/03 21:09:41]

Done. (previously)

         this.cardSlider.selectCardByValue(
             this.appsPages[Math.min(this.shownPageIndex,
                                     this.appsPages.length - 1)]);
       }
     },
 
     /**
      * Called whenever tiles should be re-arranging themselves out of the way
      * of a moving or insert tile.
      */
@@ skipping 106 matching lines @@
      * Handler for cardSlider:card_changed events from this.cardSlider.
      * @param {Event} e The cardSlider:card_changed event.
      * @private
      */
     onCardChanged_: function(e) {
       var page = e.cardSlider.currentCardValue;
 
       // Don't change shownPage until startup is done (and page changes actually
       // reflect user actions).
       if (!this.isStartingUp_()) {
-        if (page.classList.contains('apps-page')) {
-          this.setShownPage_(loadTimeData.getInteger('apps_page_id'),
-                             this.getAppsPageIndex(page));
-        } else {
+        if (page.classList.contains('apps-page'))

[Dan Beam, 2016/03/02 00:02:53]

and this is probably never false either...

[Evan Stade, 2016/03/03 19:59:53]

todo

[Dan Beam, 2016/03/03 21:09:41]

Done.

+          this.setShownPage_(this.getAppsPageIndex(page));
+        else
           console.error('unknown page selected');
-        }
       }
 
       // Update the active dot
       var curDot = this.dotList.getElementsByClassName('selected')[0];
       if (curDot)
         curDot.classList.remove('selected');
       page.navigationDot.classList.add('selected');
       this.updatePageSwitchers();
     },
 
     /**
      * Saves/updates the newly selected page to open when first loading the NTP.
-     * @param {number} shownPage The new shown page type.
      * @param {number} shownPageIndex The new shown page index.
      * @private
      */
-    setShownPage_: function(shownPage, shownPageIndex) {
+    setShownPage_: function(shownPageIndex) {
       assert(shownPageIndex >= 0);
-      this.shownPage = shownPage;
       this.shownPageIndex = shownPageIndex;
-      chrome.send('pageSelected', [this.shownPage, this.shownPageIndex]);
+      chrome.send('pageSelected', [this.shownPageIndex]);
       this.updateAppLauncherPromoHiddenState_();
     },
 
     /**
      * Listen for card additions to update the page switchers or the current
      * card accordingly.
      * @param {Event} e A card removed or added event.
      */
     onCardAdded_: function(e) {
       // When the second arg passed to insertBefore is falsey, it acts just like
@@ skipping 106 matching lines @@
       if (page.navigationDot)
         page.navigationDot.remove(opt_animate);
       this.cardSlider.removeCard(page);
     },
   };
 
   return {
     PageListView: PageListView
   };
 });