Fix ClassifyNavigation.

content/browser/frame_host/navigation_controller_impl_unittest.cc

 // Copyright 2013 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "base/basictypes.h"
 #include "base/bind.h"
 #include "base/command_line.h"
 #include "base/files/file_util.h"
 #include "base/memory/scoped_ptr.h"
 #include "base/stl_util.h"
@@ skipping 1740 matching lines @@
 
   EXPECT_EQ(controller.GetEntryCount(), 2);
   EXPECT_EQ(controller.GetLastCommittedEntryIndex(), 1);
   EXPECT_EQ(controller.GetPendingEntryIndex(), -1);
   EXPECT_TRUE(controller.GetLastCommittedEntry());
   EXPECT_FALSE(controller.GetPendingEntry());
   EXPECT_TRUE(controller.CanGoBack());
   EXPECT_FALSE(controller.CanGoForward());
 }
 
-// Two consequent navigation for the same URL entered in should be considered
+// Two consecutive navigations for the same URL entered in should be considered
 // as SAME_PAGE navigation even when we are redirected to some other page.
 TEST_F(NavigationControllerTest, Redirect) {
   NavigationControllerImpl& controller = controller_impl();
   TestNotificationTracker notifications;
   RegisterForAllNavNotifications(&notifications, &controller);
 
   const GURL url1("http://foo1");
   const GURL url2("http://foo2");  // Redirection target
 
   // First request
   controller.LoadURL(
       url1, Referrer(), ui::PAGE_TRANSITION_TYPED, std::string());
 
   EXPECT_EQ(0U, notifications.size());
-  main_test_rfh()->SendNavigate(0, url2);
+
+  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"));
+  params.redirects.push_back(GURL("http://foo2"));
+  params.should_update_history = false;
+  params.gesture = NavigationGestureAuto;
+  params.is_post = false;
+  params.page_state = PageState::CreateFromURL(url2);
+
+  LoadCommittedDetails details;
+
+  EXPECT_TRUE(controller.RendererDidNavigate(main_test_rfh(), params,
+                                             &details));
   EXPECT_EQ(1U, navigation_entry_committed_counter_);
   navigation_entry_committed_counter_ = 0;
 
   // Second request
   controller.LoadURL(
       url1, Referrer(), ui::PAGE_TRANSITION_TYPED, std::string());
 
   EXPECT_TRUE(controller.GetPendingEntry());
   EXPECT_EQ(controller.GetPendingEntryIndex(), -1);
   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"));

[Charlie Reis, 2015/04/15 17:43:08]

This whole block should get deleted, right?  It's mostly a no-op, apart from
these push_back lines which make it wrong.

[Avi (use Gerrit), 2015/04/15 19:14:16]

I'm not sure what you mean by the push_back lines making it wrong.

[Charlie Reis, 2015/04/15 19:57:36]

You've already done all of these assignments on lines 1778-1786.  |params|
hasn't changed since then (since RendererDidNavigate treats it as const), but
here you're setting page_id to 0 (again), url to url2 (again), and appending
foo1 and foo2 to redirects, which already contains them.  Now there's 4 entries
in that vector.

[Avi (use Gerrit), 2015/04/15 20:19:26]

D'oh. Yeah.

   params.redirects.push_back(GURL("http://foo2"));
   params.should_update_history = false;
   params.gesture = NavigationGestureAuto;
   params.is_post = false;
   params.page_state = PageState::CreateFromURL(url2);
 
-  LoadCommittedDetails details;
-
   EXPECT_EQ(0U, notifications.size());
   EXPECT_TRUE(controller.RendererDidNavigate(main_test_rfh(), params,
                                              &details));
   EXPECT_EQ(1U, navigation_entry_committed_counter_);
   navigation_entry_committed_counter_ = 0;
 
   EXPECT_TRUE(details.type == NAVIGATION_TYPE_SAME_PAGE);
   EXPECT_EQ(controller.GetEntryCount(), 1);
   EXPECT_EQ(controller.GetLastCommittedEntryIndex(), 0);
   EXPECT_TRUE(controller.GetLastCommittedEntry());
@@ skipping 15 matching lines @@
 
   const GURL url1("http://foo1");
   const GURL url2("http://foo2");  // Redirection target
 
   // First request as POST
   controller.LoadURL(
       url1, Referrer(), ui::PAGE_TRANSITION_TYPED, std::string());
   controller.GetVisibleEntry()->SetHasPostData(true);
 
   EXPECT_EQ(0U, notifications.size());
-  main_test_rfh()->SendNavigate(0, url2);
+
+  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"));
+  params.redirects.push_back(GURL("http://foo2"));
+  params.should_update_history = false;
+  params.gesture = NavigationGestureAuto;
+  params.is_post = true;
+  params.page_state = PageState::CreateFromURL(url2);
+
+  LoadCommittedDetails details;
+
+  EXPECT_TRUE(controller.RendererDidNavigate(main_test_rfh(), params,
+                                             &details));
   EXPECT_EQ(1U, navigation_entry_committed_counter_);
   navigation_entry_committed_counter_ = 0;
 
   // Second request
   controller.LoadURL(
       url1, Referrer(), ui::PAGE_TRANSITION_TYPED, std::string());
 
   EXPECT_TRUE(controller.GetPendingEntry());
   EXPECT_EQ(controller.GetPendingEntryIndex(), -1);
   EXPECT_EQ(url1, controller.GetVisibleEntry()->GetURL());
 
-  FrameHostMsg_DidCommitProvisionalLoad_Params params;
   params.page_id = 0;

[Charlie Reis, 2015/04/15 17:43:09]

Same.

   params.url = url2;
   params.transition = ui::PAGE_TRANSITION_SERVER_REDIRECT;
   params.redirects.push_back(GURL("http://foo1"));
   params.redirects.push_back(GURL("http://foo2"));
   params.should_update_history = false;
   params.gesture = NavigationGestureAuto;
   params.is_post = false;
   params.page_state = PageState::CreateFromURL(url2);
 
-  LoadCommittedDetails details;
-
   EXPECT_EQ(0U, notifications.size());
   EXPECT_TRUE(controller.RendererDidNavigate(main_test_rfh(), params,
                                              &details));
   EXPECT_EQ(1U, navigation_entry_committed_counter_);
   navigation_entry_committed_counter_ = 0;
 
   EXPECT_TRUE(details.type == NAVIGATION_TYPE_SAME_PAGE);
   EXPECT_EQ(controller.GetEntryCount(), 1);
   EXPECT_EQ(controller.GetLastCommittedEntryIndex(), 0);
   EXPECT_TRUE(controller.GetLastCommittedEntry());
@@ skipping 2722 matching lines @@
   {
     LoadCommittedDetails details;
     controller_impl().RendererDidNavigate(main_test_rfh(), params, &details);
     EXPECT_EQ(PAGE_TYPE_ERROR,
               controller_impl().GetLastCommittedEntry()->GetPageType());
     EXPECT_EQ(NAVIGATION_TYPE_IN_PAGE, details.type);
   }
 }
 
 }  // namespace content