Reland #1 of: Move WebUI ownership from the RenderFrameHostManager to the RenderFrameHost.

content/browser/frame_host/render_frame_host_manager_unittest.cc

Index: content/browser/frame_host/render_frame_host_manager_unittest.cc
diff --git a/content/browser/frame_host/render_frame_host_manager_unittest.cc b/content/browser/frame_host/render_frame_host_manager_unittest.cc
index 1ac1b088f2b653cfb57d5bdb268ab8d7b6ba5b2a..ef3403ff31e4a06a5f36a9b28251e743279f9e63 100644
--- a/content/browser/frame_host/render_frame_host_manager_unittest.cc
+++ b/content/browser/frame_host/render_frame_host_manager_unittest.cc
@@ -38,6 +38,7 @@
 #include "content/public/test/mock_render_process_host.h"
 #include "content/public/test/test_notification_tracker.h"
 #include "content/public/test/test_utils.h"
+#include "content/test/browser_side_navigation_test_utils.h"
 #include "content/test/test_content_browser_client.h"
 #include "content/test/test_content_client.h"
 #include "content/test/test_render_frame_host.h"
@@ -70,7 +71,8 @@ class RenderFrameHostManagerTestWebUIControllerFactory
     : public WebUIControllerFactory {
  public:
   RenderFrameHostManagerTestWebUIControllerFactory()
-    : should_create_webui_(false) {
+      : should_create_webui_(false), type_(1) {
+    CHECK_NE(reinterpret_cast<WebUI::TypeID>(type_), WebUI::kNoWebUI);
   }
   ~RenderFrameHostManagerTestWebUIControllerFactory() override {}
 
@@ -78,16 +80,34 @@ class RenderFrameHostManagerTestWebUIControllerFactory
     should_create_webui_ = should_create_webui;
   }
 
+  // This method simulates the expectation that different WebUI instance types
+  // would be created. The |type| value will be returned by GetWebUIType casted
+  // to WebUI::TypeID.
+  // As WebUI::TypeID is a typedef to void pointer, factory implementations
+  // return values that they know to be unique to their respective cases. So
+  // values set here should be safe if kept very low (just above zero).
+  void set_webui_type(uintptr_t type) {
+    CHECK_NE(reinterpret_cast<WebUI::TypeID>(type), WebUI::kNoWebUI);
+    type_ = type;
+  }
+
   // WebUIFactory implementation.
   WebUIController* CreateWebUIControllerForURL(WebUI* web_ui,
                                                const GURL& url) const override {
-    if (!(should_create_webui_ && HasWebUIScheme(url)))
-      return NULL;
-    return new WebUIController(web_ui);
+    // If WebUI creation is enabled for the test and this is a WebUI URL,
+    // returns a new instance.
+    if (should_create_webui_ && HasWebUIScheme(url))

[nasko, 2015/11/18 23:48:03]

Uh, this is weird, but probably ok at the content/ layer. There are more schemes
that get WebUI.

[carlosk, 2015/11/19 16:49:14]

Acknowledged. Note this is not a regression and for this mock it is irrelevant
as long as the developer of the test is aware of this.

+      return new WebUIController(web_ui);
+    return nullptr;
   }
 
   WebUI::TypeID GetWebUIType(BrowserContext* browser_context,
                              const GURL& url) const override {
+    // If WebUI creation is enabled for the test and this is a WebUI URL,
+    // returns a mock WebUI type.
+    if (should_create_webui_ && HasWebUIScheme(url)) {
+      return reinterpret_cast<WebUI::TypeID>(type_);

[nasko, 2015/11/18 23:48:04]

Can we avoid adding this? Can't we just come up with some unique value based on
the URL passed in? It will feel a bit more like the real implementation and it
will avoid the complexity of calling set_webui_type. 
I'm fine with even having the same if statement structure from the real code and
returning the same hardcoded small integers.

[carlosk, 2015/11/19 16:49:14]

A change like that will force tests to use actual URLs and would make so that
they have less control over WebUI transitions. Also any change in the production
code could break tests that shouldn't break (false positives) just because a
type change (or lack of it) expected in the test stopped happening.

As previously discussed [1] the problem here is that WebUIType is a very oddly
defined as a void* and each factory makes sure internally that whatever value is
passed is unique to itself.

I tried to make the usage of this scheme clear in set_webui_type comments above
and I think that with the usage examples in my tests it should be easy for
others to use it properly.

[1]
https://codereview.chromium.org/1417953002/diff/40001/content/browser/frame_h...

[nasko, 2015/11/19 19:45:24]

Why would we have tests that don't use actual URLs?
I don't see how this is relevant to this discussion.
 
It adds one more step for a developer to be aware of not to miss. I know I miss
steps that aren't clear to me and this one doesn't seem very obvious. I'd leave
it up to you, as I've laid out my arguments.

[carlosk, 2015/11/20 12:21:10]

Sorry: actual *WebUI* URLs (chrome://gpu, chrome://settings, ...). It wouldn't
be possible to just use chrome://foo.
This was to justify why this is indeed an odd solution: it mocks an odd choice
of type value.
The extra step is required if and only if the developer needs different WebUI
types to be returned by the controller mock. Among existent and new WebUI tests,
all but 1 didn't need it.

Base the return value on the provided URL would require creating a "translation
rule" from URL to some type value. This value would be arbitrary and would
require the developer to understand the rule to be able to use it properly. When
compared to this solution it would IMO make what's going on less explicit when
reading the tests.

Have an if-else table would require all tests to use and to conform to whatever
URLs are defined in this table to have the returned types working as needed.
This seems like a good alternative to me but not necessarily simpler or easier
to follow. If this seems better to you I will implement it in a follow up CL.

+    }
     return WebUI::kNoWebUI;
   }
 
@@ -103,6 +123,7 @@ class RenderFrameHostManagerTestWebUIControllerFactory
 
  private:
   bool should_create_webui_;
+  uintptr_t type_;
 
   DISALLOW_COPY_AND_ASSIGN(RenderFrameHostManagerTestWebUIControllerFactory);
 };
@@ -280,6 +301,8 @@ class RenderFrameHostManagerTest : public RenderViewHostImplTestHarness {
     factory_.set_should_create_webui(should_create_webui);
   }
 
+  void set_webui_type(int type) { factory_.set_webui_type(type); }
+
   void NavigateActiveAndCommit(const GURL& url) {
     // Note: we navigate the active RenderFrameHost because previous navigations
     // won't have committed yet, so NavigateAndCommit does the wrong thing
@@ -411,12 +434,28 @@ class RenderFrameHostManagerTest : public RenderViewHostImplTestHarness {
     FrameNavigationEntry* frame_entry = entry.root_node()->frame_entry.get();
     if (base::CommandLine::ForCurrentProcess()->HasSwitch(
         switches::kEnableBrowserSideNavigation)) {
+      NavigationControllerImpl* controller =
+          static_cast<NavigationControllerImpl*>(manager->current_frame_host()
+                                                     ->frame_tree_node()
+                                                     ->navigator()
+                                                     ->GetController());
+      // TODO(carlosk): This implementation below will not work with restore
+      // navigations. Method GetNavigationType should be exposed from
+      // navigator_impl.cc and used here to determine FrameMsg_Navigate_Type.
+      CHECK(entry.restore_type() == NavigationEntryImpl::RESTORE_NONE);
       scoped_ptr<NavigationRequest> navigation_request =
           NavigationRequest::CreateBrowserInitiated(
               manager->frame_tree_node_, frame_entry->url(),
               frame_entry->referrer(), *frame_entry, entry,
               FrameMsg_Navigate_Type::NORMAL, false, base::TimeTicks::Now(),
-              static_cast<NavigationControllerImpl*>(&controller()));
+              controller);
+
+      // Simulates request creation that triggers the 1st internal call to
+      // GetFrameHostForNavigation.
+      manager->DidCreateNavigationRequest(*navigation_request);
+
+      // And also simulates the 2nd and final call to GetFrameHostForNavigation
+      // that determines the final frame that will commit the navigation.
       TestRenderFrameHost* frame_host = static_cast<TestRenderFrameHost*>(
           manager->GetFrameHostForNavigation(*navigation_request));
       CHECK(frame_host);
@@ -1089,7 +1128,7 @@ TEST_F(RenderFrameHostManagerTest, WebUI) {
   RenderFrameHostImpl* initial_rfh = manager->current_frame_host();
 
   EXPECT_FALSE(manager->current_host()->IsRenderViewLive());
-  EXPECT_FALSE(manager->web_ui());
+  EXPECT_FALSE(manager->current_frame_host()->web_ui());
   EXPECT_TRUE(initial_rfh);
 
   const GURL kUrl("chrome://foo");
@@ -1116,13 +1155,8 @@ TEST_F(RenderFrameHostManagerTest, WebUI) {
 
   // The Web UI is committed immediately because the RenderViewHost has not been
   // used yet. UpdateStateForNavigate() took the short cut path.
-  if (base::CommandLine::ForCurrentProcess()->HasSwitch(
-          switches::kEnableBrowserSideNavigation)) {
-    EXPECT_FALSE(manager->speculative_web_ui());
-  } else {
-    EXPECT_FALSE(manager->pending_web_ui());
-  }
-  EXPECT_TRUE(manager->web_ui());
+  EXPECT_FALSE(manager->GetNavigatingWebUI());
+  EXPECT_TRUE(manager->current_frame_host()->web_ui());
 
   // Commit.
   manager->DidNavigateFrame(host, true);
@@ -1191,12 +1225,8 @@ TEST_F(RenderFrameHostManagerTest, WebUIInNewTab) {
   // No cross-process transition happens because we are already in the right
   // SiteInstance.  We should grant bindings immediately.
   EXPECT_EQ(host2, manager2->current_frame_host());
-  if (base::CommandLine::ForCurrentProcess()->HasSwitch(
-          switches::kEnableBrowserSideNavigation)) {
-    EXPECT_TRUE(manager2->speculative_web_ui());
-  } else {
-    EXPECT_TRUE(manager2->pending_web_ui());
-  }
+  EXPECT_TRUE(manager2->GetNavigatingWebUI());
+  EXPECT_FALSE(host2->web_ui());
   EXPECT_TRUE(
       host2->render_view_host()->GetEnabledBindings() & BINDINGS_POLICY_WEB_UI);
 
@@ -1210,16 +1240,14 @@ TEST_F(RenderFrameHostManagerTest, WebUIWasReused) {
   // Navigate to a WebUI page.
   const GURL kUrl1("chrome://foo");
   contents()->NavigateAndCommit(kUrl1);
-  RenderFrameHostManager* manager =
-      main_test_rfh()->frame_tree_node()->render_manager();
-  WebUIImpl* web_ui = manager->web_ui();
+  WebUIImpl* web_ui = main_test_rfh()->web_ui();
   EXPECT_TRUE(web_ui);
 
   // Navigate to another WebUI page which should be same-site and keep the
   // current WebUI.
   const GURL kUrl2("chrome://foo/bar");
   contents()->NavigateAndCommit(kUrl2);
-  EXPECT_EQ(web_ui, manager->web_ui());
+  EXPECT_EQ(web_ui, main_test_rfh()->web_ui());
 }
 
 // Tests that a WebUI is correctly cleaned up when navigating from a chrome://
@@ -1230,12 +1258,12 @@ TEST_F(RenderFrameHostManagerTest, WebUIWasCleared) {
   // Navigate to a WebUI page.
   const GURL kUrl1("chrome://foo");
   contents()->NavigateAndCommit(kUrl1);
-  EXPECT_TRUE(main_test_rfh()->frame_tree_node()->render_manager()->web_ui());
+  EXPECT_TRUE(main_test_rfh()->web_ui());
 
   // Navigate to a non-WebUI page.
   const GURL kUrl2("http://www.google.com");
   contents()->NavigateAndCommit(kUrl2);
-  EXPECT_FALSE(main_test_rfh()->frame_tree_node()->render_manager()->web_ui());
+  EXPECT_FALSE(main_test_rfh()->web_ui());
 }
 
 // Tests that we don't end up in an inconsistent state if a page does a back and
@@ -2662,4 +2690,238 @@ TEST_F(RenderFrameHostManagerTest,
   VerifyPageFocusMessage(hostC->GetProcess(), true, proxy->GetRoutingID());
 }
 
+// Checks that a restore navigation to a WebUI works.
+TEST_F(RenderFrameHostManagerTest, RestoreNavigationToWebUI) {
+  set_should_create_webui(true);
+
+  const GURL init_kUrl("chrome://foo/");

[nasko, 2015/11/18 23:48:03]

This variable name doesn't really follow any single style and mixes them all. Be
consistent with how the rest of the tests use this - kInitUrl.

[carlosk, 2015/11/19 16:49:14]

Done.

+  SiteInstanceImpl* init_instance =
+      static_cast<SiteInstanceImpl*>(SiteInstance::Create(browser_context()));
+  init_instance->SetSite(init_kUrl);
+  scoped_ptr<TestWebContents> web_contents(
+      TestWebContents::Create(browser_context(), init_instance));
+  RenderFrameHostManager* manager = web_contents->GetRenderManagerForTesting();
+  NavigationControllerImpl& controller = web_contents->GetController();
+
+  // Setup a restored entry.
+  std::vector<scoped_ptr<NavigationEntry>> entries;
+  scoped_ptr<NavigationEntry> new_entry =
+      NavigationControllerImpl::CreateNavigationEntry(
+          init_kUrl, Referrer(), ui::PAGE_TRANSITION_TYPED, false,
+          std::string(), browser_context());
+  new_entry->SetPageID(0);
+  entries.push_back(new_entry.Pass());
+  controller.Restore(
+      0, NavigationController::RESTORE_LAST_SESSION_EXITED_CLEANLY, &entries);
+  ASSERT_EQ(0u, entries.size());
+  ASSERT_EQ(1, controller.GetEntryCount());
+
+  RenderFrameHostImpl* initial_host = manager->current_frame_host();
+  ASSERT_TRUE(initial_host);
+  EXPECT_FALSE(initial_host->IsRenderFrameLive());
+  EXPECT_FALSE(initial_host->web_ui());
+
+  // Navigation request for a same-session restore navigation.

[nasko, 2015/11/18 23:48:03]

What is "same-session" navigation?

[carlosk, 2015/11/19 16:49:14]

This comment was outdated. It meant this should be a tab restore under the same
browsing session (when you press CTRL+SHIFT+T).

But I in fact based this test on what happens in
SessionRestoreTest::RestoreWebUI (session_restore_browsertest.cc) that closes
the browser and reopens it from the previous browsing session. I updated the
comment accordingly and tried to make it clearer.

+  NavigationEntryImpl entry(nullptr /* instance */, 0 /* page_id */, init_kUrl,
+                            Referrer(), base::string16() /* title */,
+                            ui::PAGE_TRANSITION_RELOAD,
+                            false /* is_renderer_init */);
+  entry.set_restore_type(
+      NavigationEntryImpl::RESTORE_LAST_SESSION_EXITED_CLEANLY);
+  NavigateToEntry(manager, entry);
+
+  // As the initial renderer was not live, the new RenderFrameHost should be
+  // made immediately active at request time.
+  EXPECT_FALSE(GetPendingFrameHost(manager));
+  TestRenderFrameHost* host =

[nasko, 2015/11/18 23:48:04]

nit: If you are calling the first RFH init_host, this one should probably be
current_host.

[carlosk, 2015/11/19 16:49:14]

Done.

+      static_cast<TestRenderFrameHost*>(manager->current_frame_host());
+  ASSERT_TRUE(host);
+  EXPECT_EQ(host, initial_host);
+  EXPECT_TRUE(host->IsRenderFrameLive());
+  WebUIImpl* web_ui = manager->GetNavigatingWebUI();
+  EXPECT_TRUE(web_ui);
+  EXPECT_FALSE(host->web_ui());
+
+  manager->DidNavigateFrame(host, true);
+  EXPECT_EQ(host, manager->current_frame_host());
+  EXPECT_EQ(web_ui, host->web_ui());
+}
+
+// Creates a test class for PlzNavigate tests.
+class RenderFrameHostManagerTestWithBrowserSideNavigation
+    : public RenderFrameHostManagerTest {
+ public:
+  void SetUp() override {
+    EnableBrowserSideNavigation();
+    RenderFrameHostManagerTest::SetUp();
+  }
+};
+
+// PlzNavigate: Tests that the correct intermediary and final navigation states
+// are reached when navigating from a renderer that is not live to a WebUI URL.
+TEST_F(RenderFrameHostManagerTestWithBrowserSideNavigation,
+       NavigateFromDeadRendererToWebUI) {
+  set_should_create_webui(true);
+  RenderViewHostChangedObserver change_observer(contents());

[nasko, 2015/11/18 23:48:04]

Why do we care about RVH? I'd rather not add more usage of RVH as we are trying
to remove it. Let's track RFH changes if you need to.

[carlosk, 2015/11/19 16:49:14]

I don't think it makes sense to track the RFH either so I removed the observer
usage.

+  RenderFrameHostManager* manager = contents()->GetRenderManagerForTesting();
+
+  RenderFrameHostImpl* initial_host = manager->current_frame_host();
+  ASSERT_TRUE(initial_host);
+  EXPECT_FALSE(initial_host->IsRenderFrameLive());
+
+  // Navigation request.
+  const GURL kUrl("chrome://foo");
+  NavigationEntryImpl entry(nullptr /* instance */, -1 /* page_id */, kUrl,
+                            Referrer(), base::string16() /* title */,
+                            ui::PAGE_TRANSITION_TYPED,
+                            false /* is_renderer_init */);
+  FrameNavigationEntry* frame_entry = entry.root_node()->frame_entry.get();
+  scoped_ptr<NavigationRequest> navigation_request =
+      NavigationRequest::CreateBrowserInitiated(
+          contents()->GetFrameTree()->root(), frame_entry->url(),
+          frame_entry->referrer(), *frame_entry, entry,
+          FrameMsg_Navigate_Type::NORMAL, false, base::TimeTicks::Now(),
+          static_cast<NavigationControllerImpl*>(&controller()));
+  manager->DidCreateNavigationRequest(*navigation_request);
+
+  // As the initial renderer was not live, the new RenderFrameHost should be
+  // made immediately active at request time.
+  EXPECT_TRUE(change_observer.DidHostChange());
+  EXPECT_FALSE(GetPendingFrameHost(manager));
+  TestRenderFrameHost* host =
+      static_cast<TestRenderFrameHost*>(manager->current_frame_host());
+  ASSERT_TRUE(host);
+  EXPECT_NE(host, initial_host);
+  EXPECT_TRUE(host->IsRenderFrameLive());
+  WebUIImpl* web_ui = host->web_ui();
+  EXPECT_TRUE(web_ui);
+
+  // Prepare to commit, update the navigating RenderFrameHost.
+  EXPECT_EQ(host, manager->GetFrameHostForNavigation(*navigation_request));
+
+  // No pending RenderFrameHost as the current one should be reused.
+  EXPECT_FALSE(GetPendingFrameHost(manager));
+
+  // But there should be a pending WebUI set to re-use the current one.

[nasko, 2015/11/18 23:48:03]

nit: reuse

[carlosk, 2015/11/19 16:49:14]

Done.

+  EXPECT_EQ(web_ui, host->web_ui());

[nasko, 2015/11/18 23:48:04]

If there should be a pending WebUI, why aren't you checking the pending_web() of
this host? Also, I don't see how we are testing "set to reuse the current one."

[carlosk, 2015/11/19 16:49:14]

Yes, this was indeed missing here and all over. I had forgotten to re-add
pending WebUI related checks in these 3 tests after I reintroduced them. Done
now.

+
+  // The RenderFrameHost committed.
+  manager->DidNavigateFrame(host, true);
+  EXPECT_FALSE(change_observer.DidHostChange());
+  EXPECT_EQ(host, manager->current_frame_host());
+  EXPECT_FALSE(GetPendingFrameHost(manager));
+  EXPECT_EQ(web_ui, host->web_ui());
+}
+
+// PlzNavigate: Tests that the correct intermediary and final navigation states
+// are reached when navigating same-site between two WebUIs of the same type.
+TEST_F(RenderFrameHostManagerTestWithBrowserSideNavigation,
+       NavigateSameSiteBetweenWebUIs) {
+  set_should_create_webui(true);
+  NavigateActiveAndCommit(GURL("chrome://foo"));
+
+  RenderFrameHostManager* manager = contents()->GetRenderManagerForTesting();
+  TestRenderFrameHost* host =
+      static_cast<TestRenderFrameHost*>(manager->current_frame_host());
+  EXPECT_TRUE(host->IsRenderFrameLive());
+  WebUIImpl* web_ui = host->web_ui();
+  EXPECT_TRUE(web_ui);
+
+  RenderViewHostChangedObserver change_observer(contents());

[nasko, 2015/11/18 23:48:03]

Same comment as above. If we care about such host changes, we should not be
tracking RVH but RFH.

[carlosk, 2015/11/19 16:49:14]

Done.

+
+  // Navigation request. No change in the returned WebUI type.
+  const GURL kUrl("chrome://foo/bar");
+  NavigationEntryImpl entry(nullptr /* instance */, -1 /* page_id */, kUrl,
+                            Referrer(), base::string16() /* title */,
+                            ui::PAGE_TRANSITION_TYPED,
+                            false /* is_renderer_init */);
+  FrameNavigationEntry* frame_entry = entry.root_node()->frame_entry.get();
+  scoped_ptr<NavigationRequest> navigation_request =
+      NavigationRequest::CreateBrowserInitiated(
+          contents()->GetFrameTree()->root(), frame_entry->url(),
+          frame_entry->referrer(), *frame_entry, entry,
+          FrameMsg_Navigate_Type::NORMAL, false, base::TimeTicks::Now(),
+          static_cast<NavigationControllerImpl*>(&controller()));
+  manager->DidCreateNavigationRequest(*navigation_request);
+
+  // The current WebUI should still be in place and there should be a new
+  // pending WebUI instance in the current RenderFrameHost.
+  EXPECT_FALSE(change_observer.DidHostChange());
+  EXPECT_FALSE(GetPendingFrameHost(manager));
+  EXPECT_EQ(web_ui, host->web_ui());
+
+  // Prepare to commit, update the navigating RenderFrameHost.
+  EXPECT_EQ(host, manager->GetFrameHostForNavigation(*navigation_request));
+
+  EXPECT_FALSE(GetPendingFrameHost(manager));
+  EXPECT_EQ(web_ui, host->web_ui());
+
+  // The RenderFrameHost committed.
+  manager->DidNavigateFrame(host, true);
+  EXPECT_FALSE(change_observer.DidHostChange());
+  EXPECT_EQ(web_ui, host->web_ui());
+}
+
+// PlzNavigate: Tests that the correct intermediary and final navigation states
+// are reached when navigating cross-site between two different WebUI types.
+TEST_F(RenderFrameHostManagerTestWithBrowserSideNavigation,
+       NavigateCrossSiteBetweenWebUIs) {
+  // Cross-site navigations will always cause the change of the WebUI instance
+  // but for consistency sake different types will be set for each navigation.
+  set_should_create_webui(true);
+  set_webui_type(1);
+  NavigateActiveAndCommit(GURL("chrome://foo"));
+
+  RenderFrameHostManager* manager = contents()->GetRenderManagerForTesting();
+  TestRenderFrameHost* host =
+      static_cast<TestRenderFrameHost*>(manager->current_frame_host());
+  EXPECT_TRUE(host->IsRenderFrameLive());
+  EXPECT_TRUE(host->web_ui());
+
+  RenderViewHostChangedObserver change_observer(contents());
+  set_webui_type(2);
+
+  // Navigation request.
+  const GURL kUrl("chrome://bar");
+  NavigationEntryImpl entry(nullptr /* instance */, -1 /* page_id */, kUrl,
+                            Referrer(), base::string16() /* title */,
+                            ui::PAGE_TRANSITION_TYPED,
+                            false /* is_renderer_init */);
+  FrameNavigationEntry* frame_entry = entry.root_node()->frame_entry.get();
+  scoped_ptr<NavigationRequest> navigation_request =
+      NavigationRequest::CreateBrowserInitiated(
+          contents()->GetFrameTree()->root(), frame_entry->url(),
+          frame_entry->referrer(), *frame_entry, entry,
+          FrameMsg_Navigate_Type::NORMAL, false, base::TimeTicks::Now(),
+          static_cast<NavigationControllerImpl*>(&controller()));
+  manager->DidCreateNavigationRequest(*navigation_request);
+
+  // The current WebUI should still be in place and there should be a new
+  // active WebUI instance in the speculative RenderFrameHost.
+  EXPECT_FALSE(change_observer.DidHostChange());
+  TestRenderFrameHost* speculative_host =
+      static_cast<TestRenderFrameHost*>(GetPendingFrameHost(manager));
+  EXPECT_TRUE(speculative_host);
+  EXPECT_TRUE(manager->current_frame_host()->web_ui());
+  WebUIImpl* next_web_ui = speculative_host->web_ui();
+  EXPECT_TRUE(next_web_ui);
+  EXPECT_NE(next_web_ui, manager->current_frame_host()->web_ui());
+
+  // Prepare to commit, update the navigating RenderFrameHost.
+  EXPECT_EQ(speculative_host,
+            manager->GetFrameHostForNavigation(*navigation_request));
+
+  EXPECT_EQ(speculative_host, GetPendingFrameHost(manager));
+  EXPECT_TRUE(manager->current_frame_host()->web_ui());
+  EXPECT_NE(next_web_ui, manager->current_frame_host()->web_ui());
+  EXPECT_EQ(next_web_ui, speculative_host->web_ui());
+
+  // The RenderFrameHost committed.
+  manager->DidNavigateFrame(speculative_host, true);
+  EXPECT_EQ(speculative_host, manager->current_frame_host());
+  EXPECT_TRUE(change_observer.DidHostChange());
+  EXPECT_EQ(next_web_ui, manager->current_frame_host()->web_ui());
+  EXPECT_FALSE(GetPendingFrameHost(manager));
+}
+
 }  // namespace content