Disallow was_within_same_page = true for a cross-process navigation

content/browser/frame_host/navigator_impl_unittest.cc

 // Copyright 2014 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 <stdint.h>
 
 #include "base/macros.h"
 #include "base/time/time.h"
 #include "build/build_config.h"
 #include "content/browser/frame_host/navigation_controller_impl.h"
@@ skipping 1128 matching lines @@
               converted_instance_1->GetSiteURL());
 
     SiteInstance* converted_instance_2 =
         ConvertToSiteInstance(rfhm, descriptor, unrelated_instance.get());
     // Should return |unrelated_instance| because its site matches and it is
     // unrelated to the current SiteInstance.
     EXPECT_EQ(unrelated_instance.get(), converted_instance_2);
   }
 }
 
+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