Take the navigation type into account when checking if the navigation is in page.

content/browser/web_contents/navigation_controller_impl_unittest.cc

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..8a92e6e0190187c37fedb88e1864aac418e4a01f 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());
@@ -2755,8 +2754,17 @@ TEST_F(NavigationControllerTest, IsInPageNavigation) {
   EXPECT_FALSE(controller.IsURLInPageNavigation(url));
   EXPECT_FALSE(controller.IsURLInPageNavigation(other_url));
   const GURL other_url_with_ref("http://www.google.com/home.html#my_other_ref");
-  EXPECT_TRUE(controller.IsURLInPageNavigation(
-      other_url_with_ref));
+  EXPECT_TRUE(controller.IsURLInPageNavigation(other_url_with_ref));
+
+  // Going to the non ref url will be considered in-page if the navigation type
+  // is IN_PAGE.
+  EXPECT_TRUE(controller.IsURLInPageNavigation(url_with_ref, true,
+      content::NAVIGATION_TYPE_UNKNOWN));

[Charlie Reis, 2013/08/07 16:59:27]

I don't understand this line.  It's identical to the check below.  As I
suggested before, the first argument should be url and not url_with_ref, and the
last argument should be IN_PAGE, not UNKNOWN.  The second argument won't affect
the outcome of the test, but it should be true because that's what it will be in
practice.

Also, I don't think you need content:: here or below, because you're already in
the content namespace.

[pstanek, 2013/08/07 17:21:46]

Looks like copy&paste without modification. I meant to test going to the same
url when renderer says it's in page and second one when going to non ref url
with IN_PAGE navigation type. But I'll change that to the one you've been
suggesting from the very beginning.

[pstanek, 2013/08/07 17:37:37]

Done.

+
+  // Going to the same url again will also be considered in-page
+  // if the renderer says it is even if the navigation type isn't IN_PAGE.
+  EXPECT_TRUE(controller.IsURLInPageNavigation(url_with_ref, true,
+      content::NAVIGATION_TYPE_UNKNOWN));

[Charlie Reis, 2013/08/07 17:49:56]

This case was a great addition, independent of the one I was suggesting.  Please
include it as well.

[pstanek, 2013/08/07 18:05:56]

Done.

 }
 
 // Some pages can have subframes with the same base URL (minus the reference) as