Partition grouping in navigation list model.

List partitions of external media devices as subdirectories underneath a
common root label to improve visual distinction. Make EntryList URL
unique for removable roots (otherwise directory change won't be
triggered).

Bug: 918795
Change-Id: I071664a333d44e0d9f4e1ca77bcb6dafda89e181
Reviewed-on: https://chromium-review.googlesource.com/c/1449472Reviewed-by: Luciano Pacheco <lucmult@chromium.org>
Commit-Queue: Alli Murray <alliemurray@google.com>
Cr-Commit-Position: refs/heads/master@{#629862}

ui/file_manager/file_manager/background/js/entry_location_impl.js

@@ -47,7 +47,8 @@ function EntryLocationImpl(volumeInfo, rootType, isRootEntry, isReadOnly) {
   /** @type{boolean} */
   this.hasFixedLabel = this.isRootEntry &&
       (rootType !== VolumeManagerCommon.RootType.TEAM_DRIVE &&
-       rootType !== VolumeManagerCommon.RootType.COMPUTER);
+       rootType !== VolumeManagerCommon.RootType.COMPUTER &&
+       rootType !== VolumeManagerCommon.RootType.REMOVABLE);
 
   Object.freeze(this);
 }

ui/file_manager/file_manager/background/js/mock_volume_manager.js

@@ -146,9 +146,11 @@ MockVolumeManager.prototype.getDriveConnectionState = function() {
  * @param {!VolumeManagerCommon.VolumeType} type Volume type.
  * @param {string} volumeId Volume id.
  * @param {string=} label Label.
+ * @param {string=} devicePath Device path.
  * @return {!VolumeInfo} Created mock VolumeInfo.
  */
-MockVolumeManager.createMockVolumeInfo = function(type, volumeId, label) {
+MockVolumeManager.createMockVolumeInfo = function(
+    type, volumeId, label, devicePath) {
   var fileSystem = new MockFileSystem(volumeId, 'filesystem:' + volumeId);
 
   // If there's no label set it to volumeId to make it shorter to write tests.
@@ -156,7 +158,7 @@ MockVolumeManager.createMockVolumeInfo = function(type, volumeId, label) {
       type, volumeId, fileSystem,
       '',                                          // error
       '',                                          // deviceType
-      '',                                          // devicePath
+      devicePath || '',                            // devicePath
       false,                                       // isReadOnly
       false,                                       // isReadOnlyRemovableDevice
       {isCurrentProfile: true, displayName: ''},   // profile

ui/file_manager/file_manager/common/js/files_app_entry_types.js

@@ -258,9 +258,9 @@ class EntryList {
    * @param {string} label: Label to be used when displaying to user, it should
    *    already translated.
    * @param {VolumeManagerCommon.RootType} rootType root type.
-   *
+   * @param {string} devicePath Device path
    */
-  constructor(label, rootType) {
+  constructor(label, rootType, devicePath = '') {
     /**
      * @private {string} label: Label to be used when displaying to user, it
      *      should be already translated. */
@@ -269,6 +269,12 @@ class EntryList {
     /** @private {VolumeManagerCommon.RootType} rootType root type. */
     this.rootType_ = rootType;
 
+    /**
+     * @private {string} devicePath Path belonging to the external media
+     * device. Partitions on the same external drive have the same device path.
+     */
+    this.devicePath_ = devicePath;
+
     /**
      * @private {!Array<!Entry|!FilesAppEntry>} children entries of
      * this EntryList instance.
@@ -317,6 +323,11 @@ class EntryList {
    * @override
    */
   toURL() {
+    // There may be multiple entry lists. Append the device path to return
+    // a unique identifiable URL for the entry list.
+    if (this.devicePath_) {
+      return 'entry-list://' + this.rootType + '/' + this.devicePath_;
+    }
     return 'entry-list://' + this.rootType;
   }
 

ui/file_manager/file_manager/foreground/css/file_types.css

@@ -455,8 +455,8 @@ tree .tree-item[selected] > .tree-row >
       url(../images/volumes/2x/phone_active.png) 2x);
 }
 
-[volume-type-icon='removable'][volume-subtype='partition'],
 [volume-type-icon='removable'][volume-subtype='unknown'],
+.tree-item .tree-item [volume-type-icon='removable'],
 [file-type-icon='removable'] {
   background-image: -webkit-image-set(
       url(../images/volumes/hard_drive.png) 1x,
@@ -464,8 +464,15 @@ tree .tree-item[selected] > .tree-row >
 }
 
 tree .tree-item[selected] > .tree-row >
-[volume-type-icon='removable'][volume-subtype='partition'],
-.tree-row[selected] [volume-type-icon='removable'][volume-subtype='unknown']  {
+.tree-row[selected] [volume-type-icon='removable'][volume-subtype='unknown'],
+.tree-item .tree-item[selected] [volume-type-icon='removable'] {
+  background-image: -webkit-image-set(
+      url(../images/volumes/hard_drive_active.png) 1x,
+      url(../images/volumes/2x/hard_drive_active.png) 2x);
+}
+
+tree .tree-item > .tree-item > .tree-row >
+[volume-type-icon='removable'] {
   background-image: -webkit-image-set(
       url(../images/volumes/hard_drive_active.png) 1x,
       url(../images/volumes/2x/hard_drive_active.png) 2x);

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

@@ -198,6 +198,14 @@ function NavigationListModel(
    */
   this.myFilesModel_ = null;
 
+  /**
+   * A collection of NavigationModel objects for Removable partition groups.
+   * Store the reference to each model here since DirectoryTree expects it to
+   * always have the same reference.
+   * @private {!Map<string, !NavigationModelFakeItem>}
+   */
+  this.removableModels_ = new Map();
+
   /**
    * True when MyFiles should be a volume and Downloads just a plain folder
    * inside it. When false MyFiles is an EntryList, which means UI only type,
@@ -504,6 +512,32 @@ NavigationListModel.prototype.orderAndNestItems_ = function() {
     return indexes.map(idx => volumeList[idx]);
   };
 
+  /**
+   * Removable volumes which share the same device path (i.e. partitions) are
+   * grouped.
+   * @return !Map<string, !Array<!NavigationModelVolumeItem>>
+   */
+  const groupRemovables = function() {
+    const removableGroups = new Map();
+    const removableVolumes =
+        getVolumes(VolumeManagerCommon.VolumeType.REMOVABLE);
+
+    for (const removable of removableVolumes) {
+      // Partitions on the same physical device share device path and drive
+      // label. Create keys using these two identifiers.
+      let key = removable.volumeInfo.devicePath + '/' +
+          removable.volumeInfo.driveLabel;
+      if (!removableGroups.has(key)) {
+        // New key, so create a new array to hold partitions.
+        removableGroups.set(key, []);
+      }
+      // Add volume to array of volumes matching device path and drive label.
+      removableGroups.get(key).push(removable);
+    }
+
+    return removableGroups;
+  };
+
   // Items as per required order.
   this.navigationItems_ = [];
 
@@ -637,11 +671,49 @@ NavigationListModel.prototype.orderAndNestItems_ = function() {
     provider.section = NavigationSection.CLOUD;
   }
 
-  // Join MTP, ARCHIVE and REMOVABLE. These types belong to same section.
+  // Add REMOVABLE volumes and partitions.
+  const removableModels = new Map();
+  for (const [devicePath, removableGroup] of groupRemovables().entries()) {
+    if (removableGroup.length == 1) {
+      // Add unpartitioned removable device as a regular volume.
+      this.navigationItems_.push(removableGroup[0]);
+      removableGroup[0].section = NavigationSection.REMOVABLE;
+      continue;
+    }
+
+    // Multiple partitions found.
+    let removableModel;
+    if (this.removableModels_.has(devicePath)) {
+      // Removable model has been seen before. Use the same reference.
+      removableModel = this.removableModels_.get(devicePath);
+    } else {
+      // Create an EntryList for new removable group.
+      const rootLabel = removableGroup[0].volumeInfo.driveLabel ?
+          removableGroup[0].volumeInfo.driveLabel :
+          /*default*/ 'External Drive';
+      const removableEntry = new EntryList(
+          rootLabel, VolumeManagerCommon.RootType.REMOVABLE, devicePath);
+      removableModel = new NavigationModelFakeItem(
+          removableEntry.label, NavigationModelItemType.ENTRY_LIST,
+          removableEntry);
+      removableModel.section = NavigationSection.REMOVABLE;
+      // Add partitions as entries.
+      for (const partition of removableGroup) {
+        // Only add partition if it doesn't exist as a child already.
+        if (removableEntry.findIndexByVolumeInfo(partition.volumeInfo) === -1) {
+          removableEntry.addEntry(new VolumeEntry(partition.volumeInfo));
+        }
+      }
+    }
+    removableModels.set(devicePath, removableModel);
+    this.navigationItems_.push(removableModel);
+  }
+  this.removableModels_ = removableModels;
+
+  // Join MTP, ARCHIVE. These types belong to same section.
   const zipIndexes = volumeIndexes[NavigationListModel.ZIP_VOLUME_TYPE] || [];
   const otherVolumes =
       [].concat(
-            getVolumes(VolumeManagerCommon.VolumeType.REMOVABLE),
             getVolumes(VolumeManagerCommon.VolumeType.ARCHIVE),
             getVolumes(VolumeManagerCommon.VolumeType.MTP),
             zipIndexes.map(idx => volumeList[idx]))

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

@@ -193,7 +193,8 @@ function testAddAndRemoveVolumes() {
 
   // Mount removable volume 'hoge'.
   volumeManager.volumeInfoList.add(MockVolumeManager.createMockVolumeInfo(
-      VolumeManagerCommon.VolumeType.REMOVABLE, 'removable:hoge'));
+      VolumeManagerCommon.VolumeType.REMOVABLE, 'removable:hoge', '',
+      'device/path/1'));
 
   assertEquals(4, model.length);
   assertEquals(
@@ -207,9 +208,11 @@ function testAddAndRemoveVolumes() {
       'removable:hoge', /** @type {!NavigationModelVolumeItem} */
       (model.item(3)).volumeInfo.volumeId);
 
-  // Mount removable volume 'fuga'.
+  // Mount removable volume 'fuga'. Not a partition, so set a different device
+  // path to 'hoge'.
   volumeManager.volumeInfoList.add(MockVolumeManager.createMockVolumeInfo(
-      VolumeManagerCommon.VolumeType.REMOVABLE, 'removable:fuga'));
+      VolumeManagerCommon.VolumeType.REMOVABLE, 'removable:fuga', '',
+      'device/path/2'));
 
   assertEquals(5, model.length);
   assertEquals(
@@ -267,14 +270,18 @@ function testOrderAndNestItems() {
       'recent-label', NavigationModelItemType.RECENT, recentFakeEntry);
 
   // Create different volumes.
-  volumeManager.volumeInfoList.add(MockVolumeManager.createMockVolumeInfo(
-      VolumeManagerCommon.VolumeType.REMOVABLE, 'removable:hoge'));
   volumeManager.volumeInfoList.add(MockVolumeManager.createMockVolumeInfo(
       VolumeManagerCommon.VolumeType.PROVIDED, 'provided:prov1'));
+  // Set the device paths of the removable volumes to different strings to
+  // test the behaviour of two physically separate external devices.
   volumeManager.volumeInfoList.add(MockVolumeManager.createMockVolumeInfo(
-      VolumeManagerCommon.VolumeType.ARCHIVE, 'archive:a-rar'));
+      VolumeManagerCommon.VolumeType.REMOVABLE, 'removable:hoge', '',
+      'device/path/1'));
   volumeManager.volumeInfoList.add(MockVolumeManager.createMockVolumeInfo(
-      VolumeManagerCommon.VolumeType.REMOVABLE, 'removable:fuga'));
+      VolumeManagerCommon.VolumeType.REMOVABLE, 'removable:fuga', '',
+      'device/path/2'));
+  volumeManager.volumeInfoList.add(MockVolumeManager.createMockVolumeInfo(
+      VolumeManagerCommon.VolumeType.ARCHIVE, 'archive:a-rar'));
   volumeManager.volumeInfoList.add(MockVolumeManager.createMockVolumeInfo(
       VolumeManagerCommon.VolumeType.MTP, 'mtp:a-phone'));
   volumeManager.volumeInfoList.add(MockVolumeManager.createMockVolumeInfo(
@@ -311,8 +318,8 @@ function testOrderAndNestItems() {
   // 10.  provided:prov2
   //
   // 11.  removable:hoge
-  // 12.  archive:a-rar  - mounted as archive
-  // 13.  removable:fuga
+  // 12.  removable:fuga
+  // 13.  archive:a-rar  - mounted as archive
   // 14.  mtp:a-phone
   // 15.  provided:"zip" - mounted as provided: $zipVolumeId
 
@@ -338,8 +345,9 @@ function testOrderAndNestItems() {
   assertEquals('provided:prov2', model.item(9).label);
 
   assertEquals('removable:hoge', model.item(10).label);
-  assertEquals('archive:a-rar', model.item(11).label);
-  assertEquals('removable:fuga', model.item(12).label);
+  assertEquals('removable:fuga', model.item(11).label);
+
+  assertEquals('archive:a-rar', model.item(12).label);
   assertEquals('mtp:a-phone', model.item(13).label);
   assertEquals(zipVolumeId, model.item(14).label);
 
@@ -372,9 +380,9 @@ function testOrderAndNestItems() {
   // MTP/Archive/Removable are grouped together.
   // removable:hoge.
   assertEquals(NavigationSection.REMOVABLE, model.item(10).section);
-  // archive:a-rar.
-  assertEquals(NavigationSection.REMOVABLE, model.item(11).section);
   // removable:fuga.
+  assertEquals(NavigationSection.REMOVABLE, model.item(11).section);
+  // archive:a-rar.
   assertEquals(NavigationSection.REMOVABLE, model.item(12).section);
   // mtp:a-phone.
   assertEquals(NavigationSection.REMOVABLE, model.item(13).section);

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

@@ -716,6 +716,7 @@ function SubDirectoryItem(label, dirEntry, parentDirItem, tree) {
     if (window.IN_TEST && location.volumeInfo) {
       item.setAttribute(
           'volume-type-for-testing', location.volumeInfo.volumeType);
+      item.setAttribute('drive-label', location.volumeInfo.driveLabel);
     }
   } else {
     const rootType = location.rootType || null;
@@ -803,6 +804,10 @@ function EntryListItem(rootType, modelItem, tree) {
   item.dirEntry_ = modelItem.entry;
   item.parentTree_ = tree;
 
+  if (rootType === VolumeManagerCommon.RootType.REMOVABLE) {
+    item.setupEjectButton_(item.rowElement);
+  }
+
   const icon = queryRequiredElement('.icon', item);
   if (window.IN_TEST && item.entry && item.entry.volumeInfo) {
     item.setAttribute(
@@ -1806,8 +1811,11 @@ DirectoryTree.createDirectoryItem = function(modelItem, tree) {
           /** @type {!NavigationModelFakeItem} */ (modelItem), tree);
       break;
     case NavigationModelItemType.ENTRY_LIST:
+      const rootType = modelItem.section === NavigationSection.REMOVABLE ?
+          VolumeManagerCommon.RootType.REMOVABLE :
+          VolumeManagerCommon.RootType.MY_FILES;
       return new EntryListItem(
-          VolumeManagerCommon.RootType.MY_FILES,
+          rootType,
           /** @type {!NavigationModelFakeItem} */ (modelItem), tree);
       break;
   }

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

@@ -75,7 +75,7 @@ LocationLine.prototype.getComponents_ = function(entry) {
 
   if (util.isFakeEntry(entry)) {
     components.push(new LocationLine.PathComponent(
-        util.getRootTypeLabel(locationInfo), entry.toURL(),
+        util.getEntryLabel(locationInfo, entry), entry.toURL(),
         /** @type {!FakeEntry} */ (entry)));
     return components;
   }