[Files app] Fix USB grouping

USB grouping wasn't updating the group when new partitions were added or
removed after the initial grouping.

This was causing devices with more than 2 partitions to only display 2
partitions if device was plugged when Files app was already open,
because each partition is mounted individually so files app updates for
every partition.

Change-Id: I27c62051774b9a73b20005967c09d48a1d2a3c30
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1506680
Commit-Queue: Luciano Pacheco <lucmult@chromium.org>
Reviewed-by: Alex Danilo <adanilo@chromium.org>
Auto-Submit: Luciano Pacheco <lucmult@chromium.org>
Cr-Commit-Position: refs/heads/master@{#638454}

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

@@ -415,6 +415,22 @@ class EntryList {
     return false;
   }
 
+  /**
+   * Removes the entry.
+   * @param {!Entry|FilesAppEntry} entry to be removed.
+   * This method is specific to EntryList and VolumeEntry instance.
+   * @return {boolean} if entry was removed.
+   */
+  removeChildEntry(entry) {
+    const childIndex =
+        this.children_.findIndex(childEntry => childEntry === entry);
+    if (childIndex !== -1) {
+      this.children_.splice(childIndex, 1);
+      return true;
+    }
+    return false;
+  }
+
   /** @override */
   getNativeEntry() {
     return null;
@@ -670,6 +686,22 @@ class VolumeEntry {
     }
     return false;
   }
+
+  /**
+   * Removes the entry.
+   * @param {!Entry|FilesAppEntry} entry to be removed.
+   * This method is specific to EntryList and VolumeEntry instance.
+   * @return {boolean} if entry was removed.
+   */
+  removeChildEntry(entry) {
+    const childIndex =
+        this.children_.findIndex(childEntry => childEntry === entry);
+    if (childIndex !== -1) {
+      this.children_.splice(childIndex, 1);
+      return true;
+    }
+    return false;
+  }
 }
 
 /**

ui/file_manager/file_manager/common/js/files_app_entry_types_unittest.js

@@ -103,7 +103,7 @@ function testEntryListAddEntry() {
 
 /**
  * Tests EntryList's methods addEntry, findIndexByVolumeInfo,
- * removeByVolumeType, removeByRootType.
+ * removeByVolumeType, removeByRootType, removeChildEntry.
  */
 function testEntryFindIndex() {
   const entryList =
@@ -140,11 +140,17 @@ function testEntryFindIndex() {
   entryList.addEntry(fakeEntry);
   assertTrue(entryList.removeByRootType(VolumeManagerCommon.RootType.CROSTINI));
   assertEquals(1, entryList.getUIChildren().length);
+
+  // Test removeChildEntry.
+  assertTrue(entryList.removeChildEntry(entryList.getUIChildren()[0]));
+  assertEquals(0, entryList.getUIChildren().length);
+  // Nothing left to remove.
+  assertFalse(entryList.removeChildEntry(/** @type {Entry} */ ({})));
 }
 
 /**
  * Tests VolumeEntry's methods findIndexByVolumeInfo, removeByVolumeType,
- * removeByRootType.
+ * removeByRootType, removeChildEntry.
  * @suppress {accessControls} to be able to access private properties.
  */
 function testVolumeEntryFindIndex() {
@@ -187,6 +193,12 @@ function testVolumeEntryFindIndex() {
   assertTrue(
       volumeEntry.removeByRootType(VolumeManagerCommon.RootType.CROSTINI));
   assertEquals(1, volumeEntry.children_.length);
+
+  // Test removeChildEntry.
+  assertTrue(volumeEntry.removeChildEntry(volumeEntry.getUIChildren()[0]));
+  assertEquals(0, volumeEntry.getUIChildren().length);
+  // Nothing left to remove.
+  assertFalse(volumeEntry.removeChildEntry(/** @type {Entry} */ ({})));
 }
 
 /** Tests method EntryList.getMetadata. */

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

@@ -659,29 +659,42 @@ class NavigationListModel extends cr.EventTarget {
 
       // Multiple partitions found.
       let removableModel;
+      let removableEntry;
       if (this.removableModels_.has(devicePath)) {
         // Removable model has been seen before. Use the same reference.
         removableModel = this.removableModels_.get(devicePath);
+        removableEntry = removableModel.entry;
       } 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(
+        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));
-          }
+      }
+
+      // Remove partitions that aren't available anymore.
+      const existingVolumeIds =
+          new Set(removableGroup.map(p => p.volumeInfo.volumeId));
+      for (const partition of removableEntry.getUIChildren()) {
+        if (!existingVolumeIds.has(partition.volumeInfo.volumeId)) {
+          removableEntry.removeChildEntry(partition);
+        }
+      }
+
+      // 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);
     }

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

@@ -478,3 +478,51 @@ function testMyFilesVolumeEnabled(callback) {
       }),
       callback);
 }
+
+/**
+ * Tests that adding a new partition to the same grouped USB will add the
+ * partition to the grouping.
+ */
+function testMultipleUsbPartitionsGrouping() {
+  const shortcutListModel = new MockFolderShortcutDataModel([]);
+  const recentItem = null;
+  const volumeManager = new MockVolumeManager();
+
+  // Use same device path so the partitions are grouped.
+  volumeManager.volumeInfoList.add(MockVolumeManager.createMockVolumeInfo(
+      VolumeManagerCommon.VolumeType.REMOVABLE, 'removable:partition1',
+      'partition1', 'device/path/1'));
+  volumeManager.volumeInfoList.add(MockVolumeManager.createMockVolumeInfo(
+      VolumeManagerCommon.VolumeType.REMOVABLE, 'removable:partition2',
+      'partition2', 'device/path/1'));
+  volumeManager.volumeInfoList.add(MockVolumeManager.createMockVolumeInfo(
+      VolumeManagerCommon.VolumeType.REMOVABLE, 'removable:partition3',
+      'partition3', 'device/path/1'));
+
+  const model = new NavigationListModel(
+      volumeManager, shortcutListModel, recentItem, directoryModel);
+
+  // Check that the common root shows 3 partitions.
+  let groupedUsbs = /** @type NavigationModelFakeItem */ (model.item(2));
+  assertEquals('External Drive', groupedUsbs.label);
+  assertEquals(3, groupedUsbs.entry.getUIChildren().length);
+
+  // Add a 4th partition, which triggers NavigationListModel to recalculate.
+  volumeManager.volumeInfoList.add(MockVolumeManager.createMockVolumeInfo(
+      VolumeManagerCommon.VolumeType.REMOVABLE, 'removable:partition4',
+      'partition4', 'device/path/1'));
+
+  // Check that the common root shows 4 partitions.
+  groupedUsbs = /** @type NavigationModelFakeItem */ (model.item(2));
+  assertEquals('External Drive', groupedUsbs.label);
+  assertEquals(4, groupedUsbs.entry.getUIChildren().length);
+
+  // Remove the 4th partition, which triggers NavigationListModel to
+  // recalculate.
+  volumeManager.volumeInfoList.remove('removable:partition4');
+
+  // Check that the common root shows 3 partitions.
+  groupedUsbs = /** @type NavigationModelFakeItem */ (model.item(2));
+  assertEquals('External Drive', groupedUsbs.label);
+  assertEquals(3, groupedUsbs.entry.getUIChildren().length);
+}