Need to track down and understand the cause of RDH_TRANSFERRING_REQUEST_NOT_FOUND renderer kill |
|
Issue descriptionThis is a follow-up for issue 659613.
,
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 |
|
Comment 1 by alex...@chromium.org
, Jan 18 2017