Clean up the SendNavigate family of functions.

BUG=none
TEST=no changes

Review URL: https://codereview.chromium.org/1181163004

Cr-Commit-Position: refs/heads/master@{#334272}

4 files changed, 83 insertions, 107 deletions

diff --git a/content/browser/frame_host/navigation_controller_impl_unittest.cc b/content/browser/frame_host/navigation_controller_impl_unittest.cc
index f813393..2e6d1a6 100644
--- a/content/browser/frame_host/navigation_controller_impl_unittest.cc
+++ b/content/browser/frame_host/navigation_controller_impl_unittest.cc
@@ -1332,6 +1332,13 @@ TEST_F(NavigationControllerTest, ReloadWithGuest) {
 }
 
 #if !defined(OS_ANDROID)  // http://crbug.com/157428
+namespace {
+void SetOriginalURL(const GURL& url,
+                    FrameHostMsg_DidCommitProvisionalLoad_Params* params) {
+  params->original_request_url = url;
+}
+}
+
 TEST_F(NavigationControllerTest, ReloadOriginalRequestURL) {
   NavigationControllerImpl& controller = controller_impl();
   TestNotificationTracker notifications;
@@ -1339,6 +1346,7 @@ TEST_F(NavigationControllerTest, ReloadOriginalRequestURL) {
 
   const GURL original_url("http://foo1");
   const GURL final_url("http://foo2");
+  auto set_original_url_callback = base::Bind(SetOriginalURL, original_url);
 
   // Load up the original URL, but get redirected.
   controller.LoadURL(
@@ -1346,8 +1354,8 @@ TEST_F(NavigationControllerTest, ReloadOriginalRequestURL) {
   int entry_id = controller.GetPendingEntry()->GetUniqueID();
   EXPECT_EQ(0U, notifications.size());
   main_test_rfh()->PrepareForCommitWithServerRedirect(final_url);
-  main_test_rfh()->SendNavigateWithOriginalRequestURL(0, entry_id, true,
-                                                      final_url, original_url);
+  main_test_rfh()->SendNavigateWithModificationCallback(
+      0, entry_id, true, final_url, set_original_url_callback);
   EXPECT_EQ(1U, navigation_entry_committed_counter_);
   navigation_entry_committed_counter_ = 0;
   entry_id = controller.GetLastCommittedEntry()->GetUniqueID();
@@ -1453,6 +1461,13 @@ TEST_F(NavigationControllerTest, ResetEntryValuesAfterCommit) {
   EXPECT_FALSE(committed_entry->should_clear_history_list());
 }
 
+namespace {
+void SetRedirects(const std::vector<GURL>& redirects,
+                  FrameHostMsg_DidCommitProvisionalLoad_Params* params) {
+  params->redirects = redirects;
+}
+}
+
 // Test that Redirects are preserved after a commit.
 TEST_F(NavigationControllerTest, RedirectsAreNotResetByCommit) {
   NavigationControllerImpl& controller = controller_impl();
@@ -1465,6 +1480,7 @@ TEST_F(NavigationControllerTest, RedirectsAreNotResetByCommit) {
   // Set up some redirect values.
   std::vector<GURL> redirects;
   redirects.push_back(url2);
+  auto set_redirects_callback = base::Bind(SetRedirects, redirects);
 
   // Set redirects on the pending entry.
   NavigationEntryImpl* pending_entry = controller.GetPendingEntry();
@@ -1474,8 +1490,8 @@ TEST_F(NavigationControllerTest, RedirectsAreNotResetByCommit) {
 
   // Normal navigation will preserve redirects in the committed entry.
   main_test_rfh()->PrepareForCommitWithServerRedirect(url2);
-  main_test_rfh()->SendNavigateWithRedirects(0, entry_id, true, url1,
-                                             redirects);
+  main_test_rfh()->SendNavigateWithModificationCallback(0, entry_id, true, url1,
+                                                        set_redirects_callback);
   NavigationEntryImpl* committed_entry = controller.GetLastCommittedEntry();
   ASSERT_EQ(1U, committed_entry->GetRedirectChain().size());
   EXPECT_EQ(url2, committed_entry->GetRedirectChain()[0]);
diff --git a/content/browser/renderer_host/render_view_host_unittest.cc b/content/browser/renderer_host/render_view_host_unittest.cc
index 8b396b9..edf12f1 100644
--- a/content/browser/renderer_host/render_view_host_unittest.cc
+++ b/content/browser/renderer_host/render_view_host_unittest.cc
@@ -9,6 +9,7 @@
 #include "content/browser/renderer_host/render_message_filter.h"
 #include "content/browser/renderer_host/render_view_host_delegate_view.h"
 #include "content/browser/renderer_host/render_widget_helper.h"
+#include "content/common/frame_messages.h"
 #include "content/common/input_messages.h"
 #include "content/common/view_messages.h"
 #include "content/public/browser/browser_context.h"
@@ -227,22 +228,35 @@ TEST_F(RenderViewHostTest, MessageWithBadHistoryItemFiles) {
   EXPECT_EQ(1, process()->bad_msg_count());
 }
 
+namespace {
+void SetBadFilePath(const GURL& url,
+                    const base::FilePath& file_path,
+                    FrameHostMsg_DidCommitProvisionalLoad_Params* params) {
+  params->page_state =
+      PageState::CreateForTesting(url, false, "data", &file_path);
+}
+}
+
 TEST_F(RenderViewHostTest, NavigationWithBadHistoryItemFiles) {
   GURL url("http://www.google.com");
   base::FilePath file_path;
   EXPECT_TRUE(PathService::Get(base::DIR_TEMP, &file_path));
   file_path = file_path.AppendASCII("bar");
+  auto set_bad_file_path_callback = base::Bind(SetBadFilePath, url, file_path);
+
   EXPECT_EQ(0, process()->bad_msg_count());
   main_test_rfh()->SendRendererInitiatedNavigationRequest(url, false);
   main_test_rfh()->PrepareForCommit();
-  contents()->GetMainFrame()->SendNavigateWithFile(1, 1, true, url, file_path);
+  contents()->GetMainFrame()->SendNavigateWithModificationCallback(
+      1, 1, true, url, set_bad_file_path_callback);
   EXPECT_EQ(1, process()->bad_msg_count());
 
   ChildProcessSecurityPolicyImpl::GetInstance()->GrantReadFile(
       process()->GetID(), file_path);
   main_test_rfh()->SendRendererInitiatedNavigationRequest(url, false);
   main_test_rfh()->PrepareForCommit();
-  contents()->GetMainFrame()->SendNavigateWithFile(2, 2, true, url, file_path);
+  contents()->GetMainFrame()->SendNavigateWithModificationCallback(
+      2, 2, true, url, set_bad_file_path_callback);
   EXPECT_EQ(1, process()->bad_msg_count());
 }
 
diff --git a/content/test/test_render_frame_host.cc b/content/test/test_render_frame_host.cc
index 504ac77..1a831ede 100644
--- a/content/test/test_render_frame_host.cc
+++ b/content/test/test_render_frame_host.cc
@@ -103,17 +103,18 @@ void TestRenderFrameHost::SendNavigate(int page_id,
                                        int nav_entry_id,
                                        bool did_create_new_entry,
                                        const GURL& url) {
-  SendNavigateWithTransition(page_id, nav_entry_id, did_create_new_entry, url,
-                             ui::PAGE_TRANSITION_LINK);
+  SendNavigateWithParameters(page_id, nav_entry_id, did_create_new_entry, url,
+                             ui::PAGE_TRANSITION_LINK, 200,
+                             ModificationCallback());
 }
 
 void TestRenderFrameHost::SendFailedNavigate(int page_id,
                                              int nav_entry_id,
                                              bool did_create_new_entry,
                                              const GURL& url) {
-  SendNavigateWithTransitionAndResponseCode(page_id, nav_entry_id,
-                                            did_create_new_entry, url,
-                                            ui::PAGE_TRANSITION_RELOAD, 500);
+  SendNavigateWithParameters(page_id, nav_entry_id, did_create_new_entry, url,
+                             ui::PAGE_TRANSITION_RELOAD, 500,
+                             ModificationCallback());
 }
 
 void TestRenderFrameHost::SendNavigateWithTransition(
@@ -122,63 +123,18 @@ void TestRenderFrameHost::SendNavigateWithTransition(
     bool did_create_new_entry,
     const GURL& url,
     ui::PageTransition transition) {
-  SendNavigateWithTransitionAndResponseCode(
-      page_id, nav_entry_id, did_create_new_entry, url, transition, 200);
-}
-
-void TestRenderFrameHost::SendNavigateWithTransitionAndResponseCode(
-    int page_id,
-    int nav_entry_id,
-    bool did_create_new_entry,
-    const GURL& url,
-    ui::PageTransition transition,
-    int response_code) {
-  // DidStartProvisionalLoad may delete the pending entry that holds |url|,
-  // so we keep a copy of it to use in SendNavigateWithParameters.
-  GURL url_copy(url);
-  OnDidStartProvisionalLoadForFrame(url_copy);
-  SendNavigateWithParameters(page_id, nav_entry_id, did_create_new_entry,
-                             url_copy, transition, url_copy, response_code, 0,
-                             std::vector<GURL>());
-}
-
-void TestRenderFrameHost::SendNavigateWithOriginalRequestURL(
-    int page_id,
-    int nav_entry_id,
-    bool did_create_new_entry,
-    const GURL& url,
-    const GURL& original_request_url) {
-  OnDidStartProvisionalLoadForFrame(url);
   SendNavigateWithParameters(page_id, nav_entry_id, did_create_new_entry, url,
-                             ui::PAGE_TRANSITION_LINK, original_request_url,
-                             200, 0, std::vector<GURL>());
+                             transition, 200, ModificationCallback());
 }
 
-void TestRenderFrameHost::SendNavigateWithFile(
+void TestRenderFrameHost::SendNavigateWithModificationCallback(
     int page_id,
     int nav_entry_id,
     bool did_create_new_entry,
     const GURL& url,
-    const base::FilePath& file_path) {
+    const ModificationCallback& callback) {
   SendNavigateWithParameters(page_id, nav_entry_id, did_create_new_entry, url,
-                             ui::PAGE_TRANSITION_LINK, url, 200, &file_path,
-                             std::vector<GURL>());
-}
-
-void TestRenderFrameHost::SendNavigateWithParams(
-    FrameHostMsg_DidCommitProvisionalLoad_Params* params) {
-  FrameHostMsg_DidCommitProvisionalLoad msg(GetRoutingID(), *params);
-  OnDidCommitProvisionalLoad(msg);
-}
-
-void TestRenderFrameHost::SendNavigateWithRedirects(
-    int page_id,
-    int nav_entry_id,
-    bool did_create_new_entry,
-    const GURL& url,
-    const std::vector<GURL>& redirects) {
-  SendNavigateWithParameters(page_id, nav_entry_id, did_create_new_entry, url,
-                             ui::PAGE_TRANSITION_LINK, url, 200, 0, redirects);
+                             ui::PAGE_TRANSITION_LINK, 200, callback);
 }
 
 void TestRenderFrameHost::SendNavigateWithParameters(
@@ -187,22 +143,20 @@ void TestRenderFrameHost::SendNavigateWithParameters(
     bool did_create_new_entry,
     const GURL& url,
     ui::PageTransition transition,
-    const GURL& original_request_url,
     int response_code,
-    const base::FilePath* file_path_for_history_item,
-    const std::vector<GURL>& redirects) {
+    const ModificationCallback& callback) {
+  // DidStartProvisionalLoad may delete the pending entry that holds |url|,
+  // so we keep a copy of it to use below.
+  GURL url_copy(url);
+  OnDidStartProvisionalLoadForFrame(url_copy);
+
   FrameHostMsg_DidCommitProvisionalLoad_Params params;
   params.page_id = page_id;
   params.nav_entry_id = nav_entry_id;
-  params.url = url;
-  params.referrer = Referrer();
+  params.url = url_copy;
   params.transition = transition;
-  params.redirects = redirects;
   params.should_update_history = true;
-  params.searchable_form_url = GURL();
-  params.searchable_form_encoding = std::string();
   params.did_create_new_entry = did_create_new_entry;
-  params.security_info = std::string();
   params.gesture = NavigationGestureUser;
   params.contents_mime_type = contents_mime_type_;
   params.is_post = false;
@@ -210,22 +164,28 @@ void TestRenderFrameHost::SendNavigateWithParameters(
   params.socket_address.set_host("2001:db8::1");
   params.socket_address.set_port(80);
   params.history_list_was_cleared = simulate_history_list_was_cleared_;
-  params.original_request_url = original_request_url;
+  params.original_request_url = url_copy;
 
   url::Replacements<char> replacements;
   replacements.ClearRef();
-  params.was_within_same_page = transition != ui::PAGE_TRANSITION_RELOAD &&
+  params.was_within_same_page =
+      transition != ui::PAGE_TRANSITION_RELOAD &&
       transition != ui::PAGE_TRANSITION_TYPED &&
-      url.ReplaceComponents(replacements) ==
+      url_copy.ReplaceComponents(replacements) ==
           GetLastCommittedURL().ReplaceComponents(replacements);
 
-  params.page_state = PageState::CreateForTesting(
-      url,
-      false,
-      file_path_for_history_item ? "data" : NULL,
-      file_path_for_history_item);
+  params.page_state =
+      PageState::CreateForTesting(url_copy, false, nullptr, nullptr);
+
+  if (!callback.is_null())
+    callback.Run(&params);
 
-  FrameHostMsg_DidCommitProvisionalLoad msg(GetRoutingID(), params);
+  SendNavigateWithParams(&params);
+}
+
+void TestRenderFrameHost::SendNavigateWithParams(
+    FrameHostMsg_DidCommitProvisionalLoad_Params* params) {
+  FrameHostMsg_DidCommitProvisionalLoad msg(GetRoutingID(), *params);
   OnDidCommitProvisionalLoad(msg);
 }
 
diff --git a/content/test/test_render_frame_host.h b/content/test/test_render_frame_host.h
index 233d501..339f861 100644
--- a/content/test/test_render_frame_host.h
+++ b/content/test/test_render_frame_host.h
@@ -70,39 +70,17 @@ class TestRenderFrameHost : public RenderFrameHostImpl,
   void SendBeforeUnloadACK(bool proceed) override;
   void SimulateSwapOutACK() override;
 
-  void SendNavigateWithTransitionAndResponseCode(int page_id,
-                                                 int nav_entry_id,
-                                                 bool did_create_new_entry,
-                                                 const GURL& url,
-                                                 ui::PageTransition transition,
-                                                 int response_code);
-  void SendNavigateWithOriginalRequestURL(int page_id,
-                                          int nav_entry_id,
-                                          bool did_create_new_entry,
-                                          const GURL& url,
-                                          const GURL& original_request_url);
-  void SendNavigateWithFile(int page_id,
-                            int nav_entry_id,
-                            bool did_create_new_entry,
-                            const GURL& url,
-                            const base::FilePath& file_path);
-  void SendNavigateWithParams(
-      FrameHostMsg_DidCommitProvisionalLoad_Params* params);
-  void SendNavigateWithRedirects(int page_id,
-                                 int nav_entry_id,
-                                 bool did_create_new_entry,
-                                 const GURL& url,
-                                 const std::vector<GURL>& redirects);
-  void SendNavigateWithParameters(
+  using ModificationCallback =
+      base::Callback<void(FrameHostMsg_DidCommitProvisionalLoad_Params*)>;
+
+  void SendNavigateWithModificationCallback(
       int page_id,
       int nav_entry_id,
       bool did_create_new_entry,
       const GURL& url,
-      ui::PageTransition transition,
-      const GURL& original_request_url,
-      int response_code,
-      const base::FilePath* file_path_for_history_item,
-      const std::vector<GURL>& redirects);
+      const ModificationCallback& callback);
+  void SendNavigateWithParams(
+      FrameHostMsg_DidCommitProvisionalLoad_Params* params);
 
   // Simulate a renderer-initiated navigation up until commit.
   void NavigateAndCommitRendererInitiated(int page_id,
@@ -142,6 +120,14 @@ class TestRenderFrameHost : public RenderFrameHostImpl,
   bool pending_commit() const { return pending_commit_; }
 
  private:
+  void SendNavigateWithParameters(int page_id,
+                                  int nav_entry_id,
+                                  bool did_create_new_entry,
+                                  const GURL& url,
+                                  ui::PageTransition transition,
+                                  int response_code,
+                                  const ModificationCallback& callback);
+
   TestRenderFrameHostCreationObserver child_creation_observer_;
 
   std::string contents_mime_type_;