From 13963b63795cb487e7fc3e8cf3ee350a8e7a8fc7 Mon Sep 17 00:00:00 2001 From: avi Date: Fri, 12 Jun 2015 16:10:58 -0700 Subject: [PATCH] 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} --- .../navigation_controller_impl_unittest.cc | 24 ++++- .../renderer_host/render_view_host_unittest.cc | 18 +++- content/test/test_render_frame_host.cc | 104 +++++++-------------- content/test/test_render_frame_host.h | 44 +++------ 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 f8133933b669..2e6d1a6dfb4b 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& 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 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 8b396b95f570..edf12f18e3ea 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 504ac771aefe..1a831ede5018 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()); -} - -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()); + 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()); -} - -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& 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& 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 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(¶ms); - FrameHostMsg_DidCommitProvisionalLoad msg(GetRoutingID(), params); + SendNavigateWithParams(¶ms); +} + +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 233d501a753d..339f86165030 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& redirects); - void SendNavigateWithParameters( + using ModificationCallback = + base::Callback; + + 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& 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_; -- 2.11.4.GIT