Check BrowsingInstance before swapping prerenders.

content/browser/frame_host/render_frame_host_manager_unittest.cc

 // Copyright 2013 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/files/file_path.h"
 #include "base/strings/utf_string_conversions.h"
 #include "content/browser/frame_host/cross_site_transferring_request.h"
 #include "content/browser/frame_host/navigation_controller_impl.h"
 #include "content/browser/frame_host/navigation_entry_impl.h"
 #include "content/browser/frame_host/navigator.h"
@@ skipping 1031 matching lines @@
                 MSG_ROUTING_NONE);
   EXPECT_FALSE(manager->current_host()->IsRenderViewLive());
 
   const GURL kUrl("chrome://foo");
   NavigationEntryImpl entry(NULL /* instance */, -1 /* page_id */, kUrl,
                             Referrer(), base::string16() /* title */,
                             PAGE_TRANSITION_TYPED,
                             false /* is_renderer_init */);
   RenderFrameHostImpl* host = manager->Navigate(entry);
 
+  // This test creates a second FrameTree for the WebContents which throws off
+  // some of the accounting. This inverts what was done in
+  // WebContentsImpl::NotifySwapped.
+  static_cast<SiteInstanceImpl*>(instance)->
+      IncrementRelatedActiveContentsCount();
+  static_cast<SiteInstanceImpl*>(host->GetSiteInstance())->
+      DecrementRelatedActiveContentsCount();

[davidben, 2014/04/28 18:34:39]

Not super-thrilled about this bit. Another possibility is some kind of
WebContentsImpl::SetUsingExternalFrameTreeForTesting(). Another possibility is
to move the accounting into top-level RFHMs.

[Charlie Reis, 2014/04/29 22:13:51]

I'm not entirely sure why we need a separate FrameTree in this test in the first
place.  Looks like it was a legacy thing of creating a fresh RVHM for the test
using the existing WebContents as a mostly-unused delegate, but I don't see a
reason why we can't just use the RFHM of the WebContents itself.  See:
https://codereview.chromium.org/117693002/diff/20001/content/browser/frame_ho...

The test seems to pass if we replace tree, manager, and the Init call with:
RenderFrameHostManager* manager = web_contents->GetRenderManagerForTesting();

Same below.

[davidben, 2014/04/29 23:02:25]

Oh. Well then. Handy. I've gone and fixed those.

+
   // We commit the pending RenderFrameHost immediately because the previous
   // RenderFrameHost was not live.  We test a case where it is live in
   // WebUIInNewTab.
   EXPECT_TRUE(host);
   EXPECT_EQ(host, manager->current_frame_host());
   EXPECT_FALSE(manager->pending_frame_host());
 
   // It's important that the site instance get set on the Web UI page as soon
   // as the navigation starts, rather than lazily after it commits, so we don't
   // try to re-use the SiteInstance/process for non Web UI things that may
@@ skipping 46 matching lines @@
   EXPECT_TRUE(
       host1->render_view_host()->GetEnabledBindings() & BINDINGS_POLICY_WEB_UI);
 
   // Commit and ensure we still have bindings.
   manager1->DidNavigateFrame(host1);
   SiteInstance* webui_instance = host1->GetSiteInstance();
   EXPECT_EQ(host1, manager1->current_frame_host());
   EXPECT_TRUE(
       host1->render_view_host()->GetEnabledBindings() & BINDINGS_POLICY_WEB_UI);
 
+  // This test creates a second FrameTree for the WebContents which throws off
+  // some of the accounting. This inverts what was done in
+  // WebContentsImpl::NotifySwapped.
+  static_cast<SiteInstanceImpl*>(blank_instance)->
+      IncrementRelatedActiveContentsCount();
+  static_cast<SiteInstanceImpl*>(host1->GetSiteInstance())->
+      DecrementRelatedActiveContentsCount();
+

[davidben, 2014/04/28 18:34:39]

Ditto.

   // Now simulate clicking a link that opens in a new tab.
   scoped_ptr<TestWebContents> web_contents2(
       TestWebContents::Create(browser_context(), webui_instance));
   FrameTree tree2(web_contents2->GetFrameTree()->root()->navigator(),
                   web_contents2.get(), web_contents2.get(),
                   web_contents2.get(), web_contents2.get());
   RenderFrameHostManager* manager2 = tree2.root()->render_manager();
   manager2->Init(
       browser_context(), webui_instance, MSG_ROUTING_NONE, MSG_ROUTING_NONE);
   // Make sure the new RVH is considered live.  This is usually done in
@@ skipping 713 matching lines @@
         pending_rfh->GetSiteInstance())->increment_active_view_count();
 
     main_test_rfh()->OnMessageReceived(
         FrameHostMsg_BeforeUnload_ACK(0, false, now, now));
     EXPECT_FALSE(contents()->cross_navigation_pending());
     EXPECT_FALSE(rvh_deleted_observer.deleted());
   }
 }
 
 }  // namespace content