New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 590845 link

Starred by 1 user

Issue metadata

Status: Started
Owner:
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: All
Pri: 2
Type: Bug

Blocked on:
issue 592107



Sign in to add a comment

base::TimeTicks::UnixEpoch() was a bad idea; so, remove it

Project Member Reported by m...@chromium.org, Feb 29 2016

Issue description

Despite 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.
 
The 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.
Cc: charliea@chromium.org
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.
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.
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.

Now - a large delta? Using Now is usually pretty sketchy in a test though.. maybe ok in this case?
Now - a large delta produces a TimeDelta. We'd have to coerce that somehow into a TimeTicks to satisfy the existing API.
Oh, woops. Well, FromInternalValue isn't terrible in a test either, is it causing some problem?
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?
Sorry I was unclear about the API, Now() - TimeDelta can give a TimeTicks. Fixing all cases in net/ will happen soon.

Comment 10 by m...@chromium.org, Aug 4 2016

Components: Internals>Core
Status: Started (was: Assigned)

Comment 11 by m...@chromium.org, Aug 4 2016

Labels: -Pri-1 -M-51 M-55 Pri-2
You started fixing this bug over two years ago. Are you still working on it? 
Yes.

Sign in to add a comment