Fix cross-site subframe navigations that transfer back to original RFH.
This fixes a bug where if an OOPIF at site A navigates to site B which
redirects back to A, the navigation timed out because Transfer() was
never called on transfer_navigation_handle_, and the pending RFH for B
was never canceled. This is because UpdateStateForNavigate returned
the current RFH early, due to checking CanSubframeSwapProcess before
reaching those steps. Not calling Transfer() later led to the
transferred request being dropped in
ResourceDispatcherHostImpl::CompleteTransfer, due to not finding a
matching entry in pending_loaders_. This CL fixes the
CanSubframeSwapProcess check to be performed after calling Transfer()
and canceling the pending RFH (if necessary).
This uncovered another complication, which is that while redirects
to data: and about: URLs are explicitly disallowed as unsafe (e.g.,
see DataProtocolHandler::IsSafeRedirectTarget), they are allowed when
an extension uses the webRequest API to redirect certain URLs.
One of the tests, ExtensionWebRequestApiTest.WebRequestDeclarative1,
was exercising this: it had a subframe which made a same-site request
that was redirected to data:,. Previously, in --site-per-process, it
worked due to the ordering problem that this CL fixes: we first
decided to transfer the request (IsRendererTransferNeededForNavigation
returned true), then we returned the current RFH early due to
CanSubframeSwapProcess returning |false| before calling Transfer().
With the fixed ordering, we were now also calling Transfer(), as we
were expecting the data: request to be put into a new SiteInstance
which didn't match the current one, yet the request was never
transferred (we weren't spinning up a new pending RFH, and we weren't
sending a Navigate IPC to the current RFH due to the
is_transfer_to_same case in NavigateToEntry, so no one actually made a
request to the data: URL).
To address this, CanSubframeSwapProcess is changed to allow a process
swap for such transfers to data: and about:. This changes behavior
where these redirects were previously kept in the original
SiteInstance, but this should be safer, and it should not break any
scripting relationships (the redirect URLs are provided by an
extension; the site that made the request that's being redirected
can't expect to script the redirect target).
BUG=681077,660407,659613
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_site_isolation
Review-Url: https://codereview.chromium.org/2636193003
Cr-Commit-Position: refs/heads/master@{#453800}
3 files changed
