Set navigationStart correctly for all load types.

content/renderer/render_view_browsertest.cc

 // Copyright (c) 2012 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 <stddef.h>
 #include <stdint.h>
 #include <tuple>
 
 #include "base/bind.h"
 #include "base/callback.h"
@@ skipping 40 matching lines @@
 #include "content/renderer/gpu/render_widget_compositor.h"
 #include "content/renderer/history_controller.h"
 #include "content/renderer/history_serialization.h"
 #include "content/renderer/navigation_state_impl.h"
 #include "content/renderer/render_frame_proxy.h"
 #include "content/renderer/render_process.h"
 #include "content/renderer/render_view_impl.h"
 #include "content/shell/browser/shell.h"
 #include "content/shell/browser/shell_browser_context.h"
 #include "content/test/mock_keyboard.h"
+#include "content/test/mock_weburlloader.h"
 #include "content/test/test_render_frame.h"
 #include "net/base/net_errors.h"
 #include "net/cert/cert_status_flags.h"
 #include "testing/gtest/include/gtest/gtest.h"
 #include "third_party/WebKit/public/platform/WebData.h"
 #include "third_party/WebKit/public/platform/WebHTTPBody.h"
 #include "third_party/WebKit/public/platform/WebString.h"
 #include "third_party/WebKit/public/platform/WebURLResponse.h"
 #include "third_party/WebKit/public/web/WebDataSource.h"
 #include "third_party/WebKit/public/web/WebDeviceEmulationParams.h"
@@ skipping 19 matching lines @@
 #include "ui/events/test/events_test_utils.h"
 #include "ui/events/test/events_test_utils_x11.h"
 #endif
 
 #if defined(USE_OZONE)
 #include "ui/events/keycodes/keyboard_code_conversion.h"
 #endif
 
 #include "url/url_constants.h"
 
+using base::TimeDelta;
 using blink::WebFrame;
 using blink::WebFrameContentDumper;
 using blink::WebInputEvent;
 using blink::WebLocalFrame;
 using blink::WebMouseEvent;
 using blink::WebRuntimeFeatures;
 using blink::WebString;
 using blink::WebTextDirection;
 using blink::WebURLError;
 
@@ skipping 72 matching lines @@
   result.unique_name =
       base::UTF16ToUTF8(base::StringPiece16(frame->uniqueName()));
   result.sandbox_flags = frame->effectiveSandboxFlags();
   // result.should_enforce_strict_mixed_content_checking is calculated in the
   // browser...
   result.origin = frame->getSecurityOrigin();
 
   return result;
 }
 
+// Returns CommonNavigationParams for a normal navigation to a data: url, with
+// navigation_start set to Now() plus the given offset.
+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;
+}
+
+blink::WebURLLoader* CreateMockURLLoader() {
+  return new testing::NiceMock<MockWebURLLoader>();
+}
+
 }  // namespace
 
 class RenderViewImplTest : public RenderViewTest {
  public:
   RenderViewImplTest() {
     // Attach a pseudo keyboard device to this object.
     mock_keyboard_.reset(new MockKeyboard());
   }
 
   ~RenderViewImplTest() override {}
@@ skipping 38 matching lines @@
     // the pending continuation.
     FrameLoadWaiter(frame()).Wait();
   }
 
   template<class T>
   typename T::Param ProcessAndReadIPC() {
     ProcessPendingMessages();
     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;
   }
 
   // Sends IPC messages that emulates a key-press event.
   int SendKeyEvent(MockKeyboard::Layout layout,
                    int key_code,
                    MockKeyboard::Modifiers modifiers,
                    base::string16* output) {
 #if defined(OS_WIN)
     // Retrieve the Unicode character for the given tuple (keyboard-layout,
@@ skipping 108 matching lines @@
   const gfx::Size& GetPreferredSize() {
     view()->CheckPreferredSize();
     return view()->preferred_size_;
   }
 
   void SetZoomLevel(double level) {
     view()->OnSetZoomLevel(
         PageMsg_SetZoomLevel_Command::USE_CURRENT_TEMPORARY_MODE, level);
   }
 
+  // Closes a view created during the test, i.e. not the |view()|. Checks that
+  // the main frame is detached and deleted, and makes sure the view does not
+  // leak.
+  void CloseRenderView(RenderViewImpl* new_view) {
+    new_view->Close();
+    EXPECT_FALSE(new_view->GetMainRenderFrame());
+    // Clean up so we don't leak it.

[clamy, 2016/08/17 13:03:55]

nit: blank line above.
nit: avoid using we in comments.

[Alexander Semashko, 2016/08/17 17:10:25]

Ok. I just removed the comment since it duplicates the comment above the
function.

+    new_view->Release();
+  }
+
  private:
   std::unique_ptr<MockKeyboard> mock_keyboard_;
 };
 
 class DevToolsAgentTest : public RenderViewImplTest {
  public:
   void Attach() {
     notifications_ = std::vector<std::string>();
     expecting_pause_ = false;
     std::string host_id = "host_id";
@@ skipping 143 matching lines @@
 // after closing it.
 TEST_F(RenderViewImplTest, RenderFrameClearedAfterClose) {
   // Create a new main frame RenderFrame so that we don't interfere with the
   // shutdown of frame() in RenderViewTest.TearDown.
   blink::WebURLRequest popup_request(GURL("http://foo.com"));
   blink::WebView* new_web_view = view()->createView(
       GetMainFrame(), popup_request, blink::WebWindowFeatures(), "foo",
       blink::WebNavigationPolicyNewForegroundTab, false);
   RenderViewImpl* new_view = RenderViewImpl::FromWebView(new_web_view);
 
-  // Close the view, causing the main RenderFrame to be detached and deleted.
-  new_view->Close();
-  EXPECT_FALSE(new_view->GetMainRenderFrame());
-
-  // Clean up after the new view so we don't leak it.
-  new_view->Release();
+  // Checks that the frame is deleted properly and cleans up the view.
+  CloseRenderView(new_view);
 }
 
 // Test that we get form state change notifications when input fields change.
 TEST_F(RenderViewImplTest, OnNavStateChanged) {
   view()->set_send_content_state_immediately(true);
   LoadHTML("<input type=\"text\" id=\"elt_text\"></input>");
 
   // We should NOT have gotten a form state change notification yet.
   EXPECT_FALSE(render_thread_->sink().GetFirstMessageMatching(
       FrameHostMsg_UpdateState::ID));
@@ skipping 256 matching lines @@
       GetMainFrame(), popup_request, blink::WebWindowFeatures(), "foo",
       blink::WebNavigationPolicyNewForegroundTab, false);
   RenderViewImpl* new_view = RenderViewImpl::FromWebView(new_web_view);
   blink::WebFrameClient::NavigationPolicyInfo popup_policy_info(popup_request);
   popup_policy_info.navigationType = blink::WebNavigationTypeLinkClicked;
   popup_policy_info.defaultPolicy = blink::WebNavigationPolicyNewForegroundTab;
   policy = static_cast<RenderFrameImpl*>(new_view->GetMainRenderFrame())->
       decidePolicyForNavigation(popup_policy_info);
   EXPECT_EQ(blink::WebNavigationPolicyIgnore, policy);
 
-  // Clean up after the new view so we don't leak it.
-  new_view->Close();
-  new_view->Release();
+  CloseRenderView(new_view);
 }
 
 // Verify that security origins are replicated properly to RenderFrameProxies
 // when swapping out.
 TEST_F(RenderViewImplTest, OriginReplicationForSwapOut) {
   // This test should only run with --site-per-process, since origin
   // replication only happens in that mode.
   if (!AreAllSitesIsolatedForTesting())
     return;
 
@@ skipping 107 matching lines @@
   // Respond to a swap out request.
   TestRenderFrame* new_main_frame =
       static_cast<TestRenderFrame*>(new_view->GetMainRenderFrame());
   new_main_frame->SwapOut(
       kProxyRoutingId, true,
       ReconstructReplicationStateForTesting(new_main_frame));
 
   // Simulate getting painted after swapping out.
   new_view->DidFlushPaint();
 
-  new_view->Close();
-  new_view->Release();
+  CloseRenderView(new_view);
 }
 
 // Verify that the renderer process doesn't crash when device scale factor
 // changes after a cross-process navigation has commited.
 // See https://crbug.com/571603.
 TEST_F(RenderViewImplTest, SetZoomLevelAfterCrossProcessNavigation) {
   // This test should only run with out-of-process iframes enabled.
   if (!SiteIsolationPolicy::AreCrossProcessFramesPossible())
     return;
 
@@ skipping 1050 matching lines @@
 
   frame()->SetAccessibilityMode(AccessibilityModeOff);
   ASSERT_EQ(AccessibilityModeOff, frame()->accessibility_mode());
   ASSERT_FALSE(frame()->render_accessibility());
 
   frame()->SetAccessibilityMode(AccessibilityModeComplete);
   ASSERT_EQ(AccessibilityModeComplete, frame()->accessibility_mode());
   ASSERT_TRUE(frame()->render_accessibility());
 }
 
+TEST_F(RenderViewImplTest, RendererNavigationStartTransmittedToBrowser) {

[clamy, 2016/08/17 13:03:55]

Could you add comments explaining what the test does? It's not clear to me what
it is doing there.

[Alexander Semashko, 2016/08/17 17:10:25]

Done.

+  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) {

[clamy, 2016/08/17 13:03:55]

Isn't this test testing that the browser navigation start is used if the initial
document was not accessed? If so, can we remove
BrowserNavigationStartUsedIfInitialDocumentWasNotAccessed, which looks a lot
more complicated?

If this the case, can we also make a similar test for initial document access
where we just access the document of the current frame?

[Alexander Semashko, 2016/08/17 17:10:25]

Assuming that these tests will always start with an untouched initial doc, it's
fine to remove. Done.
This means relying on that test-only methods for executing script will always
trigger didAccessInitialDocument, and I'm somewhat wary about this. Currently
this test mimics the real-world cases closer. Are you sure it's ok to simplify
the test?

[clamy, 2016/08/24 23:23:09]

Well it was working when you just added a subframe to the testing frame. Also
your current version relies on maintaining testing code (for routing ids) that
is not used outside of that test.

[Alexander Semashko, 2016/08/25 10:44:33]

Yeah, it is working currently. To elaborate more on my concern: we can replace
the current test with a simpler one, but also, I think, less reliable; if the
simplified version breaks in some future moment, there is a high chance that the
test will be disabled/deleted instead of digging and reinventing a complex setup
that makes it working.
Well, I just added setters for values that are implicitly used in many tests.
IMO it is a tiny bit that makes the API more complete and it's better to keep
these setters in any case.

To be clear, these are not strong objections in any way, just considerations;
I'll change the CL whatever way you think suitable.
Just waiting for your decision.

[clamy, 2016/08/25 18:00:14]

Your test is on the commit queue, which means it can only breaks if someone
explicitly disables it in order to land their patch. It is against Chromium
policy to do so: we only disable tests if they are flaky (then we file bugs to
unflake them) or if what they are testing is no longer relevant. So as long as
we want our code to follow the behavior this test is testing, it should remain
unbroken. I do think that the simpler version is easier to understand, an
requires less changes to the testing code. So I would prefer for you to use this
version.

[Alexander Semashko, 2016/08/25 21:32:10]

I meant major refactorings, where it sometimes is hard to tell whether a test is
still relevant, and some mistakes just slip through... Nevermind.

Ok, this test is now as simple as it can be.
Note that the part with routing ids is still in the CL (since you did not say
anything about it).

[clamy, 2016/08/25 22:24:29]

If the routing id part is not used in the tests, then it should be removed from
the CL.

[Alexander Semashko, 2016/08/25 22:50:19]

Done.

+  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));
+}
+
 // 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(),
                     RequestNavigationParams());
   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);
 }
 
-TEST_F(RenderViewImplTest, RendererNavigationStartTransmittedToBrowser) {
-  base::TimeTicks lower_bound_navigation_start;
-  frame()->GetWebFrame()->loadHTMLString(
-      "hello world", blink::WebURL(GURL("data:text/html,")));
+// Checks that a browser-initiated navigation in an initial document that was
+// not accessed uses browser-side timestamp.
+TEST_F(RenderViewImplTest,
+       BrowserNavigationStartUsedIfInitialDocumentWasNotAccessed) {
+  blink_platform_impl_.SetWebURLLoaderFactory(base::Bind(&CreateMockURLLoader));
+  // Open a page that will be navigated later.
+  ExecuteJavaScriptForTests("window.open('https://google.com/');");
+  // The new page starts a provisional load (it will not commit).
   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* new_main_frame =
+      static_cast<TestRenderFrame*>(RenderFrame::FromRoutingID(
+          render_thread_->new_window_main_frame_routing_id()));
+  ASSERT_TRUE(new_main_frame);
+
+  auto common_params = MakeCommonNavigationParams(-TimeDelta::FromSeconds(1));
+  new_main_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));
+
+  CloseRenderView(
+      RenderViewImpl::FromRoutingID(render_thread_->new_window_routing_id()));
+}
+
+// Checks that a browser-initiated navigation in an initial document that has
+// been accessed does not use browser-side timestamp (there may be arbitrary
+// content and/or scripts injected, including beforeunload handler that shows
+// a confirmation dialog).
+TEST_F(RenderViewImplTest,
+       BrowserNavigationStartDiscardedIfInitialDocumentWasAccessed) {
+  blink_platform_impl_.SetWebURLLoaderFactory(base::Bind(&CreateMockURLLoader));
+  // Open a page that will be navigated later.
+  ExecuteJavaScriptForTests(
+      "var w = window.open('https://google.com/');"
+      "w.document.body.appendChild(w.document.createElement('div'));");
+  // The new page starts a provisional load (it will not commit).
+  ProcessPendingMessages();
+  render_thread_->sink().ClearMessages();
+
+  auto* new_main_frame =
+      static_cast<TestRenderFrame*>(RenderFrame::FromRoutingID(
+          render_thread_->new_window_main_frame_routing_id()));
+  ASSERT_TRUE(new_main_frame);
+
+  auto common_params = MakeCommonNavigationParams(-TimeDelta::FromSeconds(1));
+  new_main_frame->Navigate(common_params, StartNavigationParams(),
+                           RequestNavigationParams());
+
+  FrameHostMsg_DidStartProvisionalLoad::Param nav_params =
+      ProcessAndReadIPC<FrameHostMsg_DidStartProvisionalLoad>();
+  EXPECT_GT(std::get<1>(nav_params), common_params.navigation_start);
+
+  CloseRenderView(
+      RenderViewImpl::FromRoutingID(render_thread_->new_window_routing_id()));
+}
+
+TEST_F(RenderViewImplTest,
+       FiringBeforeUnloadInSubFrameDiscardsBrowserNavigationStart) {

[clamy, 2016/08/17 13:03:55]

This doesn't seem correct to me: based on how the code is written, if we haven't
yet navigated anywhere we won't fire the BeforeUnload event unless the document
was accessed. So this similar to testing whether the initial document was
accessed or not.

[Alexander Semashko, 2016/08/17 17:10:25]

Well, now the test name does not reflect what is happening, I just forgot to
remove it.
It is passing because ExecuteJavaScriptForTests triggers
didAccessInitialDocument in the main frame.

+  // 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();
+  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) {

[clamy, 2016/08/17 13:03:55]

Can you rename it NavigationStartForReload?

[Alexander Semashko, 2016/08/17 17:10:25]

Done.

   const char url_string[] = "data:text/html,<div>Page</div>";
   // Navigate once, then reload.
   LoadHTML(url_string);
   ProcessPendingMessages();
   render_thread_->sink().ClearMessages();
 
   CommonNavigationParams common_params;
   common_params.url = GURL(url_string);
   common_params.navigation_type =
       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);

[clamy, 2016/08/17 13:03:55]

Can you do the following:
if (!IsBrowserSideNavigationEnabled()) {
  // The browser navigation_start should not be used because beforeunload   will
be fired during Navigate.
  EXPECT_PRED2(...
} else {
  // PlzNavigate: the browser navigation_start is always used.
  EXPECT_EQ(common_params.navigation_start, std::get<1>(nav_params));
}

Same in the history test.

[Alexander Semashko, 2016/08/17 17:10:25]

Done.

 }
 
-TEST_F(RenderViewImplTest, BrowserNavigationStartNotUsedForHistoryNavigation) {
+TEST_F(RenderViewImplTest,
+       BrowserNavigationStartNotUsedForSameProcessHistoryNavigation) {

[clamy, 2016/08/17 13:03:55]

Please rename it NavigationStartForSameProcessHistoryNavigation.

[Alexander Semashko, 2016/08/17 17:10:25]

Done. Also renamed the cross-process counterpart and
BrowserNavigationStartDiscardedIfInitialDocumentWasAccessed.

   LoadHTML("<div id=pagename>Page A</div>");
   LoadHTML("<div id=pagename>Page B</div>");
   PageState back_state = GetCurrentPageState();
   LoadHTML("<div id=pagename>Page C</div>");
   PageState forward_state = GetCurrentPageState();
   ProcessPendingMessages();
   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>");
   common_params_back.transition = ui::PAGE_TRANSITION_FORWARD_BACK;
   GoToOffsetWithParams(-1, back_state, common_params_back,
                        StartNavigationParams(), RequestNavigationParams());
   FrameHostMsg_DidStartProvisionalLoad::Param host_nav_params =
       ProcessAndReadIPC<FrameHostMsg_DidStartProvisionalLoad>();
   EXPECT_PRED2(TimeTicksGT, std::get<1>(host_nav_params),
                common_params_back.navigation_start);
   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>");
   common_params_forward.transition = ui::PAGE_TRANSITION_FORWARD_BACK;
   GoToOffsetWithParams(1, forward_state, common_params_forward,
                        StartNavigationParams(), RequestNavigationParams());
   FrameHostMsg_DidStartProvisionalLoad::Param host_nav_params2 =
       ProcessAndReadIPC<FrameHostMsg_DidStartProvisionalLoad>();
   EXPECT_PRED2(TimeTicksGT, std::get<1>(host_nav_params2),
                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>();
   EXPECT_EQ(std::get<1>(host_nav_params), common_params.navigation_start);
 }
 
 TEST_F(RenderViewImplTest, PreferredSizeZoomed) {
   LoadHTML("<body style='margin:0;'><div style='display:inline-block; "
            "width:400px; height:400px;'/></body>");
   view()->webview()->mainFrame()->setCanHaveScrollbars(false);
@@ skipping 336 matching lines @@
   ExpectPauseAndResume(3);
   blink::WebScriptSource source2(
       WebString::fromUTF8("function func2() { func1(); }; func2();"));
   frame()->GetWebFrame()->executeScriptInIsolatedWorld(17, &source2, 1, 1);
 
   EXPECT_FALSE(IsPaused());
   Detach();
 }
 
 }  // namespace content