New issue
Advanced search Search tips

Issue 699103 link

Starred by 1 user

Issue metadata

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



Sign in to add a comment

ForceRecl

Project Member Reported by kylec...@chromium.org, Mar 7 2017

Issue description

There is a bug where the referenced surfaces gets updated after CompositorFrameSinkSupport::SubmitCompositorFrame() correctly then at the next LayerTreeHostImpl::BeginCommit() the references all get cleared. This only seems to happen when using DirectCompositorFrameSink, probably because MojoCompositorFrameSink doesn't have a ForceReclaimResources.

The following stack trace roughly outlines the problem:
SurfaceFactoryClient::ReferencedSurfacesChanged
Surface::ActivateFrame
Surface::QueueFrame
Surface::EvictFrame
SurfaceFactory::ClearSurface
CompositorFrameSinkSupport::ForceReclaimResources
DirectCompositorFrameSink::ForceReclaimResources
LayerTreeHostImpl::BeginCommit

This breaks the ability to use surface references for GC, since references may or may not exist depending on the timing. This effects  normal Chrome which is using sequences + references to find the set of live surfaces

There are enough SurfaceSequences being added to stop flashing on resize but I'd imagine this could cause some subtle problems.
 
Cc: rjkroege@chromium.org sadrul@chromium.org piman@chromium.org danakj@chromium.org enne@chromium.org ericrk@chromium.org
Eric Karl had a patch to get rid o ForceReclaimResources. https://codereview.chromium.org/2609253003/

I don't think we want to do this long term because it seems to be fundamentally incompatible with an out of process display compositor. If we submit an empty CompositorFrame then we also end up releasing all resources, and a new CompositorFrame will arrive at an arbitrary time in the future. We should revive Eric's patch and try to make it stick, I think.
Ping @erick. Will you be looking at removing ForceReclaimResources soon or can we (Mus folks) poke at it? Thanks!
Components: Internals>MUS Internals>Compositing
Labels: Type-Feature
Owner: kylec...@chromium.org
Kyle is getting rid of ForceReclaimResources so I'm assigning this back to him.
Status: Fixed (was: Assigned)
Summary: ForceRecl (was: Surface references shouldn't only be stored in CompositorFrame)
ForceReclaimResources() is gone as of https://codereview.chromium.org/2822143003/.
Blocking:
Components: -Internals>MUS Internals>Services>WindowService

Sign in to add a comment