New issue
Advanced search Search tips

Issue 634368 link

Starred by 2 users

Issue metadata

Status: Fixed
Owner:
Closed: May 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: All
Pri: 3
Type: Bug



Sign in to add a comment

OOPIF: killing and re-navigating a subframe to the same site doesn't work

Project Member Reported by alex...@chromium.org, Aug 4 2016

Issue description

What steps will reproduce the problem?
(1) Start Chrome with --site-per-process
(2) Go to http://csreis.github.io/tests/cross-site-iframe.html
(3) Click "Go cross-site (simple page)"
(4) From task manager, kill the subframe process
(5) Click "Go cross-site (simple page)" again.

What is the expected output?
The subframe process should be recreated, and the subframe should navigate to the desired page.

What do you see instead?
No navigation happens, and the subframe still shows the sad tab.

Navigating the subframe to a different cross-site URL works.  This is only for the case that the destination subframe RFH is reused and isn't live.

I bumped into this while investigating the RenderView crashes.  It appears that at least part of the problem here is that in RFHM::Navigate, the case that's intended to deal with this can only handle main frames:

  // If the renderer isn't live, then try to create a new one to satisfy this
  // navigation request.
  if (!dest_render_frame_host->IsRenderFrameLive()) {
    ...
    if (!InitRenderView(dest_render_frame_host->render_view_host(), nullptr))
      return nullptr;
    ...
 
The InitRenderView() call isn't going to recreate the RenderFrame if the dest_render_frame_host is for a subframe.  Looks like we need an InitRenderFrame, and also an update to re-initialize the CrossProcessFrameConnector's RWHVCF (via SetChildRWHView).  I'll take this, since I have a draft fix in the works.
 
Project Member

Comment 1 by bugdroid1@chromium.org, Aug 12 2016

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

commit c15fab9e2bbeeb86d3188db4c4eac85dccd1d8d9
Author: alexmos <alexmos@chromium.org>
Date: Fri Aug 12 00:30:41 2016

OOPIF: Reinitialize a dead subframe correctly for same-site navigations.

Currently, if a subframe renderer dies, the subframe cannot be
re-navigated to the same site, because the corresponding code path in
RenderFrameHostManager::Navigate() uses InitRenderView which only
revives top-level RenderFrames.  This CL adds support to also revive
subframe RenderFrames.  Since PlzNavigate also relied on this logic,
duplicating it in GetFrameHostForNavigation, the new logic is
extracted into a common helper.

BUG= 634368 
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_site_isolation

Review-Url: https://codereview.chromium.org/2236753002
Cr-Commit-Position: refs/heads/master@{#411479}

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

Status: Fixed (was: Started)
Should be fixed by r411479.
Status: Assigned (was: Fixed)
The repro steps have regressed, even though the NavigateCrashedSubframeToSameSite test added in r411479 is passing.
Cc: alex...@chromium.org
Owner: lukasza@chromium.org
Status: Started (was: Assigned)
I think the first step here is to check what happens if the test navigates the iframe using javascript rather than NavigateFrameToURL.

Comment 5 by creis@chromium.org, May 2 2017

Cc: kenrb@chromium.org lfg@chromium.org
Comments 3-4: I think this might be a painting bug this time.  If you switch to another tab and come back, the iframe paints, so I'm guessing it loads the first time but just isn't being shown.  Maybe that also explains why the test is still passing?  (The fact that it isn't in Chrome's task manager either is  issue 642958 .)

If you can confirm, maybe we should file a new bug for it?
We also have  issue 682024  filed, which might be due to the same painting bug.

Comment 7 by lfg@chromium.org, May 2 2017

I haven't investigated, but the sad tab graphics are drawn by ChildFrameCompositingHelper in the parent's process when it gets the notification that the child has died from the browser process (from RenderFrameProxy::OnChildFrameProcessGone).

It should be reset in ChildFrameCompositingHelper::OnSetSurface, which should be sent by the RenderWidgetHostViewChildFrame (in RenderWidgetHostViewChildFrame::ProcessCompositorFrame when the compositor frmae is received from the new renderer), so I suspect something must be going wrong there. I would guess that local_surface_id_ needs to be resetted somewhere when the child process is killed.

Comment 8 by lfg@chromium.org, May 2 2017

Actually, RenderWidgetHostViewChildFrame should be destroyed when the child process is gone, so that can't be that. Still, something must be going wrong while setting the surface in the parent frame, and it's probably related to something being reused, as navigating to a different site works as it should.

Cc: lukasza@chromium.org
Owner: ----
Status: Fixed (was: Started)
Since the painting issue seems to be a different bug, I opened  issue 717597  to track it.
Cc: -alex...@chromium.org
Owner: alex...@chromium.org
Restoring owner since we've closed this again.

Sign in to add a comment