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.
+    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);

[clamy, 2016/07/21 11:52:44]

Why were those calls needed?

[Alexander Semashko, 2016/07/21 14:00:22]

Not sure I understood your question correctly, but...
Nothing is limiting the new view's lifetime. The test itself must be its owner,
hence it deletes the view in the end. Note that I just moved those calls to this
new method CloseRenderView.

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