Set navigationStart correctly for all load types.

content/renderer/render_view_browsertest.cc

Index: content/renderer/render_view_browsertest.cc
diff --git a/content/renderer/render_view_browsertest.cc b/content/renderer/render_view_browsertest.cc
index 57747020cd76fa2e412df893ef676a95cfbfb620..3a54e1dcaa5e6c76153be27017a57c29435191c6 100644
--- a/content/renderer/render_view_browsertest.cc
+++ b/content/renderer/render_view_browsertest.cc
@@ -97,6 +97,7 @@
 
 #include "url/url_constants.h"
 
+using base::TimeDelta;
 using blink::WebFrame;
 using blink::WebFrameContentDumper;
 using blink::WebInputEvent;
@@ -189,6 +190,18 @@ FrameReplicationState ReconstructReplicationStateForTesting(
   return result;
 }
 
+// Returns CommonNavigationParams for a normal navigation to a data: url, with
+// navigation_start set to Now() plus the given offet.
+CommonNavigationParams MakeCommonNavigationParams(
+    TimeDelta navigation_start_offset) {
+  CommonNavigationParams params;
+  params.url = GURL("data:text/html,<div>Page</div>");
+  params.navigation_start = base::TimeTicks::Now() + navigation_start_offset;
+  params.navigation_type = FrameMsg_Navigate_Type::NORMAL;
+  params.transition = ui::PAGE_TRANSITION_TYPED;
+  return params;
+}
+
 }  // namespace
 
 class RenderViewImplTest : public RenderViewTest {
@@ -247,7 +260,9 @@ class RenderViewImplTest : public RenderViewTest {
     const IPC::Message* message =
         render_thread_->sink().GetUniqueMessageMatching(T::ID);
     typename T::Param param;
-    T::Read(message, &param);
+    EXPECT_TRUE(message);
+    if (message)
+      T::Read(message, &param);
     return param;
   }
 
@@ -2039,19 +2054,60 @@ TEST_F(RenderViewImplTest, OnSetAccessibilityMode) {
   ASSERT_TRUE(frame()->render_accessibility());
 }
 
+TEST_F(RenderViewImplTest, RendererNavigationStartTransmittedToBrowser) {
+  base::TimeTicks lower_bound_navigation_start(base::TimeTicks::Now());
+  frame()->GetWebFrame()->loadHTMLString(
+      "hello world", blink::WebURL(GURL("data:text/html,")));
+
+  FrameHostMsg_DidStartProvisionalLoad::Param host_nav_params =
+      ProcessAndReadIPC<FrameHostMsg_DidStartProvisionalLoad>();
+  base::TimeTicks transmitted_start = std::get<1>(host_nav_params);
+  EXPECT_FALSE(transmitted_start.is_null());
+  EXPECT_LT(lower_bound_navigation_start, transmitted_start);
+}
+
+TEST_F(RenderViewImplTest, BrowserNavigationStart) {
+  auto common_params = MakeCommonNavigationParams(-TimeDelta::FromSeconds(1));
+
+  frame()->Navigate(common_params, StartNavigationParams(),
+                    RequestNavigationParams());
+  FrameHostMsg_DidStartProvisionalLoad::Param nav_params =
+      ProcessAndReadIPC<FrameHostMsg_DidStartProvisionalLoad>();
+  EXPECT_EQ(common_params.navigation_start, std::get<1>(nav_params));
+}
+
+TEST_F(RenderViewImplTest, BrowserNavigationStartNonStandard) {
+  // Subframes or unload handlers in initial document do not change the game.
+  ExecuteJavaScriptForTests(
+      "var handler = function() { console.log(42); };"
+      "window.onunload = handler;"
+      "var frame = document.createElement('iframe');"
+      "document.body.appendChild(frame);"
+      "frame.contentWindow.onunload = function() { return null; };");
+  // Need to drop the DidStartProvisionalLoad message from the child frame.
+  ProcessPendingMessages();
+  render_thread_->sink().ClearMessages();
+
+  auto common_params = MakeCommonNavigationParams(-TimeDelta::FromSeconds(1));
+  // The load type will be ReplaceCurrentItem instead of Standard, but it should
+  // not matter.
+  common_params.should_replace_current_entry = true;
+
+  frame()->Navigate(common_params, StartNavigationParams(),
+                    RequestNavigationParams());
+  FrameHostMsg_DidStartProvisionalLoad::Param nav_params =
+      ProcessAndReadIPC<FrameHostMsg_DidStartProvisionalLoad>();
+  EXPECT_EQ(common_params.navigation_start, std::get<1>(nav_params));
+}
+
 // Sanity check 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) {
+TEST_F(RenderViewImplTest, BrowserNavigationStartSanitized) {
   // 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().
-  CommonNavigationParams late_common_params;
-  late_common_params.url = GURL("data:text/html,<div>Another page</div>");
-  late_common_params.navigation_type = FrameMsg_Navigate_Type::NORMAL;
-  late_common_params.transition = ui::PAGE_TRANSITION_TYPED;
-  late_common_params.navigation_start =
-      base::TimeTicks::Now() + base::TimeDelta::FromDays(42);
+  auto late_common_params = MakeCommonNavigationParams(TimeDelta::FromDays(42));
   late_common_params.method = "POST";
 
   frame()->Navigate(late_common_params, StartNavigationParams(),
@@ -2065,20 +2121,39 @@ TEST_F(RenderViewImplTest, NavigationStartOverride) {
   EXPECT_LE(late_nav_reported_start, after_navigation);
 }
 
-TEST_F(RenderViewImplTest, RendererNavigationStartTransmittedToBrowser) {
-  base::TimeTicks lower_bound_navigation_start;

[Alexander Semashko, 2016/06/29 10:11:56]

Looks like it had to be initialized to Now(), right?

-  frame()->GetWebFrame()->loadHTMLString(
-      "hello world", blink::WebURL(GURL("data:text/html,")));
+TEST_F(RenderViewImplTest, FiringBeforeUnloadDiscardsBrowserNavigationStart) {
+  // Add a beforeunload handler in the initial document.
+  ExecuteJavaScriptForTests(
+      "window.onbeforeunload = function() { return null; };");
+
+  auto common_params = MakeCommonNavigationParams(-TimeDelta::FromSeconds(1));
+
+  frame()->Navigate(common_params, StartNavigationParams(),
+                    RequestNavigationParams());
+
+  FrameHostMsg_DidStartProvisionalLoad::Param host_nav_params =
+      ProcessAndReadIPC<FrameHostMsg_DidStartProvisionalLoad>();
+  EXPECT_GT(std::get<1>(host_nav_params), common_params.navigation_start);
+}
+
+TEST_F(RenderViewImplTest,
+       FiringBeforeUnloadInSubFrameDiscardsBrowserNavigationStart) {
+  // Add a beforeunload handler in the initial document.
+  ExecuteJavaScriptForTests(
+      "var frame = document.createElement('iframe');"
+      "document.body.appendChild(frame);"
+      "frame.contentWindow.onbeforeunload = function() { return null; };");
+  // Need to drop the DidStartProvisionalLoad message from the child frame.
   ProcessPendingMessages();
-  const IPC::Message* frame_navigate_msg =
-      render_thread_->sink().GetUniqueMessageMatching(
-          FrameHostMsg_DidStartProvisionalLoad::ID);
-  FrameHostMsg_DidStartProvisionalLoad::Param host_nav_params;
-  FrameHostMsg_DidStartProvisionalLoad::Read(frame_navigate_msg,
-                                                     &host_nav_params);
-  base::TimeTicks transmitted_start = std::get<1>(host_nav_params);
-  EXPECT_FALSE(transmitted_start.is_null());
-  EXPECT_LT(lower_bound_navigation_start, transmitted_start);
+  render_thread_->sink().ClearMessages();
+
+  auto common_params = MakeCommonNavigationParams(-TimeDelta::FromSeconds(1));
+  frame()->Navigate(common_params, StartNavigationParams(),
+                    RequestNavigationParams());
+
+  FrameHostMsg_DidStartProvisionalLoad::Param host_nav_params =
+      ProcessAndReadIPC<FrameHostMsg_DidStartProvisionalLoad>();
+  EXPECT_GT(std::get<1>(host_nav_params), common_params.navigation_start);
 }
 
 TEST_F(RenderViewImplTest, BrowserNavigationStartNotUsedForReload) {
@@ -2094,17 +2169,19 @@ TEST_F(RenderViewImplTest, BrowserNavigationStartNotUsedForReload) {
       FrameMsg_Navigate_Type::RELOAD_ORIGINAL_REQUEST_URL;
   common_params.transition = ui::PAGE_TRANSITION_RELOAD;
 
+  // The browser navigation_start should not be used because beforeunload will
+  // be fired during Navigate.
   frame()->Navigate(common_params, StartNavigationParams(),
                     RequestNavigationParams());
 
   FrameHostMsg_DidStartProvisionalLoad::Param host_nav_params =
       ProcessAndReadIPC<FrameHostMsg_DidStartProvisionalLoad>();
-  // The true timestamp is later than the browser initiated one.
   EXPECT_PRED2(TimeTicksGT, std::get<1>(host_nav_params),
                common_params.navigation_start);
 }
 
-TEST_F(RenderViewImplTest, BrowserNavigationStartNotUsedForHistoryNavigation) {
+TEST_F(RenderViewImplTest,
+       BrowserNavigationStartNotUsedForSameProcessHistoryNavigation) {
   LoadHTML("<div id=pagename>Page A</div>");
   LoadHTML("<div id=pagename>Page B</div>");
   PageState back_state = GetCurrentPageState();
@@ -2114,6 +2191,8 @@ TEST_F(RenderViewImplTest, BrowserNavigationStartNotUsedForHistoryNavigation) {
   render_thread_->sink().ClearMessages();
 
   // Go back.
+  // The browser navigation_start should not be used because beforeunload will
+  // be fired during GoToOffsetWithParams.
   CommonNavigationParams common_params_back;
   common_params_back.url =
       GURL("data:text/html;charset=utf-8,<div id=pagename>Page B</div>");
@@ -2127,6 +2206,8 @@ TEST_F(RenderViewImplTest, BrowserNavigationStartNotUsedForHistoryNavigation) {
   render_thread_->sink().ClearMessages();
 
   // Go forward.
+  // The browser navigation_start should not be used because beforeunload will
+  // be fired during GoToOffsetWithParams.
   CommonNavigationParams common_params_forward;
   common_params_forward.url =
       GURL("data:text/html;charset=utf-8,<div id=pagename>Page C</div>");
@@ -2139,14 +2220,20 @@ TEST_F(RenderViewImplTest, BrowserNavigationStartNotUsedForHistoryNavigation) {
                common_params_forward.navigation_start);
 }
 
-TEST_F(RenderViewImplTest, BrowserNavigationStartSuccessfullyTransmitted) {
-  CommonNavigationParams common_params;
-  common_params.url = GURL("data:text/html,<div>Page</div>");
-  common_params.navigation_type = FrameMsg_Navigate_Type::NORMAL;
-  common_params.transition = ui::PAGE_TRANSITION_TYPED;
+TEST_F(RenderViewImplTest,
+       BrowserNavigationStartUsedForCrossProcessHistoryNavigation) {
+  auto common_params = MakeCommonNavigationParams(-TimeDelta::FromSeconds(1));
+  common_params.transition = ui::PAGE_TRANSITION_FORWARD_BACK;
 
-  frame()->Navigate(common_params, StartNavigationParams(),
-                    RequestNavigationParams());
+  RequestNavigationParams request_params;
+  request_params.page_state =
+      PageState::CreateForTesting(common_params.url, false, nullptr, nullptr);
+  request_params.page_id = 1;
+  request_params.nav_entry_id = 42;
+  request_params.pending_history_list_offset = 1;
+  request_params.current_history_list_offset = 0;
+  request_params.current_history_list_length = 1;
+  frame()->Navigate(common_params, StartNavigationParams(), request_params);
 
   FrameHostMsg_DidStartProvisionalLoad::Param host_nav_params =
       ProcessAndReadIPC<FrameHostMsg_DidStartProvisionalLoad>();