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

Issues Search
    My Issues | Starred     Open | Closed | All

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

Issue 556703004: Remove page id from FrameNavigateParams. (Closed) Base URL: https://chromium.googlesource.com/chromium/src.git@master
Patch Set: cleaner Created 6 years, 3 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/navigation_controller_impl.cc ('k') | content/browser/frame_host/navigator.h » ('j') | content/browser/frame_host/render_frame_host_impl.cc » ('J')
Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
Index: content/browser/frame_host/navigation_controller_impl_unittest.cc
diff --git a/content/browser/frame_host/navigation_controller_impl_unittest.cc b/content/browser/frame_host/navigation_controller_impl_unittest.cc
index b58343b1ecf001471bc90b44dcb6765165c12283..4a3ac86bf1e3207a61e27ab72e6ba3908ea0cd2f 100644
--- a/content/browser/frame_host/navigation_controller_impl_unittest.cc
+++ b/content/browser/frame_host/navigation_controller_impl_unittest.cc
@@ -632,13 +632,12 @@ TEST_F(NavigationControllerTest, LoadURL_SamePage_DifferentMethod) {
controller.LoadURL(url1, Referrer(), PAGE_TRANSITION_TYPED, std::string());
FrameHostMsg_DidCommitProvisionalLoad_Params params;
- params.page_id = 0;
params.url = url1;
params.transition = PAGE_TRANSITION_TYPED;
params.is_post = true;
params.post_id = 123;
params.page_state = PageState::CreateForTesting(url1, false, 0, 0);
- main_test_rfh()->SendNavigateWithParams(&params);
+ main_test_rfh()->SendNavigateWithParams(0, &params);
// The post data should be visible.
NavigationEntry* entry = controller.GetVisibleEntry();
@@ -1031,7 +1030,7 @@ TEST_F(NavigationControllerTest, LoadURL_RedirectAbortDoesntShowPendingURL) {
const GURL kRedirectURL("http://foo/see");
main_test_rfh()->OnMessageReceived(
FrameHostMsg_DidRedirectProvisionalLoad(0, // routing_id
- -1, // pending page_id
+ 1, // pending page_id
kNewURL, // old url
kRedirectURL)); // new url
@@ -1693,7 +1692,6 @@ TEST_F(NavigationControllerTest, Redirect) {
EXPECT_EQ(url1, controller.GetVisibleEntry()->GetURL());
FrameHostMsg_DidCommitProvisionalLoad_Params params;
- params.page_id = 0;
params.url = url2;
params.transition = PAGE_TRANSITION_SERVER_REDIRECT;
params.redirects.push_back(GURL("http://foo1"));
@@ -1706,7 +1704,7 @@ TEST_F(NavigationControllerTest, Redirect) {
LoadCommittedDetails details;
EXPECT_EQ(0U, notifications.size());
- EXPECT_TRUE(controller.RendererDidNavigate(main_test_rfh(), params,
+ EXPECT_TRUE(controller.RendererDidNavigate(main_test_rfh(), 0, params,
&details));
EXPECT_EQ(1U, navigation_entry_committed_counter_);
navigation_entry_committed_counter_ = 0;
@@ -1751,7 +1749,6 @@ TEST_F(NavigationControllerTest, PostThenRedirect) {
EXPECT_EQ(url1, controller.GetVisibleEntry()->GetURL());
FrameHostMsg_DidCommitProvisionalLoad_Params params;
- params.page_id = 0;
params.url = url2;
params.transition = PAGE_TRANSITION_SERVER_REDIRECT;
params.redirects.push_back(GURL("http://foo1"));
@@ -1764,7 +1761,7 @@ TEST_F(NavigationControllerTest, PostThenRedirect) {
LoadCommittedDetails details;
EXPECT_EQ(0U, notifications.size());
- EXPECT_TRUE(controller.RendererDidNavigate(main_test_rfh(), params,
+ EXPECT_TRUE(controller.RendererDidNavigate(main_test_rfh(), 0, params,
&details));
EXPECT_EQ(1U, navigation_entry_committed_counter_);
navigation_entry_committed_counter_ = 0;
@@ -1799,7 +1796,6 @@ TEST_F(NavigationControllerTest, ImmediateRedirect) {
EXPECT_EQ(url1, controller.GetVisibleEntry()->GetURL());
FrameHostMsg_DidCommitProvisionalLoad_Params params;
- params.page_id = 0;
params.url = url2;
params.transition = PAGE_TRANSITION_SERVER_REDIRECT;
params.redirects.push_back(GURL("http://foo1"));
@@ -1812,7 +1808,7 @@ TEST_F(NavigationControllerTest, ImmediateRedirect) {
LoadCommittedDetails details;
EXPECT_EQ(0U, notifications.size());
- EXPECT_TRUE(controller.RendererDidNavigate(main_test_rfh(), params,
+ EXPECT_TRUE(controller.RendererDidNavigate(main_test_rfh(), 0, params,
&details));
EXPECT_EQ(1U, navigation_entry_committed_counter_);
navigation_entry_committed_counter_ = 0;
@@ -1843,7 +1839,6 @@ TEST_F(NavigationControllerTest, NewSubframe) {
const GURL url2("http://foo2");
FrameHostMsg_DidCommitProvisionalLoad_Params params;
- params.page_id = 1;
params.url = url2;
params.transition = PAGE_TRANSITION_MANUAL_SUBFRAME;
params.should_update_history = false;
@@ -1852,7 +1847,7 @@ TEST_F(NavigationControllerTest, NewSubframe) {
params.page_state = PageState::CreateFromURL(url2);
LoadCommittedDetails details;
- EXPECT_TRUE(controller.RendererDidNavigate(main_test_rfh(), params,
+ EXPECT_TRUE(controller.RendererDidNavigate(main_test_rfh(), 1, params,
&details));
EXPECT_EQ(1U, navigation_entry_committed_counter_);
navigation_entry_committed_counter_ = 0;
@@ -1866,7 +1861,7 @@ TEST_F(NavigationControllerTest, NewSubframe) {
// New entry should refer to the new page, but the old URL (entries only
// reflect the toplevel URL).
EXPECT_EQ(url1, details.entry->GetURL());
- EXPECT_EQ(params.page_id, details.entry->GetPageID());
+ EXPECT_EQ(1, details.entry->GetPageID());
}
// Some pages create a popup, then write an iframe into it. This causes a
@@ -1880,7 +1875,6 @@ TEST_F(NavigationControllerTest, SubframeOnEmptyPage) {
// Navigation controller currently has no entries.
const GURL url("http://foo2");
FrameHostMsg_DidCommitProvisionalLoad_Params params;
- params.page_id = 1;
params.url = url;
params.transition = PAGE_TRANSITION_AUTO_SUBFRAME;
params.should_update_history = false;
@@ -1889,7 +1883,7 @@ TEST_F(NavigationControllerTest, SubframeOnEmptyPage) {
params.page_state = PageState::CreateFromURL(url);
LoadCommittedDetails details;
- EXPECT_FALSE(controller.RendererDidNavigate(main_test_rfh(), params,
+ EXPECT_FALSE(controller.RendererDidNavigate(main_test_rfh(), 1, params,
&details));
EXPECT_EQ(0U, notifications.size());
}
@@ -1908,7 +1902,6 @@ TEST_F(NavigationControllerTest, AutoSubframe) {
const GURL url2("http://foo2");
FrameHostMsg_DidCommitProvisionalLoad_Params params;
- params.page_id = 0;
params.url = url2;
params.transition = PAGE_TRANSITION_AUTO_SUBFRAME;
params.should_update_history = false;
@@ -1918,7 +1911,7 @@ TEST_F(NavigationControllerTest, AutoSubframe) {
// Navigating should do nothing.
LoadCommittedDetails details;
- EXPECT_FALSE(controller.RendererDidNavigate(main_test_rfh(), params,
+ EXPECT_FALSE(controller.RendererDidNavigate(main_test_rfh(), 0, params,
&details));
EXPECT_EQ(0U, notifications.size());
@@ -1941,7 +1934,6 @@ TEST_F(NavigationControllerTest, BackSubframe) {
// First manual subframe navigation.
const GURL url2("http://foo2");
FrameHostMsg_DidCommitProvisionalLoad_Params params;
- params.page_id = 1;
params.url = url2;
params.transition = PAGE_TRANSITION_MANUAL_SUBFRAME;
params.should_update_history = false;
@@ -1951,7 +1943,7 @@ TEST_F(NavigationControllerTest, BackSubframe) {
// This should generate a new entry.
LoadCommittedDetails details;
- EXPECT_TRUE(controller.RendererDidNavigate(main_test_rfh(), params,
+ EXPECT_TRUE(controller.RendererDidNavigate(main_test_rfh(), 1, params,
&details));
EXPECT_EQ(1U, navigation_entry_committed_counter_);
navigation_entry_committed_counter_ = 0;
@@ -1959,9 +1951,8 @@ TEST_F(NavigationControllerTest, BackSubframe) {
// Second manual subframe navigation should also make a new entry.
const GURL url3("http://foo3");
- params.page_id = 2;
params.url = url3;
- EXPECT_TRUE(controller.RendererDidNavigate(main_test_rfh(), params,
+ EXPECT_TRUE(controller.RendererDidNavigate(main_test_rfh(), 2, params,
&details));
EXPECT_EQ(1U, navigation_entry_committed_counter_);
navigation_entry_committed_counter_ = 0;
@@ -1971,8 +1962,7 @@ TEST_F(NavigationControllerTest, BackSubframe) {
// Go back one.
controller.GoBack();
params.url = url2;
- params.page_id = 1;
- EXPECT_TRUE(controller.RendererDidNavigate(main_test_rfh(), params,
+ EXPECT_TRUE(controller.RendererDidNavigate(main_test_rfh(), 1, params,
&details));
EXPECT_EQ(1U, navigation_entry_committed_counter_);
navigation_entry_committed_counter_ = 0;
@@ -1984,8 +1974,7 @@ TEST_F(NavigationControllerTest, BackSubframe) {
// Go back one more.
controller.GoBack();
params.url = url1;
- params.page_id = 0;
- EXPECT_TRUE(controller.RendererDidNavigate(main_test_rfh(), params,
+ EXPECT_TRUE(controller.RendererDidNavigate(main_test_rfh(), 0, params,
&details));
EXPECT_EQ(1U, navigation_entry_committed_counter_);
navigation_entry_committed_counter_ = 0;
@@ -2035,7 +2024,6 @@ TEST_F(NavigationControllerTest, InPage) {
// Ensure main page navigation to same url respects the was_within_same_page
// hint provided in the params.
FrameHostMsg_DidCommitProvisionalLoad_Params self_params;
- self_params.page_id = 0;
self_params.url = url1;
self_params.transition = PAGE_TRANSITION_LINK;
self_params.should_update_history = false;
@@ -2045,7 +2033,7 @@ TEST_F(NavigationControllerTest, InPage) {
self_params.was_within_same_page = true;
LoadCommittedDetails details;
- EXPECT_TRUE(controller.RendererDidNavigate(main_test_rfh(), self_params,
+ EXPECT_TRUE(controller.RendererDidNavigate(main_test_rfh(), 0, self_params,
&details));
EXPECT_EQ(1U, navigation_entry_committed_counter_);
navigation_entry_committed_counter_ = 0;
@@ -2056,7 +2044,6 @@ TEST_F(NavigationControllerTest, InPage) {
// Fragment navigation to a new page_id.
const GURL url2("http://foo#a");
FrameHostMsg_DidCommitProvisionalLoad_Params params;
- params.page_id = 1;
params.url = url2;
params.transition = PAGE_TRANSITION_LINK;
params.should_update_history = false;
@@ -2066,7 +2053,7 @@ TEST_F(NavigationControllerTest, InPage) {
params.was_within_same_page = true;
// This should generate a new entry.
- EXPECT_TRUE(controller.RendererDidNavigate(main_test_rfh(), params,
+ EXPECT_TRUE(controller.RendererDidNavigate(main_test_rfh(), 1, params,
&details));
EXPECT_EQ(1U, navigation_entry_committed_counter_);
navigation_entry_committed_counter_ = 0;
@@ -2078,8 +2065,7 @@ TEST_F(NavigationControllerTest, InPage) {
FrameHostMsg_DidCommitProvisionalLoad_Params back_params(params);
controller.GoBack();
back_params.url = url1;
- back_params.page_id = 0;
- EXPECT_TRUE(controller.RendererDidNavigate(main_test_rfh(), back_params,
+ EXPECT_TRUE(controller.RendererDidNavigate(main_test_rfh(), 0, back_params,
&details));
EXPECT_EQ(1U, navigation_entry_committed_counter_);
navigation_entry_committed_counter_ = 0;
@@ -2092,8 +2078,7 @@ TEST_F(NavigationControllerTest, InPage) {
FrameHostMsg_DidCommitProvisionalLoad_Params forward_params(params);
controller.GoForward();
forward_params.url = url2;
- forward_params.page_id = 1;
- EXPECT_TRUE(controller.RendererDidNavigate(main_test_rfh(), forward_params,
+ EXPECT_TRUE(controller.RendererDidNavigate(main_test_rfh(), 1, forward_params,
&details));
EXPECT_EQ(1U, navigation_entry_committed_counter_);
navigation_entry_committed_counter_ = 0;
@@ -2108,20 +2093,19 @@ TEST_F(NavigationControllerTest, InPage) {
// one identified by an existing page ID. This would result in the second URL
// losing the reference fragment when you navigate away from it and then back.
controller.GoBack();
- EXPECT_TRUE(controller.RendererDidNavigate(main_test_rfh(), back_params,
+ EXPECT_TRUE(controller.RendererDidNavigate(main_test_rfh(), 0, back_params,
&details));
controller.GoForward();
- EXPECT_TRUE(controller.RendererDidNavigate(main_test_rfh(), forward_params,
+ EXPECT_TRUE(controller.RendererDidNavigate(main_test_rfh(), 1, forward_params,
&details));
EXPECT_EQ(forward_params.url,
controller.GetVisibleEntry()->GetURL());
// Finally, navigate to an unrelated URL to make sure in_page is not sticky.
const GURL url3("http://bar");
- params.page_id = 2;
params.url = url3;
navigation_entry_committed_counter_ = 0;
- EXPECT_TRUE(controller.RendererDidNavigate(main_test_rfh(), params,
+ EXPECT_TRUE(controller.RendererDidNavigate(main_test_rfh(), 2, params,
&details));
EXPECT_EQ(1U, navigation_entry_committed_counter_);
navigation_entry_committed_counter_ = 0;
@@ -2144,7 +2128,6 @@ TEST_F(NavigationControllerTest, InPage_Replace) {
// First navigation.
const GURL url2("http://foo#a");
FrameHostMsg_DidCommitProvisionalLoad_Params params;
- params.page_id = 0; // Same page_id
params.url = url2;
params.transition = PAGE_TRANSITION_LINK;
params.should_update_history = false;
@@ -2155,8 +2138,9 @@ TEST_F(NavigationControllerTest, InPage_Replace) {
// This should NOT generate a new entry, nor prune the list.
LoadCommittedDetails details;
- EXPECT_TRUE(controller.RendererDidNavigate(main_test_rfh(), params,
- &details));
+ EXPECT_TRUE(controller.RendererDidNavigate(main_test_rfh(),
+ 0, /* same page_id */
+ params, &details));
EXPECT_EQ(1U, navigation_entry_committed_counter_);
navigation_entry_committed_counter_ = 0;
EXPECT_TRUE(details.is_in_page);
@@ -2195,7 +2179,6 @@ TEST_F(NavigationControllerTest, ClientRedirectAfterInPageNavigation) {
{
const GURL url("http://foo2/#a");
FrameHostMsg_DidCommitProvisionalLoad_Params params;
- params.page_id = 1; // Same page_id
params.url = url;
params.transition = PAGE_TRANSITION_LINK;
params.redirects.push_back(url);
@@ -2207,8 +2190,9 @@ TEST_F(NavigationControllerTest, ClientRedirectAfterInPageNavigation) {
// This should NOT generate a new entry, nor prune the list.
LoadCommittedDetails details;
- EXPECT_TRUE(controller.RendererDidNavigate(main_test_rfh(), params,
- &details));
+ EXPECT_TRUE(controller.RendererDidNavigate(main_test_rfh(),
+ 1, /* same page_id */
+ params, &details));
EXPECT_EQ(1U, navigation_entry_committed_counter_);
navigation_entry_committed_counter_ = 0;
EXPECT_TRUE(details.is_in_page);
@@ -2220,7 +2204,6 @@ TEST_F(NavigationControllerTest, ClientRedirectAfterInPageNavigation) {
{
const GURL url("http://foo3/");
FrameHostMsg_DidCommitProvisionalLoad_Params params;
- params.page_id = 2; // New page_id
params.url = url;
params.transition = PAGE_TRANSITION_CLIENT_REDIRECT;
params.redirects.push_back(GURL("http://foo2/#a"));
@@ -2232,8 +2215,9 @@ TEST_F(NavigationControllerTest, ClientRedirectAfterInPageNavigation) {
// This SHOULD generate a new entry.
LoadCommittedDetails details;
- EXPECT_TRUE(controller.RendererDidNavigate(main_test_rfh(), params,
- &details));
+ EXPECT_TRUE(controller.RendererDidNavigate(main_test_rfh(),
+ 2, /* new page_id */
+ params, &details));
EXPECT_EQ(1U, navigation_entry_committed_counter_);
navigation_entry_committed_counter_ = 0;
EXPECT_FALSE(details.is_in_page);
@@ -2256,11 +2240,10 @@ TEST_F(NavigationControllerTest, PushStateWithoutPreviousEntry)
ASSERT_FALSE(controller_impl().GetLastCommittedEntry());
FrameHostMsg_DidCommitProvisionalLoad_Params params;
GURL url("http://foo");
- params.page_id = 1;
params.url = url;
params.page_state = PageState::CreateFromURL(url);
params.was_within_same_page = true;
- test_rvh()->SendNavigateWithParams(&params);
+ test_rvh()->SendNavigateWithParams(1, &params);
// We pass if we don't crash.
}
@@ -2401,7 +2384,6 @@ TEST_F(NavigationControllerTest, RestoreNavigate) {
// Say we navigated to that entry.
FrameHostMsg_DidCommitProvisionalLoad_Params params;
- params.page_id = 0;
params.url = url;
params.transition = PAGE_TRANSITION_LINK;
params.should_update_history = false;
@@ -2409,7 +2391,7 @@ TEST_F(NavigationControllerTest, RestoreNavigate) {
params.is_post = false;
params.page_state = PageState::CreateFromURL(url);
LoadCommittedDetails details;
- our_controller.RendererDidNavigate(our_contents->GetMainFrame(), params,
+ our_controller.RendererDidNavigate(our_contents->GetMainFrame(), 0, params,
&details);
// There should be no longer any pending entry and one committed one. This
@@ -2484,7 +2466,6 @@ TEST_F(NavigationControllerTest, RestoreNavigateAfterFailure) {
// Now the pending restored entry commits.
FrameHostMsg_DidCommitProvisionalLoad_Params params;
- params.page_id = 0;
params.url = url;
params.transition = PAGE_TRANSITION_LINK;
params.should_update_history = false;
@@ -2492,7 +2473,7 @@ TEST_F(NavigationControllerTest, RestoreNavigateAfterFailure) {
params.is_post = false;
params.page_state = PageState::CreateFromURL(url);
LoadCommittedDetails details;
- our_controller.RendererDidNavigate(our_contents->GetMainFrame(), params,
+ our_controller.RendererDidNavigate(our_contents->GetMainFrame(), 0, params,
&details);
// There should be no pending entry and one committed one.
@@ -3146,7 +3127,6 @@ TEST_F(NavigationControllerTest, SameSubframe) {
// Navigate a subframe that would normally count as in-page.
const GURL subframe("http://www.google.com/#");
FrameHostMsg_DidCommitProvisionalLoad_Params params;
- params.page_id = 0;
params.url = subframe;
params.transition = PAGE_TRANSITION_AUTO_SUBFRAME;
params.should_update_history = false;
@@ -3154,7 +3134,7 @@ TEST_F(NavigationControllerTest, SameSubframe) {
params.is_post = false;
params.page_state = PageState::CreateFromURL(subframe);
LoadCommittedDetails details;
- EXPECT_FALSE(controller.RendererDidNavigate(main_test_rfh(), params,
+ EXPECT_FALSE(controller.RendererDidNavigate(main_test_rfh(), 0, params,
&details));
// Nothing should have changed.
@@ -3262,7 +3242,6 @@ TEST_F(NavigationControllerTest, SubframeWhilePending) {
// automatically loaded. Auto subframes don't increment the page ID.
const GURL url1_sub("http://foo/subframe");
FrameHostMsg_DidCommitProvisionalLoad_Params params;
- params.page_id = controller.GetLastCommittedEntry()->GetPageID();
params.url = url1_sub;
params.transition = PAGE_TRANSITION_AUTO_SUBFRAME;
params.should_update_history = false;
@@ -3272,8 +3251,9 @@ TEST_F(NavigationControllerTest, SubframeWhilePending) {
LoadCommittedDetails details;
// This should return false meaning that nothing was actually updated.
- EXPECT_FALSE(controller.RendererDidNavigate(main_test_rfh(), params,
- &details));
+ EXPECT_FALSE(controller.RendererDidNavigate(
+ main_test_rfh(), controller.GetLastCommittedEntry()->GetPageID(), params,
+ &details));
// The notification should have updated the last committed one, and not
// the pending load.
@@ -4240,11 +4220,10 @@ TEST_F(NavigationControllerTest, PushStateUpdatesTitleAndFavicon) {
// history.pushState() is called.
FrameHostMsg_DidCommitProvisionalLoad_Params params;
GURL url("http://foo#foo");
- params.page_id = 2;
params.url = url;
params.page_state = PageState::CreateFromURL(url);
params.was_within_same_page = true;
- test_rvh()->SendNavigateWithParams(&params);
+ test_rvh()->SendNavigateWithParams(2, &params);
// The title should immediately be visible on the new NavigationEntry.
base::string16 new_title =
@@ -4313,7 +4292,6 @@ TEST_F(NavigationControllerTest, PostThenReplaceStateThenReload) {
// Submit a form.
GURL url("http://foo");
FrameHostMsg_DidCommitProvisionalLoad_Params params;
- params.page_id = 1;
params.url = url;
params.transition = PAGE_TRANSITION_FORM_SUBMIT;
params.gesture = NavigationGestureUser;
@@ -4321,11 +4299,10 @@ TEST_F(NavigationControllerTest, PostThenReplaceStateThenReload) {
params.was_within_same_page = false;
params.is_post = true;
params.post_id = 2;
- test_rvh()->SendNavigateWithParams(&params);
+ test_rvh()->SendNavigateWithParams(1, &params);
// history.replaceState() is called.
GURL replace_url("http://foo#foo");
- params.page_id = 1;
params.url = replace_url;
params.transition = PAGE_TRANSITION_LINK;
params.gesture = NavigationGestureUser;
@@ -4333,7 +4310,7 @@ TEST_F(NavigationControllerTest, PostThenReplaceStateThenReload) {
params.was_within_same_page = true;
params.is_post = false;
params.post_id = -1;
- test_rvh()->SendNavigateWithParams(&params);
+ test_rvh()->SendNavigateWithParams(1, &params);
// Now reload. replaceState overrides the POST, so we should not show a
// repost warning dialog.
« no previous file with comments | « content/browser/frame_host/navigation_controller_impl.cc ('k') | content/browser/frame_host/navigator.h » ('j') | content/browser/frame_host/render_frame_host_impl.cc » ('J')

Powered by Google App Engine
This is Rietveld 408576698