Skip to content

Commit

Permalink
Set should_replace_entry for renderer-initiated navigations
Browse files Browse the repository at this point in the history
Currently should_replace_entry never gets set when creating
NavigationEntry for a renderer-initiated navigation, causing a mismatch
between a NavigationEntry's should_replace_entry and the
NavigationRequest's common_params' should_replace_current_entry. This CL
sets the value in GetNavigationEntryForRendererInitiatedNavigation.

Bug: 977562
Change-Id: Iad788faf3a911d91279301569edb4bcb9d672d34
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2235147
Reviewed-by: Alex Moshchuk <alexmos@chromium.org>
Commit-Queue: Rakina Zata Amni <rakina@chromium.org>
Cr-Commit-Position: refs/heads/master@{#777230}
  • Loading branch information
rakina authored and Commit Bot committed Jun 11, 2020
1 parent 06faa46 commit 465ff6a
Show file tree
Hide file tree
Showing 2 changed files with 21 additions and 2 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -607,10 +607,27 @@ namespace {
// current entry.
bool RendererLocationReplace(Shell* shell, const GURL& url) {
WebContents* web_contents = shell->web_contents();
NavigationControllerImpl& controller =
static_cast<NavigationControllerImpl&>(web_contents->GetController());
WaitForLoadStop(web_contents);
TestNavigationObserver same_tab_observer(web_contents, 1);
TestNavigationManager navigation_manager(web_contents, url);
const GURL& current_url = web_contents->GetMainFrame()->GetLastCommittedURL();
EXPECT_TRUE(ExecJs(shell, JsReplace("window.location.replace($1)", url)));
same_tab_observer.Wait();
// Observe pending entry if it's not a same-document navigation. We can't
// observe same-document navigations because it might finish in the renderer,
// only telling the browser side at the end.
if (!current_url.EqualsIgnoringRef(url)) {
EXPECT_TRUE(navigation_manager.WaitForRequestStart());
// Both should_replace_entry (in the pending NavigationEntry) and
// should_replace_current_entry (in NavigationRequest params) should be
// true.
EXPECT_TRUE(controller.GetPendingEntry()->should_replace_entry());
EXPECT_TRUE(
NavigationRequest::From(navigation_manager.GetNavigationHandle())
->common_params()
.should_replace_current_entry);
}
navigation_manager.WaitForNavigationFinished();
if (!IsLastCommittedEntryOfPageType(web_contents, PAGE_TYPE_NORMAL))
return false;
return web_contents->GetLastCommittedURL() == url;
Expand Down
2 changes: 2 additions & 0 deletions content/browser/frame_host/navigator.cc
Original file line number Diff line number Diff line change
Expand Up @@ -825,6 +825,8 @@ Navigator::GetNavigationEntryForRendererInitiatedNavigation(
controller_->GetBrowserContext(),
nullptr /* blob_url_loader_factory */));

entry->set_should_replace_entry(common_params.should_replace_current_entry);

controller_->SetPendingEntry(std::move(entry));
if (delegate_)
delegate_->NotifyChangedNavigationState(content::INVALIDATE_TYPE_URL);
Expand Down

0 comments on commit 465ff6a

Please sign in to comment.