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

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: fixes Created 6 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/navigation_controller_impl.cc ('k') | content/browser/frame_host/navigator.h » ('j') | no next file with comments »
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 89644afcc5ef6fc9cdf5d0b514820dbb2956e82d..09ee3d8362f99b7eeb2fa4aa3bf0031248c7f168 100644
--- a/content/browser/frame_host/navigation_controller_impl_unittest.cc
+++ b/content/browser/frame_host/navigation_controller_impl_unittest.cc
@@ -636,13 +636,12 @@ TEST_F(NavigationControllerTest, LoadURL_SamePage_DifferentMethod) {
controller.LoadURL(
url1, Referrer(), ui::PAGE_TRANSITION_TYPED, std::string());
FrameHostMsg_DidCommitProvisionalLoad_Params params;
- params.page_id = 0;
params.url = url1;
params.transition = ui::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();
@@ -1705,7 +1704,6 @@ TEST_F(NavigationControllerTest, Redirect) {
EXPECT_EQ(url1, controller.GetVisibleEntry()->GetURL());
FrameHostMsg_DidCommitProvisionalLoad_Params params;
- params.page_id = 0;
params.url = url2;
params.transition = ui::PAGE_TRANSITION_SERVER_REDIRECT;
params.redirects.push_back(GURL("http://foo1"));
@@ -1718,7 +1716,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;
@@ -1765,7 +1763,6 @@ TEST_F(NavigationControllerTest, PostThenRedirect) {
EXPECT_EQ(url1, controller.GetVisibleEntry()->GetURL());
FrameHostMsg_DidCommitProvisionalLoad_Params params;
- params.page_id = 0;
params.url = url2;
params.transition = ui::PAGE_TRANSITION_SERVER_REDIRECT;
params.redirects.push_back(GURL("http://foo1"));
@@ -1778,7 +1775,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;
@@ -1814,7 +1811,6 @@ TEST_F(NavigationControllerTest, ImmediateRedirect) {
EXPECT_EQ(url1, controller.GetVisibleEntry()->GetURL());
FrameHostMsg_DidCommitProvisionalLoad_Params params;
- params.page_id = 0;
params.url = url2;
params.transition = ui::PAGE_TRANSITION_SERVER_REDIRECT;
params.redirects.push_back(GURL("http://foo1"));
@@ -1827,7 +1823,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;
@@ -1858,7 +1854,6 @@ TEST_F(NavigationControllerTest, NewSubframe) {
const GURL url2("http://foo2");
FrameHostMsg_DidCommitProvisionalLoad_Params params;
- params.page_id = 1;
params.url = url2;
params.transition = ui::PAGE_TRANSITION_MANUAL_SUBFRAME;
params.should_update_history = false;
@@ -1867,7 +1862,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;
@@ -1881,7 +1876,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
@@ -1895,7 +1890,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 = ui::PAGE_TRANSITION_AUTO_SUBFRAME;
params.should_update_history = false;
@@ -1904,7 +1898,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());
}
@@ -1923,7 +1917,6 @@ TEST_F(NavigationControllerTest, AutoSubframe) {
const GURL url2("http://foo2");
FrameHostMsg_DidCommitProvisionalLoad_Params params;
- params.page_id = 0;
params.url = url2;
params.transition = ui::PAGE_TRANSITION_AUTO_SUBFRAME;
params.should_update_history = false;
@@ -1933,7 +1926,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());
@@ -1956,7 +1949,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 = ui::PAGE_TRANSITION_MANUAL_SUBFRAME;
params.should_update_history = false;
@@ -1966,7 +1958,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;
@@ -1974,9 +1966,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;
@@ -1986,8 +1977,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;
@@ -1999,8 +1989,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;
@@ -2050,7 +2039,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 = ui::PAGE_TRANSITION_LINK;
self_params.should_update_history = false;
@@ -2060,7 +2048,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;
@@ -2071,7 +2059,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 = ui::PAGE_TRANSITION_LINK;
params.should_update_history = false;
@@ -2081,7 +2068,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;
@@ -2093,8 +2080,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;
@@ -2107,8 +2093,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;
@@ -2123,20 +2108,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;
@@ -2159,7 +2143,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 = ui::PAGE_TRANSITION_LINK;
params.should_update_history = false;
@@ -2170,8 +2153,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);
@@ -2210,7 +2194,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 = ui::PAGE_TRANSITION_LINK;
params.redirects.push_back(url);
@@ -2222,8 +2205,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);
@@ -2235,7 +2219,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 = ui::PAGE_TRANSITION_CLIENT_REDIRECT;
params.redirects.push_back(GURL("http://foo2/#a"));
@@ -2247,8 +2230,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);
@@ -2271,11 +2255,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;
- contents()->GetMainFrame()->SendNavigateWithParams(&params);
+ contents()->GetMainFrame()->SendNavigateWithParams(1, &params);
// We pass if we don't crash.
}
@@ -2416,7 +2399,6 @@ TEST_F(NavigationControllerTest, RestoreNavigate) {
// Say we navigated to that entry.
FrameHostMsg_DidCommitProvisionalLoad_Params params;
- params.page_id = 0;
params.url = url;
params.transition = ui::PAGE_TRANSITION_LINK;
params.should_update_history = false;
@@ -2424,7 +2406,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
@@ -2499,7 +2481,6 @@ TEST_F(NavigationControllerTest, RestoreNavigateAfterFailure) {
// Now the pending restored entry commits.
FrameHostMsg_DidCommitProvisionalLoad_Params params;
- params.page_id = 0;
params.url = url;
params.transition = ui::PAGE_TRANSITION_LINK;
params.should_update_history = false;
@@ -2507,7 +2488,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.
@@ -3162,7 +3143,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 = ui::PAGE_TRANSITION_AUTO_SUBFRAME;
params.should_update_history = false;
@@ -3170,7 +3150,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.
@@ -3278,7 +3258,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 = ui::PAGE_TRANSITION_AUTO_SUBFRAME;
params.should_update_history = false;
@@ -3288,8 +3267,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.
@@ -4256,11 +4236,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;
- contents()->GetMainFrame()->SendNavigateWithParams(&params);
+ contents()->GetMainFrame()->SendNavigateWithParams(2, &params);
// The title should immediately be visible on the new NavigationEntry.
base::string16 new_title =
@@ -4329,7 +4308,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 = ui::PAGE_TRANSITION_FORM_SUBMIT;
params.gesture = NavigationGestureUser;
@@ -4337,11 +4315,10 @@ TEST_F(NavigationControllerTest, PostThenReplaceStateThenReload) {
params.was_within_same_page = false;
params.is_post = true;
params.post_id = 2;
- contents()->GetMainFrame()->SendNavigateWithParams(&params);
+ contents()->GetMainFrame()->SendNavigateWithParams(1, &params);
// history.replaceState() is called.
GURL replace_url("http://foo#foo");
- params.page_id = 1;
params.url = replace_url;
params.transition = ui::PAGE_TRANSITION_LINK;
params.gesture = NavigationGestureUser;
@@ -4349,7 +4326,7 @@ TEST_F(NavigationControllerTest, PostThenReplaceStateThenReload) {
params.was_within_same_page = true;
params.is_post = false;
params.post_id = -1;
- contents()->GetMainFrame()->SendNavigateWithParams(&params);
+ contents()->GetMainFrame()->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') | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698