Change directory_tree.js to use Metadata update event and cached metadata |
||||||
Issue descriptiondirectory_tree.js currently requests metadata for each sub-directory individually, which impacts the overall performance of the application. Instead it should use the metadata update event to have the latest metadata and request the metadata in batches. Internal doc: go/files-app-metadata-audit
,
Sep 6
,
Sep 10
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/f57f5f8ee29f223cefee213a86a1542e50f04b8b commit f57f5f8ee29f223cefee213a86a1542e50f04b8b Author: Luciano Pacheco <lucmult@chromium.org> Date: Mon Sep 10 07:39:01 2018 Change directory tree to use Metadata update event Metadata model provides an "update" event which is triggered anytime a metadata item has been updated from the backend. Any UI part that to display latest metadata should listen to it and update the UI accordingly. Change the directory tree to listen to Metadata "update" event to update the "shared" icon. A listener is added for every expanded folder to listen to updates for its sub-folders. When a folder is collapsed its listener is removed. Change |onExpand_| function to fetch all metadata values from |LIST_CONTAINER_METADATA_PREFETCH_PROPERTY_NAMES|, instead of |DIRECTORY_TREE_METADATA_PREFETCH_PROPERTY_NAMES|. Directory tree needs the metadata "can*" to setup permission accordingly in the context menu. This is only fetched for Drive roots, since only Drive implements these metadata. Change |updateSharedStatusIcon| function to use the cached metadata value. This fixes the issue where it would trigger metadata requests to backend too early, because this |updateSharedStatusIcon| is called from SubDirectoryItem constructor. Remove Metadata cache invalidation calls (notifyEntriesChanged and notifyEntriesRemoved) because directory tree doesn't update metadata directly. Fix MockVolumeManager to work with other volumes other than Drive, by extracting the VolumeManager implementation to detect RootType to a common function |getRootTypeFromVolumeType|. Remove constant |DIRECTORY_TREE_METADATA_PREFETCH_PROPERTY_NAMES| since its use has been removed in directory_tree.js. Add MetadataModel |removeEventListener| to be able to remove listener of MetadataModel "update" event. manual tests for Drive "shared" icon. New integration tests for "shared" icon will be added in follow up CL. Bug: 880187 , 880189 Test: Unittest for |insideMyDrive| and |insideDrive| functions and Change-Id: Ibe8320e7b0981d8e9556ebb8a922d7db2f16860c Reviewed-on: https://chromium-review.googlesource.com/1203596 Commit-Queue: Luciano Pacheco <lucmult@chromium.org> Reviewed-by: Noel Gordon <noel@chromium.org> Cr-Commit-Position: refs/heads/master@{#589846} [modify] https://crrev.com/f57f5f8ee29f223cefee213a86a1542e50f04b8b/ui/file_manager/file_manager/background/js/mock_volume_manager.js [modify] https://crrev.com/f57f5f8ee29f223cefee213a86a1542e50f04b8b/ui/file_manager/file_manager/background/js/volume_manager_impl.js [modify] https://crrev.com/f57f5f8ee29f223cefee213a86a1542e50f04b8b/ui/file_manager/file_manager/common/js/volume_manager_common.js [modify] https://crrev.com/f57f5f8ee29f223cefee213a86a1542e50f04b8b/ui/file_manager/file_manager/foreground/js/constants.js [modify] https://crrev.com/f57f5f8ee29f223cefee213a86a1542e50f04b8b/ui/file_manager/file_manager/foreground/js/metadata/metadata_model.js [modify] https://crrev.com/f57f5f8ee29f223cefee213a86a1542e50f04b8b/ui/file_manager/file_manager/foreground/js/ui/directory_tree.js [modify] https://crrev.com/f57f5f8ee29f223cefee213a86a1542e50f04b8b/ui/file_manager/file_manager/foreground/js/ui/directory_tree_unittest.js
,
Sep 24
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/7f622acdece990d212e293904db728a461f9203e commit 7f622acdece990d212e293904db728a461f9203e Author: Luciano Pacheco <lucmult@chromium.org> Date: Mon Sep 24 05:35:29 2018 Add test to check Metadata operations calls Add test for the following use cases: 1. Navigate to a Downloads sub-sub-folder using directory tree. 2. Navigate to a Drive sub-sub-folder using directory tree. 3. Navigate to a Drive sub-folder with 50 files and 50 siblings folders, using directory tree. 4. Navigate to Team Drives root using directory tree. Add MetadataStats class to hold metadata operation counters. Change MetadataModel to count operations only during tests using a global instance of MetadataStats. Add getMetadataStats test util function to return the MetadataStats global value to test extension. Add waitFor remote test function to wait for any remote test function to return an expected result. Bug: 880187 Change-Id: I024938cfe09c2599968100b978a071675072b2d8 Reviewed-on: https://chromium-review.googlesource.com/1188046 Commit-Queue: Luciano Pacheco <lucmult@chromium.org> Reviewed-by: Noel Gordon <noel@chromium.org> Cr-Commit-Position: refs/heads/master@{#593477} [modify] https://crrev.com/7f622acdece990d212e293904db728a461f9203e/chrome/browser/chromeos/file_manager/file_manager_browsertest.cc [modify] https://crrev.com/7f622acdece990d212e293904db728a461f9203e/ui/file_manager/file_manager/background/js/test_util_base.js [modify] https://crrev.com/7f622acdece990d212e293904db728a461f9203e/ui/file_manager/file_manager/foreground/js/metadata/metadata_model.js [add] https://crrev.com/7f622acdece990d212e293904db728a461f9203e/ui/file_manager/integration_tests/file_manager/metadata.js [modify] https://crrev.com/7f622acdece990d212e293904db728a461f9203e/ui/file_manager/integration_tests/file_manager_test_manifest.json [modify] https://crrev.com/7f622acdece990d212e293904db728a461f9203e/ui/file_manager/integration_tests/remote_call.js
,
Sep 24
Requesting to merge crrev.com/c/1203596 and crrev.com/c/1203352. They've landed on: 71.0.3549.0 and 71.0.3544.0 I've tested on Canary image: 71.0.3558.0 and I also merged them in the branch-heads/3538, tested running locally and our Files app integration tests; all of them are working. These are performance improvements for Files app, not change in user features.
,
Sep 27
This issue has been approved for a merge. Please merge the fix to any appropriate branches as soon as possible! If all merges have been completed, please remove any remaining Merge-Approved labels from this issue. Thanks for your time! To disable nags, add the Disable-Nags label. For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
,
Sep 27
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/75263a99f0b6cd3b038d1a90f31321588fda779f commit 75263a99f0b6cd3b038d1a90f31321588fda779f Author: Luciano Pacheco <lucmult@chromium.org> Date: Thu Sep 27 20:45:24 2018 Add entriesMap and property names to Metadata cache event Add property names set to update event so event listeners can check if the metadata that they want has been updated in constant time. Add entriesMap to update event so event listeners can check if the relevant entry has been updated in constant time. These will be used in the directory_tree.js to update the 'shared' icon for sub-directories. Bug: 880187 , 880189 Change-Id: I5bb21e8b82b208c8d971e3193840f8b6afb8546e Reviewed-on: https://chromium-review.googlesource.com/1203352 Reviewed-by: Luciano Pacheco <lucmult@chromium.org> Reviewed-by: Noel Gordon <noel@chromium.org> Commit-Queue: Luciano Pacheco <lucmult@chromium.org> Cr-Original-Commit-Position: refs/heads/master@{#588758}(cherry picked from commit 25cb4e0cb2bcc9f223dd50cda2a416918222cb7e) Reviewed-on: https://chromium-review.googlesource.com/1249463 Cr-Commit-Position: refs/branch-heads/3538@{#713} Cr-Branched-From: 79f7c91a2b2a2932cd447fa6f865cb6662fa8fa6-refs/heads/master@{#587811} [modify] https://crrev.com/75263a99f0b6cd3b038d1a90f31321588fda779f/ui/file_manager/file_manager/foreground/js/metadata/metadata_cache_set.js [modify] https://crrev.com/75263a99f0b6cd3b038d1a90f31321588fda779f/ui/file_manager/file_manager/foreground/js/metadata/metadata_cache_set_unittest.js [modify] https://crrev.com/75263a99f0b6cd3b038d1a90f31321588fda779f/ui/file_manager/file_manager/foreground/js/metadata/metadata_model.js
,
Sep 27
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/08b84980b8291568db24dea726ba7f1d5bd6d0d0 commit 08b84980b8291568db24dea726ba7f1d5bd6d0d0 Author: Luciano Pacheco <lucmult@chromium.org> Date: Thu Sep 27 20:46:20 2018 Change directory tree to use Metadata update event Metadata model provides an "update" event which is triggered anytime a metadata item has been updated from the backend. Any UI part that to display latest metadata should listen to it and update the UI accordingly. Change the directory tree to listen to Metadata "update" event to update the "shared" icon. A listener is added for every expanded folder to listen to updates for its sub-folders. When a folder is collapsed its listener is removed. Change |onExpand_| function to fetch all metadata values from |LIST_CONTAINER_METADATA_PREFETCH_PROPERTY_NAMES|, instead of |DIRECTORY_TREE_METADATA_PREFETCH_PROPERTY_NAMES|. Directory tree needs the metadata "can*" to setup permission accordingly in the context menu. This is only fetched for Drive roots, since only Drive implements these metadata. Change |updateSharedStatusIcon| function to use the cached metadata value. This fixes the issue where it would trigger metadata requests to backend too early, because this |updateSharedStatusIcon| is called from SubDirectoryItem constructor. Remove Metadata cache invalidation calls (notifyEntriesChanged and notifyEntriesRemoved) because directory tree doesn't update metadata directly. Fix MockVolumeManager to work with other volumes other than Drive, by extracting the VolumeManager implementation to detect RootType to a common function |getRootTypeFromVolumeType|. Remove constant |DIRECTORY_TREE_METADATA_PREFETCH_PROPERTY_NAMES| since its use has been removed in directory_tree.js. Add MetadataModel |removeEventListener| to be able to remove listener of MetadataModel "update" event. manual tests for Drive "shared" icon. New integration tests for "shared" icon will be added in follow up CL. Bug: 880187 , 880189 Test: Unittest for |insideMyDrive| and |insideDrive| functions and Change-Id: Ibe8320e7b0981d8e9556ebb8a922d7db2f16860c Reviewed-on: https://chromium-review.googlesource.com/1203596 Commit-Queue: Luciano Pacheco <lucmult@chromium.org> Reviewed-by: Noel Gordon <noel@chromium.org> Cr-Original-Commit-Position: refs/heads/master@{#589846}(cherry picked from commit f57f5f8ee29f223cefee213a86a1542e50f04b8b) Reviewed-on: https://chromium-review.googlesource.com/1249464 Reviewed-by: Luciano Pacheco <lucmult@chromium.org> Cr-Commit-Position: refs/branch-heads/3538@{#714} Cr-Branched-From: 79f7c91a2b2a2932cd447fa6f865cb6662fa8fa6-refs/heads/master@{#587811} [modify] https://crrev.com/08b84980b8291568db24dea726ba7f1d5bd6d0d0/ui/file_manager/file_manager/background/js/mock_volume_manager.js [modify] https://crrev.com/08b84980b8291568db24dea726ba7f1d5bd6d0d0/ui/file_manager/file_manager/background/js/volume_manager_impl.js [modify] https://crrev.com/08b84980b8291568db24dea726ba7f1d5bd6d0d0/ui/file_manager/file_manager/common/js/volume_manager_common.js [modify] https://crrev.com/08b84980b8291568db24dea726ba7f1d5bd6d0d0/ui/file_manager/file_manager/foreground/js/constants.js [modify] https://crrev.com/08b84980b8291568db24dea726ba7f1d5bd6d0d0/ui/file_manager/file_manager/foreground/js/metadata/metadata_model.js [modify] https://crrev.com/08b84980b8291568db24dea726ba7f1d5bd6d0d0/ui/file_manager/file_manager/foreground/js/ui/directory_tree.js [modify] https://crrev.com/08b84980b8291568db24dea726ba7f1d5bd6d0d0/ui/file_manager/file_manager/foreground/js/ui/directory_tree_unittest.js
,
Sep 27
,
Sep 27
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/08b84980b8291568db24dea726ba7f1d5bd6d0d0 Commit: 08b84980b8291568db24dea726ba7f1d5bd6d0d0 Author: lucmult@chromium.org Commiter: lucmult@chromium.org Date: 2018-09-27 20:46:20 +0000 UTC Change directory tree to use Metadata update event Metadata model provides an "update" event which is triggered anytime a metadata item has been updated from the backend. Any UI part that to display latest metadata should listen to it and update the UI accordingly. Change the directory tree to listen to Metadata "update" event to update the "shared" icon. A listener is added for every expanded folder to listen to updates for its sub-folders. When a folder is collapsed its listener is removed. Change |onExpand_| function to fetch all metadata values from |LIST_CONTAINER_METADATA_PREFETCH_PROPERTY_NAMES|, instead of |DIRECTORY_TREE_METADATA_PREFETCH_PROPERTY_NAMES|. Directory tree needs the metadata "can*" to setup permission accordingly in the context menu. This is only fetched for Drive roots, since only Drive implements these metadata. Change |updateSharedStatusIcon| function to use the cached metadata value. This fixes the issue where it would trigger metadata requests to backend too early, because this |updateSharedStatusIcon| is called from SubDirectoryItem constructor. Remove Metadata cache invalidation calls (notifyEntriesChanged and notifyEntriesRemoved) because directory tree doesn't update metadata directly. Fix MockVolumeManager to work with other volumes other than Drive, by extracting the VolumeManager implementation to detect RootType to a common function |getRootTypeFromVolumeType|. Remove constant |DIRECTORY_TREE_METADATA_PREFETCH_PROPERTY_NAMES| since its use has been removed in directory_tree.js. Add MetadataModel |removeEventListener| to be able to remove listener of MetadataModel "update" event. manual tests for Drive "shared" icon. New integration tests for "shared" icon will be added in follow up CL. Bug: 880187 , 880189 Test: Unittest for |insideMyDrive| and |insideDrive| functions and Change-Id: Ibe8320e7b0981d8e9556ebb8a922d7db2f16860c Reviewed-on: https://chromium-review.googlesource.com/1203596 Commit-Queue: Luciano Pacheco <lucmult@chromium.org> Reviewed-by: Noel Gordon <noel@chromium.org> Cr-Original-Commit-Position: refs/heads/master@{#589846}(cherry picked from commit f57f5f8ee29f223cefee213a86a1542e50f04b8b) Reviewed-on: https://chromium-review.googlesource.com/1249464 Reviewed-by: Luciano Pacheco <lucmult@chromium.org> Cr-Commit-Position: refs/branch-heads/3538@{#714} Cr-Branched-From: 79f7c91a2b2a2932cd447fa6f865cb6662fa8fa6-refs/heads/master@{#587811}
,
Sep 27
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/75263a99f0b6cd3b038d1a90f31321588fda779f Commit: 75263a99f0b6cd3b038d1a90f31321588fda779f Author: lucmult@chromium.org Commiter: lucmult@chromium.org Date: 2018-09-27 20:45:24 +0000 UTC Add entriesMap and property names to Metadata cache event Add property names set to update event so event listeners can check if the metadata that they want has been updated in constant time. Add entriesMap to update event so event listeners can check if the relevant entry has been updated in constant time. These will be used in the directory_tree.js to update the 'shared' icon for sub-directories. Bug: 880187 , 880189 Change-Id: I5bb21e8b82b208c8d971e3193840f8b6afb8546e Reviewed-on: https://chromium-review.googlesource.com/1203352 Reviewed-by: Luciano Pacheco <lucmult@chromium.org> Reviewed-by: Noel Gordon <noel@chromium.org> Commit-Queue: Luciano Pacheco <lucmult@chromium.org> Cr-Original-Commit-Position: refs/heads/master@{#588758}(cherry picked from commit 25cb4e0cb2bcc9f223dd50cda2a416918222cb7e) Reviewed-on: https://chromium-review.googlesource.com/1249463 Cr-Commit-Position: refs/branch-heads/3538@{#713} Cr-Branched-From: 79f7c91a2b2a2932cd447fa6f865cb6662fa8fa6-refs/heads/master@{#587811}
,
Nov 20
|
||||||
►
Sign in to add a comment |
||||||
Comment 1 by bugdroid1@chromium.org
, Sep 5