Refactor ordering of NavigationListModel items

Replace the logic in NavigationListModel.prototype.item and
NavigationListModel.prototype.length_ to instead calculate the order
and preserve it in an array in reorderNavigationItems_.  This
makes the ordering rules clearer and should also give faster
performance for the more frequently accessed item function.

Update tests to include recent.

This change in done in advance of adding crostini Linux Files.

Bug: 834103
Cq-Include-Trybots: master.tryserver.chromium.linux:closure_compilation
Change-Id: I08c3222b4317b0d38524c52e320545d6717ef9b0
Reviewed-on: https://chromium-review.googlesource.com/1039207
Commit-Queue: Joel Hockey <joelhockey@chromium.org>
Reviewed-by: Sasha Morrissey <sashab@chromium.org>
Cr-Commit-Position: refs/heads/master@{#555649}

ui/file_manager/file_manager/foreground/js/file_manager.js

@@ -1166,11 +1166,6 @@ FileManager.prototype = /** @struct */ {
                            fakeEntriesVisible);
     directoryTree.dataModel = new NavigationListModel(
         assert(this.volumeManager_), assert(this.folderShortcutsModel_),
-        addNewServicesVisible ?
-            new NavigationModelMenuItem(
-                str('ADD_NEW_SERVICES_BUTTON_LABEL'), '#add-new-services-menu',
-                'add-new-services') :
-            null,
         fakeEntriesVisible &&
                 !DialogType.isFolderDialog(this.launchParams_.type) ?
             new NavigationModelRecentItem(str('RECENT_ROOT_LABEL'), {
@@ -1181,6 +1176,11 @@ FileManager.prototype = /** @struct */ {
               },
               sourceRestriction: this.getSourceRestriction_()
             }) :
+            null,
+        addNewServicesVisible ?
+            new NavigationModelMenuItem(
+                str('ADD_NEW_SERVICES_BUTTON_LABEL'), '#add-new-services-menu',
+                'add-new-services') :
             null);
     this.ui_.initDirectoryTree(directoryTree);
   };

ui/file_manager/file_manager/foreground/js/navigation_list_model.js

@@ -135,15 +135,13 @@ NavigationModelRecentItem.prototype = /** @struct */ {
  * @param {!VolumeManagerWrapper} volumeManager VolumeManagerWrapper instance.
  * @param {(!cr.ui.ArrayDataModel|!FolderShortcutsDataModel)} shortcutListModel
  *     The list of folder shortcut.
- * @param {NavigationModelMenuItem} menuModelItem Menu button at the end of the
- *     list.
- * @param {NavigationModelRecentItem} recentModelItem Recent folder below the
- *     Downloads volume in the list.
+ * @param {NavigationModelRecentItem} recentModelItem Recent folder.
+ * @param {NavigationModelMenuItem} addNewServicesItem Add new services item.
  * @constructor
  * @extends {cr.EventTarget}
  */
 function NavigationListModel(
-    volumeManager, shortcutListModel, menuModelItem, recentModelItem) {
+    volumeManager, shortcutListModel, recentModelItem, addNewServicesItem) {
   cr.EventTarget.call(this);
 
   /**
@@ -159,16 +157,23 @@ function NavigationListModel(
   this.shortcutListModel_ = shortcutListModel;
 
   /**
-   * @private {NavigationModelMenuItem}
+   * @private {NavigationModelRecentItem}
    * @const
    */
-  this.menuModelItem_ = menuModelItem;
+  this.recentModelItem_ = recentModelItem;
 
   /**
-   * @private {NavigationModelRecentItem}
+   * @private {NavigationModelMenuItem}
    * @const
    */
-  this.recentModelItem_ = recentModelItem;
+  this.addNewServicesItem_ = addNewServicesItem;
+
+
+  /**
+   * All root navigation items in display order.
+   * @private {!Array<!NavigationModelItem>}
+   */
+  this.navigationItems_ = [];
 
   var volumeInfoToModelItem = function(volumeInfo) {
     return new NavigationModelVolumeItem(
@@ -205,7 +210,11 @@ function NavigationListModel(
     this.shortcutList_.push(entryToModelItem(shortcutEntry));
   }
 
-  // Generates a combined 'permuted' event from an event of either list.
+  // Reorder volumes, shortcuts, and optional items.
+  this.reorderNavigationItems_();
+
+  // Generates a combined 'permuted' event from an event of either volumeList or
+  // shortcutList.
   var permutedHandler = function(listType, event) {
     var permutation;
 
@@ -288,6 +297,9 @@ function NavigationListModel(
       this.shortcutList_ = newList;
     }
 
+    // Reorder items after permutation.
+    this.reorderNavigationItems_();
+
     // Dispatch permuted event.
     var permutedEvent = new Event('permuted');
     permutedEvent.newLength =
@@ -316,28 +328,34 @@ NavigationListModel.prototype = {
   get folderShortcutList() { return this.shortcutList_; }
 };
 
+/**
+ * Reorder navigation items in the following order:
+ *  1. Volumes.
+ *  2. If Downloads exists, then immediately after Downloads should be:
+ *  2a. Recent if it exists.
+ *  3. Shortcuts.
+ *  4. Add new services if it exists.
+ * @private
+ */
+NavigationListModel.prototype.reorderNavigationItems_ = function() {
+  // Items as per required order.
+  this.navigationItems_ = this.volumeList_.slice();
+  var downloadsVolumeIndex = this.findDownloadsVolumeIndex_();
+  if (this.recentModelItem_ && downloadsVolumeIndex >= 0)
+    this.navigationItems_.splice(
+        downloadsVolumeIndex + 1, 0, this.recentModelItem_);
+  Array.prototype.push.apply(this.navigationItems_, this.shortcutList_);
+  if (this.addNewServicesItem_)
+    this.navigationItems_.push(this.addNewServicesItem_);
+};
+
 /**
  * Returns the item at the given index.
  * @param {number} index The index of the entry to get.
  * @return {NavigationModelItem|undefined} The item at the given index.
  */
 NavigationListModel.prototype.item = function(index) {
-  // If we should show "Recent" folder, insert it just below Downloads volume.
-  var downloadsVolumeIndex = this.findDownloadsVolumeIndex_();
-  var indexWithoutRecent = index;
-  if (this.recentModelItem_ && downloadsVolumeIndex >= 0) {
-    if (index == downloadsVolumeIndex + 1)
-      return this.recentModelItem_;
-    if (index > downloadsVolumeIndex + 1)
-      indexWithoutRecent--;
-  }
-  if (indexWithoutRecent < this.volumeList_.length)
-    return this.volumeList_[indexWithoutRecent];
-  if (indexWithoutRecent < this.volumeList_.length + this.shortcutList_.length)
-    return this.shortcutList_[indexWithoutRecent - this.volumeList_.length];
-  if (index === this.length_() - 1)
-    return this.menuModelItem_;
-  return undefined;
+  return this.navigationItems_[index];
 };
 
 /**
@@ -346,9 +364,7 @@ NavigationListModel.prototype.item = function(index) {
  * @private
  */
 NavigationListModel.prototype.length_ = function() {
-  return this.volumeList_.length + this.shortcutList_.length +
-      (this.menuModelItem_ ? 1 : 0) +
-      (this.recentModelItem_ && this.findDownloadsVolumeIndex_() >= 0 ? 1 : 0);
+  return this.navigationItems_.length;
 };
 
 /**

ui/file_manager/file_manager/foreground/js/navigation_list_model_unittest.js

@@ -27,10 +27,36 @@ function testModel() {
   var volumeManager = new MockVolumeManagerWrapper();
   var shortcutListModel = new MockFolderShortcutDataModel(
       [new MockFileEntry(drive, '/root/shortcut')]);
-  var menuModel = new NavigationModelMenuItem(
+  var fakeEntry = {
+    toURL: function() {
+      return 'fake-entry://recent';
+    }
+  };
+  var recentItem = new NavigationModelRecentItem('recent-label', fakeEntry);
+  var addNewServicesItem = new NavigationModelMenuItem(
       'menu-button-label', '#add-new-services', 'menu-button-icon');
   var model = new NavigationListModel(
-      volumeManager, shortcutListModel, menuModel);
+      volumeManager, shortcutListModel, recentItem, addNewServicesItem);
+
+  assertEquals(5, model.length);
+  assertEquals('drive', model.item(0).volumeInfo.volumeId);
+  assertEquals('downloads', model.item(1).volumeInfo.volumeId);
+  assertEquals('fake-entry://recent', model.item(2).entry.toURL());
+  assertEquals('/root/shortcut', model.item(3).entry.fullPath);
+  assertEquals('menu-button-label', model.item(4).label);
+  assertEquals('#add-new-services', model.item(4).menu);
+  assertEquals('menu-button-icon', model.item(4).icon);
+}
+
+function testNoRecent() {
+  var volumeManager = new MockVolumeManagerWrapper();
+  var shortcutListModel = new MockFolderShortcutDataModel(
+      [new MockFileEntry(drive, '/root/shortcut')]);
+  var recentItem = null;
+  var addNewServicesItem = new NavigationModelMenuItem(
+      'menu-button-label', '#add-new-services', 'menu-button-icon');
+  var model = new NavigationListModel(
+      volumeManager, shortcutListModel, recentItem, addNewServicesItem);
 
   assertEquals(4, model.length);
   assertEquals('drive', model.item(0).volumeInfo.volumeId);
@@ -45,7 +71,10 @@ function testAddAndRemoveShortcuts() {
   var volumeManager = new MockVolumeManagerWrapper();
   var shortcutListModel = new MockFolderShortcutDataModel(
       [new MockFileEntry(drive, '/root/shortcut')]);
-  var model = new NavigationListModel(volumeManager, shortcutListModel, null);
+  var recentItem = null;
+  var addNewServicesItem = null;
+  var model = new NavigationListModel(
+      volumeManager, shortcutListModel, recentItem, addNewServicesItem);
 
   assertEquals(3, model.length);
 
@@ -77,7 +106,10 @@ function testAddAndRemoveVolumes() {
   var volumeManager = new MockVolumeManagerWrapper();
   var shortcutListModel = new MockFolderShortcutDataModel(
       [new MockFileEntry(drive, '/root/shortcut')]);
-  var model = new NavigationListModel(volumeManager, shortcutListModel, null);
+  var recentItem = null;
+  var addNewServicesItem = null;
+  var model = new NavigationListModel(
+      volumeManager, shortcutListModel, recentItem, addNewServicesItem);
 
   assertEquals(3, model.length);