Make RenderFrameHostManager swap RenderFrameHosts, not RenderViewHosts.

content/browser/web_contents/web_contents_impl_unittest.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/logging.h"
 #include "base/strings/utf_string_conversions.h"
 #include "content/browser/frame_host/interstitial_page_impl.h"
 #include "content/browser/frame_host/navigation_entry_impl.h"
 #include "content/browser/renderer_host/render_view_host_impl.h"
 #include "content/browser/site_instance_impl.h"
@@ skipping 163 matching lines @@
   void CommandReceived() {
     command_received_count_++;
   }
 
   void set_delegate(Delegate* delegate) {
     delegate_ = delegate;
   }
 
  protected:
   virtual RenderViewHost* CreateRenderViewHost() OVERRIDE {
     return new TestRenderViewHost(

[Charlie Reis, 2013/12/17 19:39:22]

This TestRVH was leaking in the test, since we now use the RVH from the
interstitial page's frame tree instead.  As it turns out, this override was
unnecessary because we create TestRenderViewHosts from the factory anyway.

         SiteInstance::Create(web_contents()->GetBrowserContext()),
-        this, this, this, MSG_ROUTING_NONE, MSG_ROUTING_NONE, false);
+        this, this, MSG_ROUTING_NONE, MSG_ROUTING_NONE, false);
   }
 
   virtual WebContentsView* CreateWebContentsView() OVERRIDE {
     return NULL;
   }
 
  private:
   InterstitialState* state_;
   bool* deleted_;
   int command_received_count_;
@@ skipping 224 matching lines @@
   controller().LoadURL(
       url, Referrer(), PAGE_TRANSITION_GENERATED, std::string());
   EXPECT_TRUE(controller().GetVisibleEntry() == NULL);
 }
 
 // Test that navigating across a site boundary creates a new RenderViewHost
 // with a new SiteInstance.  Going back should do the same.
 TEST_F(WebContentsImplTest, CrossSiteBoundaries) {
   contents()->transition_cross_site = true;
   TestRenderViewHost* orig_rvh = test_rvh();
+  RenderFrameHostImpl* orig_rfh =
+      contents()->GetFrameTree()->root()->current_frame_host();
   int orig_rvh_delete_count = 0;
   orig_rvh->set_delete_counter(&orig_rvh_delete_count);
   SiteInstance* instance1 = contents()->GetSiteInstance();
 
   // Navigate to URL.  First URL should use first RenderViewHost.
   const GURL url("http://www.google.com");
   controller().LoadURL(
       url, Referrer(), PAGE_TRANSITION_TYPED, std::string());
   contents()->TestDidNavigate(orig_rvh, 1, url, PAGE_TRANSITION_TYPED);
 
@@ skipping 12 matching lines @@
   const GURL url2("http://www.yahoo.com");
   controller().LoadURL(
       url2, Referrer(), PAGE_TRANSITION_TYPED, std::string());
   EXPECT_TRUE(contents()->cross_navigation_pending());
   EXPECT_EQ(url, contents()->GetLastCommittedURL());
   EXPECT_EQ(url2, contents()->GetVisibleURL());
   TestRenderViewHost* pending_rvh =
       static_cast<TestRenderViewHost*>(contents()->GetPendingRenderViewHost());
   int pending_rvh_delete_count = 0;
   pending_rvh->set_delete_counter(&pending_rvh_delete_count);
+  RenderFrameHostImpl* pending_rfh = contents()->GetFrameTree()->root()->
+      render_manager()->pending_frame_host();
 
   // Navigations should be suspended in pending_rvh until ShouldCloseACK.
   EXPECT_TRUE(pending_rvh->are_navigations_suspended());
   orig_rvh->SendShouldCloseACK(true);
   EXPECT_FALSE(pending_rvh->are_navigations_suspended());
 
   // DidNavigate from the pending page
   contents()->TestDidNavigate(
       pending_rvh, 1, url2, PAGE_TRANSITION_TYPED);
   SiteInstance* instance2 = contents()->GetSiteInstance();
 
   // Keep the number of active views in pending_rvh's SiteInstance
   // non-zero so that orig_rvh doesn't get deleted when it gets
   // swapped out.
   static_cast<SiteInstanceImpl*>(pending_rvh->GetSiteInstance())->
       increment_active_view_count();
 
   EXPECT_FALSE(contents()->cross_navigation_pending());
   EXPECT_EQ(pending_rvh, contents()->GetRenderViewHost());
   EXPECT_EQ(url2, contents()->GetLastCommittedURL());
   EXPECT_EQ(url2, contents()->GetVisibleURL());
   EXPECT_NE(instance1, instance2);
   EXPECT_TRUE(contents()->GetPendingRenderViewHost() == NULL);
-  // We keep the original RVH around, swapped out.
+  // We keep the original RFH around, swapped out.
   EXPECT_TRUE(contents()->GetRenderManagerForTesting()->IsOnSwappedOutList(
-      orig_rvh));
+      orig_rfh));
   EXPECT_EQ(orig_rvh_delete_count, 0);
 
   // Going back should switch SiteInstances again.  The first SiteInstance is
   // stored in the NavigationEntry, so it should be the same as at the start.
   // We should use the same RVH as before, swapping it back in.
   controller().GoBack();
   TestRenderViewHost* goback_rvh =
       static_cast<TestRenderViewHost*>(contents()->GetPendingRenderViewHost());
   EXPECT_EQ(orig_rvh, goback_rvh);
   EXPECT_TRUE(contents()->cross_navigation_pending());
 
   // Navigations should be suspended in goback_rvh until ShouldCloseACK.
   EXPECT_TRUE(goback_rvh->are_navigations_suspended());
   pending_rvh->SendShouldCloseACK(true);
   EXPECT_FALSE(goback_rvh->are_navigations_suspended());
 
   // DidNavigate from the back action
   contents()->TestDidNavigate(
       goback_rvh, 1, url2, PAGE_TRANSITION_TYPED);
   EXPECT_FALSE(contents()->cross_navigation_pending());
   EXPECT_EQ(goback_rvh, contents()->GetRenderViewHost());
   EXPECT_EQ(instance1, contents()->GetSiteInstance());
-  // The pending RVH should now be swapped out, not deleted.
+  // The pending RFH should now be swapped out, not deleted.
   EXPECT_TRUE(contents()->GetRenderManagerForTesting()->
-      IsOnSwappedOutList(pending_rvh));
+      IsOnSwappedOutList(pending_rfh));
   EXPECT_EQ(pending_rvh_delete_count, 0);
 
   // Close contents and ensure RVHs are deleted.
   DeleteContents();
   EXPECT_EQ(orig_rvh_delete_count, 1);
   EXPECT_EQ(pending_rvh_delete_count, 1);
 }
 
 // Test that navigating across a site boundary after a crash creates a new
 // RVH without requiring a cross-site transition (i.e., PENDING state).
@@ skipping 102 matching lines @@
   // Both contentses should now be in the same SiteInstance.
   EXPECT_EQ(instance2a, instance2b);
 }
 
 TEST_F(WebContentsImplTest, NavigateDoesNotUseUpSiteInstance) {
   WebContentsImplTestBrowserClient browser_client;
   SetBrowserClientForTesting(&browser_client);
 
   contents()->transition_cross_site = true;
   TestRenderViewHost* orig_rvh = test_rvh();
+  RenderFrameHostImpl* orig_rfh =
+      contents()->GetFrameTree()->root()->current_frame_host();
   int orig_rvh_delete_count = 0;
   orig_rvh->set_delete_counter(&orig_rvh_delete_count);
   SiteInstanceImpl* orig_instance =
       static_cast<SiteInstanceImpl*>(contents()->GetSiteInstance());
 
   browser_client.set_assign_site_for_url(false);
   // Navigate to an URL that will not assign a new SiteInstance.
   const GURL native_url("non-site-url://stuffandthings");
   controller().LoadURL(
       native_url, Referrer(), PAGE_TRANSITION_TYPED, std::string());
@@ skipping 50 matching lines @@
   contents()->TestDidNavigate(
       pending_rvh, 1, url2, PAGE_TRANSITION_TYPED);
   SiteInstance* new_instance = contents()->GetSiteInstance();
 
   EXPECT_FALSE(contents()->cross_navigation_pending());
   EXPECT_EQ(pending_rvh, contents()->GetRenderViewHost());
   EXPECT_EQ(url2, contents()->GetLastCommittedURL());
   EXPECT_EQ(url2, contents()->GetVisibleURL());
   EXPECT_NE(new_instance, orig_instance);
   EXPECT_FALSE(contents()->GetPendingRenderViewHost());
-  // We keep the original RVH around, swapped out.
+  // We keep the original RFH around, swapped out.
   EXPECT_TRUE(contents()->GetRenderManagerForTesting()->IsOnSwappedOutList(
-      orig_rvh));
+      orig_rfh));
   EXPECT_EQ(orig_rvh_delete_count, 0);
 
   // Close contents and ensure RVHs are deleted.
   DeleteContents();
   EXPECT_EQ(orig_rvh_delete_count, 1);
   EXPECT_EQ(pending_rvh_delete_count, 1);
 }
 
 // Test that we can find an opener RVH even if it's pending.
 // http://crbug.com/176252.
@@ skipping 1442 matching lines @@
 TEST_F(WebContentsImplTest, PendingContents) {
   scoped_ptr<TestWebContents> other_contents(
       static_cast<TestWebContents*>(CreateTestWebContents()));
   contents()->AddPendingContents(other_contents.get());
   int route_id = other_contents->GetRenderViewHost()->GetRoutingID();
   other_contents.reset();
   EXPECT_EQ(NULL, contents()->GetCreatedWindow(route_id));
 }
 
 }  // namespace content