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

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

Issue 21544005: Take the navigation type into account when checking if the navigation is in page. (Closed) Base URL: https://chromium.googlesource.com/chromium/src.git@master
Patch Set: More generic and brave fix Created 7 years, 4 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: content/browser/web_contents/navigation_controller_impl_unittest.cc
diff --git a/content/browser/web_contents/navigation_controller_impl_unittest.cc b/content/browser/web_contents/navigation_controller_impl_unittest.cc
index a1904d04e2de3d9da87c046157cdf89cb64bbf2a..69adf5ff2ac32b3d13d9e89e70192d911bd132dd 100644
--- a/content/browser/web_contents/navigation_controller_impl_unittest.cc
+++ b/content/browser/web_contents/navigation_controller_impl_unittest.cc
@@ -1921,6 +1921,7 @@ TEST_F(NavigationControllerTest, InPage) {
params.gesture = NavigationGestureUser;
params.is_post = false;
params.page_state = PageState::CreateFromURL(url2);
+ params.was_within_same_page = true;
// This should generate a new entry.
EXPECT_TRUE(controller.RendererDidNavigate(params, &details));
@@ -1938,9 +1939,7 @@ TEST_F(NavigationControllerTest, InPage) {
EXPECT_TRUE(controller.RendererDidNavigate(back_params, &details));
EXPECT_EQ(1U, navigation_entry_committed_counter_);
navigation_entry_committed_counter_ = 0;
- // is_in_page is false in that case but should be true.
- // See comment in AreURLsInPageNavigation() in navigation_controller.cc
- // EXPECT_TRUE(details.is_in_page);
+ EXPECT_TRUE(details.is_in_page);
EXPECT_EQ(2, controller.GetEntryCount());
EXPECT_EQ(0, controller.GetCurrentEntryIndex());
EXPECT_EQ(back_params.url, controller.GetActiveEntry()->GetURL());
@@ -2741,22 +2740,33 @@ TEST_F(NavigationControllerTest, IsInPageNavigation) {
test_rvh()->SendNavigate(0, url);
// Reloading the page is not an in-page navigation.
- EXPECT_FALSE(controller.IsURLInPageNavigation(url));
+ EXPECT_FALSE(controller.IsURLInPageNavigation(url,
+ content::NAVIGATION_TYPE_SAME_PAGE));
Charlie Reis 2013/08/06 18:48:08 Don't need to pass the type in any of these but th
const GURL other_url("http://www.google.com/add.html");
- EXPECT_FALSE(controller.IsURLInPageNavigation(other_url));
+ EXPECT_FALSE(controller.IsURLInPageNavigation(other_url,
+ content::NAVIGATION_TYPE_NEW_PAGE));
const GURL url_with_ref("http://www.google.com/home.html#my_ref");
- EXPECT_TRUE(controller.IsURLInPageNavigation(url_with_ref));
+ EXPECT_TRUE(controller.IsURLInPageNavigation(url_with_ref,
+ content::NAVIGATION_TYPE_IN_PAGE));
// Navigate to URL with refs.
test_rvh()->SendNavigate(1, url_with_ref);
// Reloading the page is not an in-page navigation.
- EXPECT_FALSE(controller.IsURLInPageNavigation(url_with_ref));
- EXPECT_FALSE(controller.IsURLInPageNavigation(url));
- EXPECT_FALSE(controller.IsURLInPageNavigation(other_url));
+ EXPECT_FALSE(controller.IsURLInPageNavigation(url_with_ref,
+ content::NAVIGATION_TYPE_SAME_PAGE));
+ EXPECT_FALSE(controller.IsURLInPageNavigation(url,
+ content::NAVIGATION_TYPE_NEW_PAGE));
+ EXPECT_FALSE(controller.IsURLInPageNavigation(other_url,
+ content::NAVIGATION_TYPE_NEW_PAGE));
const GURL other_url_with_ref("http://www.google.com/home.html#my_other_ref");
EXPECT_TRUE(controller.IsURLInPageNavigation(
- other_url_with_ref));
+ other_url_with_ref, content::NAVIGATION_TYPE_IN_PAGE));
+
+ // Going to the same url again will also be considered in-page
+ // if the renderer says it is even the navigation type isn't IN_PAGE.
Charlie Reis 2013/08/06 18:48:08 nit: even if
+ EXPECT_TRUE(controller.IsURLInPageNavigation(url_with_ref, true,
+ content::NAVIGATION_TYPE_UNKNOWN));
Charlie Reis 2013/08/06 18:48:08 This case is good to test, but can you also add th
pstanek 2013/08/07 11:39:35 Ok. But I'll modify it a bit since it's the naviga
}
// Some pages can have subframes with the same base URL (minus the reference) as

Powered by Google App Engine
This is Rietveld 408576698