New issue
Advanced search Search tips

Issue 733434 link

Starred by 3 users

Issue metadata

Status: Assigned
Owner:
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Feature

Blocking:
issue 646242



Sign in to add a comment

SurfaceManager should retain ownership of all Surfaces

Project Member Reported by fsam...@chromium.org, Jun 14 2017

Issue description

Currently CompositorFrameSinkSupport retains ownership of the latest surface for a given client. This makes frame eviction clumsy. If no client refers to a surface then it becomes a candidate for eviction. If a client wishes to keep some surfaces around for an extra while, it keeps them in |referenced_surfaces|.

We should not transfer ownership of cc::Surfaces from SurfaceManager to
CompositorFrameSinkSupport and back. Instead SurfaceManager should always own all surfaces, and decide when to reclaim them when signaled. This was suggested by samans@. samans@, kylechar@, and I chatted about this today
and we think this is a good refactor and it takes us a step closer
to display compositor managed frame eviction.
 
Project Member

Comment 1 by bugdroid1@chromium.org, Jun 28 2017

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

commit c4d8e3bfc8190090ae22ce114d73d7313135c29c
Author: samans <samans@chromium.org>
Date: Wed Jun 28 22:09:31 2017

cc: Move ownership of surfaces to SurfaceManager

SurfaceManager::CreateSurface gives up the ownership of the surface
it creates to CompositorFrameSinkSupport by returning a unique_ptr.
CompositorFrameSinkSupport later passes back the unique_ptr to
SurfaceManager::DestroySurface when it wants to destroy it. Eventually
we want SurfaceManager::DestroySurface to go away and give full
authority to SurfaceManager to destroy surfaces when no one is
referencing them without involving CompsositorFrameSinkSupport. To this
end, this CL amends CreateSurface so it returns a raw pointer and
keeps the unique_ptr in SurfaceManager.

BUG=733434
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_trusty_blink_rel

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

[modify] https://crrev.com/c4d8e3bfc8190090ae22ce114d73d7313135c29c/cc/surfaces/compositor_frame_sink_support.cc
[modify] https://crrev.com/c4d8e3bfc8190090ae22ce114d73d7313135c29c/cc/surfaces/compositor_frame_sink_support.h
[modify] https://crrev.com/c4d8e3bfc8190090ae22ce114d73d7313135c29c/cc/surfaces/surface.cc
[modify] https://crrev.com/c4d8e3bfc8190090ae22ce114d73d7313135c29c/cc/surfaces/surface.h
[modify] https://crrev.com/c4d8e3bfc8190090ae22ce114d73d7313135c29c/cc/surfaces/surface_manager.cc
[modify] https://crrev.com/c4d8e3bfc8190090ae22ce114d73d7313135c29c/cc/surfaces/surface_manager.h
[modify] https://crrev.com/c4d8e3bfc8190090ae22ce114d73d7313135c29c/cc/surfaces/surface_synchronization_unittest.cc

Sign in to add a comment