New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 835203 link

Starred by 4 users

Issue metadata

Status: Assigned
Owner:
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Bug

Blocked on:
issue 858935



Sign in to add a comment

Change 'Entries' to use a custom class in the Files App

Project Member Reported by sashab@chromium.org, Apr 20 2018

Issue description

We currently use Entry (FileEntry and DirectoryEntry) objects for representing files in the Files App (relatedly, we need to finish converting remaining URLs to use isolated entries in issue 507210).

This has some drawbacks; for example, we can't store whether a file/folder is read-only and fetch it synchronously.

Investigate creating either a subclass of Entry or a new class that contains Entry that can store this additional metadata. Then, remove redundant calls to getEntryProperties() and replace it with synchronous code (e.g. the code for checking whether folders are read-only).
 

Comment 1 by sashab@chromium.org, Apr 20 2018

Cc: sashab@chromium.org
We have some partial mechanism for this. We can obtain extra metadata deduced from the volume metadata by calling synchronous VolumeManager.getLocationInfo(Entry).

This won't work for required file properties that can't be deduced from the volume metadata. For those properties an asynchronous call is needed via MetadataModel which calls getEntryProperties private API under the hood. IIRC there was a best effort synchronous call in MetadataModel, but it can return null if the data is not cached.

There is a reason for choosing such design. Fetching all metadata for each file may be very heavy. And if we want to have all metadata synchronously then we have to prefetch everything. That may very negatively affect performance.

That's why this has been done on case by case basis. If some properties were needed synchronously, they were prefetched asynchronously and then queried synchronously.

Comment 3 by sashab@google.com, May 24 2018

Labels: -M-68 M-69

Comment 4 by sashab@google.com, May 24 2018

Labels: -M-69

Comment 5 by sashab@chromium.org, May 31 2018

Owner: lucmult@chromium.org
Status: Assigned (was: Available)
Blockedon: 858935
Project Member

Comment 7 by bugdroid1@chromium.org, Jul 4

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/d0bca74166ccd0d8534ffaf3ead5beaf779a8350

commit d0bca74166ccd0d8534ffaf3ead5beaf779a8350
Author: Luciano Pacheco <lucmult@chromium.org>
Date: Wed Jul 04 06:41:00 2018

Implement base interface and types for new navigation

Add interface FilesAppEntry which is the base interface that moving
forward the app UI will converge as base type that can be displayed on
different UI components such as: navigation tree, file list and
breadcrumbs, eventually superseding Entry type.

Add VolumeEntry which implements interface FilesAppEntry to represent a
Volume, this will allow to display Volumes on file list/Right Hand Side
(RHS).

Add EntryList which implements interface FilesAppEntry to represent a
list of entries.  This will be used to implement "My Files" which will
contain a list of VolumeEntry for the volumes: Downloads, Linux Files
(Crostini) and Play Files (ARC++).

Design doc: https://docs.google.com/document/d/1X5XSLKJd0yerL-qFhpb2z9ibUVb_W3gG_tfIV7T_Qt0

Bug:  846587 , 835203
Test: Unit-test for the new types.
Cq-Include-Trybots: luci.chromium.try:closure_compilation
Change-Id: Ia2fce233338f8b8e0969b77daf4c77139852c441
Reviewed-on: https://chromium-review.googlesource.com/1086680
Commit-Queue: Luciano Pacheco <lucmult@chromium.org>
Reviewed-by: Noel Gordon <noel@chromium.org>
Reviewed-by: Naoki Fukino <fukino@chromium.org>
Reviewed-by: Sasha Morrissey <sashab@chromium.org>
Cr-Commit-Position: refs/heads/master@{#572496}
[modify] https://crrev.com/d0bca74166ccd0d8534ffaf3ead5beaf779a8350/chrome/browser/chromeos/file_manager/file_manager_jstest.cc
[modify] https://crrev.com/d0bca74166ccd0d8534ffaf3ead5beaf779a8350/ui/file_manager/file_manager/common/js/BUILD.gn
[add] https://crrev.com/d0bca74166ccd0d8534ffaf3ead5beaf779a8350/ui/file_manager/file_manager/common/js/files_app_entry_types.js
[add] https://crrev.com/d0bca74166ccd0d8534ffaf3ead5beaf779a8350/ui/file_manager/file_manager/common/js/files_app_entry_types_unittest.html
[add] https://crrev.com/d0bca74166ccd0d8534ffaf3ead5beaf779a8350/ui/file_manager/file_manager/common/js/files_app_entry_types_unittest.js

Sign in to add a comment