Remove SiteIsolationPolicy::IsSwappedOutStateForbidden().

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 "content/browser/frame_host/render_frame_host_manager.h"
 
 #include <stdint.h>
 #include <utility>
 
 #include "base/command_line.h"
@@ skipping 342 matching lines @@
     int32_t max_page_id =
         contents()->GetMaxPageIDForSiteInstance(active_rfh->GetSiteInstance());
 
     // Use an observer to avoid accessing a deleted renderer later on when the
     // state is being checked.
     RenderFrameDeletedObserver rfh_observer(old_rfh);
     RenderViewHostDeletedObserver rvh_observer(old_rfh->GetRenderViewHost());
     active_rfh->SendNavigate(max_page_id + 1, entry_id, true, url);
 
     // Make sure that we start to run the unload handler at the time of commit.
-    bool expecting_rfh_shutdown = false;
     if (old_rfh != active_rfh && !rfh_observer.deleted()) {
       EXPECT_EQ(RenderFrameHostImpl::STATE_PENDING_SWAP_OUT,
                 old_rfh->rfh_state());
-      if (!old_rfh->GetSiteInstance()->active_frame_count() ||
-          SiteIsolationPolicy::IsSwappedOutStateForbidden()) {
-        expecting_rfh_shutdown = true;
-        EXPECT_TRUE(
+      EXPECT_TRUE(
             old_rfh->frame_tree_node()->render_manager()->IsPendingDeletion(
                 old_rfh));
-      }
     }
 
     // Simulate the swap out ACK coming from the pending renderer.  This should
     // either shut down the old RFH or leave it in a swapped out state.
     if (old_rfh != active_rfh) {
       old_rfh->OnSwappedOut();
-      if (expecting_rfh_shutdown) {
-        EXPECT_TRUE(rfh_observer.deleted());
-        if (!SiteIsolationPolicy::IsSwappedOutStateForbidden()) {
-          EXPECT_TRUE(rvh_observer.deleted());
-        }
-      } else {
-        EXPECT_EQ(RenderFrameHostImpl::STATE_SWAPPED_OUT,
-                  old_rfh->rfh_state());
-      }
+      EXPECT_TRUE(rfh_observer.deleted());
     }
     EXPECT_EQ(active_rfh, contents()->GetMainFrame());
     EXPECT_EQ(NULL, contents()->GetPendingMainFrame());
   }
 
   bool ShouldSwapProcesses(RenderFrameHostManager* manager,
                            const NavigationEntryImpl* current_entry,
                            const NavigationEntryImpl* new_entry) const {
     CHECK(new_entry);
     BrowserContext* browser_context =
         manager->delegate_->GetControllerForRenderManager().GetBrowserContext();
     const GURL& current_effective_url = current_entry ?
         SiteInstanceImpl::GetEffectiveURL(browser_context,
                                           current_entry->GetURL()) :
         manager->render_frame_host_->GetSiteInstance()->GetSiteURL();
     bool current_is_view_source_mode = current_entry ?
         current_entry->IsViewSourceMode() : new_entry->IsViewSourceMode();
     return manager->ShouldSwapBrowsingInstancesForNavigation(
         current_effective_url,
         current_is_view_source_mode,
         new_entry->site_instance(),
         SiteInstanceImpl::GetEffectiveURL(browser_context, new_entry->GetURL()),
         new_entry->IsViewSourceMode());
   }
 
-  // Creates a test RenderFrameHost that's swapped out.
-  TestRenderFrameHost* CreateSwappedOutRenderFrameHost() {
+  // Creates a test RenderViewHost that's swapped out.

[dcheng, 2016/03/11 20:32:28]

It seems weird that the function is called CreateSwappedOutRenderViewHost but it
returns a RenderFrameHost. The return value also appears to be unused.

[nasko, 2016/03/11 22:35:33]

Done.

+  TestRenderFrameHost* CreateSwappedOutRenderViewHost() {
     const GURL kChromeURL("chrome://foo");
     const GURL kDestUrl("http://www.google.com/");
 
     // Navigate our first tab to a chrome url and then to the destination.
     NavigateActiveAndCommit(kChromeURL);
     TestRenderFrameHost* ntp_rfh = contents()->GetMainFrame();
 
     // Navigate to a cross-site URL.
     contents()->GetController().LoadURL(
         kDestUrl, Referrer(), ui::PAGE_TRANSITION_LINK, std::string());
     int entry_id = contents()->GetController().GetPendingEntry()->GetUniqueID();
     contents()->GetMainFrame()->PrepareForCommit();
     EXPECT_TRUE(contents()->CrossProcessNavigationPending());
 
     // Manually increase the number of active frames in the
     // SiteInstance that ntp_rfh belongs to, to prevent it from being
     // destroyed when it gets swapped out.
     ntp_rfh->GetSiteInstance()->IncrementActiveFrameCount();
 
     TestRenderFrameHost* dest_rfh = contents()->GetPendingMainFrame();
     CHECK(dest_rfh);
     EXPECT_NE(ntp_rfh, dest_rfh);
 
     // BeforeUnload finishes.
     ntp_rfh->SendBeforeUnloadACK(true);
 
     dest_rfh->SendNavigate(101, entry_id, true, kDestUrl);
     ntp_rfh->OnSwappedOut();
 
-    EXPECT_TRUE(ntp_rfh->is_swapped_out());
     return ntp_rfh;
   }
 
   // Returns the RenderFrameHost that should be used in the navigation to
   // |entry|.
   RenderFrameHostImpl* NavigateToEntry(
       RenderFrameHostManager* manager,
       const NavigationEntryImpl& entry) {
     // Tests currently only navigate using main frame FrameNavigationEntries.
     FrameNavigationEntry* frame_entry = entry.root_node()->frame_entry.get();
@@ skipping 180 matching lines @@
 
   // The old renderer, being slow, now updates the favicon. It should be
   // filtered out and not take effect.
   {
     PluginFaviconMessageObserver observer(contents());
     EXPECT_TRUE(
         ntp_rvh->GetWidget()->OnMessageReceived(ViewHostMsg_UpdateFaviconURL(
             dest_rfh->GetRenderViewHost()->GetRoutingID(), icons)));
     EXPECT_FALSE(observer.favicon_received());
   }
-
-  // In --site-per-process, the RenderFrameHost is deleted on cross-process
-  // navigation, so the rest of the test case doesn't apply.
-  if (SiteIsolationPolicy::IsSwappedOutStateForbidden()) {
-    return;
-  }
-
-#if defined(ENABLE_PLUGINS)
-  // The same logic should apply to RenderFrameHosts as well and routing through
-  // swapped out RFH shouldn't be allowed. Use a PluginCrashObserver to check
-  // if the IPC message is allowed through or not.
-  {
-    PluginFaviconMessageObserver observer(contents());
-    EXPECT_TRUE(ntp_rfh->OnMessageReceived(
-                    FrameHostMsg_PluginCrashed(
-                        ntp_rfh->GetRoutingID(), base::FilePath(), 0)));
-    EXPECT_FALSE(observer.plugin_crashed());
-  }
-#endif
-
-  // We cannot filter out synchronous IPC messages, because the renderer would
-  // be left waiting for a reply.  We pick RunBeforeUnloadConfirm as an example
-  // that can run easily within a unit test, and that needs to receive a reply
-  // without showing an actual dialog.
-  MockRenderProcessHost* ntp_process_host = ntp_rfh->GetProcess();
-  ntp_process_host->sink().ClearMessages();
-  const base::string16 msg = base::ASCIIToUTF16("Message");
-  bool result = false;
-  base::string16 unused;
-  FrameHostMsg_RunBeforeUnloadConfirm before_unload_msg(
-      ntp_rfh->GetRoutingID(), kChromeURL, msg, false, &result, &unused);
-  EXPECT_TRUE(ntp_rfh->OnMessageReceived(before_unload_msg));
-  EXPECT_TRUE(ntp_process_host->sink().GetUniqueMessageMatching(IPC_REPLY_ID));
-
-  // Also test RunJavaScriptMessage.
-  ntp_process_host->sink().ClearMessages();
-  FrameHostMsg_RunJavaScriptMessage js_msg(
-      ntp_rfh->GetRoutingID(), msg, msg, kChromeURL,
-      JAVASCRIPT_MESSAGE_TYPE_CONFIRM, &result, &unused);
-  js_msg.EnableMessagePumping();
-  EXPECT_TRUE(ntp_rfh->OnMessageReceived(js_msg));
-  EXPECT_TRUE(ntp_process_host->sink().GetUniqueMessageMatching(IPC_REPLY_ID));
 }
 
 // Test that the ViewHostMsg_UpdateFaviconURL IPC message is ignored if the
 // renderer is in the STATE_PENDING_SWAP_OUT_STATE. The favicon code assumes
 // that it only gets ViewHostMsg_UpdateFaviconURL messages for the most recently
 // committed navigation for each WebContentsImpl.
 TEST_F(RenderFrameHostManagerTest, UpdateFaviconURLWhilePendingSwapOut) {
   const GURL kChromeURL("chrome://foo");
   const GURL kDestUrl("http://www.google.com/");
   std::vector<FaviconURL> icons;
@@ skipping 39 matching lines @@
   // be ignored.
   {
     PluginFaviconMessageObserver observer(contents());
     EXPECT_TRUE(rfh1->GetRenderViewHost()->GetWidget()->OnMessageReceived(
         ViewHostMsg_UpdateFaviconURL(rfh1->GetRenderViewHost()->GetRoutingID(),
                                      icons)));
     EXPECT_FALSE(observer.favicon_received());
   }
 }
 
-// Ensure that frames aren't added to the frame tree, if the message is coming
-// from a process different than the parent frame's current RenderFrameHost
-// process. Otherwise it is possible to have collisions of routing ids, as they
-// are scoped per process. See https://crbug.com/415059.
-TEST_F(RenderFrameHostManagerTest, DropCreateChildFrameWhileSwappedOut) {
-  const GURL kUrl1("http://foo.com");
-  const GURL kUrl2("http://www.google.com/");
-
-  // This test is invalid in --site-per-process mode, as swapped-out is no
-  // longer used.
-  if (SiteIsolationPolicy::IsSwappedOutStateForbidden()) {
-    return;
-  }
-
-  // Navigate to the first site.
-  NavigateActiveAndCommit(kUrl1);
-  TestRenderFrameHost* initial_rfh = contents()->GetMainFrame();
-  {
-    RenderFrameHostCreatedObserver observer(contents());
-    initial_rfh->OnCreateChildFrame(
-        initial_rfh->GetProcess()->GetNextRoutingID(),
-        blink::WebTreeScopeType::Document, std::string(), "uniqueName1",
-        blink::WebSandboxFlags::None, blink::WebFrameOwnerProperties());
-    EXPECT_TRUE(observer.created());
-  }
-
-  // Create one more frame in the same SiteInstance where initial_rfh
-  // exists so that initial_rfh doesn't get deleted on navigation to another
-  // site.
-  initial_rfh->GetSiteInstance()->IncrementActiveFrameCount();
-
-  // Navigate to a cross-site URL.
-  NavigateActiveAndCommit(kUrl2);
-  EXPECT_TRUE(initial_rfh->is_swapped_out());
-
-  TestRenderFrameHost* dest_rfh = contents()->GetMainFrame();
-  ASSERT_TRUE(dest_rfh);
-  EXPECT_NE(initial_rfh, dest_rfh);
-
-  {
-    // Since the old RFH is now swapped out, it shouldn't process any messages
-    // to create child frames.
-    RenderFrameHostCreatedObserver observer(contents());
-    initial_rfh->OnCreateChildFrame(
-        initial_rfh->GetProcess()->GetNextRoutingID(),
-        blink::WebTreeScopeType::Document, std::string(), "uniqueName2",
-        blink::WebSandboxFlags::None, blink::WebFrameOwnerProperties());
-    EXPECT_FALSE(observer.created());
-  }
-}
-
-TEST_F(RenderFrameHostManagerTest, WhiteListSwapCompositorFrame) {
-  // TODO(nasko): Check with kenrb whether this test can be rewritten and
-  // whether it makes sense when swapped out is replaced with proxies.
-  if (SiteIsolationPolicy::IsSwappedOutStateForbidden()) {
-    return;
-  }
-  TestRenderFrameHost* swapped_out_rfh = CreateSwappedOutRenderFrameHost();
-  TestRenderWidgetHostView* swapped_out_rwhv =
-      static_cast<TestRenderWidgetHostView*>(
-          swapped_out_rfh->GetRenderViewHost()->GetWidget()->GetView());
-  EXPECT_FALSE(swapped_out_rwhv->did_swap_compositor_frame());
-
-  MockRenderProcessHost* process_host = swapped_out_rfh->GetProcess();
-  process_host->sink().ClearMessages();
-
-  cc::CompositorFrame frame;
-  ViewHostMsg_SwapCompositorFrame msg(
-      rvh()->GetRoutingID(), 0, frame, std::vector<IPC::Message>());
-
-  EXPECT_TRUE(
-      swapped_out_rfh->render_view_host()->GetWidget()->OnMessageReceived(msg));
-  EXPECT_TRUE(swapped_out_rwhv->did_swap_compositor_frame());
-}
-
 // Test if RenderViewHost::GetRenderWidgetHosts() only returns active
 // widgets.
 TEST_F(RenderFrameHostManagerTest, GetRenderWidgetHostsReturnsActiveViews) {
-  // This test is invalid in --site-per-process mode, as swapped-out is no

[Charlie Reis, 2016/03/11 21:56:56]

Is there a reasons to keep the rest of this test?

[nasko, 2016/03/11 22:35:33]

I want to keep these two around, as it isn't quite clear yet what "swapped out
RVH" means. I want to ensure that we aren't returning these out to callers when
we aren't supposed to. I forsee them disappearing once we have the RV/RW split
out complete and the IPCs are no longer routed through RVHs.

-  // longer used.
-  if (SiteIsolationPolicy::IsSwappedOutStateForbidden()) {
-    return;
-  }
-
-  TestRenderFrameHost* swapped_out_rfh = CreateSwappedOutRenderFrameHost();
-  EXPECT_TRUE(swapped_out_rfh->is_swapped_out());
-
+  CreateSwappedOutRenderViewHost();
   scoped_ptr<RenderWidgetHostIterator> widgets(
       RenderWidgetHost::GetRenderWidgetHosts());
+
   // We know that there is the only one active widget. Another view is
   // now swapped out, so the swapped out view is not included in the
   // list.
   RenderWidgetHost* widget = widgets->GetNextHost();
   EXPECT_FALSE(widgets->GetNextHost());
   RenderViewHost* rvh = RenderViewHost::From(widget);
   EXPECT_TRUE(static_cast<RenderViewHostImpl*>(rvh)->is_active());
 }
 
 // Test if RenderViewHost::GetRenderWidgetHosts() returns a subset of
 // RenderViewHostImpl::GetAllRenderWidgetHosts().
 // RenderViewHost::GetRenderWidgetHosts() returns only active widgets, but
 // RenderViewHostImpl::GetAllRenderWidgetHosts() returns everything
 // including swapped out ones.
 TEST_F(RenderFrameHostManagerTest,
        GetRenderWidgetHostsWithinGetAllRenderWidgetHosts) {
-  // This test is invalid in --site-per-process mode, as swapped-out is no

[Charlie Reis, 2016/03/11 21:56:56]

Is there a reason to keep the rest of this test?

[nasko, 2016/03/11 22:35:33]

See the above comment.

-  // longer used.
-  if (SiteIsolationPolicy::IsSwappedOutStateForbidden()) {
-    return;
-  }
-
-  TestRenderFrameHost* swapped_out_rfh = CreateSwappedOutRenderFrameHost();
-  EXPECT_TRUE(swapped_out_rfh->is_swapped_out());
-
+  CreateSwappedOutRenderViewHost();
   scoped_ptr<RenderWidgetHostIterator> widgets(
       RenderWidgetHost::GetRenderWidgetHosts());
 
   while (RenderWidgetHost* w = widgets->GetNextHost()) {
     bool found = false;
     scoped_ptr<RenderWidgetHostIterator> all_widgets(
         RenderWidgetHostImpl::GetAllRenderWidgetHosts());
     while (RenderWidgetHost* widget = all_widgets->GetNextHost()) {
       if (w == widget) {
         found = true;
@@ skipping 565 matching lines @@
   EXPECT_TRUE(rfh2->IsWaitingForUnloadACK());
   EXPECT_EQ(RenderFrameHostImpl::STATE_PENDING_SWAP_OUT, rfh2->rfh_state());
 
   // We should be able to navigate forward.
   contents()->GetController().GoForward();
   contents()->GetMainFrame()->PrepareForCommit();
   const NavigationEntry* entry2 = contents()->GetController().GetPendingEntry();
   contents()->GetPendingMainFrame()->SendNavigate(
       entry2->GetPageID(), entry2->GetUniqueID(), false, entry2->GetURL());
   EXPECT_EQ(RenderFrameHostImpl::STATE_DEFAULT, main_test_rfh()->rfh_state());
-  if (!SiteIsolationPolicy::IsSwappedOutStateForbidden()) {
-    EXPECT_EQ(rfh2, main_test_rfh());
-    EXPECT_EQ(RenderFrameHostImpl::STATE_PENDING_SWAP_OUT, rfh1->rfh_state());
-    rfh1->OnSwappedOut();
-    EXPECT_TRUE(rfh1->is_swapped_out());
-    EXPECT_EQ(RenderFrameHostImpl::STATE_SWAPPED_OUT, rfh1->rfh_state());
-  }
 }
 
 // Test that we create swapped out RFHs for the opener chain when navigating an
 // opened tab cross-process.  This allows us to support certain cross-process
 // JavaScript calls (http://crbug.com/99202).
 TEST_F(RenderFrameHostManagerTest, CreateSwappedOutOpenerRFHs) {
   const GURL kUrl1("http://www.google.com/");
   const GURL kUrl2("http://www.chromium.org/");
   const GURL kChromeUrl("chrome://foo");
 
@@ skipping 21 matching lines @@
 
   // Navigate to a cross-site URL (different SiteInstance but same
   // BrowsingInstance).
   contents()->NavigateAndCommit(kUrl2);
   TestRenderFrameHost* rfh2 = main_test_rfh();
   TestRenderViewHost* rvh2 = test_rvh();
   EXPECT_NE(site_instance1, rfh2->GetSiteInstance());
   EXPECT_TRUE(site_instance1->IsRelatedSiteInstance(rfh2->GetSiteInstance()));
 
   // Ensure rvh1 is placed on swapped out list of the current tab.
-  if (!SiteIsolationPolicy::IsSwappedOutStateForbidden()) {
-    EXPECT_TRUE(manager->IsRVHOnSwappedOutList(rvh1));
-    EXPECT_FALSE(rfh1_deleted_observer.deleted());
-    EXPECT_TRUE(manager->IsOnSwappedOutList(rfh1));
-    EXPECT_EQ(rfh1,
-              manager->GetRenderFrameProxyHost(site_instance1.get())
-                  ->render_frame_host());
-  } else {
-    EXPECT_TRUE(rfh1_deleted_observer.deleted());
-    EXPECT_TRUE(manager->GetRenderFrameProxyHost(site_instance1.get()));
-  }
+  EXPECT_TRUE(rfh1_deleted_observer.deleted());
+  EXPECT_TRUE(manager->GetRenderFrameProxyHost(site_instance1.get()));
   EXPECT_EQ(rvh1,
             manager->GetSwappedOutRenderViewHost(rvh1->GetSiteInstance()));
 
   // Ensure a swapped out RFH and RFH is created in the first opener tab.
   RenderFrameProxyHost* opener1_proxy =
       opener1_manager->GetRenderFrameProxyHost(rfh2->GetSiteInstance());
   RenderFrameHostImpl* opener1_rfh = opener1_proxy->render_frame_host();
   TestRenderViewHost* opener1_rvh = static_cast<TestRenderViewHost*>(
       opener1_manager->GetSwappedOutRenderViewHost(rvh2->GetSiteInstance()));
-  if (!SiteIsolationPolicy::IsSwappedOutStateForbidden()) {
-    EXPECT_TRUE(opener1_manager->IsOnSwappedOutList(opener1_rfh));
-    EXPECT_TRUE(opener1_manager->IsRVHOnSwappedOutList(opener1_rvh));
-    EXPECT_TRUE(opener1_rfh->is_swapped_out());
-  } else {
-    EXPECT_FALSE(opener1_rfh);
-  }
+  EXPECT_FALSE(opener1_rfh);
   EXPECT_FALSE(opener1_rvh->is_active());
 
   // Ensure a swapped out RFH and RVH is created in the second opener tab.
   RenderFrameProxyHost* opener2_proxy =
       opener2_manager->GetRenderFrameProxyHost(rfh2->GetSiteInstance());
   RenderFrameHostImpl* opener2_rfh = opener2_proxy->render_frame_host();
   TestRenderViewHost* opener2_rvh = static_cast<TestRenderViewHost*>(
       opener2_manager->GetSwappedOutRenderViewHost(rvh2->GetSiteInstance()));
-  if (!SiteIsolationPolicy::IsSwappedOutStateForbidden()) {
-    EXPECT_TRUE(opener2_manager->IsOnSwappedOutList(opener2_rfh));
-    EXPECT_TRUE(opener2_manager->IsRVHOnSwappedOutList(opener2_rvh));
-    EXPECT_TRUE(opener2_rfh->is_swapped_out());
-  } else {
-    EXPECT_FALSE(opener2_rfh);
-  }
+  EXPECT_FALSE(opener2_rfh);
   EXPECT_FALSE(opener2_rvh->is_active());
 
   // Navigate to a cross-BrowsingInstance URL.
   contents()->NavigateAndCommit(kChromeUrl);
   TestRenderFrameHost* rfh3 = main_test_rfh();
   EXPECT_NE(site_instance1, rfh3->GetSiteInstance());
   EXPECT_FALSE(site_instance1->IsRelatedSiteInstance(rfh3->GetSiteInstance()));
 
   // No scripting is allowed across BrowsingInstances, so we should not create
   // swapped out RVHs for the opener chain in this case.
@@ skipping 226 matching lines @@
   EXPECT_NE(rvh1->GetSiteInstance(), rvh2->GetSiteInstance());
   EXPECT_TRUE(rvh1->GetSiteInstance()->IsRelatedSiteInstance(
                   rvh2->GetSiteInstance()));
 
   // Ensure a swapped out RFH and RVH is created in the first opener tab.
   RenderFrameProxyHost* opener1_proxy =
       opener1_manager->GetRenderFrameProxyHost(rvh2->GetSiteInstance());
   RenderFrameHostImpl* opener1_rfh = opener1_proxy->render_frame_host();
   TestRenderViewHost* opener1_rvh = static_cast<TestRenderViewHost*>(
       opener1_manager->GetSwappedOutRenderViewHost(rvh2->GetSiteInstance()));
-  if (!SiteIsolationPolicy::IsSwappedOutStateForbidden()) {
-    EXPECT_TRUE(opener1_manager->IsOnSwappedOutList(opener1_rfh));
-    EXPECT_TRUE(opener1_manager->IsRVHOnSwappedOutList(opener1_rvh));
-    EXPECT_TRUE(opener1_rfh->is_swapped_out());
-  } else {
-    EXPECT_FALSE(opener1_rfh);
-  }
+  EXPECT_FALSE(opener1_rfh);
   EXPECT_FALSE(opener1_rvh->is_active());
 
   // Ensure the new RVH has WebUI bindings.
   EXPECT_TRUE(rvh2->GetEnabledBindings() & BINDINGS_POLICY_WEB_UI);
 }
 
 // Test that we reuse the same guest SiteInstance if we navigate across sites.
 TEST_F(RenderFrameHostManagerTest, NoSwapOnGuestNavigations) {
   GURL guest_url(std::string(kGuestScheme).append("://abc123"));
   SiteInstance* instance =
@@ skipping 216 matching lines @@
                               ui::PAGE_TRANSITION_TYPED);
   EXPECT_FALSE(contents()->CrossProcessNavigationPending());
   EXPECT_EQ(rfh2, contents()->GetMainFrame());
   EXPECT_TRUE(contents()->GetPendingMainFrame() == NULL);
   EXPECT_EQ(RenderFrameHostImpl::STATE_DEFAULT, rfh2->rfh_state());
   EXPECT_EQ(RenderFrameHostImpl::STATE_PENDING_SWAP_OUT, rfh1->rfh_state());
 
   // Simulate the swap out ack.
   rfh1->OnSwappedOut();
 
-  // rfh1 should be swapped out or deleted in --site-per-process.
-  if (!SiteIsolationPolicy::IsSwappedOutStateForbidden()) {
-    EXPECT_FALSE(rfh_deleted_observer.deleted());
-    EXPECT_TRUE(rfh1->is_swapped_out());
-  } else {
-    EXPECT_TRUE(rfh_deleted_observer.deleted());
-  }
+  // rfh1 should be deleted.
+  EXPECT_TRUE(rfh_deleted_observer.deleted());
 }
 
 // Test that the RenderViewHost is properly swapped out if a navigation in the
 // new renderer commits before sending the SwapOut message to the old renderer.
 // This simulates a cross-site navigation to a synchronously committing URL
 // (e.g., a data URL) and ensures it works properly.
 TEST_F(RenderFrameHostManagerTest,
        CommitNewNavigationBeforeSendingSwapOut) {
   const GURL kUrl1("http://www.google.com/");
   const GURL kUrl2("http://www.chromium.org/");
@@ skipping 22 matching lines @@
                               ui::PAGE_TRANSITION_TYPED);
   EXPECT_FALSE(contents()->CrossProcessNavigationPending());
   EXPECT_EQ(rfh2, contents()->GetMainFrame());
   EXPECT_TRUE(contents()->GetPendingMainFrame() == NULL);
   EXPECT_EQ(RenderFrameHostImpl::STATE_DEFAULT, rfh2->rfh_state());
   EXPECT_EQ(RenderFrameHostImpl::STATE_PENDING_SWAP_OUT, rfh1->rfh_state());
 
   // Simulate the swap out ack.
   rfh1->OnSwappedOut();
 
-  // rfh1 should be swapped out.
-  if (SiteIsolationPolicy::IsSwappedOutStateForbidden()) {
-    EXPECT_TRUE(rfh_deleted_observer.deleted());
-    EXPECT_TRUE(contents()->GetFrameTree()->root()->render_manager()
-                ->GetRenderFrameProxyHost(site_instance.get()));
-  } else {
-    EXPECT_FALSE(rfh_deleted_observer.deleted());
-    EXPECT_TRUE(rfh1->is_swapped_out());
-  }
+  // rfh1 should be deleted

[Charlie Reis, 2016/03/11 21:56:56]

nit: Add period.

[nasko, 2016/03/11 22:35:33]

Done.

+  EXPECT_TRUE(rfh_deleted_observer.deleted());
+  EXPECT_TRUE(contents()->GetFrameTree()->root()->render_manager()
+              ->GetRenderFrameProxyHost(site_instance.get()));
 }
 
 // Test that a RenderFrameHost is properly deleted when a cross-site navigation
 // is cancelled.
 TEST_F(RenderFrameHostManagerTest,
        CancelPendingProperlyDeletesOrSwaps) {
   const GURL kUrl1("http://www.google.com/");
   const GURL kUrl2("http://www.chromium.org/");
   RenderFrameHostImpl* pending_rfh = NULL;
   base::TimeTicks now = base::TimeTicks::Now();
@@ skipping 31 matching lines @@
     // cause the pending RFH to be deleted and a RenderFrameProxyHost to be
     // created.
     scoped_refptr<SiteInstanceImpl> site_instance =
         pending_rfh->GetSiteInstance();
     site_instance->IncrementActiveFrameCount();
 
     contents()->GetMainFrame()->OnMessageReceived(
         FrameHostMsg_BeforeUnload_ACK(0, false, now, now));
     EXPECT_FALSE(contents()->CrossProcessNavigationPending());
 
-    if (SiteIsolationPolicy::IsSwappedOutStateForbidden()) {
-      EXPECT_TRUE(rfh_deleted_observer.deleted());
-      EXPECT_TRUE(contents()->GetFrameTree()->root()->render_manager()
-                  ->GetRenderFrameProxyHost(site_instance.get()));
-    } else {
-      EXPECT_FALSE(rfh_deleted_observer.deleted());
-    }
+    EXPECT_TRUE(rfh_deleted_observer.deleted());
+    EXPECT_TRUE(contents()->GetFrameTree()->root()->render_manager()
+                ->GetRenderFrameProxyHost(site_instance.get()));
   }
 }
 
 class RenderFrameHostManagerTestWithSiteIsolation
     : public RenderFrameHostManagerTest {
  public:
   RenderFrameHostManagerTestWithSiteIsolation() {
     IsolateAllSitesForTesting(base::CommandLine::ForCurrentProcess());
   }
 };
@@ skipping 1152 matching lines @@
   commit_params.should_enforce_strict_mixed_content_checking = false;
   child_host->SendNavigateWithParams(&commit_params);
   EXPECT_NO_FATAL_FAILURE(CheckMixedContentIPC(
       main_test_rfh(), false, proxy_to_parent->GetRoutingID()));
   EXPECT_FALSE(root->child_at(0)
                    ->current_replication_state()
                    .should_enforce_strict_mixed_content_checking);
 }
 
 }  // namespace content