New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 717597 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: May 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux , Windows , Chrome , Mac
Pri: 3
Type: Bug



Sign in to add a comment

Killed and renavigated iframe has painting problems

Project Member Reported by lukasza@chromium.org, May 2 2017

Issue description

What steps will reproduce the problem? (I've reproed the problem on Goobuntu at r467143)
(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?
a. The subframe process should be recreated
b. The subframe should navigate to the desired page.
c. The subframe should paint.

What do you see instead?
a. The subframe process is recreated
b. The subframe navigates and is "live" (i.e. can run javascript and post)
c. The subframe doesn't paint (unless I resize content_browsertests to minimal size and back;  I couldn't induce a paint in ToT chrome).

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.

The repro steps are exactly the same as  issue 634368 , but the symptoms are slightly different (painting problem here VS general process management problem in the other issue).  This bug might also be related or a duplicate of  issue 682024 , but I am keeping them separate before we investigate and confirm.
 
Cc: lfg@chromium.org
Alternative repro steps:

1. Patch in https://codereview.chromium.org/2857693002
2. Run: out/gn/content_browsertests --gtest_filter=*NavigateCrashedSubframeToSameSite* --enable-pixel-output-in-tests --ui-test-action-max-timeout=1000000 --test-launcher-timeout=1000000
3. Wait until the frame is killed and renavigated (extra sleeps inserted for some ad-hoc debugging)
4. Observe: the frame stays black
5. Resize the window to minimal size horizontally and back
6. Observe: the frame paints
Info related to this bug (thanks lfg@) can be found in  https://crbug.com/634368#c7  and  https://crbug.com/634368#c8 
It seems that things start painting again after content::RenderWidgetHostImpl::SubmitCompositorFrame() is called for the parent widget trigerring:

#1 0x7f41171a9663 content::CrossProcessFrameConnector::SetChildFrameSurface()
#2 0x7f41171fb16f content::RenderWidgetHostViewChildFrame::ProcessCompositorFrame()
#3 0x7f41171fb4d2 content::RenderWidgetHostViewChildFrame::SubmitCompositorFrame()
#4 0x7f41173da35c content::RenderWidgetHostImpl::SubmitCompositorFrame()

which later tickles down into the parent renderer:

#1 0x7f3ec230ed98 content::ChildFrameCompositingHelper::UpdateWebLayer()
#2 0x7f3ec230f764 content::ChildFrameCompositingHelper::OnSetSurface()
#3 0x7f3ec23c5011 content::RenderFrameProxy::OnSetChildFrameSurface()

which resets the sad-face layer.
Owner: lfg@chromium.org
Status: Assigned (was: Untriaged)
Lucas, I wonder if you could PTAL?  I've tried to poke the code yesterday to understand which part of the code is supposed to trigger replacing of the sad face layer, but I didn't get very far... :-)

Comment 5 by lfg@chromium.org, May 3 2017

This happens because in RenderWidgetHostImpl::RendererExited, we mark crashed renderers to be invisble: https://cs.chromium.org/chromium/src/content/browser/renderer_host/render_widget_host_impl.cc?l=1667.

Then, when we re-create the process, the code that handles unhiding the widget doesn't handle OOPIFs, in RenderFrameHostManager::EnsureRenderFrameHostVisibilityConsistent, https://cs.chromium.org/chromium/src/content/browser/frame_host/render_frame_host_manager.cc?l=2851 (i.e., we unhide the frame, but we ignore the page, aka the swapped out RenderView).

Comment 6 by lfg@chromium.org, May 4 2017

Labels: OS-Chrome OS-Mac OS-Windows
Status: Started (was: Assigned)
Project Member

Comment 7 by bugdroid1@chromium.org, May 8 2017

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

commit b592bfa82d1cad4997766f751002f44ef37bd8a5
Author: lfg <lfg@chromium.org>
Date: Mon May 08 20:47:00 2017

Use the WebContents visibility as the initial visibility when
creating a swapped-out RenderView.

This CL changes the RenderView creation to use the WebContents
visibility when creating a swapped-out RenderView. This is
important because the browser doesn't track changes to the
visibility state of swapped-out RenderViewHosts, so the renderer
can end up having the wrong visibility when recovering from a
crash.

BUG= 717597 , 718213 

CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_site_isolation

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

[modify] https://crrev.com/b592bfa82d1cad4997766f751002f44ef37bd8a5/content/browser/frame_host/render_frame_host_manager.cc
[modify] https://crrev.com/b592bfa82d1cad4997766f751002f44ef37bd8a5/content/browser/renderer_host/render_view_host_impl.cc
[modify] https://crrev.com/b592bfa82d1cad4997766f751002f44ef37bd8a5/content/browser/site_per_process_browsertest.cc

Lucas, can we close this as fixed?  I've just verified that the bug doesn't repro anymore in 60.0.3095.5 canary, which includes your fix.

Comment 9 by lfg@chromium.org, May 15 2017

Status: Fixed (was: Started)

Sign in to add a comment