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 162 matching lines @@
   }
 
   void CommandReceived() {
     command_received_count_++;
   }
 
   void set_delegate(Delegate* delegate) {
     delegate_ = delegate;
   }
 
  protected:

[Charlie Reis, 2013/12/17 22:40:21]

Note: The CreateRenderViewHost method has already been removed in
http://crrev.com/241363, so the memory leak should be gone.

   virtual WebContentsView* CreateWebContentsView() OVERRIDE {
     return NULL;
   }
 
  private:
   InterstitialState* state_;
   bool* deleted_;
   int command_received_count_;
   Delegate* delegate_;
 };
@@ skipping 222 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