Adding the ability to use a simulated clock for unit tests.

webrtc/base/timeutils.cc

Index: webrtc/base/timeutils.cc
diff --git a/webrtc/base/timeutils.cc b/webrtc/base/timeutils.cc
index de3e6afb28d824ecb9dfc26f1029dd0d0958fd24..f176589638e62ffbd341deabdd8a4c594db0e0f6 100644
--- a/webrtc/base/timeutils.cc
+++ b/webrtc/base/timeutils.cc
@@ -32,50 +32,75 @@ namespace rtc {
 
 const uint32_t HALF = 0x80000000;
 
-uint64_t TimeNanos() {
-  int64_t ticks = 0;
 #if defined(WEBRTC_MAC)
-  static mach_timebase_info_data_t timebase;
-  if (timebase.denom == 0) {
-    // Get the timebase if this is the first time we run.
-    // Recommended by Apple's QA1398.
-    if (mach_timebase_info(&timebase) != KERN_SUCCESS) {
-      RTC_DCHECK(false);
+class MacClock : public ClockInterface {
+  uint64_t TimeNanos() const override {
+    static mach_timebase_info_data_t timebase;
+    if (timebase.denom == 0) {
+      // Get the timebase if this is the first time we run.
+      // Recommended by Apple's QA1398.
+      if (mach_timebase_info(&timebase) != KERN_SUCCESS) {
+        RTC_DCHECK(false);
+      }
     }
+    // Use timebase to convert absolute time tick units into nanoseconds.
+    return mach_absolute_time() * timebase.numer / timebase.denom;
   }
-  // Use timebase to convert absolute time tick units into nanoseconds.
-  ticks = mach_absolute_time() * timebase.numer / timebase.denom;
+};
+const MacClock default_clock_;

[pthatcher1, 2016/05/24 18:05:46]

Is Chrome going to be OK with this?  They don't allow any static initializers. 
I don't know exactly what is OK and what's not, but I know they are more strict
in their check then we are, so you may want to try and compile chrome to see if
it works.

[pthatcher1, 2016/05/24 18:05:46]

I think it would make more sense to do a typedef MacClock Clock inside of each
ifdef.  Then, one could construct a new Clock() without knowing the OS they are
on.

[Taylor Brandstetter, 2016/05/24 21:47:59]

Done.

[Taylor Brandstetter, 2016/05/24 21:48:00]

Good catch. Fixing this would need to use some kind of singleton implementation
which is a lot of overkill for something that was working fine before, and
doesn't really need to be converted to an object since it has no state. I'll
just change it back, since objectify-ing the existing time code is orthogonal to
this CL.

 #elif defined(WEBRTC_POSIX)
-  struct timespec ts;
-  // TODO: Do we need to handle the case when CLOCK_MONOTONIC
-  // is not supported?
-  clock_gettime(CLOCK_MONOTONIC, &ts);
-  ticks = kNumNanosecsPerSec * static_cast<int64_t>(ts.tv_sec) +
-          static_cast<int64_t>(ts.tv_nsec);
+class PosixClock : public ClockInterface {
+  uint64_t TimeNanos() const override {
+    struct timespec ts;
+    // TODO(deadbeef): Do we need to handle the case when CLOCK_MONOTONIC
+    // is not supported?
+    clock_gettime(CLOCK_MONOTONIC, &ts);
+    return kNumNanosecsPerSec * static_cast<int64_t>(ts.tv_sec) +
+           static_cast<int64_t>(ts.tv_nsec);
+  }
+};
+const PosixClock default_clock_;
 #elif defined(WEBRTC_WIN)
-  static volatile LONG last_timegettime = 0;
-  static volatile int64_t num_wrap_timegettime = 0;
-  volatile LONG* last_timegettime_ptr = &last_timegettime;
-  DWORD now = timeGetTime();
-  // Atomically update the last gotten time
-  DWORD old = InterlockedExchange(last_timegettime_ptr, now);
-  if (now < old) {
-    // If now is earlier than old, there may have been a race between
-    // threads.
-    // 0x0fffffff ~3.1 days, the code will not take that long to execute
-    // so it must have been a wrap around.
-    if (old > 0xf0000000 && now < 0x0fffffff) {
-      num_wrap_timegettime++;
+class WindowsClock : public ClockInterface {
+  uint64_t TimeNanos() const override {
+    int64_t ticks = 0;
+    static volatile LONG last_timegettime = 0;
+    static volatile int64_t num_wrap_timegettime = 0;
+    volatile LONG* last_timegettime_ptr = &last_timegettime;
+    DWORD now = timeGetTime();
+    // Atomically update the last gotten time
+    DWORD old = InterlockedExchange(last_timegettime_ptr, now);
+    if (now < old) {
+      // If now is earlier than old, there may have been a race between
+      // threads.
+      // 0x0fffffff ~3.1 days, the code will not take that long to execute
+      // so it must have been a wrap around.
+      if (old > 0xf0000000 && now < 0x0fffffff) {
+        num_wrap_timegettime++;
+      }
     }
+    ticks = now + (num_wrap_timegettime << 32);
+    // TODO(deadbeef): Calculate with nanosecond precision. Otherwise, we're
+    // just wasting a multiply and divide when doing Time() on Windows.
+    ticks = ticks * kNumNanosecsPerMillisec;
+    return ticks;
   }
-  ticks = now + (num_wrap_timegettime << 32);
-  // TODO: Calculate with nanosecond precision.  Otherwise, we're just
-  // wasting a multiply and divide when doing Time() on Windows.
-  ticks = ticks * kNumNanosecsPerMillisec;
+};
+const WindowsClock default_clock_;
+#elif defined(WEBRTC_WIN)
 #else
 #error Unsupported platform.
 #endif
-  return ticks;
+
+const ClockInterface* clock_ = &default_clock_;

[pthatcher1, 2016/05/24 18:05:46]

I've not seen other code where we have the member variable name style "foo_"
used for global constants.  Do we have other places where we do that?

[Taylor Brandstetter, 2016/05/24 21:47:59]

I don't remember why I named it that, but it looks like the proper pattern is
"g_name". Fixed.

+
+void SetClock(ClockInterface* clock) {
+  clock_ = clock ? clock : &default_clock_;

[pthatcher1, 2016/05/24 18:05:46]

Does this take ownership of the clock?  Should it delete it when it's cleared? 
At least make it clear.  

[Taylor Brandstetter, 2016/05/24 21:48:00]

No it doesn't; I made the comment in the header file more clear.

+}
+
+uint64_t TimeNanos() {
+  RTC_CHECK(clock_);
+  return clock_->TimeNanos();

[pthatcher1, 2016/05/24 18:05:46]

I think instead of having a global clock that TimeNanos() calls, would it be
feasible to inject Clock objects into all the places that call TimeNanos()?  

We already do that in several places with the Timing class.

[Taylor Brandstetter, 2016/05/24 21:47:59]

It's feasible, and it would be ideal (since it would mean no global variable,
and people could implement a clock for their own platform and pass it in). But I
think it would be quite the expansive change. Could it be done in a separate CL?

 }
 
 uint32_t Time32() {