Send navigation_start to the browser in DidStartProvisionalLoad IPC

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 "base/basictypes.h"
 #include "base/bind.h"
 #include "base/callback.h"
 #include "base/location.h"
 #include "base/single_thread_task_runner.h"
 #include "base/strings/string_util.h"
@@ skipping 150 matching lines @@
   }
 
   int view_page_id() {
     return view()->page_id_;
   }
 
   TestRenderFrame* frame() {
     return static_cast<TestRenderFrame*>(view()->GetMainRenderFrame());
   }
 
+  void GoToOffsetWithParams(int offset,
+                            const PageState& state,
+                            const CommonNavigationParams common_params,
+                            const StartNavigationParams start_params,
+                            RequestNavigationParams request_params) {
+    EXPECT_TRUE(common_params.transition & ui::PAGE_TRANSITION_FORWARD_BACK);
+    int history_list_length = view()->historyBackListCount() +
+                              view()->historyForwardListCount() + 1;
+    int pending_offset = offset + view()->history_list_offset_;

[nasko, 2015/11/13 21:52:54]

Hmmm, not sure if asking the view() is the right thing long term, as we are
moving history navigation around. Will add avi@ to take a poke at this one.

[Charlie Harrison, 2015/11/14 00:25:00]

For context, most of this logic is copied from render_view_test's GoToOffset. I
just needed to finely tune the parameters.

+
+    request_params.page_state = state;
+    request_params.page_id = view()->page_id_ + offset;
+    request_params.nav_entry_id = pending_offset + 1;
+    request_params.pending_history_list_offset = pending_offset;
+    request_params.current_history_list_offset = view()->history_list_offset_;
+    request_params.current_history_list_length = history_list_length;
+    frame()->Navigate(common_params, start_params, request_params);
+
+    // The load actually happens asynchronously, so we pump messages to process
+    // the pending continuation.
+    FrameLoadWaiter(frame()).Wait();
+  }
+
   // 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,
     // key-code, and modifiers).
     // Exit when a keyboard-layout driver cannot assign a Unicode character to
     // the tuple to prevent sending an invalid key code to the RenderView
@@ skipping 2116 matching lines @@
                     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,")));
+  ProcessPendingMessages();
+  const IPC::Message* frame_navigate_msg =
+      render_thread_->sink().GetUniqueMessageMatching(
+          FrameHostMsg_DidStartProvisionalLoadForFrame::ID);
+  FrameHostMsg_DidStartProvisionalLoadForFrame::Param host_nav_params;
+  FrameHostMsg_DidStartProvisionalLoadForFrame::Read(frame_navigate_msg,
+                                                     &host_nav_params);
+  base::TimeTicks transmitted_start = base::get<1>(host_nav_params);
+  EXPECT_FALSE(transmitted_start.is_null());
+  EXPECT_LT(lower_bound_navigation_start, transmitted_start);
+}
+
+TEST_F(RenderViewImplTest, BrowserNavigationStartNotUsedForReload) {
+  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_LINK;

[nasko, 2015/11/13 21:52:54]

Why not PAGE_TRANSITION_RELOAD?

[Charlie Harrison, 2015/11/14 00:25:00]

No good reason. Fixed.

+
+  frame()->Navigate(common_params, StartNavigationParams(),
+                    RequestNavigationParams());
+  ProcessPendingMessages();
+
+  const IPC::Message* frame_navigate_msg =
+      render_thread_->sink().GetUniqueMessageMatching(
+          FrameHostMsg_DidStartProvisionalLoadForFrame::ID);
+  FrameHostMsg_DidStartProvisionalLoadForFrame::Param host_nav_params;
+  FrameHostMsg_DidStartProvisionalLoadForFrame::Read(frame_navigate_msg,
+                                                     &host_nav_params);
+  // The true timestamp is later than the browser initiated one.
+  EXPECT_GT(base::get<1>(host_nav_params), common_params.navigation_start);

[nasko, 2015/11/13 21:52:54]

Do we have enough granularity to ensure we are not going to collide if the test
runs too fast? It can lead to flakiness if not. Maybe check if the ticks object
is high resolution or not?
https://code.google.com/p/chromium/codesearch#chromium/src/base/time/time.h&l...

[Charlie Harrison, 2015/11/14 00:25:00]

Good idea. It should be monotonic either way, so I'll make the check GTE if
we're using the low resolution timer, because we could possibly mark the
navigationStart in the renderer within 15ms (assuming comment in time.h is
correct).

+}
+
+TEST_F(RenderViewImplTest, BrowserNavigationStartNotUsedForHistoryNavigation) {
+  LoadHTML("<div id=pagename>Page A</div>");
+  LoadHTML("<div id=pagename>Page B</div>");
+  PageState back_state =
+      HistoryEntryToPageState(view()->history_controller()->GetCurrentEntry());
+  LoadHTML("<div id=pagename>Page C</div>");
+  PageState forward_state =
+      HistoryEntryToPageState(view()->history_controller()->GetCurrentEntry());
+  ProcessPendingMessages();
+  render_thread_->sink().ClearMessages();
+
+  CommonNavigationParams common_params;
+  common_params.transition = ui::PAGE_TRANSITION_FORWARD_BACK;
+  // Go back.
+  GoToOffsetWithParams(-1, back_state, common_params, StartNavigationParams(),
+                       RequestNavigationParams());
+  ProcessPendingMessages();
+  const IPC::Message* frame_navigate_msg =
+      render_thread_->sink().GetUniqueMessageMatching(
+          FrameHostMsg_DidStartProvisionalLoadForFrame::ID);
+  FrameHostMsg_DidStartProvisionalLoadForFrame::Param host_nav_params;
+  FrameHostMsg_DidStartProvisionalLoadForFrame::Read(frame_navigate_msg,
+                                                     &host_nav_params);

[nasko, 2015/11/13 21:52:54]

This code for reading the IPC is done enough times that it might be good to
extract it into a helper method called by all the tests using it.

[Charlie Harrison, 2015/11/14 00:25:00]

Sounds good to me. This might not end up being a win for readability unless we
can templatize it. Uploading a draft now to apply to just my tests. I can
probably make a separate CL later cleaning up other tests.

[nasko, 2015/11/14 00:33:24]

This looks good.

+  EXPECT_GT(base::get<1>(host_nav_params), common_params.navigation_start);
+  render_thread_->sink().ClearMessages();
+
+  // Go forward.
+  GoToOffsetWithParams(1, forward_state, common_params,
+                             StartNavigationParams(),
+                             RequestNavigationParams());
+  ProcessPendingMessages();
+  const IPC::Message* frame_navigate_msg2 =
+      render_thread_->sink().GetUniqueMessageMatching(
+          FrameHostMsg_DidStartProvisionalLoadForFrame::ID);
+  FrameHostMsg_DidStartProvisionalLoadForFrame::Param host_nav_params2;
+  FrameHostMsg_DidStartProvisionalLoadForFrame::Read(frame_navigate_msg2,
+                                                     &host_nav_params2);
+  EXPECT_GT(base::get<1>(host_nav_params2), common_params.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;
+
+  frame()->Navigate(common_params, StartNavigationParams(),
+                    RequestNavigationParams());
+  ProcessPendingMessages();
+
+  const IPC::Message* frame_navigate_msg =
+      render_thread_->sink().GetUniqueMessageMatching(
+          FrameHostMsg_DidStartProvisionalLoadForFrame::ID);
+  FrameHostMsg_DidStartProvisionalLoadForFrame::Param host_nav_params;
+  FrameHostMsg_DidStartProvisionalLoadForFrame::Read(frame_navigate_msg,
+                                                     &host_nav_params);
+  EXPECT_EQ(base::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);
   EnablePreferredSizeMode();
 
   gfx::Size size = GetPreferredSize();
   EXPECT_EQ(gfx::Size(400, 400), size);
 
   SetZoomLevel(ZoomFactorToZoomLevel(2.0));
@@ skipping 57 matching lines @@
       FROM_HERE,
       base::Bind(&DevToolsAgentTest::CloseWhilePaused, base::Unretained(this)));
   ExecuteJavaScriptForTests("debugger;");
 
   // CloseWhilePaused should resume execution and continue here.
   EXPECT_FALSE(IsPaused());
   Detach();
 }
 
 }  // namespace content