I suspect this never really worked.

A long time ago, values were returned in milliseconds (yay), but with an offset of around -4G for me (boo).

This appears to have changed in https://codereview.chromium.org/2206053002 that touched android key event code and mistakenly added an extra divide-by-1000. Values were now in in seconds, and with a weird offset of around -2M for me.

Right now I always get 0, this because https://codereview.chromium.org/2615533002 merged two conversion functions and added clamping to 0 (this flapped between a DCHECK and clamping to 0 a couple times, see  bug 680623 .

The real issue, having a nonzero offset, appears to be caused by an unfortunate "java long" -> "c++ long" conversion in android code, values get garbled at the JNI boundary.

The issue is more apparent if you happen to have a machine with uptime of about 25 days aka over 2^31 microseconds.

Thanks for the investigation.  It looks like this is the only place in the code where we made the jlong -> long mistake.

I made the changes that caused Event.timeStamp to always return 0, which is one of the side-effects that i ignored when i was pushing the change. The intent of my change was to reuse The monotonicTimeToDOMHighResTimeStamp(...) implementation for PerformanceResourceTiming and PerformanceNavigationTiming. And we figured, it doesn't make much sense to let monotonicTimeToDOMHighResTimeStamp(...) return any negative values. That's why two explicit checks were added to make sure that no negative values get returned. Apparently, monotonicTimeToDOMHighResTimeStamp(...) is also being used for other purposes than returning performance-related timing values. I am happy to take on the bug to revert our changes if needed.

Don't worry, you don't need to revert.  tsniatowski@ has a fix for the root cause at https://codereview.chromium.org/2755453004/

Sounds good!

I guess event.timeStamp is technically a performance-related timing value... but that doesn't stop, say, a site that I won't name, from tying business logic to having the event timestamps changing over time.

I can't guarantee there isn't some other code path that'll end up getting negative values through the conversion function, but the linked CL should definitely help. Anyway, the clamping only made the underlying problem need more immediate fixing, I didn't need to touch it to fix this bug.

And, aelias@, thanks for checking the other long/jlongs, I haven't had the time to look around too much.

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/6bda2b55ca3d254ca021560906d0703e8b0df21e

commit 6bda2b55ca3d254ca021560906d0703e8b0df21e
Author: tsniatowski <tsniatowski@opera.com>
Date: Thu Mar 16 08:17:50 2017

Fix android key event timestamps

Pass a Java long to C++ as a jlong type, not a C++ long which can have a
different size to avoid broken / negative event timestamp values.
ImeAdapter's Java side uses "long" in SendKeyEvent, so the C++ side must
use a jlong or int64_t, and not a C++ long. Otherwise things don't work
well when system uptime is over 2^31ms (~25 days).

Additionally, do not do an extra divide-by-1000 when the used helper
function will do the milliseconds to seconds conversion already, so the
timestamps are correctly measured in milliseconds.

The resulting keyboard event timestamps end up nicely sane and positive,
and no longer clamped to 0 in PerformanceBase.cpp.

BUG= 701726 
R=aelias@chromium.org

Review-Url: https://codereview.chromium.org/2755453004
Cr-Commit-Position: refs/heads/master@{#457369}

[modify] https://crrev.com/6bda2b55ca3d254ca021560906d0703e8b0df21e/content/browser/renderer_host/ime_adapter_android.cc
[modify] https://crrev.com/6bda2b55ca3d254ca021560906d0703e8b0df21e/content/browser/renderer_host/ime_adapter_android.h

> that doesn't stop, say, a site that I won't name, from tying business logic to having the event timestamps changing over time.

If this is a public-facing website, it would be useful to note here which site was broken by this problem.  Then we can dupe any other bug that may be filed about this site into this one.  (Fine not to describe it if it's an internal customer.)

yeah, not a public-facing website