base::TimeTicks::UnixEpoch() was a bad idea; so, remove it |
||||
Issue descriptionDespite it being correctly used for a few specific use cases, it seems this function is now being used in many places inappropriately. The function takes a single snapshot the first time it is used and, therefore, does not account for clock drift between base::Time and base::TimeTicks (especially when the user puts their machine to sleep!). This recently led to bugs in the networking stack! This bug tracks efforts to deprecate and remove the function, and provide existing use cases with reasonable alternatives.
,
Mar 7 2016
Could you give some context on why Netlog wants to find the time since the Unix epoch in TimeTicks? It seems that the main problem is that this question is just inherently unanswerable. I guess my problem with "approximate" is that it's vague as to just how far off the answer really is.
,
Mar 7 2016
I'm going to tentatively say "nevermind," as I'm looking through the code. It feels like the best thing to do is to just log a reference Time and TimeTicks at netlog creation time, and perform all Time conversions using the initial Time reference plus a TimeDelta calculated via TimeTicks.
,
Mar 8 2016
Another question: https://code.google.com/p/chromium/codesearch#chromium/src/net/base/network_quality_estimator_unittest.cc&l=360 This test uses UnixEpoch to mock a "very old" timestamp. What's the canonical way to do this? I imagine there's something better than using FromInternalValue.
,
Mar 8 2016
Now - a large delta? Using Now is usually pretty sketchy in a test though.. maybe ok in this case?
,
Mar 8 2016
Now - a large delta produces a TimeDelta. We'd have to coerce that somehow into a TimeTicks to satisfy the existing API.
,
Mar 8 2016
Oh, woops. Well, FromInternalValue isn't terrible in a test either, is it causing some problem?
,
Mar 8 2016
https://code.google.com/p/chromium/codesearch#chromium/src/base/time/time.h&l=329 Are you sure it produces a delta? Which operator are you meaning?
,
Mar 29 2016
Sorry I was unclear about the API, Now() - TimeDelta can give a TimeTicks. Fixing all cases in net/ will happen soon.
,
Aug 4 2016
,
Aug 4 2016
,
Jan 11
You started fixing this bug over two years ago. Are you still working on it?
,
Jan 13
Yes. |
||||
►
Sign in to add a comment |
||||
Comment 1 by csharrison@chromium.org
, Mar 4 2016The netlog uses a timeTickOffset to convert TimeTicks values to Dates for display. I imagine most TimeTicks use this kind of approach. Should we expose a function that performs the same job as UnixEpoch, but just provides TimeTicks::Now() as the reference time? TimeTicks ApproximateUnixEpoch() { return TimeTicks::Now() - (Time::Now() - Time::UnixEpoch()); } Or, should we instead provide a function like: TimeDelta ApproximateTimeSinceEpoch(TimeTicks t) { return (Time::Now() - Time::UnixEpoch()) - (TimeTicks::Now() - t); } WDYT? Netlog would probably like option (1), but option (2) is a better interface. Who wants Epoch time in TimeTicks format? Seems like a bad idea. We could also expose an interface from TimeTicks => Time.