Skip to content

GitLab

T
tangled
Commit 6520278f authored by Doug Arnett's avatar Doug Arnett Committed by Commit Bot
Browse files
Options

Uses original request URL for ReloadType::DISABLE_PREVIEWS 

When the user is viewing a preview page and selects "Show original",
the client asks the NavigationController to Reload(DISABLE_PREVIEWS).
It currently reloads the last committed URL which is a problem if
the preview caused a client redirect to occur so that the last
committed URL is a preview-specific URL. Really, we want to reload
the original requested URL to show the original page.

This CL updates the NavigatorImpl::NavigateToEntry() logic to use
the original request URL for ReloadType::DISABLE_PREVIEWS just as it
currently does for ReloadType::ORIGINAL_REQUEST_URL.

Bug: 795940
Change-Id: Ic167afa48dca9e40eac3f0dabaf0fb5c5b6ddcdc
Reviewed-on: https://chromium-review.googlesource.com/879026
Commit-Queue: Doug Arnett <dougarnett@chromium.org>
Reviewed-by: default avatarCharlie Reis <creis@chromium.org>
Cr-Commit-Position: refs/heads/master@{#533293}
parent f22c91d3
Hide whitespace changes
Inline Side-by-side
Showing with 172 additions and 6 deletions
content/browser/frame_host/navigation_controller_impl_browsertest.cc
View file @ 6520278f
...@@ -24,6 +24,7 @@ ...@@ -24,6 +24,7 @@
#include "content/browser/frame_host/frame_tree.h" #include "content/browser/frame_host/frame_tree.h"
#include "content/browser/frame_host/navigation_entry_impl.h" #include "content/browser/frame_host/navigation_entry_impl.h"
#include "content/browser/frame_host/navigation_handle_impl.h" #include "content/browser/frame_host/navigation_handle_impl.h"
#include "content/browser/frame_host/navigation_request.h"
#include "content/browser/frame_host/render_frame_host_impl.h" #include "content/browser/frame_host/render_frame_host_impl.h"
#include "content/browser/loader/resource_dispatcher_host_impl.h" #include "content/browser/loader/resource_dispatcher_host_impl.h"
#include "content/browser/web_contents/web_contents_impl.h" #include "content/browser/web_contents/web_contents_impl.h"
...@@ -7854,6 +7855,120 @@ IN_PROC_BROWSER_TEST_F( ...@@ -7854,6 +7855,120 @@ IN_PROC_BROWSER_TEST_F(
EXPECT_EQ(0U, entry->root_node()->children[0]->children.size()); EXPECT_EQ(0U, entry->root_node()->children[0]->children.size());
} }
class NavigationControllerControllableResponseBrowserTest
: public ContentBrowserTest {
protected:
void SetUpOnMainThread() override {
host_resolver()->AddRule("*", "127.0.0.1");
content::SetupCrossSiteRedirector(embedded_test_server());
}
};
IN_PROC_BROWSER_TEST_F(NavigationControllerControllableResponseBrowserTest,
ReloadDisablePreviewReloadsOriginalRequestURL) {
const std::string kOriginalPath = "/original.html";
const std::string kRedirectPath = "/redirect.html";
net::test_server::ControllableHttpResponse original_response1(
embedded_test_server(), kOriginalPath);
net::test_server::ControllableHttpResponse original_response2(
embedded_test_server(), kOriginalPath);
net::test_server::ControllableHttpResponse redirect_response1(
embedded_test_server(), kRedirectPath);
net::test_server::ControllableHttpResponse redirect_response2(
embedded_test_server(), kRedirectPath);
EXPECT_TRUE(embedded_test_server()->Start());
const GURL kOriginalURL =
embedded_test_server()->GetURL("a.com", kOriginalPath);
const GURL kRedirectURL =
embedded_test_server()->GetURL("b.com", kRedirectPath);
const GURL kReloadRedirectURL =
embedded_test_server()->GetURL("c.com", kRedirectPath);
// First navigate to the initial URL. This page will have a cross-site
// redirect to a 2nd domain.
shell()->LoadURL(kOriginalURL);
original_response1.WaitForRequest();
original_response1.Send(
"HTTP/1.1 302 FOUND\r\n"
"Location: " +
kRedirectURL.spec() +
"\r\n"
"\r\n");
original_response1.Done();
redirect_response1.WaitForRequest();
redirect_response1.Send(
"HTTP/1.1 200 OK\r\n"
"Content-Type: text/html; charset=utf-8\r\n"
"\r\n");
redirect_response1.Send(
"<html>"
"<body></body>"
"</html>");
redirect_response1.Done();
EXPECT_TRUE(WaitForLoadStop(shell()->web_contents()));
EXPECT_EQ(kRedirectURL, shell()->web_contents()->GetVisibleURL());
if (content::AreAllSitesIsolatedForTesting()) {
RenderFrameHostImpl* rfh =
static_cast<WebContentsImpl*>(shell()->web_contents())->GetMainFrame();
EXPECT_EQ(GURL("http://b.com"), rfh->GetSiteInstance()->GetSiteURL());
}
// Now simulate a 'Show original' reload via ReloadType::DISABLE_PREVIEWS.
// This reload will have a cross-site redirect to a 3rd domain.
TestNavigationManager reload(shell()->web_contents(), kOriginalURL);
shell()->web_contents()->GetController().Reload(ReloadType::DISABLE_PREVIEWS,
false);
EXPECT_TRUE(reload.WaitForRequestStart());
// Verify reload is using the original request URL and no previews allowed.
EXPECT_EQ(kOriginalURL, reload.GetNavigationHandle()->GetURL());
NavigationRequest* navigation_request =
static_cast<WebContentsImpl*>(shell()->web_contents())
->GetFrameTree()
->root()
->navigation_request();
CHECK(navigation_request);
EXPECT_EQ(content::PREVIEWS_NO_TRANSFORM,
navigation_request->common_params().previews_state);
reload.ResumeNavigation();
original_response2.WaitForRequest();
original_response2.Send(
"HTTP/1.1 302 FOUND\r\n"
"Location: " +
kReloadRedirectURL.spec() +
"\r\n"
"\r\n");
original_response2.Done();
redirect_response2.WaitForRequest();
// Verify now using new redirect URL.
EXPECT_EQ(kReloadRedirectURL, reload.GetNavigationHandle()->GetURL());
redirect_response2.Send(
"HTTP/1.1 200 OK\r\n"
"Content-Type: text/html; charset=utf-8\r\n"
"\r\n");
redirect_response2.Send(
"<html>"
"<body></body>"
"</html>");
redirect_response2.Done();
EXPECT_TRUE(reload.WaitForResponse());
reload.WaitForNavigationFinished();
EXPECT_TRUE(WaitForLoadStop(shell()->web_contents()));
EXPECT_EQ(kReloadRedirectURL, shell()->web_contents()->GetVisibleURL());
if (content::AreAllSitesIsolatedForTesting()) {
RenderFrameHostImpl* rfh =
static_cast<WebContentsImpl*>(shell()->web_contents())->GetMainFrame();
EXPECT_EQ(GURL("http://c.com"), rfh->GetSiteInstance()->GetSiteURL());
}
}
// This test reproduces issue 769645. It happens when the user reloads the page // This test reproduces issue 769645. It happens when the user reloads the page
// and an "unload" event triggers a back navigation. If the reload navigation // and an "unload" event triggers a back navigation. If the reload navigation
// has reached the ReadyToCommit stage but has not committed, the back // has reached the ReadyToCommit stage but has not committed, the back
......
content/browser/frame_host/navigation_controller_impl_unittest.cc
View file @ 6520278f
...@@ -39,6 +39,7 @@ ...@@ -39,6 +39,7 @@
#include "content/public/common/browser_side_navigation_policy.h" #include "content/public/common/browser_side_navigation_policy.h"
#include "content/public/common/page_state.h" #include "content/public/common/page_state.h"
#include "content/public/common/page_type.h" #include "content/public/common/page_type.h"
#include "content/public/common/previews_state.h"
#include "content/public/common/url_constants.h" #include "content/public/common/url_constants.h"
#include "content/public/test/browser_side_navigation_test_utils.h" #include "content/public/test/browser_side_navigation_test_utils.h"
#include "content/public/test/mock_render_process_host.h" #include "content/public/test/mock_render_process_host.h"
...@@ -245,6 +246,13 @@ class NavigationControllerTest ...@@ -245,6 +246,13 @@ class NavigationControllerTest
return navigation_request->common_params().url; return navigation_request->common_params().url;
} }
content::PreviewsState GetLastNavigationPreviewsState() {
NavigationRequest* navigation_request =
contents()->GetFrameTree()->root()->navigation_request();
CHECK(navigation_request);
return navigation_request->common_params().previews_state;
}
protected: protected:
GURL navigated_url_; GURL navigated_url_;
size_t navigation_entry_committed_counter_ = 0; size_t navigation_entry_committed_counter_ = 0;
...@@ -1481,7 +1489,6 @@ TEST_F(NavigationControllerTest, ReloadWithGuest) { ...@@ -1481,7 +1489,6 @@ TEST_F(NavigationControllerTest, ReloadWithGuest) {
EXPECT_EQ(entry1, entry2); EXPECT_EQ(entry1, entry2);
} }
#if !defined(OS_ANDROID) // http://crbug.com/157428
namespace { namespace {
void SetOriginalURL(const GURL& url, void SetOriginalURL(const GURL& url,
FrameHostMsg_DidCommitProvisionalLoad_Params* params) { FrameHostMsg_DidCommitProvisionalLoad_Params* params) {
...@@ -1546,7 +1553,48 @@ TEST_F(NavigationControllerTest, ReloadOriginalRequestURL) { ...@@ -1546,7 +1553,48 @@ TEST_F(NavigationControllerTest, ReloadOriginalRequestURL) {
EXPECT_FALSE(controller.CanGoForward()); EXPECT_FALSE(controller.CanGoForward());
} }
#endif // !defined(OS_ANDROID) TEST_F(NavigationControllerTest,
ReloadDisablePreviewReloadsOriginalRequestURL) {
NavigationControllerImpl& controller = controller_impl();
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(original_url, Referrer(), ui::PAGE_TRANSITION_TYPED,
std::string());
int entry_id = controller.GetPendingEntry()->GetUniqueID();
EXPECT_EQ(0U, navigation_entry_changed_counter_);
EXPECT_EQ(0U, navigation_list_pruned_counter_);
main_test_rfh()->PrepareForCommitWithServerRedirect(final_url);
main_test_rfh()->SendNavigateWithModificationCallback(
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();
// The NavigationEntry should save both the original URL and the final
// redirected URL.
EXPECT_EQ(original_url,
controller.GetVisibleEntry()->GetOriginalRequestURL());
EXPECT_EQ(final_url, controller.GetVisibleEntry()->GetURL());
// Reload with previews disabled.
controller.Reload(ReloadType::DISABLE_PREVIEWS, false);
EXPECT_EQ(0U, navigation_entry_changed_counter_);
EXPECT_EQ(0U, navigation_list_pruned_counter_);
// The reload is pending. The request should point to the original URL.
EXPECT_EQ(original_url, navigated_url());
EXPECT_EQ(controller.GetEntryCount(), 1);
EXPECT_EQ(controller.GetLastCommittedEntryIndex(), 0);
EXPECT_EQ(controller.GetPendingEntryIndex(), 0);
EXPECT_TRUE(controller.GetLastCommittedEntry());
EXPECT_TRUE(controller.GetPendingEntry());
EXPECT_TRUE(HasNavigationRequest());
EXPECT_EQ(content::PREVIEWS_NO_TRANSFORM, GetLastNavigationPreviewsState());
}
// Test that certain non-persisted NavigationEntryImpl values get reset after // Test that certain non-persisted NavigationEntryImpl values get reset after
// commit. // commit.
......
content/browser/frame_host/navigator_impl.cc
View file @ 6520278f
...@@ -268,12 +268,15 @@ bool NavigatorImpl::NavigateToEntry( ...@@ -268,12 +268,15 @@ bool NavigatorImpl::NavigateToEntry(
GURL dest_url = frame_entry.url(); GURL dest_url = frame_entry.url();
Referrer dest_referrer = frame_entry.referrer(); Referrer dest_referrer = frame_entry.referrer();
if (reload_type == ReloadType::ORIGINAL_REQUEST_URL && if ((reload_type == ReloadType::ORIGINAL_REQUEST_URL ||
reload_type == ReloadType::DISABLE_PREVIEWS) &&
entry.GetOriginalRequestURL().is_valid() && !entry.GetHasPostData()) { entry.GetOriginalRequestURL().is_valid() && !entry.GetHasPostData()) {
// We may have been redirected when navigating to the current URL. // We may have been redirected when navigating to the current URL.
// Use the URL the user originally intended to visit, if it's valid and if a // Use the URL the user originally intended to visit as signaled by the
// POST wasn't involved; the latter case avoids issues with sending data to // ReloadType, if it's valid and if a POST wasn't involved; the latter
// the wrong page. // case avoids issues with sending data to the wrong page. The
// DISABLE_PREVIEWS case is triggered from a user action to view the
// original URL without any preview intervention treatment.
dest_url = entry.GetOriginalRequestURL(); dest_url = entry.GetOriginalRequestURL();
dest_referrer = Referrer(); dest_referrer = Referrer();
} }
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment