As much as possilble, the VolumeManager interface should be used rather than the VolumeManagerWrapper. The VolumeManagerWrapper should implement VolumeManager and be an optional wrapper to restrict visibility of specific volumes when required.
VolumeManager / VolumeInfo / VolumeInfoList are core FilesApp interfaces defined in filesapp externs. https://cs.chromium.org/chromium/src/ui/file_manager/externs/volume_manager.js https://cs.chromium.org/chromium/src/ui/file_manager/externs/volume_info_list.js https://cs.chromium.org/chromium/src/ui/file_manager/externs/volume_info.js VolumeManager interface defines property volumeInfoList of type VolumeInfoList. Within filesapp, the implementation of these interfaces is provided in background: https://cs.chromium.org/chromium/src/ui/file_manager/file_manager/background/js/volume_manager_impl.js https://cs.chromium.org/chromium/src/ui/file_manager/file_manager/background/js/volume_info_list_impl.js My guess is that the intention is that the interfaces are visibled and used in both background and foreground, but the implementations are only defined and visible in background. I think this is a good design approach. Within foreground, a VolumeManagerWrapper type is defined https://cs.chromium.org/chromium/src/ui/file_manager/file_manager/foreground/js/volume_manager_wrapper.js which seems to be primarily for the purpose of filtering volumes. From class JS doc: "This class also filters some "disallowed" volumes; for example, Drive volumes are dropped if Drive is disabled, and read-only volumes are dropped in save-as dialogs." Strangely, the VolumeManagerWrapper class does not itself implement the VolumeManager interface when it would be appropriate for it to do so. Instead, the VolumeManagerWrapper type is used throughout foreground classes. This creates some mess especially in the MockVolumeManager class where it must separately implement both VolumeManager and VolumeManagerWrapper. The comments in this class include a TODO(mtomasz) to fix this. https://cs.chromium.org/chromium/src/ui/file_manager/file_manager/background/js/mock_volume_manager.js?l=178&rcl=38b3243afbabdc4fb1fb1a4be85a2f8281a3684b VolumeManagerWrapper does not provide any specific typing on its volumeInfoList property when it is clear that this should be of type VolumeInfoList.
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/a25ce958362ec7e1efb5a9e7f33d344eec78a7b5 commit a25ce958362ec7e1efb5a9e7f33d344eec78a7b5 Author: Joel Hockey <joelhockey@chromium.org> Date: Tue Sep 18 02:01:50 2018 CrOS FilesApp: Clean up VolumeManagerWrapper Make VolumeManagerWrapper inherit from VolumeManager and use the VolumeManager interface as much as possible within FilesApp. A new class VolumeInfoListWrapper is defined in volume_manager_wrapper.js which implements the minimal VolumeInfoList interface required in foreground. Now that VolumeManagerWrapper implments VolumeManager, we have some type safety for VolumeManagerWrapper.volumeInfoList property which must implement the VolumeInfoList interface. MockVolumeManager no longer requires to implement VolumeManagerWrapper and a lot of code can be deleted, fixing a TODO left by tomascz. VolumeInfoListImpl no longer requires to sort volumes, since this is done in NavigationListModel code. The directory_tree_unittest required modification to stop inserting volumes in an illegal manner which would not happen in prod, and cannot happen now that the correct VolumeInfoList interface is being used. Bug: 884107 Change-Id: I03078d0423b1ad85ae8909d633c58786135cd4b6 Reviewed-on: https://chromium-review.googlesource.com/1226755 Commit-Queue: Joel Hockey <joelhockey@chromium.org> Reviewed-by: Luciano Pacheco <lucmult@chromium.org> Cr-Commit-Position: refs/heads/master@{#591915} [modify] https://crrev.com/a25ce958362ec7e1efb5a9e7f33d344eec78a7b5/ui/file_manager/externs/command_handler_deps.js [modify] https://crrev.com/a25ce958362ec7e1efb5a9e7f33d344eec78a7b5/ui/file_manager/file_manager/background/js/mock_volume_manager.js [modify] https://crrev.com/a25ce958362ec7e1efb5a9e7f33d344eec78a7b5/ui/file_manager/file_manager/background/js/volume_info_list_impl.js [modify] https://crrev.com/a25ce958362ec7e1efb5a9e7f33d344eec78a7b5/ui/file_manager/file_manager/common/js/volume_manager_common.js [modify] https://crrev.com/a25ce958362ec7e1efb5a9e7f33d344eec78a7b5/ui/file_manager/file_manager/foreground/js/BUILD.gn [modify] https://crrev.com/a25ce958362ec7e1efb5a9e7f33d344eec78a7b5/ui/file_manager/file_manager/foreground/js/actions_controller.js [modify] https://crrev.com/a25ce958362ec7e1efb5a9e7f33d344eec78a7b5/ui/file_manager/file_manager/foreground/js/actions_model.js [modify] https://crrev.com/a25ce958362ec7e1efb5a9e7f33d344eec78a7b5/ui/file_manager/file_manager/foreground/js/crostini.js [modify] https://crrev.com/a25ce958362ec7e1efb5a9e7f33d344eec78a7b5/ui/file_manager/file_manager/foreground/js/dialog_action_controller.js [modify] https://crrev.com/a25ce958362ec7e1efb5a9e7f33d344eec78a7b5/ui/file_manager/file_manager/foreground/js/directory_model.js [modify] https://crrev.com/a25ce958362ec7e1efb5a9e7f33d344eec78a7b5/ui/file_manager/file_manager/foreground/js/file_manager_commands.js [modify] https://crrev.com/a25ce958362ec7e1efb5a9e7f33d344eec78a7b5/ui/file_manager/file_manager/foreground/js/file_selection.js [modify] https://crrev.com/a25ce958362ec7e1efb5a9e7f33d344eec78a7b5/ui/file_manager/file_manager/foreground/js/file_tasks.js [modify] https://crrev.com/a25ce958362ec7e1efb5a9e7f33d344eec78a7b5/ui/file_manager/file_manager/foreground/js/file_transfer_controller.js [modify] https://crrev.com/a25ce958362ec7e1efb5a9e7f33d344eec78a7b5/ui/file_manager/file_manager/foreground/js/folder_shortcuts_data_model.js [modify] https://crrev.com/a25ce958362ec7e1efb5a9e7f33d344eec78a7b5/ui/file_manager/file_manager/foreground/js/list_thumbnail_loader.js [modify] https://crrev.com/a25ce958362ec7e1efb5a9e7f33d344eec78a7b5/ui/file_manager/file_manager/foreground/js/main_window_component.js [modify] https://crrev.com/a25ce958362ec7e1efb5a9e7f33d344eec78a7b5/ui/file_manager/file_manager/foreground/js/metadata/multi_metadata_provider_unittest.js [modify] https://crrev.com/a25ce958362ec7e1efb5a9e7f33d344eec78a7b5/ui/file_manager/file_manager/foreground/js/navigation_list_model.js [modify] https://crrev.com/a25ce958362ec7e1efb5a9e7f33d344eec78a7b5/ui/file_manager/file_manager/foreground/js/navigation_list_model_unittest.js [modify] https://crrev.com/a25ce958362ec7e1efb5a9e7f33d344eec78a7b5/ui/file_manager/file_manager/foreground/js/navigation_uma.js [modify] https://crrev.com/a25ce958362ec7e1efb5a9e7f33d344eec78a7b5/ui/file_manager/file_manager/foreground/js/providers_model.js [modify] https://crrev.com/a25ce958362ec7e1efb5a9e7f33d344eec78a7b5/ui/file_manager/file_manager/foreground/js/providers_model_unittest.js [modify] https://crrev.com/a25ce958362ec7e1efb5a9e7f33d344eec78a7b5/ui/file_manager/file_manager/foreground/js/quick_view_controller.js [modify] https://crrev.com/a25ce958362ec7e1efb5a9e7f33d344eec78a7b5/ui/file_manager/file_manager/foreground/js/quick_view_uma.js [modify] https://crrev.com/a25ce958362ec7e1efb5a9e7f33d344eec78a7b5/ui/file_manager/file_manager/foreground/js/task_controller.js [modify] https://crrev.com/a25ce958362ec7e1efb5a9e7f33d344eec78a7b5/ui/file_manager/file_manager/foreground/js/ui/BUILD.gn [modify] https://crrev.com/a25ce958362ec7e1efb5a9e7f33d344eec78a7b5/ui/file_manager/file_manager/foreground/js/ui/banners.js [modify] https://crrev.com/a25ce958362ec7e1efb5a9e7f33d344eec78a7b5/ui/file_manager/file_manager/foreground/js/ui/directory_tree.js [modify] https://crrev.com/a25ce958362ec7e1efb5a9e7f33d344eec78a7b5/ui/file_manager/file_manager/foreground/js/ui/directory_tree_unittest.js [modify] https://crrev.com/a25ce958362ec7e1efb5a9e7f33d344eec78a7b5/ui/file_manager/file_manager/foreground/js/ui/file_grid.js [modify] https://crrev.com/a25ce958362ec7e1efb5a9e7f33d344eec78a7b5/ui/file_manager/file_manager/foreground/js/ui/file_table.js [modify] https://crrev.com/a25ce958362ec7e1efb5a9e7f33d344eec78a7b5/ui/file_manager/file_manager/foreground/js/ui/location_line.js [modify] https://crrev.com/a25ce958362ec7e1efb5a9e7f33d344eec78a7b5/ui/file_manager/file_manager/foreground/js/volume_manager_wrapper.js [modify] https://crrev.com/a25ce958362ec7e1efb5a9e7f33d344eec78a7b5/ui/file_manager/gallery/js/BUILD.gn [modify] https://crrev.com/a25ce958362ec7e1efb5a9e7f33d344eec78a7b5/ui/file_manager/gallery/js/gallery.js [modify] https://crrev.com/a25ce958362ec7e1efb5a9e7f33d344eec78a7b5/ui/file_manager/gallery/js/gallery_data_model.js [modify] https://crrev.com/a25ce958362ec7e1efb5a9e7f33d344eec78a7b5/ui/file_manager/gallery/js/gallery_item.js [modify] https://crrev.com/a25ce958362ec7e1efb5a9e7f33d344eec78a7b5/ui/file_manager/gallery/js/gallery_item_unittest.js [modify] https://crrev.com/a25ce958362ec7e1efb5a9e7f33d344eec78a7b5/ui/file_manager/gallery/js/gallery_util.js [modify] https://crrev.com/a25ce958362ec7e1efb5a9e7f33d344eec78a7b5/ui/file_manager/gallery/js/slide_mode.js
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/b5f0ba4492bd6c35aff70375422ccaaae73e13ab commit b5f0ba4492bd6c35aff70375422ccaaae73e13ab Author: Joel Hockey <joelhockey@chromium.org> Date: Tue Sep 18 08:32:31 2018 Remove VolumeManagerCommon.VolumeInfoProvider VolumeManager in externs should not be depending on this type, and it has only a small purpose in import_controller which can be replaced by using VolumeManager directly. As a result of this change, VolumeInfoList.findByEntry(Entry) is now replaced by VolumeManager.getVolumeInfo(Entry). Bug: 884107 Change-Id: I7429513429dcf8c9c04a6b20fc309813de3ab0a2 Reviewed-on: https://chromium-review.googlesource.com/1229716 Reviewed-by: Luciano Pacheco <lucmult@chromium.org> Commit-Queue: Joel Hockey <joelhockey@chromium.org> Cr-Commit-Position: refs/heads/master@{#591973} [modify] https://crrev.com/b5f0ba4492bd6c35aff70375422ccaaae73e13ab/ui/file_manager/externs/volume_info_list.js [modify] https://crrev.com/b5f0ba4492bd6c35aff70375422ccaaae73e13ab/ui/file_manager/externs/volume_manager.js [modify] https://crrev.com/b5f0ba4492bd6c35aff70375422ccaaae73e13ab/ui/file_manager/file_manager/background/js/mock_volume_manager.js [modify] https://crrev.com/b5f0ba4492bd6c35aff70375422ccaaae73e13ab/ui/file_manager/file_manager/background/js/volume_info_list_impl.js [modify] https://crrev.com/b5f0ba4492bd6c35aff70375422ccaaae73e13ab/ui/file_manager/file_manager/background/js/volume_manager_impl.js [modify] https://crrev.com/b5f0ba4492bd6c35aff70375422ccaaae73e13ab/ui/file_manager/file_manager/common/js/BUILD.gn [modify] https://crrev.com/b5f0ba4492bd6c35aff70375422ccaaae73e13ab/ui/file_manager/file_manager/common/js/importer_common.js [modify] https://crrev.com/b5f0ba4492bd6c35aff70375422ccaaae73e13ab/ui/file_manager/file_manager/common/js/volume_manager_common.js [modify] https://crrev.com/b5f0ba4492bd6c35aff70375422ccaaae73e13ab/ui/file_manager/file_manager/foreground/js/column_visibility_controller.js [modify] https://crrev.com/b5f0ba4492bd6c35aff70375422ccaaae73e13ab/ui/file_manager/file_manager/foreground/js/import_controller.js [modify] https://crrev.com/b5f0ba4492bd6c35aff70375422ccaaae73e13ab/ui/file_manager/file_manager/foreground/js/import_controller_unittest.js [modify] https://crrev.com/b5f0ba4492bd6c35aff70375422ccaaae73e13ab/ui/file_manager/file_manager/foreground/js/metadata/BUILD.gn [modify] https://crrev.com/b5f0ba4492bd6c35aff70375422ccaaae73e13ab/ui/file_manager/file_manager/foreground/js/metadata/metadata_model.js [modify] https://crrev.com/b5f0ba4492bd6c35aff70375422ccaaae73e13ab/ui/file_manager/file_manager/foreground/js/metadata/multi_metadata_provider.js [modify] https://crrev.com/b5f0ba4492bd6c35aff70375422ccaaae73e13ab/ui/file_manager/file_manager/foreground/js/volume_manager_wrapper.js
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/a9e1be2b634e9b09919c769ee852faa93c3e8528 commit a9e1be2b634e9b09919c769ee852faa93c3e8528 Author: Joel Hockey <joelhockey@chromium.org> Date: Tue Sep 18 09:19:23 2018 Move functions out of VolumeInfoList. This should be a simple interface for a list which allows add, remove and listening for 'splice' event. * findIndex - removed, only implemented in VolumeInfoListImpl as helper. * findByEntry - removed, it already has a duplicate in VolumeManager.getVolumeInfo(). * findByDevicePath - moved into VolumeManager. * findByVolumeId - removed, not needed. * whenVolumeInfoReady - moved into VolumeManager. Bug: 884107 Change-Id: Ifd2aa176d53e4a3ea69eff56519b1d067471fdb4 Reviewed-on: https://chromium-review.googlesource.com/1229876 Commit-Queue: Joel Hockey <joelhockey@chromium.org> Reviewed-by: Luciano Pacheco <lucmult@chromium.org> Cr-Commit-Position: refs/heads/master@{#591985} [modify] https://crrev.com/a9e1be2b634e9b09919c769ee852faa93c3e8528/ui/file_manager/externs/volume_info_list.js [modify] https://crrev.com/a9e1be2b634e9b09919c769ee852faa93c3e8528/ui/file_manager/externs/volume_manager.js [modify] https://crrev.com/a9e1be2b634e9b09919c769ee852faa93c3e8528/ui/file_manager/file_manager/background/js/BUILD.gn [modify] https://crrev.com/a9e1be2b634e9b09919c769ee852faa93c3e8528/ui/file_manager/file_manager/background/js/background.js [modify] https://crrev.com/a9e1be2b634e9b09919c769ee852faa93c3e8528/ui/file_manager/file_manager/background/js/device_handler.js [modify] https://crrev.com/a9e1be2b634e9b09919c769ee852faa93c3e8528/ui/file_manager/file_manager/background/js/device_handler_unittest.html [modify] https://crrev.com/a9e1be2b634e9b09919c769ee852faa93c3e8528/ui/file_manager/file_manager/background/js/duplicate_finder_unittest.html [modify] https://crrev.com/a9e1be2b634e9b09919c769ee852faa93c3e8528/ui/file_manager/file_manager/background/js/media_import_handler_unittest.html [modify] https://crrev.com/a9e1be2b634e9b09919c769ee852faa93c3e8528/ui/file_manager/file_manager/background/js/mock_volume_manager.js [modify] https://crrev.com/a9e1be2b634e9b09919c769ee852faa93c3e8528/ui/file_manager/file_manager/background/js/volume_info_list_impl.js [modify] https://crrev.com/a9e1be2b634e9b09919c769ee852faa93c3e8528/ui/file_manager/file_manager/background/js/volume_manager_impl.js [modify] https://crrev.com/a9e1be2b634e9b09919c769ee852faa93c3e8528/ui/file_manager/file_manager/background/js/volume_manager_unittest.js [modify] https://crrev.com/a9e1be2b634e9b09919c769ee852faa93c3e8528/ui/file_manager/file_manager/common/js/importer_common_unittest.html [modify] https://crrev.com/a9e1be2b634e9b09919c769ee852faa93c3e8528/ui/file_manager/file_manager/foreground/js/import_controller_unittest.html [modify] https://crrev.com/a9e1be2b634e9b09919c769ee852faa93c3e8528/ui/file_manager/file_manager/foreground/js/navigation_list_model_unittest.html [modify] https://crrev.com/a9e1be2b634e9b09919c769ee852faa93c3e8528/ui/file_manager/file_manager/foreground/js/providers_model_unittest.html [modify] https://crrev.com/a9e1be2b634e9b09919c769ee852faa93c3e8528/ui/file_manager/file_manager/foreground/js/ui/directory_tree_unittest.html [modify] https://crrev.com/a9e1be2b634e9b09919c769ee852faa93c3e8528/ui/file_manager/file_manager/foreground/js/volume_manager_wrapper.js [modify] https://crrev.com/a9e1be2b634e9b09919c769ee852faa93c3e8528/ui/file_manager/file_manager/test/js/chrome_file_manager_private_test_impl.js
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/6d839cdfbfc661e698cab06c0f30df8cf6cd7fd0 commit 6d839cdfbfc661e698cab06c0f30df8cf6cd7fd0 Author: Joel Hockey <joelhockey@chromium.org> Date: Wed Sep 19 06:19:39 2018 CrOS FilesApp rename VolumeManagerWrapper to FilteredVolumeManager Renamed: ui/file_manager/file_manager/foreground/js/VolumeManagerWrapper to ui/file_manager/base/js/FilteredVolumeManager Bug: 884107 Change-Id: Iece36d99f152be1f921c8a20d428415ee0921acb Reviewed-on: https://chromium-review.googlesource.com/1229740 Commit-Queue: Joel Hockey <joelhockey@chromium.org> Reviewed-by: Trent Apted <tapted@chromium.org> Reviewed-by: Luciano Pacheco <lucmult@chromium.org> Cr-Commit-Position: refs/heads/master@{#592327} [modify] https://crrev.com/6d839cdfbfc661e698cab06c0f30df8cf6cd7fd0/chrome/test/BUILD.gn [modify] https://crrev.com/6d839cdfbfc661e698cab06c0f30df8cf6cd7fd0/ui/file_manager/BUILD.gn [modify] https://crrev.com/6d839cdfbfc661e698cab06c0f30df8cf6cd7fd0/ui/file_manager/audio_player/js/BUILD.gn [modify] https://crrev.com/6d839cdfbfc661e698cab06c0f30df8cf6cd7fd0/ui/file_manager/audio_player/js/audio_player.js [modify] https://crrev.com/6d839cdfbfc661e698cab06c0f30df8cf6cd7fd0/ui/file_manager/audio_player/js/audio_player_scripts.js [add] https://crrev.com/6d839cdfbfc661e698cab06c0f30df8cf6cd7fd0/ui/file_manager/base/js/BUILD.gn [rename] https://crrev.com/6d839cdfbfc661e698cab06c0f30df8cf6cd7fd0/ui/file_manager/base/js/filtered_volume_manager.js [modify] https://crrev.com/6d839cdfbfc661e698cab06c0f30df8cf6cd7fd0/ui/file_manager/file_manager/background/js/volume_manager_impl.js [modify] https://crrev.com/6d839cdfbfc661e698cab06c0f30df8cf6cd7fd0/ui/file_manager/file_manager/foreground/js/BUILD.gn [modify] https://crrev.com/6d839cdfbfc661e698cab06c0f30df8cf6cd7fd0/ui/file_manager/file_manager/foreground/js/file_manager.js [modify] https://crrev.com/6d839cdfbfc661e698cab06c0f30df8cf6cd7fd0/ui/file_manager/file_manager/foreground/js/folder_shortcuts_data_model.js [modify] https://crrev.com/6d839cdfbfc661e698cab06c0f30df8cf6cd7fd0/ui/file_manager/file_manager/foreground/js/main_scripts.js [modify] https://crrev.com/6d839cdfbfc661e698cab06c0f30df8cf6cd7fd0/ui/file_manager/gallery/js/BUILD.gn [modify] https://crrev.com/6d839cdfbfc661e698cab06c0f30df8cf6cd7fd0/ui/file_manager/gallery/js/gallery.js [modify] https://crrev.com/6d839cdfbfc661e698cab06c0f30df8cf6cd7fd0/ui/file_manager/gallery/js/gallery_scripts.js [modify] https://crrev.com/6d839cdfbfc661e698cab06c0f30df8cf6cd7fd0/ui/file_manager/video_player/js/BUILD.gn [modify] https://crrev.com/6d839cdfbfc661e698cab06c0f30df8cf6cd7fd0/ui/file_manager/video_player/js/video_player.js [modify] https://crrev.com/6d839cdfbfc661e698cab06c0f30df8cf6cd7fd0/ui/file_manager/video_player/js/video_player_scripts.js
Comment 1 by joelhockey@chromium.org
, Sep 17