Commit 7290ea71 authored by Camille Lamy's avatar Camille Lamy Committed by Commit Bot

Add finer checks for url in NavigationController

This CL updates the checks in
NavigationControllerImpl::CreateNavigationRequestFromLoadParams so that
we can distinguish which of the main frame vs subframe case is
triggering.

It also adds additional checks in

FrameNavigationEntry url starts to diverge from the computed
url_to_load.

NavigationControllerImpl: :NavigateWithoutEntry to understand when the
Bug: 896028
Change-Id: I4bf4fbb9e0dc668e88456181fae425c6cb79570a
Reviewed-on: https://chromium-review.googlesource.com/c/1318978Reviewed-by: default avatarJohn Abd-El-Malek <jam@chromium.org>
Commit-Queue: Camille Lamy <clamy@chromium.org>
Cr-Commit-Position: refs/heads/master@{#606014}
parent 1dbfd810
...@@ -2496,6 +2496,20 @@ void NavigationControllerImpl::NavigateWithoutEntry( ...@@ -2496,6 +2496,20 @@ void NavigationControllerImpl::NavigateWithoutEntry(
if (!node) if (!node)
node = delegate_->GetFrameTree()->root(); node = delegate_->GetFrameTree()->root();
// Compute the url_to_load to check that it doesn't change through the
// function.
// TODO(clamy): Remove this once the root cause behind
// https://crbug.com/896028 has been found.
GURL url_to_load;
GURL virtual_url;
if (node->IsMainFrame()) {
bool reverse_on_redirect = false;
RewriteUrlForNavigation(params.url, browser_context_, &url_to_load,
&virtual_url, &reverse_on_redirect);
} else {
url_to_load = params.url;
}
// Compute overrides to the LoadURLParams for |override_user_agent|, // Compute overrides to the LoadURLParams for |override_user_agent|,
// |should_replace_current_entry| and |has_user_gesture| that will be used // |should_replace_current_entry| and |has_user_gesture| that will be used
// both in the creation of the NavigationEntry and the NavigationRequest. // both in the creation of the NavigationEntry and the NavigationRequest.
...@@ -2530,8 +2544,10 @@ void NavigationControllerImpl::NavigateWithoutEntry( ...@@ -2530,8 +2544,10 @@ void NavigationControllerImpl::NavigateWithoutEntry(
entry = CreateNavigationEntryFromLoadParams( entry = CreateNavigationEntryFromLoadParams(
node, params, override_user_agent, should_replace_current_entry, node, params, override_user_agent, should_replace_current_entry,
has_user_gesture); has_user_gesture);
CHECK_EQ(url_to_load, entry->GetFrameEntry(node)->url());
DiscardPendingEntry(false); DiscardPendingEntry(false);
SetPendingEntry(std::move(entry)); SetPendingEntry(std::move(entry));
CHECK_EQ(url_to_load, pending_entry_->GetFrameEntry(node)->url());
} }
// Renderer-debug URLs are sent to the renderer process immediately for // Renderer-debug URLs are sent to the renderer process immediately for
...@@ -2570,6 +2586,7 @@ void NavigationControllerImpl::NavigateWithoutEntry( ...@@ -2570,6 +2586,7 @@ void NavigationControllerImpl::NavigateWithoutEntry(
DCHECK(node->IsMainFrame() || !params.navigation_ui_data); DCHECK(node->IsMainFrame() || !params.navigation_ui_data);
DCHECK(pending_entry_); DCHECK(pending_entry_);
CHECK_EQ(url_to_load, pending_entry_->GetFrameEntry(node)->url());
std::unique_ptr<NavigationRequest> request = std::unique_ptr<NavigationRequest> request =
CreateNavigationRequestFromLoadParams( CreateNavigationRequestFromLoadParams(
node, params, override_user_agent, should_replace_current_entry, node, params, override_user_agent, should_replace_current_entry,
...@@ -2736,13 +2753,14 @@ NavigationControllerImpl::CreateNavigationRequestFromLoadParams( ...@@ -2736,13 +2753,14 @@ NavigationControllerImpl::CreateNavigationRequestFromLoadParams(
// TODO(clamy): In order to remove the pending NavigationEntry, // TODO(clamy): In order to remove the pending NavigationEntry,
// |virtual_url| and |reverse_on_redirect| should be stored in the // |virtual_url| and |reverse_on_redirect| should be stored in the
// NavigationRequest. // NavigationRequest.
CHECK(virtual_url == entry.GetVirtualURL());
CHECK_EQ(url_to_load, frame_entry->url());
} else { } else {
url_to_load = params.url; url_to_load = params.url;
virtual_url = params.url; virtual_url = params.url;
CHECK_EQ(url_to_load, frame_entry->url());
} }
CHECK(!node->IsMainFrame() || virtual_url == entry.GetVirtualURL());
CHECK_EQ(url_to_load, frame_entry->url());
if (!IsValidURLForNavigation(node->IsMainFrame(), virtual_url, url_to_load)) if (!IsValidURLForNavigation(node->IsMainFrame(), virtual_url, url_to_load))
return nullptr; return nullptr;
......
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