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

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

With WebFileInfo now recording modification time in milliseconds via
modificationTimeMS, switch FileMetadata over to using it and keep a
milliseconds-based modification time field also. Also have the fileapi's
File object's snapshot times be milliseconds-based.

To make it clearer what units the modification time values are over,
append a MS suffix. This is temporary until the conversion is complete.

This patch is the third one in the following series,

1: [blink] add WebFileInfo::modificationTimeMS [ https://codereview.chromium.org/873723004/ ]
2: [chromium] fill in modificationTimeMS [ https://codereview.chromium.org/884413002/ ] 
3: [blink] *this one* [ 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,jsbell
BUG=451747
Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=189475

Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=189755

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

mkwst@chromium.org changed reviewers:
+ mkwst@chromium.org

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

LGTM.

[sof, 2015-02-03 06:22:26]

The CQ bit was checked by sigbjornf@opera.com

[commit-bot: I haz the power, 2015-02-03 06:22:52]

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

[commit-bot: I haz the power, 2015-02-03 08:10:18]

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

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

Try jobs failed on following builders:
  linux_blink_rel on tryserver.blink
(http://build.chromium.org/p/tryserver.blink/builders/linux_blink_rel/builds/4...)

[sof, 2015-02-03 15:30:53]

The initial patchset didn't address the change in interpretation of
last-modified timestamps when serializing File objects. Updated the patch to do
so, requiring a version bump for the wire format. 

Please take another look?

(+Cc: jsbell@ )

[jsbell, 2015-02-03 17:43:45]

jsbell@chromium.org changed reviewers:
+ jsbell@chromium.org

[jsbell, 2015-02-03 17:43:46]

lgtm

https://codereview.chromium.org/884393002/diff/40001/Source/core/fileapi/File.h
File Source/core/fileapi/File.h (right):

https://codereview.chromium.org/884393002/diff/40001/Source/core/fileapi/File...
Source/core/fileapi/File.h:135: void captureSnapshot(long long& snapshotSize,
double& snapshotModificationTimeInMS) const;
Nit: this is the only place that is "TimeInMS" rather than just "TimeMS"

[jsbell, 2015-02-03 17:46:30]

(And this is probably covered everywhere, but do we care about filesystems that
have don't have ms accuracy? I haven't gone through the entire CL set so I'm not
sure if that is even relevant.)

[sof, 2015-02-03 19:07:46]

On 2015/02/03 17:46:30, jsbell wrote:
> (And this is probably covered everywhere, but do we care about filesystems
that
> have don't have ms accuracy? I haven't gone through the entire CL set so I'm
not
> sure if that is even relevant.)

Good question, hadn't considered. As the file times originate from
base::File::last_modified Time values, I would expect the fs-specific routines
to map into Time values at whatever granularity the underlying FS supports.
Which is then converted into milliseconds as WebFileInfo.lastModified(MS)

Metadata.modificationTime returns the time in ms (which we no longer need to
scale.)

[sof, 2015-02-03 19:46:25]

https://codereview.chromium.org/884393002/diff/40001/Source/core/fileapi/File.h
File Source/core/fileapi/File.h (right):

https://codereview.chromium.org/884393002/diff/40001/Source/core/fileapi/File...
Source/core/fileapi/File.h:135: void captureSnapshot(long long& snapshotSize,
double& snapshotModificationTimeInMS) const;
On 2015/02/03 17:43:45, jsbell wrote:
> Nit: this is the only place that is "TimeInMS" rather than just "TimeMS"

Thanks, made consistent.

[sof, 2015-02-03 19:46:33]

The CQ bit was checked by sigbjornf@opera.com

[commit-bot: I haz the power, 2015-02-03 19:46:54]

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

[commit-bot: I haz the power, 2015-02-03 22:04:11]

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

[commit-bot: I haz the power, 2015-02-03 22:04:13]

Try jobs failed on following builders:
  mac_blink_rel on tryserver.blink
(http://build.chromium.org/p/tryserver.blink/builders/mac_blink_rel/builds/41083)

[sof, 2015-02-03 22:35:18]

The CQ bit was checked by sigbjornf@opera.com

[commit-bot: I haz the power, 2015-02-03 22:36:19]

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

[commit-bot: I haz the power, 2015-02-04 01:14:14]

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

[commit-bot: I haz the power, 2015-02-04 01:14:15]

Try jobs failed on following builders:
  mac_blink_rel on tryserver.blink
(http://build.chromium.org/p/tryserver.blink/builders/mac_blink_rel/builds/41131)

[sof, 2015-02-04 06:24:51]

The CQ bit was checked by sigbjornf@opera.com

[commit-bot: I haz the power, 2015-02-04 06:25:30]

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

[commit-bot: I haz the power, 2015-02-04 09:04:05]

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

[commit-bot: I haz the power, 2015-02-04 09:04:06]

Try jobs failed on following builders:
  mac_blink_rel on tryserver.blink
(http://build.chromium.org/p/tryserver.blink/builders/mac_blink_rel/builds/41252)

[sof, 2015-02-04 09:22:20]

The CQ bit was checked by sigbjornf@opera.com

[commit-bot: I haz the power, 2015-02-04 09:23:48]

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

[commit-bot: I haz the power, 2015-02-04 10:27:40]

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

[sof, 2015-02-04 12:52:47]

A revert of this CL (patchset #4 id:60001) has been created in
https://codereview.chromium.org/900773003/ by sigbjornf@opera.com.

The reason for reverting is: Two linux32 failures coming through,

http://test-results.appspot.com/dashboards/flakiness_dashboard.html#master=Ch....

[sof, 2015-02-06 10:11:09]

Addressed linux32 failures; please take another look?

It is unsound to continue using time_t for getFileModificationTime() when we're
now working over milliseconds. Some platforms (linux32) map time_t to e.g. "long
int", which doesn't have the required range. And it doesn't have to; switched to
using 'double' instead.

[sof, 2015-02-06 19:32:26]

jsbell@: time_t => double argument change looks fine?

(sorry, i'm just keen to get this train back on track again :-) )

[sof, 2015-02-08 14:39:11]

Relanding now, but will follow-up should there be any questions surrounding the
last patchset.

[sof, 2015-02-08 14:39:18]

The CQ bit was checked by sigbjornf@opera.com

[commit-bot: I haz the power, 2015-02-08 14:39:36]

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

[commit-bot: I haz the power, 2015-02-08 14:42:49]

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

[jsbell, 2015-02-09 18:24:08]

On 2015/02/06 19:32:26, sof wrote:
> jsbell@: time_t => double argument change looks fine?
> 
> (sorry, i'm just keen to get this train back on track again :-) )

lgtm after the fact

(I think I was on an airplane at the time)

[sof, 2015-02-09 18:40:45]

On 2015/02/09 18:24:08, jsbell wrote:
> On 2015/02/06 19:32:26, sof wrote:
> > jsbell@: time_t => double argument change looks fine?
> > 
> > (sorry, i'm just keen to get this train back on track again :-) )
> 
> lgtm after the fact
> 
> (I think I was on an airplane at the time)

thanks for checking it over.