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

Issue 760194 link

Starred by 1 user
Status: Fixed
Owner:
kylec...@chromium.org
Closed: Aug 2017
Cc:
█ fsam...@chromium.org
Components:
EstimatedDays: ----
NextAction: ----
OS: All
Pri: 1
Type: Bug

Blocking:
issue 676384



Sign in to add a comment

PDF viewer doesn't register frame set hierarchy

Project Member Reported by kylec...@chromium.org, Aug 29 2017 
The FrameSinkId for the PDF plugin process never gets registered as part of the frame sink hierarchy. When you open a PDF document there look to be 3 processes that submit CompositorFrames. There is a surface embedding hierarchy like this:

SurfaceId(FrameSinkId(0, 1), LocalSurfaceId(N, ...)) <- browser
  SurfaceId(FrameSinkId(6, 1), LocalSurfaceId(N, ...)) <- renderer
    SurfaceId(FrameSinkId(1, 1), LocalSurfaceId(N, ...)) <- pdf plugin

The hierarchy should look like this: 
FrameSinkId(0, 1) <- browser
  FrameSinkId(6, 1) <- renderer
    FrameSinkId(7, 1) <- pdf plugin

We never register anything for pdf plugin so it actually looks like this:
FrameSinkId(0, 1) <- browser
  FrameSinkId(6, 1) <- renderer

When FrameSinkId(7, 1) creates a new Surface, HostFrameSinkManager doesn't know what FrameSinkId should embed it and drops the temporary reference.
Project Member

Comment 1 by bugdroid1@chromium.org, Aug 29 2017 

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

commit e4b6f0c7c4bb48603d35f9928be8ecc8ca4d73fc
Author: kylechar <kylechar@chromium.org>
Date: Tue Aug 29 19:36:50 2017

PDF viewer registers frame sink hierarchy.

We were not registering the frame sink hierarchy for the PDF viewer.
HostFrameSinkManager didn't know what FrameSinkId was expected to embed
new Surfaces from the PDF viewer and would drop the temporary
references. This meant that during resize it was possible to get into a
state where renderer embedded a PDF viewer Surface that had already been
deleted.

RenderWidgetHostViewGuest is built on RenderWidgetHostViewChildFrame
which is supposed to handle registering frame sink hierarchy. The parent
FrameSinkId is only set when using CrossProcessFrameConnector and PDF
viewer is a special case that doesn't use CrossProcessFrameConnector.

When creating RenderWidgetHostViewGuest set the parent FrameSinkId by
getting it from the owner. The owner is a renderer created to embed the
PDF viewer, so there shouldn't be any reparenting concerns.
RenderWidgetHostViewChildFrame will handle registering and unregistering
frame sink hierarchy as expected after this change.

Bug:  760194 ,  676384 
Cq-Include-Trybots: master.tryserver.chromium.linux:linux_site_isolation
Change-Id: I689426b32fb8a18bdd877ef04116777d4870c7a6
Reviewed-on: https://chromium-review.googlesource.com/641654
Reviewed-by: Fady Samuel <fsamuel@chromium.org>
Reviewed-by: Antoine Labour <piman@chromium.org>
Reviewed-by: Ehsan Karamad <ekaramad@chromium.org>
Reviewed-by: Ken Buchanan <kenrb@chromium.org>
Commit-Queue: kylechar <kylechar@chromium.org>
Cr-Commit-Position: refs/heads/master@{#498202}
[modify] https://crrev.com/e4b6f0c7c4bb48603d35f9928be8ecc8ca4d73fc/content/browser/frame_host/render_widget_host_view_guest.cc
[modify] https://crrev.com/e4b6f0c7c4bb48603d35f9928be8ecc8ca4d73fc/content/browser/renderer_host/render_widget_host_view_child_frame.cc
[modify] https://crrev.com/e4b6f0c7c4bb48603d35f9928be8ecc8ca4d73fc/content/browser/renderer_host/render_widget_host_view_child_frame.h

Comment 2 by kylec...@chromium.org, Aug 31 2017

Status: Fixed (was: Assigned)

Comment 3 by laforge@google.com, Nov 8 2017

Components: -Internals>Viz Internals>Services>Viz
Migrating from Internals>Viz to Internals>Services>Viz.
►

Sign in to add a comment