Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(117)

Unified Diff: content/browser/frame_host/render_frame_host_manager_unittest.cc

Issue 1417953002: PlzNavigate: Add tests to check WebUI navigations. (Closed) Base URL: https://chromium.googlesource.com/chromium/src.git@master
Patch Set: Addressing CR comments (still needs a rebase). Created 5 years, 2 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
« no previous file with comments | « content/browser/frame_host/render_frame_host_manager.cc ('k') | no next file » | no next file with comments »
Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
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 bac4f6df8215295e4fe88f03dc88fe333798b34d..80354171be0952015b6e089d2963966b08cddd60 100644
--- a/content/browser/frame_host/render_frame_host_manager_unittest.cc
+++ b/content/browser/frame_host/render_frame_host_manager_unittest.cc
@@ -37,6 +37,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"
@@ -54,7 +55,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 {}
@@ -62,6 +64,17 @@ class RenderFrameHostManagerTestWebUIControllerFactory
should_create_webui_ = should_create_webui;
}
+ // This method allows simulating the expectation that different WebUI instance
clamy 2015/10/27 11:54:44 nit: s/allows simulating/simulates
carlosk 2015/11/12 10:40:39 Done. (it will show up on the other change I merge
+ // 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 {
@@ -77,8 +90,7 @@ class RenderFrameHostManagerTestWebUIControllerFactory
// 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 const_cast<RenderFrameHostManagerTestWebUIControllerFactory*>(
- this);
+ return reinterpret_cast<WebUI::TypeID>(type_);
}
return WebUI::kNoWebUI;
}
@@ -95,6 +107,7 @@ class RenderFrameHostManagerTestWebUIControllerFactory
private:
bool should_create_webui_;
+ uintptr_t type_;
DISALLOW_COPY_AND_ASSIGN(RenderFrameHostManagerTestWebUIControllerFactory);
};
@@ -272,6 +285,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
@@ -409,6 +424,13 @@ class RenderFrameHostManagerTest : public RenderViewHostImplTestHarness {
frame_entry->referrer(), *frame_entry, entry,
FrameMsg_Navigate_Type::NORMAL, false, base::TimeTicks::Now(),
static_cast<NavigationControllerImpl*>(&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);
@@ -1108,13 +1130,17 @@ 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_TRUE(manager->web_ui());
if (base::CommandLine::ForCurrentProcess()->HasSwitch(
switches::kEnableBrowserSideNavigation)) {
- EXPECT_FALSE(manager->speculative_web_ui());
+ // For PlzNavigate as GetFrameHostForNavigation is called twice in a row, a
+ // speculative WebUI will exist because of the 2nd call but it should just
+ // be the re-using of the current one that has been committed.
+ EXPECT_TRUE(manager->speculative_web_ui());
+ EXPECT_EQ(manager->web_ui(), manager->speculative_web_ui());
} else {
EXPECT_FALSE(manager->pending_web_ui());
}
- EXPECT_TRUE(manager->web_ui());
// Commit.
manager->DidNavigateFrame(host, true);
@@ -2488,4 +2514,200 @@ TEST_F(RenderFrameHostManagerTest, TraverseComplexOpenerChain) {
nodes_with_back_links.end());
}
+// 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());
+ 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(NULL /* 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);
+ EXPECT_FALSE(manager->speculative_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.
+ EXPECT_EQ(web_ui, host->web_ui());
+ EXPECT_EQ(web_ui, manager->speculative_web_ui());
+ EXPECT_EQ(web_ui, host->pending_web_ui());
+
+ // 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());
+ EXPECT_FALSE(manager->speculative_web_ui());
+ EXPECT_FALSE(host->pending_web_ui());
+}
+
+// PlzNavigate: Tests that the correct intermediary and final navigation states
+// are reached when navigating same-site between two different WebUI types.
+TEST_F(RenderFrameHostManagerTestWithBrowserSideNavigation,
+ NavigateSameSiteBetweenWebUIs) {
+ 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://foo/bar");
+ NavigationEntryImpl entry(NULL /* 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_TRUE(host->web_ui());
+ WebUIImpl* next_web_ui = manager->speculative_web_ui();
+ EXPECT_TRUE(next_web_ui);
+ EXPECT_NE(next_web_ui, host->web_ui());
+ EXPECT_EQ(next_web_ui, host->pending_web_ui());
+
+ // Prepare to commit, update the navigating RenderFrameHost.
+ EXPECT_EQ(host, manager->GetFrameHostForNavigation(*navigation_request));
+
+ EXPECT_FALSE(GetPendingFrameHost(manager));
+ EXPECT_TRUE(host->web_ui());
+ EXPECT_NE(next_web_ui, host->web_ui());
+ EXPECT_EQ(next_web_ui, manager->speculative_web_ui());
+ EXPECT_EQ(next_web_ui, host->pending_web_ui());
+
+ // The RenderFrameHost committed.
+ manager->DidNavigateFrame(host, true);
+ EXPECT_FALSE(change_observer.DidHostChange());
+ EXPECT_EQ(next_web_ui, host->web_ui());
+ EXPECT_FALSE(manager->speculative_web_ui());
+ EXPECT_FALSE(host->pending_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(NULL /* 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 = manager->speculative_web_ui();
+ EXPECT_TRUE(next_web_ui);
+ EXPECT_NE(next_web_ui, manager->current_frame_host()->web_ui());
+ EXPECT_EQ(next_web_ui, speculative_host->web_ui());
+ EXPECT_FALSE(manager->current_frame_host()->pending_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, manager->speculative_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(manager->speculative_web_ui());
+ EXPECT_FALSE(speculative_host->pending_web_ui());
+}
+
} // namespace content
« no previous file with comments | « content/browser/frame_host/render_frame_host_manager.cc ('k') | no next file » | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698