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

Unified Diff: trunk/src/content/browser/web_contents/navigation_controller_impl_unittest.cc

Issue 13966012: Revert 195553 "Allow showing pending URL for new tab navigations..." (Closed) Base URL: svn://svn.chromium.org/chrome/
Patch Set: Created 7 years, 8 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
Index: trunk/src/content/browser/web_contents/navigation_controller_impl_unittest.cc
===================================================================
--- trunk/src/content/browser/web_contents/navigation_controller_impl_unittest.cc (revision 195969)
+++ trunk/src/content/browser/web_contents/navigation_controller_impl_unittest.cc (working copy)
@@ -812,7 +812,10 @@
EXPECT_FALSE(contents()->GetDelegate());
contents()->SetDelegate(delegate.get());
- // Start with a pending new navigation.
+ // Without any navigations, the renderer starts at about:blank.
+ const GURL kExistingURL("about:blank");
+
+ // Now make a pending new navigation.
const GURL kNewURL("http://eh");
controller.LoadURL(
kNewURL, Referrer(), PAGE_TRANSITION_TYPED, std::string());
@@ -841,15 +844,7 @@
EXPECT_TRUE(controller.GetPendingEntry());
EXPECT_EQ(-1, controller.GetLastCommittedEntryIndex());
EXPECT_EQ(1, delegate->navigation_state_change_count());
- NavigationEntry* pending_entry = controller.GetPendingEntry();
- // Ensure that a reload keeps the same pending entry.
- controller.Reload(true);
- EXPECT_EQ(-1, controller.GetPendingEntryIndex());
- EXPECT_TRUE(controller.GetPendingEntry());
- EXPECT_EQ(pending_entry, controller.GetPendingEntry());
- EXPECT_EQ(-1, controller.GetLastCommittedEntryIndex());
-
contents()->SetDelegate(NULL);
}
@@ -860,21 +855,16 @@
TestNotificationTracker notifications;
RegisterForAllNavNotifications(&notifications, &controller);
- // First make an existing committed entry.
- const GURL kExistingURL("http://foo/eh");
- controller.LoadURL(kExistingURL, content::Referrer(),
- content::PAGE_TRANSITION_TYPED, std::string());
- test_rvh()->SendNavigate(0, kExistingURL);
- EXPECT_TRUE(notifications.Check1AndReset(
- content::NOTIFICATION_NAV_ENTRY_COMMITTED));
-
// Set a WebContentsDelegate to listen for state changes.
scoped_ptr<TestWebContentsDelegate> delegate(new TestWebContentsDelegate());
EXPECT_FALSE(contents()->GetDelegate());
contents()->SetDelegate(delegate.get());
+ // Without any navigations, the renderer starts at about:blank.
+ const GURL kExistingURL("about:blank");
+
// Now make a pending new navigation, initiated by the renderer.
- const GURL kNewURL("http://foo/bee");
+ const GURL kNewURL("http://eh");
NavigationController::LoadURLParams load_url_params(kNewURL);
load_url_params.transition_type = PAGE_TRANSITION_TYPED;
load_url_params.is_renderer_initiated = true;
@@ -882,14 +872,16 @@
EXPECT_EQ(0U, notifications.size());
EXPECT_EQ(-1, controller.GetPendingEntryIndex());
EXPECT_TRUE(controller.GetPendingEntry());
- EXPECT_EQ(0, controller.GetLastCommittedEntryIndex());
- EXPECT_EQ(0, delegate->navigation_state_change_count());
+ EXPECT_EQ(-1, controller.GetLastCommittedEntryIndex());
+ EXPECT_EQ(1, delegate->navigation_state_change_count());
- // The visible entry should be the last committed URL, not the pending one.
- EXPECT_EQ(kExistingURL, controller.GetVisibleEntry()->GetURL());
+ // There should be no visible entry (resulting in about:blank in the
+ // omnibox), because it was renderer-initiated and there's no last committed
+ // entry.
+ EXPECT_FALSE(controller.GetVisibleEntry());
// Now the navigation redirects.
- const GURL kRedirectURL("http://foo/see");
+ const GURL kRedirectURL("http://bee");
test_rvh()->OnMessageReceived(
ViewHostMsg_DidRedirectProvisionalLoad(0, // routing_id
-1, // pending page_id
@@ -917,12 +909,12 @@
// change.
EXPECT_EQ(-1, controller.GetPendingEntryIndex());
EXPECT_TRUE(controller.GetPendingEntry());
- EXPECT_EQ(0, controller.GetLastCommittedEntryIndex());
- EXPECT_EQ(0, delegate->navigation_state_change_count());
+ EXPECT_EQ(-1, controller.GetLastCommittedEntryIndex());
+ EXPECT_EQ(1, delegate->navigation_state_change_count());
- // The visible entry should be the last committed URL, not the pending one,
- // so that no spoof is possible.
- EXPECT_EQ(kExistingURL, controller.GetVisibleEntry()->GetURL());
+ // There should be no visible entry (resulting in about:blank in the
+ // omnibox), ensuring no spoof is possible.
+ EXPECT_FALSE(controller.GetVisibleEntry());
contents()->SetDelegate(NULL);
}
@@ -2499,86 +2491,6 @@
notifications.Reset();
}
-// Tests that the URLs for renderer-initiated navigations in new tabs are
-// displayed to the user before commit, as long as the initial about:blank
-// page has not been modified. If so, we must revert to showing about:blank.
-// See http://crbug.com/9682.
-TEST_F(NavigationControllerTest, ShowRendererURLInNewTabUntilModified) {
- NavigationControllerImpl& controller = controller_impl();
- TestNotificationTracker notifications;
- RegisterForAllNavNotifications(&notifications, &controller);
-
- const GURL url("http://foo");
-
- // For renderer-initiated navigations in new tabs (with no committed entries),
- // we show the pending entry's URL as long as the about:blank page is not
- // modified.
- NavigationController::LoadURLParams load_url_params(url);
- load_url_params.transition_type = PAGE_TRANSITION_LINK;
- load_url_params.is_renderer_initiated = true;
- controller.LoadURLWithParams(load_url_params);
- EXPECT_EQ(url, controller.GetActiveEntry()->GetURL());
- EXPECT_EQ(url, controller.GetVisibleEntry()->GetURL());
- EXPECT_TRUE(
- NavigationEntryImpl::FromNavigationEntry(controller.GetPendingEntry())->
- is_renderer_initiated());
- EXPECT_TRUE(controller.IsInitialNavigation());
- EXPECT_FALSE(test_rvh()->has_accessed_initial_document());
-
- // There should be no title yet.
- EXPECT_TRUE(contents()->GetTitle().empty());
-
- // If something else modifies the contents of the about:blank page, then
- // we must revert to showing about:blank to avoid a URL spoof.
- test_rvh()->OnMessageReceived(
- ViewHostMsg_DidAccessInitialDocument(0));
- EXPECT_TRUE(test_rvh()->has_accessed_initial_document());
- EXPECT_FALSE(controller.GetVisibleEntry());
- EXPECT_EQ(url, controller.GetActiveEntry()->GetURL());
-
- notifications.Reset();
-}
-
-TEST_F(NavigationControllerTest, DontShowRendererURLInNewTabAfterCommit) {
- NavigationControllerImpl& controller = controller_impl();
- TestNotificationTracker notifications;
- RegisterForAllNavNotifications(&notifications, &controller);
-
- const GURL url1("http://foo/eh");
- const GURL url2("http://foo/bee");
-
- // For renderer-initiated navigations in new tabs (with no committed entries),
- // we show the pending entry's URL as long as the about:blank page is not
- // modified.
- NavigationController::LoadURLParams load_url_params(url1);
- load_url_params.transition_type = PAGE_TRANSITION_LINK;
- load_url_params.is_renderer_initiated = true;
- controller.LoadURLWithParams(load_url_params);
- EXPECT_EQ(url1, controller.GetActiveEntry()->GetURL());
- EXPECT_EQ(url1, controller.GetVisibleEntry()->GetURL());
- EXPECT_TRUE(
- NavigationEntryImpl::FromNavigationEntry(controller.GetPendingEntry())->
- is_renderer_initiated());
- EXPECT_TRUE(controller.IsInitialNavigation());
- EXPECT_FALSE(test_rvh()->has_accessed_initial_document());
-
- // Simulate a commit and then starting a new pending navigation.
- test_rvh()->SendNavigate(0, url1);
- NavigationController::LoadURLParams load_url2_params(url2);
- load_url2_params.transition_type = PAGE_TRANSITION_LINK;
- load_url2_params.is_renderer_initiated = true;
- controller.LoadURLWithParams(load_url2_params);
-
- // We should not consider this an initial navigation, and thus should
- // not show the pending URL.
- EXPECT_FALSE(test_rvh()->has_accessed_initial_document());
- EXPECT_FALSE(controller.IsInitialNavigation());
- EXPECT_TRUE(controller.GetVisibleEntry());
- EXPECT_EQ(url1, controller.GetVisibleEntry()->GetURL());
-
- notifications.Reset();
-}
-
// Tests that IsInPageNavigation returns appropriate results. Prevents
// regression for bug 1126349.
TEST_F(NavigationControllerTest, IsInPageNavigation) {
@@ -2644,10 +2556,8 @@
NavigationControllerImpl& controller = controller_impl();
const GURL url1("http://foo1");
const GURL url2("http://foo2");
- const string16 title(ASCIIToUTF16("Title"));
NavigateAndCommit(url1);
- controller.GetActiveEntry()->SetTitle(title);
NavigateAndCommit(url2);
scoped_ptr<WebContents> clone(controller.GetWebContents()->Clone());
@@ -2657,10 +2567,6 @@
clone->GetController().GoBack();
// Navigating back should have triggered needs_reload_ to go false.
EXPECT_FALSE(clone->GetController().NeedsReload());
-
- // Ensure that the pending URL and its title are visible.
- EXPECT_EQ(url1, clone->GetController().GetVisibleEntry()->GetURL());
- EXPECT_EQ(title, clone->GetTitle());
}
// Make sure that cloning a WebContentsImpl doesn't copy interstitials.

Powered by Google App Engine
This is Rietveld 408576698