Files.app: Move ContentMetadataProvider from ThumbnailModel to FileSystemMetadata.

Files.app: Move ContentMetadataProvider from ThumbnailModel to FileSystemMetadata.

Audio player refers 'media.title' and 'media.artist' properties that are fetched
by ContentMetadataProvider. Because these properties are not related with
thumbnail, the CL moves ContentMetadataProvider to FileSystemMetadata for
preparing to use new metadata class in Audio player.

BUG=410766
TEST=FileManagerJsTest.FileSystemMetadata

Committed: https://crrev.com/14295058048d3dd07a1e8c86c85711711adfb044
Cr-Commit-Position: refs/heads/master@{#317015}

[hirono, 2015-02-19 05:21:42]

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

[hirono, 2015-02-19 05:21:42]

PTAL, thanks!

[yawano, 2015-02-19 06:03:50]

https://codereview.chromium.org/932343003/diff/1/ui/file_manager/file_manager...
File ui/file_manager/file_manager/foreground/js/metadata/file_system_metadata.js
(right):

https://codereview.chromium.org/932343003/diff/1/ui/file_manager/file_manager...
ui/file_manager/file_manager/foreground/js/metadata/file_system_metadata.js:108:
this.contentMetadataProvider_.get(entries, contentPropertyNames)
Is it okay to get metadata of entries which are not present from
ContentMetadataProvider?

https://codereview.chromium.org/932343003/diff/1/ui/file_manager/file_manager...
File
ui/file_manager/file_manager/foreground/js/metadata/file_system_metadata_unittest.js
(right):

https://codereview.chromium.org/932343003/diff/1/ui/file_manager/file_manager...
ui/file_manager/file_manager/foreground/js/metadata/file_system_metadata_unittest.js:35:
// Mocking ExternalMetadataProvider.
nit: ContentMetadataProvider?

https://codereview.chromium.org/932343003/diff/1/ui/file_manager/file_manager...
ui/file_manager/file_manager/foreground/js/metadata/file_system_metadata_unittest.js:39:
assertEquals('filesystem://A', urls[0].toURL());
The variable 'urls' seems to be 'entries' here.

[hirono, 2015-02-19 06:27:33]

Thanks!

https://codereview.chromium.org/932343003/diff/1/ui/file_manager/file_manager...
File ui/file_manager/file_manager/foreground/js/metadata/file_system_metadata.js
(right):

https://codereview.chromium.org/932343003/diff/1/ui/file_manager/file_manager...
ui/file_manager/file_manager/foreground/js/metadata/file_system_metadata.js:108:
this.contentMetadataProvider_.get(entries, contentPropertyNames)
On 2015/02/19 06:03:50, yawano wrote:
> Is it okay to get metadata of entries which are not present from
> ContentMetadataProvider?

I think the client code (e.g. ThumbnailModel) should care about it. In audio
player, we need to fetch the title of music even if the file is not present
though fetching the metadata generates local cache of music files.

https://codereview.chromium.org/932343003/diff/1/ui/file_manager/file_manager...
File
ui/file_manager/file_manager/foreground/js/metadata/file_system_metadata_unittest.js
(right):

https://codereview.chromium.org/932343003/diff/1/ui/file_manager/file_manager...
ui/file_manager/file_manager/foreground/js/metadata/file_system_metadata_unittest.js:35:
// Mocking ExternalMetadataProvider.
On 2015/02/19 06:03:50, yawano wrote:
> nit: ContentMetadataProvider?

Done.

https://codereview.chromium.org/932343003/diff/1/ui/file_manager/file_manager...
ui/file_manager/file_manager/foreground/js/metadata/file_system_metadata_unittest.js:39:
assertEquals('filesystem://A', urls[0].toURL());
On 2015/02/19 06:03:50, yawano wrote:
> The variable 'urls' seems to be 'entries' here.

Done.

[yawano, 2015-02-19 06:37:28]

lgtm. Thank you!

https://codereview.chromium.org/932343003/diff/1/ui/file_manager/file_manager...
File ui/file_manager/file_manager/foreground/js/metadata/file_system_metadata.js
(right):

https://codereview.chromium.org/932343003/diff/1/ui/file_manager/file_manager...
ui/file_manager/file_manager/foreground/js/metadata/file_system_metadata.js:108:
this.contentMetadataProvider_.get(entries, contentPropertyNames)
On 2015/02/19 06:27:32, hirono wrote:
> On 2015/02/19 06:03:50, yawano wrote:
> > Is it okay to get metadata of entries which are not present from
> > ContentMetadataProvider?
> 
> I think the client code (e.g. ThumbnailModel) should care about it. In audio
> player, we need to fetch the title of music even if the file is not present
> though fetching the metadata generates local cache of music files.
> 

Acknowledged.

[hirono, 2015-02-19 06:41:03]

The CQ bit was checked by hirono@chromium.org

[hirono, 2015-02-19 06:41:10]

On 2015/02/19 06:37:28, yawano wrote:
> lgtm. Thank you!
> 
>
https://codereview.chromium.org/932343003/diff/1/ui/file_manager/file_manager...
> File
ui/file_manager/file_manager/foreground/js/metadata/file_system_metadata.js
> (right):
> 
>
https://codereview.chromium.org/932343003/diff/1/ui/file_manager/file_manager...
>
ui/file_manager/file_manager/foreground/js/metadata/file_system_metadata.js:108:
> this.contentMetadataProvider_.get(entries, contentPropertyNames)
> On 2015/02/19 06:27:32, hirono wrote:
> > On 2015/02/19 06:03:50, yawano wrote:
> > > Is it okay to get metadata of entries which are not present from
> > > ContentMetadataProvider?
> > 
> > I think the client code (e.g. ThumbnailModel) should care about it. In audio
> > player, we need to fetch the title of music even if the file is not present
> > though fetching the metadata generates local cache of music files.
> > 
> 
> Acknowledged.

Thanks!

[commit-bot: I haz the power, 2015-02-19 06:41:45]

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

[commit-bot: I haz the power, 2015-02-19 07:08:21]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2015-02-19 07:08:21]

Try jobs failed on following builders:
  chromium_presubmit on tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)

[hirono, 2015-02-19 08:33:20]

The CQ bit was checked by hirono@chromium.org

[commit-bot: I haz the power, 2015-02-19 08:34:34]

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

[commit-bot: I haz the power, 2015-02-19 08:53:44]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2015-02-19 08:54:30]

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