Amend sanitization of browser-side navigationStart override.

content/renderer/render_frame_impl.cc

Index: content/renderer/render_frame_impl.cc
diff --git a/content/renderer/render_frame_impl.cc b/content/renderer/render_frame_impl.cc
index af2987fda5b30c540abe6ed6ac8ce081e87930e2..727a194d82359485c37389b72bfd0c3ca8a0077b 100644
--- a/content/renderer/render_frame_impl.cc
+++ b/content/renderer/render_frame_impl.cc
@@ -886,6 +886,9 @@ void RenderFrameImpl::OnNavigate(const FrameMsg_Navigate_Params& params) {
       request.setHTTPBody(http_body);
     }
 
+    // Record this before starting the load, we need a lower bound of this time
+    // to sanitize the navigationStart override set below.
+    base::TimeTicks renderer_navigation_start = base::TimeTicks::Now();
     frame->loadRequest(request);
 
     // The browser provides the navigation_start time to bootstrap the
@@ -894,11 +897,12 @@ void RenderFrameImpl::OnNavigate(const FrameMsg_Navigate_Params& params) {
     // finishing the onbeforeunload handler of the previous page.
     DCHECK(!params.browser_navigation_start.is_null());
     if (frame->provisionalDataSource()) {
-      // browser_navigation_start is likely before this process existed, so we
-      // can't use InterProcessTimeTicksConverter. Instead, the best we can do
-      // is just ensure we don't report a bogus value in the future.
+      // |browser_navigation_start| is likely before this process existed, so we
+      // can't use InterProcessTimeTicksConverter. We need at least to ensure
+      // that the browser-side navigation start we set is not later than the one
+      // on the renderer side.
       base::TimeTicks navigation_start = std::min(

[pasko, 2014/07/17 13:18:00]

This min() could be a reason of bi-modal distribution in navigation metrics,
which makes it quite hard to analyse the metric with statistical methods.

If we record the time upon sending and receiving the IPC in the corresponding
processes as |browser_sent_navigation| and |renderer_received_navigation|, then
we can account for this offset in renderer-side navigation timings.

Proposal:

navigation_start = renderer_received_navigation - (browser_sent_navigation -
params.browser_navigation_start)

This way we will be discounting for the IPC travel time, which is not ideal, but
I like thinking that IPC travel time depends more on how the OS behaves and is
somewhat out of our control. I have no better way in mind for synchronizing time
across processes, but this min() time feels confusing, non-monotonic and
<censored>.

[ppi, 2014/07/17 13:27:43]

This is an excellent idea, also solving the problem of big clock differences
between processes on Windows. Let me file a bug and let's see what the other
people think.

Thanks!!

-          base::TimeTicks::Now(), params.browser_navigation_start);
+          params.browser_navigation_start, renderer_navigation_start);
       double navigation_start_seconds =
           (navigation_start - base::TimeTicks()).InSecondsF();
       frame->provisionalDataSource()->setNavigationStartTime(