Files.app: Add MetadataItem class.

Files.app: Add MetadataItem class.

Previously Files.app uses @typedef for expressing metadata item.
The CL introduces new class instead, for better compiler check.

BUG=410766
TEST=None

Committed: https://crrev.com/d88d2844eef98871fdf0753429f5b0e63053cf7a
Cr-Commit-Position: refs/heads/master@{#316991}

[hirono, 2015-02-18 13:50:50]

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

[hirono, 2015-02-18 13:50:51]

PTAL, thanks!

[mtomasz, 2015-02-18 14:14:03]

https://codereview.chromium.org/939653002/diff/20001/ui/file_manager/file_man...
File ui/file_manager/file_manager/foreground/js/metadata/metadata_item.js
(right):

https://codereview.chromium.org/939653002/diff/20001/ui/file_manager/file_man...
ui/file_manager/file_manager/foreground/js/metadata/metadata_item.js:81: 
nit: \n\n -> \n

https://codereview.chromium.org/939653002/diff/40001/ui/file_manager/file_man...
File
ui/file_manager/file_manager/foreground/js/metadata/content_metadata_provider.js
(right):

https://codereview.chromium.org/939653002/diff/40001/ui/file_manager/file_man...
ui/file_manager/file_manager/foreground/js/metadata/content_metadata_provider.js:80:
item.contentImageTransform = metadata ['imageTransform'];
Hm, is the new version better? Before, we didn't allow to return "size" from a
ContentMetadataProvider, but the new version would pass the closure compiler
type checks.

https://codereview.chromium.org/939653002/diff/40001/ui/file_manager/file_man...
File
ui/file_manager/file_manager/foreground/js/metadata/new_metadata_provider.js
(right):

https://codereview.chromium.org/939653002/diff/40001/ui/file_manager/file_man...
ui/file_manager/file_manager/foreground/js/metadata/new_metadata_provider.js:40:
* @return {!Promise<!Array<!MetadataItem>>} Promise with obtained metadata. It
Can we have separate objects like MetadataItem for each provider, to have even
more compile time check? WDYT?

[hirono, 2015-02-19 03:42:48]

https://codereview.chromium.org/939653002/diff/20001/ui/file_manager/file_man...
File ui/file_manager/file_manager/foreground/js/metadata/metadata_item.js
(right):

https://codereview.chromium.org/939653002/diff/20001/ui/file_manager/file_man...
ui/file_manager/file_manager/foreground/js/metadata/metadata_item.js:81: 
On 2015/02/18 14:14:03, mtomasz wrote:
> nit: \n\n -> \n

Done.

https://codereview.chromium.org/939653002/diff/40001/ui/file_manager/file_man...
File
ui/file_manager/file_manager/foreground/js/metadata/external_metadata_provider.js
(right):

https://codereview.chromium.org/939653002/diff/40001/ui/file_manager/file_man...
ui/file_manager/file_manager/foreground/js/metadata/external_metadata_provider.js:82:
item.availableOffline = properties.availableOffline;
> Hm, is the new version better? Before, we didn't allow to return "size" from a
> ContentMetadataProvider, but the new version would pass the closure compiler
> type checks.
> Can we have separate objects like MetadataItem for each provider, to have even
> more compile time check? WDYT?

There is a trade-off between single MetadataItem and multiple MetadataItem for
each provider.

Good points of single MetadataItem is less maintenance costs. And we can regard
it as a master of metadata type.
We can have all comments about the property in one place. And the master forces
us to use common names of properties everywhere.
If we have multiple MetadataItem for each provider, we may name the properties
that have same meanings in different names.
e.g. Currently ExternalProvider has imageRotation property while ContentProvider
imageTransformation property.
Also we may have typo thumbnailUrl vs thumbnailURL, which are not checked
closure compiler because we copied properties by using for in loop.

Bad points of single MetadataItem is, as you pointed out, it does not check
unintentional property assign.
To prevent this, we can define specific metadata type for each provider.
But I think there is not huge difference between:
 * listing up the property names that can be assigned by the provider in the
constructor of the specific metadata type, and
 * listing up the property names here (the code where the properties actually
assigned by each provider)

WDYT?

[mtomasz, 2015-02-19 04:44:51]

salgtm = sgtm + lgtm :)

[hirono, 2015-02-19 04:56:34]

The CQ bit was checked by hirono@chromium.org

[hirono, 2015-02-19 04:56:38]

On 2015/02/19 04:44:51, mtomasz wrote:
> salgtm = sgtm + lgtm :)

Thanks!

[commit-bot: I haz the power, 2015-02-19 04:57:04]

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

[commit-bot: I haz the power, 2015-02-19 05:00:20]

Committed patchset #4 (id:60001)

[commit-bot: I haz the power, 2015-02-19 05:00:54]

Patchset 4 (id:??) landed as
https://crrev.com/d88d2844eef98871fdf0753429f5b0e63053cf7a
Cr-Commit-Position: refs/heads/master@{#316991}