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

Issue 659227 link

Starred by 2 users

Issue metadata

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

Blocked on:
issue 680184
issue 683738



Sign in to add a comment

Implement cc::Surface ref counting

Project Member Reported by kylec...@chromium.org, Oct 25 2016

Issue description

Currently cc::Surface lifetime is controlled via the SurfaceSequence mechanism. This should be replaced by a mechanism that is better suited for the multi-process architecture of mustash. fsamuel@ suggested a ref counting based lifetime management mechanism.
 
Project Member

Comment 2 by bugdroid1@chromium.org, Oct 26 2016

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

commit 09c16e13a45528abf1994e036f488a09dc97d76b
Author: kylechar <kylechar@chromium.org>
Date: Wed Oct 26 15:48:40 2016

Pass root ServerWindow id to FrameGenerator.

The FrameSinkId for surfaces corresponding to a ServerWindow encodes the
ServerWindow WindowId as the client_id. This doesn't happen for
FrameGenerator because the display root ServerWindow was created after
the display FrameGenerator. Change the order of creation and pass the
WindowId to FrameGenerator.

BUG= 659227 

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

[modify] https://crrev.com/09c16e13a45528abf1994e036f488a09dc97d76b/services/ui/ws/display.cc
[modify] https://crrev.com/09c16e13a45528abf1994e036f488a09dc97d76b/services/ui/ws/display.h
[modify] https://crrev.com/09c16e13a45528abf1994e036f488a09dc97d76b/services/ui/ws/frame_generator.cc
[modify] https://crrev.com/09c16e13a45528abf1994e036f488a09dc97d76b/services/ui/ws/frame_generator.h
[modify] https://crrev.com/09c16e13a45528abf1994e036f488a09dc97d76b/services/ui/ws/frame_generator_unittest.cc
[modify] https://crrev.com/09c16e13a45528abf1994e036f488a09dc97d76b/services/ui/ws/ids.h
[modify] https://crrev.com/09c16e13a45528abf1994e036f488a09dc97d76b/services/ui/ws/platform_display.cc
[modify] https://crrev.com/09c16e13a45528abf1994e036f488a09dc97d76b/services/ui/ws/platform_display_delegate.h
[modify] https://crrev.com/09c16e13a45528abf1994e036f488a09dc97d76b/services/ui/ws/platform_display_init_params.cc
[modify] https://crrev.com/09c16e13a45528abf1994e036f488a09dc97d76b/services/ui/ws/platform_display_init_params.h
[modify] https://crrev.com/09c16e13a45528abf1994e036f488a09dc97d76b/services/ui/ws/test_utils.cc

Project Member

Comment 3 by bugdroid1@chromium.org, Nov 3 2016

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

commit 0de3732f5b55c4e7ccbaec6487acfe0742a33e2b
Author: kylechar <kylechar@chromium.org>
Date: Thu Nov 03 15:25:57 2016

Add cc::Surface references.

Start framework for tracking reference in cc::SurfaceManager. This will
replace the SurfaceSequence dependency tracking.

The general concept is parent surface references a child surface it is
embedding. When the parent surface no longer needs the child surface, it
removes the reference. We can GC all surfaces with 0 references.

1. Add helper methods to add or remove a reference from a SurfaceId to
   another SurfaceId. Add maps to track references in both directions.
2. Add check in SurfaceManager::GarbageCollectSurfaces() if references
   are zero. If both references and sequences are zero we can garbage
   collect the surface. This will allow references and sequences to both
   work until sequences can be replaced.
3. Add unit tests that check references works correctly.

The surface reference code is unused outside tests at this point. It's
designed so that dependency tracking and references can be used together
while code is converted.

The next CL will start replacing surface dependencies with surface
references in mus-ws.

BUG= 659227 
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_precise_blink_rel

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

[modify] https://crrev.com/0de3732f5b55c4e7ccbaec6487acfe0742a33e2b/cc/BUILD.gn
[modify] https://crrev.com/0de3732f5b55c4e7ccbaec6487acfe0742a33e2b/cc/surfaces/surface_id.h
[modify] https://crrev.com/0de3732f5b55c4e7ccbaec6487acfe0742a33e2b/cc/surfaces/surface_manager.cc
[modify] https://crrev.com/0de3732f5b55c4e7ccbaec6487acfe0742a33e2b/cc/surfaces/surface_manager.h
[add] https://crrev.com/0de3732f5b55c4e7ccbaec6487acfe0742a33e2b/cc/surfaces/surface_manager_ref_unittest.cc

Project Member

Comment 4 by bugdroid1@chromium.org, Nov 9 2016

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

commit 8f4cf6a6f7c47173fe85f7f6022bd5f64b84eb04
Author: kylechar <kylechar@chromium.org>
Date: Wed Nov 09 03:17:17 2016

Remove SurfaceSequence from mus client code.

This is a first step in replacing all SurfaceSequences in mus. We were
passing a sequence from WT to WTC and the client was storing it. The
client would later return the sequence. However, we never registered the
sequence as a dependency so it was one big no-op.

Remove all of the no-op code. Drop OnWindowSurfacedDetached() since it's
not needed now. Something along these lines will need to be added in the
future, but it will need to be synchronized with SubmitCompositorFrame()
so would need to be in a different interface anyways.

BUG= 659227 

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

[modify] https://crrev.com/8f4cf6a6f7c47173fe85f7f6022bd5f64b84eb04/services/ui/public/cpp/surface_id_handler.h
[modify] https://crrev.com/8f4cf6a6f7c47173fe85f7f6022bd5f64b84eb04/services/ui/public/cpp/tests/test_window_tree.cc
[modify] https://crrev.com/8f4cf6a6f7c47173fe85f7f6022bd5f64b84eb04/services/ui/public/cpp/tests/test_window_tree.h
[modify] https://crrev.com/8f4cf6a6f7c47173fe85f7f6022bd5f64b84eb04/services/ui/public/cpp/window.cc
[modify] https://crrev.com/8f4cf6a6f7c47173fe85f7f6022bd5f64b84eb04/services/ui/public/cpp/window_tree_client.cc
[modify] https://crrev.com/8f4cf6a6f7c47173fe85f7f6022bd5f64b84eb04/services/ui/public/cpp/window_tree_client.h
[modify] https://crrev.com/8f4cf6a6f7c47173fe85f7f6022bd5f64b84eb04/services/ui/public/interfaces/window_tree.mojom
[modify] https://crrev.com/8f4cf6a6f7c47173fe85f7f6022bd5f64b84eb04/services/ui/ws/server_window_compositor_frame_sink_manager.cc
[modify] https://crrev.com/8f4cf6a6f7c47173fe85f7f6022bd5f64b84eb04/services/ui/ws/server_window_compositor_frame_sink_manager.h
[modify] https://crrev.com/8f4cf6a6f7c47173fe85f7f6022bd5f64b84eb04/services/ui/ws/test_change_tracker.cc
[modify] https://crrev.com/8f4cf6a6f7c47173fe85f7f6022bd5f64b84eb04/services/ui/ws/test_change_tracker.h
[modify] https://crrev.com/8f4cf6a6f7c47173fe85f7f6022bd5f64b84eb04/services/ui/ws/test_utils.cc
[modify] https://crrev.com/8f4cf6a6f7c47173fe85f7f6022bd5f64b84eb04/services/ui/ws/test_utils.h
[modify] https://crrev.com/8f4cf6a6f7c47173fe85f7f6022bd5f64b84eb04/services/ui/ws/window_server.h
[modify] https://crrev.com/8f4cf6a6f7c47173fe85f7f6022bd5f64b84eb04/services/ui/ws/window_tree.cc
[modify] https://crrev.com/8f4cf6a6f7c47173fe85f7f6022bd5f64b84eb04/services/ui/ws/window_tree.h
[modify] https://crrev.com/8f4cf6a6f7c47173fe85f7f6022bd5f64b84eb04/services/ui/ws/window_tree_client_unittest.cc
[modify] https://crrev.com/8f4cf6a6f7c47173fe85f7f6022bd5f64b84eb04/ui/aura/mus/surface_id_handler.h
[modify] https://crrev.com/8f4cf6a6f7c47173fe85f7f6022bd5f64b84eb04/ui/aura/mus/window_port_mus.cc
[modify] https://crrev.com/8f4cf6a6f7c47173fe85f7f6022bd5f64b84eb04/ui/aura/mus/window_tree_client.cc
[modify] https://crrev.com/8f4cf6a6f7c47173fe85f7f6022bd5f64b84eb04/ui/aura/mus/window_tree_client.h
[modify] https://crrev.com/8f4cf6a6f7c47173fe85f7f6022bd5f64b84eb04/ui/aura/test/mus/test_window_tree.cc
[modify] https://crrev.com/8f4cf6a6f7c47173fe85f7f6022bd5f64b84eb04/ui/aura/test/mus/test_window_tree.h

Project Member

Comment 5 by bugdroid1@chromium.org, Nov 21 2016

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

commit a9194624d84580648fb7967c00c2800a880a9939
Author: kylechar <kylechar@chromium.org>
Date: Mon Nov 21 18:25:41 2016

Convert mustash use surface references.

FrameGenerator uses surface references instead of sequences with this
patch. When mus-ws learns about a new surface, it adds a reference from
the embedding surface to the new surface. If there an existing surface
for the FrameSink, the reference to the existing surface is removed when
the next CompositorFrame is submitted.

FrameGenerator owns the display root surface here and plays the role of
the embedding client. The display root surface gets a reference from the
top level root. All embedded surfaces get a reference from the display
root surface.

When a new surface is created in the gpu process, DisplayCompositor
immediately adds a temporary reference. This is so the surface doesn't
get deleted before the mus-ws process can add the correct reference.

BUG= 659227 

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

[modify] https://crrev.com/a9194624d84580648fb7967c00c2800a880a9939/services/ui/surfaces/display_compositor.cc
[modify] https://crrev.com/a9194624d84580648fb7967c00c2800a880a9939/services/ui/surfaces/display_compositor.h
[modify] https://crrev.com/a9194624d84580648fb7967c00c2800a880a9939/services/ui/ws/frame_generator.cc
[modify] https://crrev.com/a9194624d84580648fb7967c00c2800a880a9939/services/ui/ws/frame_generator.h
[modify] https://crrev.com/a9194624d84580648fb7967c00c2800a880a9939/services/ui/ws/platform_display.h
[modify] https://crrev.com/a9194624d84580648fb7967c00c2800a880a9939/services/ui/ws/platform_display_default.cc
[modify] https://crrev.com/a9194624d84580648fb7967c00c2800a880a9939/services/ui/ws/platform_display_default.h
[modify] https://crrev.com/a9194624d84580648fb7967c00c2800a880a9939/services/ui/ws/test_utils.cc
[modify] https://crrev.com/a9194624d84580648fb7967c00c2800a880a9939/services/ui/ws/window_server.cc

Project Member

Comment 6 by bugdroid1@chromium.org, Dec 2 2016

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

commit 4f2334d832ffe204b1702dcb652846519874a461
Author: kylechar <kylechar@chromium.org>
Date: Fri Dec 02 03:13:24 2016

Add DisplayCompositorTest.

Implement tests to ensure that DisplayCompositor has correct interaction
with DisplayCompositorClient and add/removes temporary references
appropriately.

Add interface SurfaceReferenceManager that encapsulates part of
SurfaceManager functionality. SurfaceManager implements this interface
without changes and we can create a test only version to verify that
DisplayCompositor makes the correct calls.

BUG= 659227 

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

[modify] https://crrev.com/4f2334d832ffe204b1702dcb652846519874a461/cc/surfaces/BUILD.gn
[modify] https://crrev.com/4f2334d832ffe204b1702dcb652846519874a461/cc/surfaces/surface_manager.cc
[modify] https://crrev.com/4f2334d832ffe204b1702dcb652846519874a461/cc/surfaces/surface_manager.h
[add] https://crrev.com/4f2334d832ffe204b1702dcb652846519874a461/cc/surfaces/surface_reference_manager.h
[modify] https://crrev.com/4f2334d832ffe204b1702dcb652846519874a461/services/ui/surfaces/display_compositor.cc
[modify] https://crrev.com/4f2334d832ffe204b1702dcb652846519874a461/services/ui/surfaces/display_compositor.h
[add] https://crrev.com/4f2334d832ffe204b1702dcb652846519874a461/services/ui/surfaces/display_compositor_unittest.cc
[modify] https://crrev.com/4f2334d832ffe204b1702dcb652846519874a461/services/ui/ws/BUILD.gn

Project Member

Comment 7 by bugdroid1@chromium.org, Dec 2 2016

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

commit a886f220c05a5d12d65ccf97bfc86ec1985dc93e
Author: kylechar <kylechar@chromium.org>
Date: Fri Dec 02 17:52:16 2016

Add SurfaceManager option to use references exclusively.

SurfaceManager should be able to walk from root and delete all destroyed
and unreachable surfaces. This doesn't work in conjunction with surface
sequence dependencies (like regular chrome) but works fine for mustash
which only uses surface refrences. Add an optional parameter to
SurfaceManager. This parameter would be removed when everything uses
surface references.

BUG= 659227 

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

[modify] https://crrev.com/a886f220c05a5d12d65ccf97bfc86ec1985dc93e/cc/surfaces/surface_manager.cc
[modify] https://crrev.com/a886f220c05a5d12d65ccf97bfc86ec1985dc93e/cc/surfaces/surface_manager.h
[modify] https://crrev.com/a886f220c05a5d12d65ccf97bfc86ec1985dc93e/services/ui/surfaces/display_compositor.cc

Project Member

Comment 8 by bugdroid1@chromium.org, Dec 5 2016

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

commit c142d1d1f52fd97e463944d0666702c920eb58b2
Author: kylechar <kylechar@chromium.org>
Date: Mon Dec 05 22:23:08 2016

Add SurfaceReference class.

Add a simple class SurfaceReference that holds a parent and child
SurfaceId. Also add Mojo struct and StructTraits for use over IPC.

Update SurfaceId and LocalFrameId struct traits slightly to avoid
unnecessary copies. This requires a friend declaration but it seems
reasonable and is done elsewhere.

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

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

[modify] https://crrev.com/c142d1d1f52fd97e463944d0666702c920eb58b2/cc/ipc/BUILD.gn
[modify] https://crrev.com/c142d1d1f52fd97e463944d0666702c920eb58b2/cc/ipc/local_frame_id_struct_traits.h
[modify] https://crrev.com/c142d1d1f52fd97e463944d0666702c920eb58b2/cc/ipc/struct_traits_unittest.cc
[modify] https://crrev.com/c142d1d1f52fd97e463944d0666702c920eb58b2/cc/ipc/surface_id_struct_traits.h
[add] https://crrev.com/c142d1d1f52fd97e463944d0666702c920eb58b2/cc/ipc/surface_reference.mojom
[add] https://crrev.com/c142d1d1f52fd97e463944d0666702c920eb58b2/cc/ipc/surface_reference.typemap
[add] https://crrev.com/c142d1d1f52fd97e463944d0666702c920eb58b2/cc/ipc/surface_reference_struct_traits.h
[modify] https://crrev.com/c142d1d1f52fd97e463944d0666702c920eb58b2/cc/ipc/traits_test_service.mojom
[modify] https://crrev.com/c142d1d1f52fd97e463944d0666702c920eb58b2/cc/ipc/typemaps.gni
[modify] https://crrev.com/c142d1d1f52fd97e463944d0666702c920eb58b2/cc/surfaces/BUILD.gn
[add] https://crrev.com/c142d1d1f52fd97e463944d0666702c920eb58b2/cc/surfaces/DEPS
[modify] https://crrev.com/c142d1d1f52fd97e463944d0666702c920eb58b2/cc/surfaces/local_frame_id.h
[modify] https://crrev.com/c142d1d1f52fd97e463944d0666702c920eb58b2/cc/surfaces/surface_id.h
[add] https://crrev.com/c142d1d1f52fd97e463944d0666702c920eb58b2/cc/surfaces/surface_reference.cc
[add] https://crrev.com/c142d1d1f52fd97e463944d0666702c920eb58b2/cc/surfaces/surface_reference.h

Project Member

Comment 9 by bugdroid1@chromium.org, Dec 6 2016

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

commit 2255ff5852f6d1ab187601fc18231f97409d3031
Author: kylechar <kylechar@chromium.org>
Date: Tue Dec 06 20:42:35 2016

Add/remove surface references via MojoCompositorFrameSink.

Both adding/removing surface references and submitting compositor frames
happen via IPC. There needs to be synchronization so that a reference is
added before a CompositorFrame is submitted that embeds the Surface.
Likewise, a reference should only be removed after a new CompositorFrame
is submitted that doesn't embed the Surfacee.

Having add/remove references on the same Mojo interface as submitting
CompositorFrames is the simplest way to achieve this synchronization.
Remove the old IPCs from mojom::DisplayCompositor and add new IPCs to
mojom::MojoCompositorFrameSink.

Also add OnDisplayCompositorCreated() to mojom::DisplayCompositorClient.
This is sent as the first message to the DisplayCompositorClient and it
contains the top level root surface id. This allows the privileged
process to add references to the display roots. This also makes it
impossible for non-privileged clients to add references from the top
level root, since the SurfaceId is unguessable.

BUG= 659227 
TBR=junov@chromium.org

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

[modify] https://crrev.com/2255ff5852f6d1ab187601fc18231f97409d3031/cc/ipc/display_compositor.mojom
[modify] https://crrev.com/2255ff5852f6d1ab187601fc18231f97409d3031/cc/ipc/mojo_compositor_frame_sink.mojom
[modify] https://crrev.com/2255ff5852f6d1ab187601fc18231f97409d3031/content/browser/renderer_host/offscreen_canvas_compositor_frame_sink.cc
[modify] https://crrev.com/2255ff5852f6d1ab187601fc18231f97409d3031/content/browser/renderer_host/offscreen_canvas_compositor_frame_sink.h
[modify] https://crrev.com/2255ff5852f6d1ab187601fc18231f97409d3031/services/ui/surfaces/display_compositor.cc
[modify] https://crrev.com/2255ff5852f6d1ab187601fc18231f97409d3031/services/ui/surfaces/display_compositor.h
[modify] https://crrev.com/2255ff5852f6d1ab187601fc18231f97409d3031/services/ui/surfaces/display_compositor_unittest.cc
[modify] https://crrev.com/2255ff5852f6d1ab187601fc18231f97409d3031/services/ui/surfaces/gpu_compositor_frame_sink.cc
[modify] https://crrev.com/2255ff5852f6d1ab187601fc18231f97409d3031/services/ui/surfaces/gpu_compositor_frame_sink.h
[modify] https://crrev.com/2255ff5852f6d1ab187601fc18231f97409d3031/services/ui/ws/frame_generator.cc
[modify] https://crrev.com/2255ff5852f6d1ab187601fc18231f97409d3031/services/ui/ws/frame_generator.h
[modify] https://crrev.com/2255ff5852f6d1ab187601fc18231f97409d3031/services/ui/ws/server_window_delegate.h
[modify] https://crrev.com/2255ff5852f6d1ab187601fc18231f97409d3031/services/ui/ws/test_server_window_delegate.cc
[modify] https://crrev.com/2255ff5852f6d1ab187601fc18231f97409d3031/services/ui/ws/test_server_window_delegate.h
[modify] https://crrev.com/2255ff5852f6d1ab187601fc18231f97409d3031/services/ui/ws/window_server.cc
[modify] https://crrev.com/2255ff5852f6d1ab187601fc18231f97409d3031/services/ui/ws/window_server.h

Blocking: 601863
Labels: displaycompositor
Cc: rjkroege@chromium.org sadrul@chromium.org piman@chromium.org enne@chromium.org
Cc: samans@chromium.org
+samans@

Saman is working on a SurfaceEmbedding object used in clients.

SurfaceEmbedding ~= SurfaceInfo (surface ID + frame size + device scale factor) + SurfaceReference or SurfaceSequence (which is an implementation detail). The intent is SurfaceLayer just cares about SurfaceEmbeddings and the system they use under the hood is abstracted away from there.
Blockedon: 680184
Project Member

Comment 14 by bugdroid1@chromium.org, Jan 11 2017

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

commit d358fb4f2ad43329a2109fde0f84e9ffe4f1feae
Author: kylechar <kylechar@chromium.org>
Date: Wed Jan 11 21:54:00 2017

Separate client surface reference tracking from FrameGenerator.

The logic to track surface references for surface embeddings was built
as part of FrameGenerator. Generalize the logic and remove from
FrameGenerator, creating the class EmbeddedSurfaceTracker.

EmbeddedSurfaceTracker keeps track of surface references from one client
surface to many embedded surfaces. It also handles generating new
references when the client surface changes. Includes unit tests.

This simplifies FrameGenerator and should make it easier to use surface
references in other clients.

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

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

[modify] https://crrev.com/d358fb4f2ad43329a2109fde0f84e9ffe4f1feae/cc/BUILD.gn
[modify] https://crrev.com/d358fb4f2ad43329a2109fde0f84e9ffe4f1feae/cc/surfaces/BUILD.gn
[add] https://crrev.com/d358fb4f2ad43329a2109fde0f84e9ffe4f1feae/cc/surfaces/embedded_surface_tracker.cc
[add] https://crrev.com/d358fb4f2ad43329a2109fde0f84e9ffe4f1feae/cc/surfaces/embedded_surface_tracker.h
[add] https://crrev.com/d358fb4f2ad43329a2109fde0f84e9ffe4f1feae/cc/surfaces/embedded_surface_tracker_unittest.cc
[modify] https://crrev.com/d358fb4f2ad43329a2109fde0f84e9ffe4f1feae/services/ui/ws/frame_generator.cc
[modify] https://crrev.com/d358fb4f2ad43329a2109fde0f84e9ffe4f1feae/services/ui/ws/frame_generator.h

Project Member

Comment 15 by bugdroid1@chromium.org, Jan 18 2017

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

commit 2226adf3f8999d114147ffcfc79089cc2e136076
Author: kylechar <kylechar@chromium.org>
Date: Wed Jan 18 19:07:37 2017

Add debug method to print surface references.

SurfaceManager::SurfaceReferencesToString() prints current surface
references starting at the top level root to a string. Only compiled for
debug/DCHECK builds. Helpful to visualize current references.

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

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

[modify] https://crrev.com/2226adf3f8999d114147ffcfc79089cc2e136076/cc/surfaces/surface.cc
[modify] https://crrev.com/2226adf3f8999d114147ffcfc79089cc2e136076/cc/surfaces/surface.h
[modify] https://crrev.com/2226adf3f8999d114147ffcfc79089cc2e136076/cc/surfaces/surface_manager.cc
[modify] https://crrev.com/2226adf3f8999d114147ffcfc79089cc2e136076/cc/surfaces/surface_manager.h

Project Member

Comment 16 by bugdroid1@chromium.org, Jan 19 2017

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

commit 4e728beb8a92c33105c7562bf11bac2e3ece1eb7
Author: kylechar <kylechar@chromium.org>
Date: Thu Jan 19 17:39:30 2017

Move ReferencedSurfaceTracker into GpuCompositorFrameSink.

Move the logic for adding/removing references from the client to the
GPU. We already pass a list of referenced surfaces, so just use that
list to compute a diff from the last submitted frame. This will
simplifying all clients and make removing SurfaceSequences easier in the
future. Add ReferencedSurfaceTracker to simplify this.

Also move add/removing top-level root references into
GpuCompositorFrameSink. This simplifies code in the mus Window Server.
FrameGenerator just needs to fill in
CompositorFrameMetadata::referenced_surfaces and clients that use
SurfaceLayers should just work.

This change only affects mustash, since non-mustash Chrome doesn't use
the Mojo interface for submitting CompositorFrames yet.

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

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

[modify] https://crrev.com/4e728beb8a92c33105c7562bf11bac2e3ece1eb7/cc/BUILD.gn
[modify] https://crrev.com/4e728beb8a92c33105c7562bf11bac2e3ece1eb7/cc/ipc/display_compositor.mojom
[modify] https://crrev.com/4e728beb8a92c33105c7562bf11bac2e3ece1eb7/cc/ipc/mojo_compositor_frame_sink.mojom
[modify] https://crrev.com/4e728beb8a92c33105c7562bf11bac2e3ece1eb7/cc/surfaces/BUILD.gn
[delete] https://crrev.com/4b470c5b3ffd5b74476cb3ffbcf3ff416ea30d72/cc/surfaces/embedded_surface_tracker.cc
[delete] https://crrev.com/4b470c5b3ffd5b74476cb3ffbcf3ff416ea30d72/cc/surfaces/embedded_surface_tracker.h
[delete] https://crrev.com/4b470c5b3ffd5b74476cb3ffbcf3ff416ea30d72/cc/surfaces/embedded_surface_tracker_unittest.cc
[add] https://crrev.com/4e728beb8a92c33105c7562bf11bac2e3ece1eb7/cc/surfaces/referenced_surface_tracker.cc
[add] https://crrev.com/4e728beb8a92c33105c7562bf11bac2e3ece1eb7/cc/surfaces/referenced_surface_tracker.h
[add] https://crrev.com/4e728beb8a92c33105c7562bf11bac2e3ece1eb7/cc/surfaces/referenced_surface_tracker_unittest.cc
[modify] https://crrev.com/4e728beb8a92c33105c7562bf11bac2e3ece1eb7/components/exo/compositor_frame_sink.cc
[modify] https://crrev.com/4e728beb8a92c33105c7562bf11bac2e3ece1eb7/components/exo/compositor_frame_sink.h
[modify] https://crrev.com/4e728beb8a92c33105c7562bf11bac2e3ece1eb7/content/browser/renderer_host/offscreen_canvas_compositor_frame_sink.cc
[modify] https://crrev.com/4e728beb8a92c33105c7562bf11bac2e3ece1eb7/content/browser/renderer_host/offscreen_canvas_compositor_frame_sink.h
[modify] https://crrev.com/4e728beb8a92c33105c7562bf11bac2e3ece1eb7/services/ui/surfaces/display_compositor.cc
[modify] https://crrev.com/4e728beb8a92c33105c7562bf11bac2e3ece1eb7/services/ui/surfaces/gpu_compositor_frame_sink.cc
[modify] https://crrev.com/4e728beb8a92c33105c7562bf11bac2e3ece1eb7/services/ui/surfaces/gpu_compositor_frame_sink.h
[modify] https://crrev.com/4e728beb8a92c33105c7562bf11bac2e3ece1eb7/services/ui/ws/frame_generator.cc
[modify] https://crrev.com/4e728beb8a92c33105c7562bf11bac2e3ece1eb7/services/ui/ws/frame_generator.h
[modify] https://crrev.com/4e728beb8a92c33105c7562bf11bac2e3ece1eb7/services/ui/ws/server_window_delegate.h
[modify] https://crrev.com/4e728beb8a92c33105c7562bf11bac2e3ece1eb7/services/ui/ws/test_server_window_delegate.cc
[modify] https://crrev.com/4e728beb8a92c33105c7562bf11bac2e3ece1eb7/services/ui/ws/test_server_window_delegate.h
[modify] https://crrev.com/4e728beb8a92c33105c7562bf11bac2e3ece1eb7/services/ui/ws/window_server.cc
[modify] https://crrev.com/4e728beb8a92c33105c7562bf11bac2e3ece1eb7/services/ui/ws/window_server.h

Project Member

Comment 17 by bugdroid1@chromium.org, Jan 25 2017

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

commit 43117a2e3860fed908e46bed85624b17358fe63b
Author: kylechar <kylechar@chromium.org>
Date: Wed Jan 25 17:45:59 2017

Fix surface reference assumptions in SurfaceManager.

The original plan was to use the surface reference graph plus
SurfaceSequences to help migrate code to use surface reference graph.
That doesn't look to be necessary and/or workable now so cleanup
SurfaceMananger.

1. Remove checking count of surface references in the surface reference
   graph. We won't add references if we are using the old system.
2. If a surface drops to having 0 incoming references, don't delete all
   references it holds. We are doing garbage collection via top-level
   root, so there is no need to cleanup references. This wouldn't work
   if a cycle got introduced anyways.
3. Add cleanup of references when a surface is deleted. Since (2) no
   longer happens we have to cleanup references on delete.
4. Cleanup unit tests. The older tests are improved to actually check
   the specific references exist instead of just having the right
   number. Also fix some style differences across new + old tests to
   make them easier to read.

This also sets things up to remove the SurfaceReferenceManager interface
that is now unused.

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

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

[modify] https://crrev.com/43117a2e3860fed908e46bed85624b17358fe63b/cc/surfaces/surface_factory_unittest.cc
[modify] https://crrev.com/43117a2e3860fed908e46bed85624b17358fe63b/cc/surfaces/surface_manager.cc
[modify] https://crrev.com/43117a2e3860fed908e46bed85624b17358fe63b/cc/surfaces/surface_manager.h
[modify] https://crrev.com/43117a2e3860fed908e46bed85624b17358fe63b/cc/surfaces/surface_manager_ref_unittest.cc

Components: Internals>Compositing
Project Member

Comment 19 by bugdroid1@chromium.org, Jan 27 2017

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

commit a5af59cbbaccb5f6705e2e3b6c62984c21a398e1
Author: kylechar <kylechar@chromium.org>
Date: Fri Jan 27 16:45:27 2017

Remove SurfaceReferenceManager interface.

The interface is no longer necessary for tests. Delete the interface and
move method comments to SurfaceManager. Cleanup the last references to
the interface in DisplayCompositor.

Also removes the GetSurfaceReferenceCount() / GetReferenceSurfaceCount()
methods that are unused. We don't care about the number of references,
we care about reachable from the top-level root.

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

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

[modify] https://crrev.com/a5af59cbbaccb5f6705e2e3b6c62984c21a398e1/cc/surfaces/BUILD.gn
[modify] https://crrev.com/a5af59cbbaccb5f6705e2e3b6c62984c21a398e1/cc/surfaces/surface_manager.cc
[modify] https://crrev.com/a5af59cbbaccb5f6705e2e3b6c62984c21a398e1/cc/surfaces/surface_manager.h
[modify] https://crrev.com/a5af59cbbaccb5f6705e2e3b6c62984c21a398e1/cc/surfaces/surface_manager_ref_unittest.cc
[delete] https://crrev.com/f329d552a1d32251a9c20ba06183fe5aa203040d/cc/surfaces/surface_reference_manager.h
[modify] https://crrev.com/a5af59cbbaccb5f6705e2e3b6c62984c21a398e1/services/ui/surfaces/display_compositor.cc
[modify] https://crrev.com/a5af59cbbaccb5f6705e2e3b6c62984c21a398e1/services/ui/surfaces/display_compositor.h

Blockedon: 683738
This is implemented in mus+ash now. There is still the matter of getting it work with non-mus+ash Chrome but that is being tracked in  crbug.com/676384 .
Status: Fixed (was: Started)
Blocking: -601863

Sign in to add a comment