PlzNavigate: improve tracking of improper subframe swap-out.

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 "base/command_line.h"
 #include "base/macros.h"
 #include "base/time/time.h"
 #include "content/browser/frame_host/navigation_controller_impl.h"
 #include "content/browser/frame_host/navigation_entry_impl.h"
 #include "content/browser/frame_host/navigation_request.h"
@@ skipping 256 matching lines @@
 }
 
 // PlzNavigate: Test that a proper NavigationRequest is created by
 // RequestNavigation.
 TEST_F(NavigatorTestWithBrowserSideNavigation, BeginNavigation) {
   const GURL kUrl1("http://www.google.com/");
   const GURL kUrl2("http://www.chromium.org/");
   const GURL kUrl3("http://www.gmail.com/");
 
   contents()->NavigateAndCommit(kUrl1);
+  process()->sink().ClearMessages();
 
   // Add a subframe.
   FrameTreeNode* root_node = contents()->GetFrameTree()->root();
+  NavigatorImpl* navigator =
+      static_cast<NavigatorImpl*>(root_node->navigator());
   TestRenderFrameHost* subframe_rfh = main_test_rfh()->AppendChild("Child");
   ASSERT_TRUE(subframe_rfh);
 
   // Start a navigation at the subframe.
   FrameTreeNode* subframe_node = subframe_rfh->frame_tree_node();
   RequestNavigation(subframe_node, kUrl2);
   NavigationRequest* subframe_request =
-      GetNavigationRequestForFrameTreeNode(subframe_node);
+      navigator->GetNavigationRequestForNodeForTesting(subframe_node);
   TestNavigationURLLoader* subframe_loader =
       GetLoaderForNavigationRequest(subframe_request);
 
   // Subframe navigations should start right away as they don't have to request
   // beforeUnload to run at the renderer.
   ASSERT_TRUE(subframe_request);
   ASSERT_TRUE(subframe_loader);
   EXPECT_EQ(NavigationRequest::STARTED, subframe_request->state());
   EXPECT_EQ(kUrl2, subframe_request->common_params().url);
   EXPECT_EQ(kUrl2, subframe_loader->request_info()->common_params.url);
@@ skipping 10 matching lines @@
   if (base::CommandLine::ForCurrentProcess()->HasSwitch(
           switches::kSitePerProcess)) {
     EXPECT_TRUE(GetSpeculativeRenderFrameHost(subframe_node));
   } else {
     EXPECT_FALSE(GetSpeculativeRenderFrameHost(subframe_node));
   }
 
   // Now start a navigation at the root node.
   RequestNavigation(root_node, kUrl3);
   NavigationRequest* main_request =
-      GetNavigationRequestForFrameTreeNode(root_node);
+      navigator->GetNavigationRequestForNodeForTesting(root_node);
   ASSERT_TRUE(main_request);
   EXPECT_EQ(NavigationRequest::WAITING_FOR_RENDERER_RESPONSE,
             main_request->state());
   EXPECT_FALSE(GetSpeculativeRenderFrameHost(root_node));
 
   // Simulate a BeforeUnloadACK IPC on the main frame.
   main_test_rfh()->SendBeforeUnloadACK(true);
   TestNavigationURLLoader* main_loader =
       GetLoaderForNavigationRequest(main_request);
   EXPECT_EQ(kUrl3, main_request->common_params().url);
   EXPECT_EQ(kUrl3, main_loader->request_info()->common_params.url);
   EXPECT_EQ(kUrl3, main_loader->request_info()->first_party_for_cookies);
   EXPECT_TRUE(main_loader->request_info()->is_main_frame);
   EXPECT_FALSE(main_loader->request_info()->parent_is_main_frame);
   EXPECT_TRUE(main_request->browser_initiated());
   // BeforeUnloadACK was received from the renderer so the navigation should
   // have started.
   EXPECT_EQ(NavigationRequest::STARTED, main_request->state());
 
   // Main frame navigation to a different site should use a speculative
   // RenderFrameHost.
   EXPECT_TRUE(GetSpeculativeRenderFrameHost(root_node));
 
   // As the main frame hasn't yet committed the subframe still exists. Thus, the
   // above situation regarding subframe navigations is valid here.
+  ASSERT_TRUE(navigator->GetNavigationRequestForNodeForTesting(subframe_node));
   if (base::CommandLine::ForCurrentProcess()->HasSwitch(
           switches::kSitePerProcess)) {
     EXPECT_TRUE(GetSpeculativeRenderFrameHost(subframe_node));
   } else {
     EXPECT_FALSE(GetSpeculativeRenderFrameHost(subframe_node));
   }
+
+  // Have the root node's speculative RenderFrameHost commit the navigation.
+  scoped_refptr<ResourceResponse> response(new ResourceResponse);
+  main_loader->CallOnResponseStarted(response, MakeEmptyStream());
+  TestRenderFrameHost* speculative_root_rfh =
+      GetSpeculativeRenderFrameHost(root_node);
+  ASSERT_TRUE(speculative_root_rfh);
+  EXPECT_TRUE(DidRenderFrameHostRequestCommit(speculative_root_rfh));
+  EXPECT_FALSE(DidRenderFrameHostRequestCommit(main_test_rfh()));
+
+  // The subframe request is still there as the subframe still exists.
+  ASSERT_TRUE(navigator->GetNavigationRequestForNodeForTesting(subframe_node));
+
+  // Commit the navigation.
+  speculative_root_rfh->SendNavigate(1, kUrl3);
+
+  // The subframe request should have been cleaned up with the subframe.
+  ASSERT_FALSE(navigator->GetNavigationRequestForNodeForTesting(subframe_node));

[carlosk, 2015/04/08 09:01:48]

As I removed the N::CancelNavigation from FTN::~FrameTreeNode I wanted to be
sure subframe navigations were still correctly cleaned up. Hence these additions
to this test.

[clamy, 2015/04/08 11:29:11]

Well since it's failing they are not.

[carlosk, 2015/04/08 16:05:39]

Yeah. It did pass on my workstation but I only tested in Debug. Now I see it
does not in Release...

 }
 
 // PlzNavigate: Test that committing an HTTP 204 or HTTP 205 response cancels
 // the navigation.
 TEST_F(NavigatorTestWithBrowserSideNavigation, NoContent) {
   const GURL kUrl1("http://www.chromium.org/");
   const GURL kUrl2("http://www.google.com/");
 
   // Load a URL.
   contents()->NavigateAndCommit(kUrl1);
@@ skipping 770 matching lines @@
 
     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 content