Method to convet base::Time to Java timestamp.

Added method to convert base::Time to a Java timestamp.
BUG=247382
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=224744

[Kibeom Kim (inactive), 2013-08-23 22:31:25]

lgtm

[newt (away), 2013-08-23 22:59:52]

Can you rename the title to make it clear what your change does? It's not clear
if "ForeignSessionHelper returns POSIX timestamps" describes the current bad
behavior or the new super awesome behavior. I'd say something like "Make
ForeignSessionHelper return POSIX timestamps".

A question and a comment:
 1. why is this change needed? 
 B. if we really need POSIX timestamps, we should probably add a function
WindowsToPosixTime() in time.h or time_util.h. That seems far less error prone
than trying to remember whether to add or subtract
kWindowsEpochDeltaMicroseconds all over the codebase.

[newt (away), 2013-08-24 01:02:08]

can you change the subject too?

android bits lgtm

[commit-bot: I haz the power, 2013-09-12 00:18:19]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/apiccion@chromium.org/22791013/5001

[commit-bot: I haz the power, 2013-09-12 00:38:42]

Retried try job too often on chromium_presubmit for step(s) presubmit
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=chromium_p...

[apiccion, 2013-09-20 03:03:57]

Mark or Darin could you have a look please?

[Mark Mentovai, 2013-09-20 15:46:25]

https://codereview.chromium.org/22791013/diff/5001/base/time/time.cc
File base/time/time.cc (right):

https://codereview.chromium.org/22791013/diff/5001/base/time/time.cc#newcode141
base/time/time.cc:141: int64 Time::ToJavaTime() const {
It looks like, given this and ToJsTime’s equivalence, you could use a templated
friend or private member function to do the internal work, and have ToJsTime and
ToJavaTime call that internal helper.

I won’t insist on this as long as there are only two, but I would insist if
there were three.

https://codereview.chromium.org/22791013/diff/5001/base/time/time.cc#newcode143
base/time/time.cc:143: return 0;
You seem to have copied this from ToJsTime, but you lost the comment that came
with this return statement.

https://codereview.chromium.org/22791013/diff/5001/base/time/time.cc#newcode147
base/time/time.cc:147: return std::numeric_limits<long>::max();
long is the wrong type here.

[apiccion, 2013-09-20 18:21:48]

https://codereview.chromium.org/22791013/diff/5001/base/time/time.cc
File base/time/time.cc (right):

https://codereview.chromium.org/22791013/diff/5001/base/time/time.cc#newcode141
base/time/time.cc:141: int64 Time::ToJavaTime() const {
Thank you!

On 2013/09/20 15:46:25, Mark Mentovai wrote:
> It looks like, given this and ToJsTime’s equivalence, you could use a
templated
> friend or private member function to do the internal work, and have ToJsTime
and
> ToJavaTime call that internal helper.
> 
> I won’t insist on this as long as there are only two, but I would insist if
> there were three.

https://codereview.chromium.org/22791013/diff/5001/base/time/time.cc#newcode143
base/time/time.cc:143: return 0;
On 2013/09/20 15:46:25, Mark Mentovai wrote:
> You seem to have copied this from ToJsTime, but you lost the comment that came
> with this return statement.

Done.

https://codereview.chromium.org/22791013/diff/5001/base/time/time.cc#newcode147
base/time/time.cc:147: return std::numeric_limits<long>::max();
On 2013/09/20 15:46:25, Mark Mentovai wrote:
> long is the wrong type here.

Done.

[Mark Mentovai, 2013-09-20 20:44:05]

You seem to have lost chrome/browser/android/foreign_session_helper.cc in patch
set 3. Intentional?

[apiccion, 2013-09-20 20:51:50]

Thanks for catching this. Moved those changes to:
https://codereview.chromium.org/23514039/

Should be good. Renamed subject/description.

[Mark Mentovai, 2013-09-20 21:32:27]

LGTM for base OWNERS approval.

[commit-bot: I haz the power, 2013-09-20 21:52:40]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/apiccion@chromium.org/22791013/21001

[commit-bot: I haz the power, 2013-09-21 00:32:58]

Retried try job too often on linux_chromeos for step(s) browser_tests
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=linux_chro...

[commit-bot: I haz the power, 2013-09-23 06:01:22]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/apiccion@chromium.org/22791013/21001

[commit-bot: I haz the power, 2013-09-23 10:52:02]

Retried try job too often on win_rel for step(s) sync_integration_tests
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=win_rel&nu...

[commit-bot: I haz the power, 2013-09-23 16:27:57]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/apiccion@chromium.org/22791013/21001

[commit-bot: I haz the power, 2013-09-23 17:52:43]

Change committed as 224744