Amend sanitization of browser-side navigationStart override.

When setting the navigationStart recorded on the browser side for
browser-initiated navigations, we need to ensure the time is lower than
the time of starting the load on the renderer side. Otherwise with heavy
clock differences between processes and particularly fast navigation
start-ups we could end up pushing the navigationStart *forward* and
hitting an assert that is being deployed in
https://codereview.chromium.org/379873002/ .

BUG=376004

Review URL: https://codereview.chromium.org/384503009

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@283761 0039d316-1c4b-4281-b951-d872f2087c98

2 files changed, 55 insertions, 4 deletions

diff --git a/content/renderer/render_frame_impl.cc b/content/renderer/render_frame_impl.cc
index 8470b30..dba0887 100644
--- a/content/renderer/render_frame_impl.cc
+++ b/content/renderer/render_frame_impl.cc
@@ -893,6 +893,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
@@ -901,11 +904,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(
-          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(
diff --git a/content/renderer/render_view_browsertest.cc b/content/renderer/render_view_browsertest.cc
index 7af38eda..83e282f 100644
--- a/content/renderer/render_view_browsertest.cc
+++ b/content/renderer/render_view_browsertest.cc
@@ -49,6 +49,7 @@
 #include "third_party/WebKit/public/web/WebDeviceEmulationParams.h"
 #include "third_party/WebKit/public/web/WebHistoryItem.h"
 #include "third_party/WebKit/public/web/WebLocalFrame.h"
+#include "third_party/WebKit/public/web/WebPerformance.h"
 #include "third_party/WebKit/public/web/WebRuntimeFeatures.h"
 #include "third_party/WebKit/public/web/WebView.h"
 #include "third_party/WebKit/public/web/WebWindowFeatures.h"
@@ -2389,4 +2390,50 @@ TEST_F(RenderViewImplTest, ScreenMetricsEmulation) {
   // Don't disable here to test that emulation is being shutdown properly.
 }
 
+// Sanity checks for the Navigation Timing API |navigationStart| override. We
+// are asserting only most basic constraints, as TimeTicks (passed as the
+// override) are not comparable with the wall time (returned by the Blink API).
+TEST_F(RenderViewImplTest, NavigationStartOverride) {
+  // Verify that a navigation that claims to have started at the earliest
+  // possible TimeTicks is indeed reported as one that started before
+  // OnNavigate() is called.
+  base::Time before_navigation = base::Time::Now();
+  FrameMsg_Navigate_Params early_nav_params;
+  early_nav_params.url = GURL("data:text/html,<div>Page</div>");
+  early_nav_params.navigation_type = FrameMsg_Navigate_Type::NORMAL;
+  early_nav_params.transition = PAGE_TRANSITION_TYPED;
+  early_nav_params.page_id = -1;
+  early_nav_params.is_post = true;
+  early_nav_params.browser_navigation_start =
+      base::TimeTicks::FromInternalValue(1);
+
+  frame()->OnNavigate(early_nav_params);
+  ProcessPendingMessages();
+
+  base::Time early_nav_reported_start =
+      base::Time::FromDoubleT(GetMainFrame()->performance().navigationStart());
+  EXPECT_LT(early_nav_reported_start, before_navigation);
+
+  // Verify that a navigation that claims to have started in the future - 42
+  // days from now is *not* reported as one that starts in the future; as we
+  // sanitize the override allowing a maximum of ::Now().
+  FrameMsg_Navigate_Params late_nav_params;
+  late_nav_params.url = GURL("data:text/html,<div>Another page</div>");
+  late_nav_params.navigation_type = FrameMsg_Navigate_Type::NORMAL;
+  late_nav_params.transition = PAGE_TRANSITION_TYPED;
+  late_nav_params.page_id = -1;
+  late_nav_params.is_post = true;
+  late_nav_params.browser_navigation_start =
+      base::TimeTicks::Now() + base::TimeDelta::FromDays(42);
+
+  frame()->OnNavigate(late_nav_params);
+  ProcessPendingMessages();
+  base::Time after_navigation =
+      base::Time::Now() + base::TimeDelta::FromDays(1);
+
+  base::Time late_nav_reported_start =
+      base::Time::FromDoubleT(GetMainFrame()->performance().navigationStart());
+  EXPECT_LE(late_nav_reported_start, after_navigation);
+}
+
 }  // namespace content