Upgrade Blink to milliseconds-based last modified filetimes, part 1.

Upgrade Blink to milliseconds-based last modified filetimes, part 1.

This is the initial change towards switching Blink and Chromium to
work over milliseconds-based WebFileInfo modification times.

It adds a temporary millisecond-based extra field to WebFileInfo
recording time in milliseconds, alongside the current seconds-based
one.

This is the first patch in the following Blink+Chromium series:

1: [blink] *this one* [ https://codereview.chromium.org/873723004/ ]
2: [chromium] fill in modificationTimeMS [ https://codereview.chromium.org/884413002/ ] 
3: [blink] make use of modificationTimeMS [ https://codereview.chromium.org/884393002/ ]
4: [chromium] set modificationTime to something msec-based [ https://codereview.chromium.org/862203003/ ]
5: [blink] switch to using modificationTime instead of *MS [ https://codereview.chromium.org/882343002/ ]
6: [chromium] stop setting modificationTimeMS [ https://codereview.chromium.org/890523002/ ]
7: [blink] remove modificationTimeMS [ https://codereview.chromium.org/869613005/ ]

R=mkwst,kinuko
BUG=451747
Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=189238

[sof, 2015-01-29 09:53:19]

sigbjornf@opera.com changed reviewers:
+ jochen@chromium.org, kinuko@chromium.org, mkwst@chromium.org,
nhiroki@chromium.org

[sof, 2015-01-29 09:53:20]

Please take a look.

I think we should try to transition WebFileInfo up/over to something
milliseconds-based. And Blink in general could with benefit use milliseconds
throughout for modification times, in due course.

This patch series gets Blink & Chromium to agree on WebFileInfo modification
times, along with switching FileMetadata over too in the process. Would one or
more of you be willing to help out getting this patch series onto both trunks?
Thanks.

[Mike West, 2015-01-29 11:38:01]

I'll defer to Kinuko-san regarding the sanity of the end result. It makes sense
to me, and this series of patches (7!) looks like a reasonable way to achieve
it.

[sof, 2015-01-29 11:39:42]

On 2015/01/29 11:38:01, Mike West wrote:
> I'll defer to Kinuko-san regarding the sanity of the end result. It makes
sense
> to me, and this series of patches (7!) looks like a reasonable way to achieve
> it.

ok, thanks. If a poster child for repo merging is needed, look no further?

[kinuko, 2015-01-30 05:11:46]

I agree that this change makes sense (seconds-based resolution used to be a
problem), and thanks for working on this!

lgtm, with one comment.

I'm totally ok with these 7 patches as you've already made them (that must have
been tough!), but I think we can also probably decrease the # of necessary
patches by using transient ifdef tricks.

E.g. we can land chromium changes first, without introducing a new xxxMS field
like this:

 // TODO(sof): this ifdef should be removed once blink side change is rolled.
 #ifdef FILE_INFO_EXPECT_MSEC
    web_file_info->modificationTime = file_info.last_modified.ToJsTime();
 #else
    web_file_info->modificationTime = file_info.last_modified.ToDoubleT();
 #endif

And then land blink patch to switch everything, which also switch the chromium
impl by including the define in the public header:

 // WebFileInfo.h
 #define FILE_INFO_EXPECT_MSEC

It's not the cleanest way to make patches, but it could be useful when we need
to make complicated multi-sided patches.

[sof, 2015-01-30 07:45:03]

On 2015/01/30 05:11:46, kinuko wrote:
> I agree that this change makes sense (seconds-based resolution used to be a
> problem), and thanks for working on this!
> 
> lgtm, with one comment.
> 
> I'm totally ok with these 7 patches as you've already made them (that must
have
> been tough!), but I think we can also probably decrease the # of necessary
> patches by using transient ifdef tricks.
> 
> E.g. we can land chromium changes first, without introducing a new xxxMS field
> like this:
> 
>  // TODO(sof): this ifdef should be removed once blink side change is rolled.
>  #ifdef FILE_INFO_EXPECT_MSEC
>     web_file_info->modificationTime = file_info.last_modified.ToJsTime();
>  #else
>     web_file_info->modificationTime = file_info.last_modified.ToDoubleT();
>  #endif
> 
> And then land blink patch to switch everything, which also switch the chromium
> impl by including the define in the public header:
> 
>  // WebFileInfo.h
>  #define FILE_INFO_EXPECT_MSEC
> 
> It's not the cleanest way to make patches, but it could be useful when we need
> to make complicated multi-sided patches.

Thanks for the suggestion to lessen the overhead, I might just make use of such
a shortcut when making a follow-through change for blob data modification times.

With the CLs in place here & there being no rush, I will proceed with these.

[sof, 2015-01-30 07:49:05]

mkwst@: whenever there's time; public/ change looks alright?

[Mike West, 2015-01-30 08:18:27]

LGTM.

[sof, 2015-01-30 08:20:04]

The CQ bit was checked by sigbjornf@opera.com

[commit-bot: I haz the power, 2015-01-30 08:20:28]

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

[commit-bot: I haz the power, 2015-01-30 09:50:33]

Committed patchset #1 (id:1) as
https://src.chromium.org/viewvc/blink?view=rev&revision=189238