Disallow was_within_same_page = true for a cross-process navigation

content/browser/frame_host/navigator_impl_unittest.cc

Index: content/browser/frame_host/navigator_impl_unittest.cc
diff --git a/content/browser/frame_host/navigator_impl_unittest.cc b/content/browser/frame_host/navigator_impl_unittest.cc
index efb85766a817bd414a0de3a5063f90886c805664..223ac973f137d4e47726a7f2c200d4830f5b0016 100644
--- a/content/browser/frame_host/navigator_impl_unittest.cc
+++ b/content/browser/frame_host/navigator_impl_unittest.cc
@@ -1146,4 +1146,42 @@ TEST_F(NavigatorTestWithBrowserSideNavigation,
   }
 }
 
+namespace {
+void SetWithinPage(const GURL& url,
+                   FrameHostMsg_DidCommitProvisionalLoad_Params* params) {
+  params->was_within_same_page = true;
+  params->url = url;
+}
+}
+
+// A renderer might try and claim that a cross site navigation was within
+// the same page by setting was_within_same_page = true for
+// FrameHostMsg_DidCommitProvisionalLoad. Such case should be detected
+// on the browser side and the renderer should be killed.

[nasko, 2016/03/01 00:27:01]

nit: s/renderer/renderer process/ in first and last line of the comment.

[gzobqq, 2016/03/01 08:31:42]

Done.

+TEST_F(NavigatorTestWithBrowserSideNavigation, CrossSiteClaimWithinPage) {
+  const GURL kUrl1("http://www.chromium.org/");
+  const GURL kUrl2("http://www.google.com/");
+
+  contents()->NavigateAndCommit(kUrl1);
+  FrameTreeNode* node = main_test_rfh()->frame_tree_node();
+
+  // Navigate to a different site.
+  int entry_id = RequestNavigation(node, kUrl2);
+  NavigationRequest* main_request = node->navigation_request();
+  TestRenderFrameHost* speculative_rfh = GetSpeculativeRenderFrameHost(node);
+
+  // Receive the beforeUnload ACK.
+  main_test_rfh()->SendBeforeUnloadACK(true);

[nasko, 2016/03/01 00:27:01]

Why not call PrepareForCommit? It should abstract all the details of
beforeunload ack and possibly other parts of the state machine.

[gzobqq, 2016/03/01 08:31:42]

That's nice, done.

+
+  scoped_refptr<ResourceResponse> response(new ResourceResponse);
+  GetLoaderForNavigationRequest(main_request)
+      ->CallOnResponseStarted(response, MakeEmptyStream());
+
+  // Claim that the navigation was within same page.
+  int bad_msg_count = process()->bad_msg_count();

[nasko, 2016/03/01 00:27:01]

Is process() the right one to use? Shouldn't the bad message be associated with
the speculative_rfh, which is in a different process?

[gzobqq, 2016/03/01 08:31:42]

process() should be good, it has logic to select the speculative RFH if one
exists.

+  speculative_rfh->SendNavigateWithModificationCallback(
+      0, entry_id, true, kUrl2, base::Bind(SetWithinPage, kUrl1));
+  EXPECT_EQ(process()->bad_msg_count(), bad_msg_count + 1);
+}
+
 }  // namespace content