Replace TimeTicks::UnixEpoch with Time::UnixEpoch for the net log

net/log/net_log_util.cc

 // Copyright 2014 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "net/log/net_log_util.h"
 
 #include <algorithm>
 #include <string>
 #include <utility>
 #include <vector>
@@ skipping 256 matching lines @@
 
     dict->SetInteger("ADDRESS_FAMILY_UNSPECIFIED", ADDRESS_FAMILY_UNSPECIFIED);
     dict->SetInteger("ADDRESS_FAMILY_IPV4", ADDRESS_FAMILY_IPV4);
     dict->SetInteger("ADDRESS_FAMILY_IPV6", ADDRESS_FAMILY_IPV6);
 
     constants_dict->Set("addressFamily", std::move(dict));
   }
 
   // Information about how the "time ticks" values we have given it relate to
   // actual system times.  Time ticks are used throughout since they are stable
-  // across system clock changes.
+  // across system clock changes. Note: |timeTickOffset| is only comparable to
+  // TimeTicks values in milliseconds.
+  // TODO(csharrison): This is an imprecise way to convert TimeTicks to unix
+  // time. In fact, there isn't really a good way to do this unless we log Time
+  // and TimeTicks values side by side for every event. crbug.com/593157 tracks
+  // a change where the user will be notified if a timing anomaly occured that
+  // would skew the conversion (i.e. the machine entered suspend mode while
+  // logging).
   {
+    base::TimeDelta time_since_epoch =
+        base::Time::Now() - base::Time::UnixEpoch();
+    base::TimeDelta reference_time_ticks =
+        base::TimeTicks::Now() - base::TimeTicks();

[mmenke, 2016/03/10 22:19:35]

This seems to completely defeat the purpose of using TimeTicks to get more
accurate times in the first place...No?  If we're going to do this, we should
switch to Time everywhere instead...Or use a constant delta between Time and
TimeTicks, unless there's a sudden jump.

[Charlie Harrison, 2016/03/10 22:22:25]

You have a point, but I assumed that we needed TimeTicks in the netlog
internally to properly order event / make the waterfall, etc. If a timing
anomaly occurred, it would make things extremely hard to reason about if events
started displaying out of order.

My assumption might be wrong though, I don't know the netlog code very well.

[eroman, 2016/03/10 23:18:08]

I disagree that using Time everywhere is equivalent.

One of my concerns with Time is that it is subject to change at any moment, and
fundamentally seems ill-suited for measuring elapsed time deltas (I have similar
concerns on its liberal use throughout Chrome).

A standard configuration these days is for your computer to pull time
information from the network and automatically adjust its system clock.... So we
aren't just talking about manual interventions where the user goes and changes
their system time so they can play candy crush levels, but also small (or large)
corrections to Time that happen during the normal course of operation.

It seems wrong for something measuring network timings to carry a dependency on
the network itself (i.e. NTP).

I don't think this is just an academic concern:  I have a crufy workstation at
home where my CMOS battery is presumably dead. Since each time I power it on the
clock is way off. But after a couple minutes of operation (why so long I don't
know...) it sets itself back to something reasonable.

Unless there is a good reason, I would say stick with TimeTicks.

     int64_t tick_to_unix_time_ms =
-        (base::TimeTicks() - base::TimeTicks::UnixEpoch()).InMilliseconds();
+        (time_since_epoch - reference_time_ticks).InMilliseconds();
 
     // Pass it as a string, since it may be too large to fit in an integer.
     constants_dict->SetString("timeTickOffset",
                               base::Int64ToString(tick_to_unix_time_ms));
   }
 
   // "clientInfo" key is required for some WriteToFileNetLogObserver log
   // readers. Provide a default empty value for compatibility.
   constants_dict->Set("clientInfo", new base::DictionaryValue());
 
@@ skipping 238 matching lines @@
     // fine, since GetRequestStateAsValue() ignores the capture mode.
     NetLog::EntryData entry_data(
         NetLog::TYPE_REQUEST_ALIVE, request->net_log().source(),
         NetLog::PHASE_BEGIN, request->creation_time(), &callback);
     NetLog::Entry entry(&entry_data, NetLogCaptureMode::Default());
     observer->OnAddEntry(entry);
   }
 }
 
 }  // namespace net