Commit 0076d8b9 authored by creis@chromium.org's avatar creis@chromium.org

Revert 215836 "Ensure that renderer-initiated pending entries ca..."

Caused flakiness on Linux ASAN PlatformAppBrowserTest.DisallowNavigation.

> Ensure that renderer-initiated pending entries can be replaced when a new
> navigation is started.
> 
> We don't replace browser-initiated pending entries in didStartProvisionalLoad
> because either (1) they were just created for this navigation and are already
> correct, (2) they're for a different SiteInstance and won't be used upon
> commit, or (3) they're same-site (see below).
> 
> Note that pending entries for same-site browser-initiated navigations can be
> incorrectly used for renderer-initiated navigations that interrupt them, but
> that's a long standing bug that we can address separately.
> 
> BUG=266922
> TEST=See bug for repro steps.
> 
> Review URL: https://chromiumcodereview.appspot.com/21913003

TBR=creis@chromium.org

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

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@215904 0039d316-1c4b-4281-b951-d872f2087c98
parent e2906ae9
...@@ -311,12 +311,8 @@ void TestRenderViewHost::SendNavigateWithFile( ...@@ -311,12 +311,8 @@ void TestRenderViewHost::SendNavigateWithFile(
void TestRenderViewHost::SendNavigateWithTransitionAndResponseCode( void TestRenderViewHost::SendNavigateWithTransitionAndResponseCode(
int page_id, const GURL& url, PageTransition transition, int page_id, const GURL& url, PageTransition transition,
int response_code) { int response_code) {
// DidStartProvisionalLoad may delete the pending entry that holds |url|, OnDidStartProvisionalLoadForFrame(kFrameId, -1, true, url);
// so we keep a copy of it to use in SendNavigateWithParameters. SendNavigateWithParameters(page_id, url, transition, url, response_code, 0);
GURL url_copy(url);
OnDidStartProvisionalLoadForFrame(kFrameId, -1, true, url_copy);
SendNavigateWithParameters(page_id, url_copy, transition, url_copy,
response_code, 0);
} }
void TestRenderViewHost::SendNavigateWithParameters( void TestRenderViewHost::SendNavigateWithParameters(
......
...@@ -2612,44 +2612,6 @@ TEST_F(NavigationControllerTest, ReloadTransient) { ...@@ -2612,44 +2612,6 @@ TEST_F(NavigationControllerTest, ReloadTransient) {
EXPECT_EQ(controller.GetEntryAtIndex(1)->GetURL(), transient_url); EXPECT_EQ(controller.GetEntryAtIndex(1)->GetURL(), transient_url);
} }
// Ensure that renderer initiated pending entries get replaced, so that we
// don't show a stale virtual URL when a navigation commits.
// See http://crbug.com/266922.
TEST_F(NavigationControllerTest, RendererInitiatedPendingEntries) {
NavigationControllerImpl& controller = controller_impl();
const GURL url1("nonexistent:12121");
const GURL url1_fixed("http://nonexistent:12121/");
const GURL url2("http://foo");
// We create pending entries for renderer-initiated navigations so that we
// can show them in new tabs when it is safe.
contents()->DidStartProvisionalLoadForFrame(
test_rvh(), 1, -1, true, url1);
// Simulate what happens if a BrowserURLHandler rewrites the URL, causing
// the virtual URL to differ from the URL.
controller.GetPendingEntry()->SetURL(url1_fixed);
controller.GetPendingEntry()->SetVirtualURL(url1);
EXPECT_EQ(url1_fixed, controller.GetPendingEntry()->GetURL());
EXPECT_EQ(url1, controller.GetPendingEntry()->GetVirtualURL());
EXPECT_TRUE(
NavigationEntryImpl::FromNavigationEntry(controller.GetPendingEntry())->
is_renderer_initiated());
// If the user clicks another link, we should replace the pending entry.
contents()->DidStartProvisionalLoadForFrame(
test_rvh(), 1, -1, true, url2);
EXPECT_EQ(url2, controller.GetPendingEntry()->GetURL());
EXPECT_EQ(url2, controller.GetPendingEntry()->GetVirtualURL());
// Once it commits, the URL and virtual URL should reflect the actual page.
test_rvh()->SendNavigate(0, url2);
EXPECT_EQ(url2, controller.GetLastCommittedEntry()->GetURL());
EXPECT_EQ(url2, controller.GetLastCommittedEntry()->GetVirtualURL());
}
// Tests that the URLs for renderer-initiated navigations are not displayed to // Tests that the URLs for renderer-initiated navigations are not displayed to
// the user until the navigation commits, to prevent URL spoof attacks. // the user until the navigation commits, to prevent URL spoof attacks.
// See http://crbug.com/99016. // See http://crbug.com/99016.
......
...@@ -2133,30 +2133,24 @@ void WebContentsImpl::DidStartProvisionalLoadForFrame( ...@@ -2133,30 +2133,24 @@ void WebContentsImpl::DidStartProvisionalLoadForFrame(
render_view_host->GetProcess(); render_view_host->GetProcess();
RenderViewHost::FilterURL(render_process_host, false, &validated_url); RenderViewHost::FilterURL(render_process_host, false, &validated_url);
if (is_main_frame) { if (is_main_frame)
DidChangeLoadProgress(0); DidChangeLoadProgress(0);
// If there is no browser-initiated pending entry for this navigation, // Create a pending entry for this provisional load (if none exists) using the
// create one using the current SiteInstance, and ensure the address bar // current SiteInstance, and ensure the address bar updates accordingly.
// updates accordingly. We don't know the referrer or extra headers at this // We don't know the referrer or extra headers at this point, but the referrer
// point, but the referrer will be set properly upon commit. // will be set properly upon commit.
NavigationEntry* pending_entry = controller_.GetPendingEntry(); if (is_main_frame && !controller_.GetPendingEntry()) {
bool has_browser_initiated_pending_entry = pending_entry && NavigationEntryImpl* entry = NavigationEntryImpl::FromNavigationEntry(
!NavigationEntryImpl::FromNavigationEntry(pending_entry)-> controller_.CreateNavigationEntry(validated_url,
is_renderer_initiated(); content::Referrer(),
if (!has_browser_initiated_pending_entry) { content::PAGE_TRANSITION_LINK,
NavigationEntryImpl* entry = NavigationEntryImpl::FromNavigationEntry( true /* is_renderer_initiated */,
controller_.CreateNavigationEntry(validated_url, std::string(), GetBrowserContext()));
content::Referrer(), entry->set_site_instance(
content::PAGE_TRANSITION_LINK, static_cast<SiteInstanceImpl*>(GetSiteInstance()));
true /* is_renderer_initiated */, controller_.SetPendingEntry(entry);
std::string(), NotifyNavigationStateChanged(content::INVALIDATE_TYPE_URL);
GetBrowserContext()));
entry->set_site_instance(
static_cast<SiteInstanceImpl*>(GetSiteInstance()));
controller_.SetPendingEntry(entry);
NotifyNavigationStateChanged(content::INVALIDATE_TYPE_URL);
}
} }
// Notify observers about the start of the provisional load. // Notify observers about the start of the provisional load.
......
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