New issue
Advanced search Search tips

Issue 660407 link

Starred by 2 users

Issue metadata

Status: Untriaged
Owner: ----
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Bug



Sign in to add a comment

Need to track down and understand the cause of RDH_TRANSFERRING_REQUEST_NOT_FOUND renderer kill

Project Member Reported by lukasza@chromium.org, Oct 28 2016

Issue description

This is a follow-up for issue 659613.
 
Cc: alex...@chromium.org
Note that I think the repro in  issue 681077  might be hitting this code path.  See my analysis at https://bugs.chromium.org/p/chromium/issues/detail?id=681077#c10.
Project Member

Comment 2 by bugdroid1@chromium.org, Mar 1 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/0431b6bc6c9bf5624cccf1121285392b07b74ec6

commit 0431b6bc6c9bf5624cccf1121285392b07b74ec6
Author: alexmos <alexmos@chromium.org>
Date: Wed Mar 01 02:52:43 2017

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}

[modify] https://crrev.com/0431b6bc6c9bf5624cccf1121285392b07b74ec6/content/browser/frame_host/render_frame_host_manager.cc
[modify] https://crrev.com/0431b6bc6c9bf5624cccf1121285392b07b74ec6/content/browser/frame_host/render_frame_host_manager.h
[modify] https://crrev.com/0431b6bc6c9bf5624cccf1121285392b07b74ec6/content/browser/site_per_process_browsertest.cc

Sign in to add a comment