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

webrtc/base/timeutils.cc

 /*
  *  Copyright 2004 The WebRTC Project Authors. All rights reserved.
  *
  *  Use of this source code is governed by a BSD-style license
  *  that can be found in the LICENSE file in the root of the source
  *  tree. An additional intellectual property rights grant can be found
  *  in the file PATENTS.  All contributing project authors may
  *  be found in the AUTHORS file in the root of the source tree.
  */
 
@@ skipping 14 matching lines @@
 #include <mmsystem.h>
 #endif
 
 #include "webrtc/base/checks.h"
 #include "webrtc/base/timeutils.h"
 
 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() {
   return static_cast<uint32_t>(TimeNanos() / kNumNanosecsPerMillisec);
 }
 
 int64_t Time64() {
   return static_cast<int64_t>(TimeNanos() / kNumNanosecsPerMillisec);
 }
 
@@ skipping 85 matching lines @@
   if (expiry_in_leap_year && month <= 2 - 1) // |month| is zero based.
     day -= 1;
 
   // Combine all variables into seconds from 1970-01-01 00:00 (except |month|
   // which was accumulated into |day| above).
   return (((static_cast<int64_t>
             (year - 1970) * 365 + day) * 24 + hour) * 60 + min) * 60 + sec;
 }
 
 } // namespace rtc