Files.app: Update metadata by using new metadata cache event.

Files.app: Update metadata by using new metadata cache event.

BUG=410766
TEST=None

Committed: https://crrev.com/64d92fe9882e05da90d150a7a5bf26a9325de750
Cr-Commit-Position: refs/heads/master@{#316543}

[hirono, 2015-02-16 14:12:26]

hirono@chromium.org changed reviewers:
+ yawano@chromium.org

[hirono, 2015-02-16 14:12:27]

PTAL, thanks!

[yawano, 2015-02-17 03:26:46]

https://codereview.chromium.org/921683003/diff/20001/ui/file_manager/file_man...
File ui/file_manager/file_manager/foreground/js/metadata_update_controller.js
(right):

https://codereview.chromium.org/921683003/diff/20001/ui/file_manager/file_man...
ui/file_manager/file_manager/foreground/js/metadata_update_controller.js:91:
'external', event.entries);
onWatcherMetadataChanged_ can be called with metadata type 'thumbnail'. -
https://code.google.com/p/chromium/codesearch#chromium/src/ui/file_manager/fi...
Don't you need to consider this also in the new CL?

https://codereview.chromium.org/921683003/diff/20001/ui/file_manager/file_man...
ui/file_manager/file_manager/foreground/js/metadata_update_controller.js:100:
this.fileSystemMetadata_.get(entries, ['modificationTime']).then(function() {
We don't need to get modificationTime again to update date field for a daily
update.

[hirono, 2015-02-17 07:28:26]

https://codereview.chromium.org/921683003/diff/20001/ui/file_manager/file_man...
File ui/file_manager/file_manager/foreground/js/metadata_update_controller.js
(right):

https://codereview.chromium.org/921683003/diff/20001/ui/file_manager/file_man...
ui/file_manager/file_manager/foreground/js/metadata_update_controller.js:91:
'external', event.entries);
On 2015/02/17 03:26:46, yawano wrote:
> onWatcherMetadataChanged_ can be called with metadata type 'thumbnail'. -
>
https://code.google.com/p/chromium/codesearch#chromium/src/ui/file_manager/fi...
> Don't you need to consider this also in the new CL?

The thumbnail change for list views is handled by ThumbnailListLoader.

https://codereview.chromium.org/921683003/diff/20001/ui/file_manager/file_man...
ui/file_manager/file_manager/foreground/js/metadata_update_controller.js:100:
this.fileSystemMetadata_.get(entries, ['modificationTime']).then(function() {
On 2015/02/17 03:26:46, yawano wrote:
> We don't need to get modificationTime again to update date field for a daily
> update.

I'd like to avoid using getCache if we can. The result of getCache depends on
global cache state, and we cannot ensure the result of getCache is always
obtained correctly (if we don't know the assumption that all metadata of file
list has been cached)

Here even if we use 'get', if the values are cached, they are just returns
without API requesting. So there is no drawback to use 'get' method.

WDYT?

[yawano, 2015-02-17 07:42:08]

lgtm. Thank you!

https://codereview.chromium.org/921683003/diff/20001/ui/file_manager/file_man...
File ui/file_manager/file_manager/foreground/js/metadata_update_controller.js
(right):

https://codereview.chromium.org/921683003/diff/20001/ui/file_manager/file_man...
ui/file_manager/file_manager/foreground/js/metadata_update_controller.js:91:
'external', event.entries);
On 2015/02/17 07:28:25, hirono wrote:
> On 2015/02/17 03:26:46, yawano wrote:
> > onWatcherMetadataChanged_ can be called with metadata type 'thumbnail'. -
> >
>
https://code.google.com/p/chromium/codesearch#chromium/src/ui/file_manager/fi...
> > Don't you need to consider this also in the new CL?
> 
> The thumbnail change for list views is handled by ThumbnailListLoader.

Acknowledged.

https://codereview.chromium.org/921683003/diff/20001/ui/file_manager/file_man...
ui/file_manager/file_manager/foreground/js/metadata_update_controller.js:100:
this.fileSystemMetadata_.get(entries, ['modificationTime']).then(function() {
On 2015/02/17 07:28:25, hirono wrote:
> On 2015/02/17 03:26:46, yawano wrote:
> > We don't need to get modificationTime again to update date field for a daily
> > update.
> 
> I'd like to avoid using getCache if we can. The result of getCache depends on
> global cache state, and we cannot ensure the result of getCache is always
> obtained correctly (if we don't know the assumption that all metadata of file
> list has been cached)
> 
> Here even if we use 'get', if the values are cached, they are just returns
> without API requesting. So there is no drawback to use 'get' method.
> 
> WDYT?
> 

If we don't have the assumption (we don't require) that all metadata of the file
list are cached, this seems good to me. Thank you!

[hirono, 2015-02-17 08:17:15]

The CQ bit was checked by hirono@chromium.org

[hirono, 2015-02-17 08:17:31]

On 2015/02/17 07:42:08, yawano wrote:
> lgtm. Thank you!
> 
>
https://codereview.chromium.org/921683003/diff/20001/ui/file_manager/file_man...
> File ui/file_manager/file_manager/foreground/js/metadata_update_controller.js
> (right):
> 
>
https://codereview.chromium.org/921683003/diff/20001/ui/file_manager/file_man...
> ui/file_manager/file_manager/foreground/js/metadata_update_controller.js:91:
> 'external', event.entries);
> On 2015/02/17 07:28:25, hirono wrote:
> > On 2015/02/17 03:26:46, yawano wrote:
> > > onWatcherMetadataChanged_ can be called with metadata type 'thumbnail'. -
> > >
> >
>
https://code.google.com/p/chromium/codesearch#chromium/src/ui/file_manager/fi...
> > > Don't you need to consider this also in the new CL?
> > 
> > The thumbnail change for list views is handled by ThumbnailListLoader.
> 
> Acknowledged.
> 
>
https://codereview.chromium.org/921683003/diff/20001/ui/file_manager/file_man...
> ui/file_manager/file_manager/foreground/js/metadata_update_controller.js:100:
> this.fileSystemMetadata_.get(entries, ['modificationTime']).then(function() {
> On 2015/02/17 07:28:25, hirono wrote:
> > On 2015/02/17 03:26:46, yawano wrote:
> > > We don't need to get modificationTime again to update date field for a
daily
> > > update.
> > 
> > I'd like to avoid using getCache if we can. The result of getCache depends
on
> > global cache state, and we cannot ensure the result of getCache is always
> > obtained correctly (if we don't know the assumption that all metadata of
file
> > list has been cached)
> > 
> > Here even if we use 'get', if the values are cached, they are just returns
> > without API requesting. So there is no drawback to use 'get' method.
> > 
> > WDYT?
> > 
> 
> If we don't have the assumption (we don't require) that all metadata of the
file
> list are cached, this seems good to me. Thank you!

Thanks!

[commit-bot: I haz the power, 2015-02-17 08:17:50]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/921683003/20001

[commit-bot: I haz the power, 2015-02-17 08:20:29]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2015-02-17 08:21:13]

Patchset 2 (id:??) landed as
https://crrev.com/64d92fe9882e05da90d150a7a5bf26a9325de750
Cr-Commit-Position: refs/heads/master@{#316543}