Commit 085854f7 authored by carlosk's avatar carlosk Committed by Commit bot

PlzNavigate: change behavior when navigating with a non-live RFH.

To fix a few tests the current behavior when navigating from a non-live RFH was changed so that the new, speculative one is immediately set as active matching what is done in RenderFrameHostManager::UpdateStateForNavigate. This replaced the behavior of simply bringing the current RFH back to life.

With this change 6 tests are fixed for PlzNavigate:

    RenderFrameHostManagerTest.WebUI
    WebContentsImplTest.NTPViewSource
    RenderFrameHostManagerTest.NewTabPageProcesses
    WebContentsImplTest.CrossSiteBoundariesAfterCrash
    WebContentsImplTest.CrossSiteComparesAgainstCurrentPage
    WebContentsImplTest.ShowInterstitialFromBrowserNewNavigationProceed

BUG=439423

Review URL: https://codereview.chromium.org/892123002

Cr-Commit-Position: refs/heads/master@{#314543}
parent 947ebfb6
......@@ -716,17 +716,6 @@ RenderFrameHostImpl* RenderFrameHostManager::GetFrameHostForNavigation(
CleanUpNavigation();
navigation_rfh = render_frame_host_.get();
} else {
// If the current render_frame_host_ isn't live, we should create it so
// that we don't show a sad tab while the navigation is ongoing.
// (Bug 1145340)
if (!render_frame_host_->IsRenderFrameLive()) {
// Note: we don't call InitRenderView here because we are navigating away
// soon anyway, and we don't have the NavigationEntry for this host.
delegate_->CreateRenderViewForRenderManager(
render_frame_host_->render_view_host(), MSG_ROUTING_NONE,
MSG_ROUTING_NONE, frame_tree_node_->IsMainFrame());
}
// If the SiteInstance for the final URL doesn't match the one from the
// speculatively created RenderFrameHost, create a new RenderFrameHost using
// this new SiteInstance.
......@@ -741,6 +730,16 @@ RenderFrameHostImpl* RenderFrameHostManager::GetFrameHostForNavigation(
}
DCHECK(speculative_render_frame_host_);
navigation_rfh = speculative_render_frame_host_.get();
// Check if our current RFH is live.
if (!render_frame_host_->IsRenderFrameLive()) {
// The current RFH is not live. There's no reason to sit around with a
// sad tab or a newly created RFH while we wait for the navigation to
// complete. Just switch to the speculative RFH now and go back to non
// cross-navigating (Note that we don't care about on{before}unload
// handlers if the current RFH isn't live.)
CommitPending();
}
}
DCHECK(navigation_rfh &&
(navigation_rfh == render_frame_host_.get() ||
......
......@@ -234,6 +234,13 @@ class CONTENT_EXPORT RenderFrameHostManager : public NotificationObserver {
pending_and_current_web_ui_.get();
}
// PlzNavigate
// Returns the speculative WebUI for the navigation (a newly created one or
// the current one if it should be reused). If none is set returns nullptr.
WebUIImpl* speculative_web_ui_for_testing() const {
return should_reuse_web_ui_ ? web_ui_.get() : speculative_web_ui_.get();
}
// Called when we want to instruct the renderer to navigate to the given
// navigation entry. It may create a new RenderFrameHost or re-use an existing
// one. The RenderFrameHost to navigate will be returned. Returns NULL if one
......
......@@ -941,8 +941,11 @@ TEST_F(RenderFrameHostManagerTest, WebUI) {
scoped_ptr<TestWebContents> web_contents(
TestWebContents::Create(browser_context(), instance));
RenderFrameHostManager* manager = web_contents->GetRenderManagerForTesting();
RenderFrameHostImpl* initial_rfh = manager->current_frame_host();
EXPECT_FALSE(manager->current_host()->IsRenderViewLive());
EXPECT_FALSE(manager->web_ui());
EXPECT_TRUE(initial_rfh);
const GURL kUrl("chrome://foo");
NavigationEntryImpl entry(NULL /* instance */, -1 /* page_id */, kUrl,
......@@ -955,6 +958,7 @@ TEST_F(RenderFrameHostManagerTest, WebUI) {
// RenderFrameHost was not live. We test a case where it is live in
// WebUIInNewTab.
EXPECT_TRUE(host);
EXPECT_NE(initial_rfh, host);
EXPECT_EQ(host, manager->current_frame_host());
EXPECT_FALSE(GetPendingFrameHost(manager));
......@@ -967,7 +971,12 @@ TEST_F(RenderFrameHostManagerTest, WebUI) {
// The Web UI is committed immediately because the RenderViewHost has not been
// used yet. UpdateStateForNavigate() took the short cut path.
EXPECT_FALSE(manager->pending_web_ui());
if (base::CommandLine::ForCurrentProcess()->HasSwitch(
switches::kEnableBrowserSideNavigation)) {
EXPECT_FALSE(manager->speculative_web_ui_for_testing());
} else {
EXPECT_FALSE(manager->pending_web_ui());
}
EXPECT_TRUE(manager->web_ui());
// Commit.
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment