GitLab

r705290 moved when exactly a renderer/NTP-initiated navigation starts to
be treated as if it was a browser/bookmark-initiated navigation.  This
regressed omnibox behavior which stopped showing the URL of a navigation
initiated by an NTP click.  The main cause of the regression is that
after r705290 the NavigationEntryImpl::is_renderer_initiated() would
return true (inconsistent with NavigationRequest).

This CL makes sure that NavigationEntry associated with a
renderer/NTP-initiated navigation also indicates that this navigation
should be treated as browser-initiated.  This is achieved by adding an
extra call to ContentBrowserClient::OverrideNavigationParams *before*
creating the NavigationEntry and *before* the new Omnibox display text
is calculated by the following callstack:
    LocationBarModelImpl::GetFormattedURL()
    LocationBarModelImpl::GetURLForDisplay()
    OmniboxEditModel::ResetDisplayTexts()
    OmniboxViewViews::Update()
    LocationBarView::Update()
    ToolbarView::Update()
    Browser::UpdateToolbar()
    Browser::ScheduleUIUpdate()
    Browser::NavigationStateChanged()
    content::WebContentsImpl::NotifyNavigationStateChanged()
    content::NavigatorImpl::GetNavigationEntryForRendererInitiatedNavigation()
    content::NavigatorImpl::OnBeginNavigation()
    content::RenderFrameHostImpl::BeginNavigation()

The decision whether a navigation is NTP-initiated needs to be based on
the |source_site_instance|.  The decision cannot be based on the
|initiator_origin|, because we want https://example.com to be treated
differently from https://example.com/chrome/ntp (the latter might have a
different effective Site URL when it is designated as a "remote NTP").
This requirement necessitates adding a |source_site_instance| parameter
to NavigationController::CreateNavigationEntry().  In the NTP scenario,
the NavigationEntry is created by the following callstack:
    NavigationEntryImpl::NavigationEntryImpl()
    NavigationController::CreateNavigationEntry()
    NavigatorImpl::GetNavigationEntryForRendererInitiatedNavigation()
    NavigatorImpl::OnBeginNavigation()
    RenderFrameHostImpl::BeginNavigation()

To account for DCHECKs that ensure that NavigationRequest and
NavigationEntryImpl are compatible, the CL also had to move the older
OverrideNavigationParams call from NavigationRequest::StartNavigation to
the NavigationRequest's constructor.

This CL adds a regression test: LocalNTPTest.PendingNavigations
Additionally, I've manually tested that repro steps from
https://crbug.com/1020610#c1 have expected behavior after this CL.

Bug: 1020610
Change-Id: I7a16b28eb9426c348f37f432e7b4813bee55ddd6
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1896295Reviewed-by: Nasko Oskov <nasko@chromium.org>
Reviewed-by: Dan Beam <dbeam@chromium.org>
Auto-Submit: Łukasz Anforowicz <lukasza@chromium.org>
Commit-Queue: Łukasz Anforowicz <lukasza@chromium.org>
Cr-Commit-Position: refs/heads/master@{#713592}