Use int64 time stamp when storing metadata to the HTTP cache.

Use int64 time stamp when storing metadata to the HTTP cache.

This is part 2/4 of a clean up to switch to int64 time stamps when caching metadata. The background to this is that converting int64 time stamps to double and back to int64 is lossy and causes false time stamp mismatches.

BUG=453298
Committed: https://crrev.com/7fbf4c1ca967c4dd4674622f2c2ea059d08566b7
Cr-Commit-Position: refs/heads/master@{#317421}

[Yang, 2015-02-18 09:24:31]

yangguo@chromium.org changed reviewers:
+ davidben@chromium.org, mkwst@chromium.org, rvargas@chromium.org

[Yang, 2015-02-18 09:46:46]

Please take a look. This is part of a series of CLs between blink and chromium.

Part 1/4: https://codereview.chromium.org/938643002/
Part 2/4: https://codereview.chromium.org/935963002/
Part 3/4: https://codereview.chromium.org/940443002/
Part 4/4: https://codereview.chromium.org/935983002/

[rvargas (doing something else), 2015-02-18 18:38:41]

lgtm

[davidben, 2015-02-18 18:50:45]

https://codereview.chromium.org/935963002/diff/1/content/child/web_url_loader...
File content/child/web_url_loader_impl.cc (right):

https://codereview.chromium.org/935963002/diff/1/content/child/web_url_loader...
content/child/web_url_loader_impl.cc:977:
response->setOriginalResponseTime(info.response_time.ToInternalValue());
(It's a little weird to be storing two copies of the same information on a
WebURLResponse, but I'll defer to part 1's review there.)

https://codereview.chromium.org/935963002/diff/1/content/common/view_messages.h
File content/common/view_messages.h (right):

https://codereview.chromium.org/935963002/diff/1/content/common/view_messages...
content/common/view_messages.h:1475: int64 /* expected_response_time */,
base::Time? We don't seem to have issues putting a base::Time in an IPC, going
by codesearch.

https://codereview.chromium.org/935963002/diff/1/content/renderer/renderer_bl...
File content/renderer/renderer_blink_platform_impl.cc (right):

https://codereview.chromium.org/935963002/diff/1/content/renderer/renderer_bl...
content/renderer/renderer_blink_platform_impl.cc:345: size_t size) {
Why not have this function do the conversion from double to int64 and call the
int64 cacheMetadata overload? Then you don't need to duplicate the IPC and
everything else up the stack.

[Yang, 2015-02-19 08:08:48]

New patchsets have been uploaded after l-g-t-m from rvargas@chromium.org

[Yang, 2015-02-19 08:39:44]

Thanks for the review. I addressed your comments.

https://codereview.chromium.org/935963002/diff/20001/content/child/web_url_lo...
File content/child/web_url_loader_impl.cc (right):

https://codereview.chromium.org/935963002/diff/20001/content/child/web_url_lo...
content/child/web_url_loader_impl.cc:978: static_cast<long
long>(info.response_time.ToInternalValue()));
This static_cast will be removed in part 4 once the overloaded
setResponseTime(double) is removed in part 3.

[Mike West, 2015-02-19 09:37:10]

LGTM, given that you're getting rid of the duplication in future patches in the
series.

[Yang, 2015-02-19 09:53:27]

New patchsets have been uploaded after l-g-t-m from mkwst@chromium.org

[davidben, 2015-02-19 21:40:08]

lgtm

https://codereview.chromium.org/935963002/diff/40001/content/child/web_url_lo...
File content/child/web_url_loader_impl.cc (right):

https://codereview.chromium.org/935963002/diff/40001/content/child/web_url_lo...
content/child/web_url_loader_impl.cc:978: static_cast<long
long>(info.response_time.ToInternalValue()));
This was really confusing, but I see that they go to different fields and the
other's going away.

Maybe add a comment in the meantime (to be removed later when line 976 goes)
just so it doesn't freak out anyone else who sees it in a transition state. :-)

[Yang, 2015-02-20 10:11:30]

New patchsets have been uploaded after l-g-t-m from davidben@chromium.org

[Yang, 2015-02-20 21:09:55]

The CQ bit was checked by yangguo@chromium.org

[commit-bot: I haz the power, 2015-02-20 21:11:25]

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

[commit-bot: I haz the power, 2015-02-20 21:25:32]

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

[commit-bot: I haz the power, 2015-02-20 21:25:34]

Try jobs failed on following builders:
  ios_dbg_simulator on tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios_dbg_simulator...)

[Yang, 2015-02-20 21:32:35]

The CQ bit was checked by yangguo@chromium.org

[commit-bot: I haz the power, 2015-02-20 21:33:19]

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

[commit-bot: I haz the power, 2015-02-20 22:17:12]

Committed patchset #5 (id:80001)

[commit-bot: I haz the power, 2015-02-20 22:17:51]

Patchset 5 (id:??) landed as
https://crrev.com/7fbf4c1ca967c4dd4674622f2c2ea059d08566b7
Cr-Commit-Position: refs/heads/master@{#317421}