Note that given the parent may decide it wants to immediately discard a temporary reference before it even uses it, "returned references" needs to be a field separate from "referenced_surfaces" in my opinion. Perhaps those references can be returned via a separate IPC?

I'll take this one.

SurfaceReferences are a blocker for an out-of-process display compositor in general, and for more sophisticated deadline scheduling for surface synchronization so I'm marking this as P2.

I think this is possible to do in a reasonable way with surface synchronization. As long as we can guarantee the parent CompositorFrameSink (CFS) exists before the child CFS it should be fine. The WS needs to know the parent CFS to assign a temporary reference to it. The WS doesn't know what ServerWindow has the CFS until the CFS exists. SurfaceManager also wouldn't know if the parent has already been deleted or if it doesn't exist yet which I think is an even bigger challenge.

We also need to keep register and unregister for FrameSinkId. We need to know when a CFS is no longer valid, which corresponds to when it unregisters itself.

If there is no guarantee on the CFS creation order, I think some sort of timeout based expiry is the only reasonable solution. However, I think that surface synchronization guarantees this order and everything is fine. If the parent CFS is what generates the child CFSs next SurfaceId, then the child can't submit a CF without the parent already existing.

Here are some notes just so I can remember what I was thinking about after the long weekend:

(1) The parent CFS submits a CF first. This CF will include the new child SurfaceId and a reference will be added before child surface gets created. There is no temporary reference in this case so nothing needs to get assigned. Done.
or
(1) The child CFS submits a CF first. A temporary reference is added to the new child SurfaceId here. SurfaceManager needs the parent FrameSinkId.
(2) DisplayCompositor sends an IPC to WindowServer for OnSurfaceCreated(). Something in this IPC indicates the temporary referenced needs to be assigned.
(3a) WindowServer finds that the OnSurfaceCreated() corresponds to a ServerWindow that doesn't exist or has no parent. WindowServer sends an IPC back to DisplayCompositor to drop the temporary reference immediately.
or
(3b) WindowServer finds the parent ServerWindow to forward OnSurfaceCreated() to. WindowServer replies to DisplayCompositor with the parent FrameSinkId.
(4) DisplayCompositor forwards the FrameSinkId of the parent back to SurfaceManager. 
(5) SurfaceManager checks if the parent FrameSinkId still exists.
(6a) The parent FrameSinkId doesn't exist, so the CFS must have already been unregistered. The temporary reference is also deleted. Done.
or
(6b) The FrameSinkId does exist. 
(7) SurfaceManager checks if the temporary reference still exists.
(8a) The temporary reference doesn't exist. A real reference must have been added. Done.
or
(8b) The temporary reference does exist. It gets associated with the parent FrameSinkId.

When a CFS gets unregistered, any temporary references associated to it get destroyed. This handles crashing or misbehaving parent. Any temporary references associated with the parent gets cleaned up, and any that will get associated with the parent will get cleaned up when SurfaceManager can't find the parent.

If a client CFSs submits a CF that the parent CFS didn't request then the parent CFS will get an unexpected OnSurfaceCreated(). There needs to be a channel for the parent CFS to reject this OnSurfaceCreated(). The temporary reference would get deleted and it would signal something is wrong the client.

I think if (3b) happens [The window server sees that the parent exists] then the FrameSinkId exists in the display compositor and so (6a) shouldn't happen.

Note that a GpuCompositorFrameSink's lifetime is bound to two MessagePipes, the client pipe and the private pipe. We don't tear down GpuCompositorFrameSink until BOTH pipes are gone.

If the client pipe goes away then the display compositor tells the window server about the lost connection, and then the window server tears down its private pipe. 

This guarantees that if the window server believes the parent still exists, then the corresponding GpuCompositorFrameSink still exists.

I generally don't like the idea of "FrameSinkId registration" happening before GpuCompositorFrameSink creation because it complicates GPU restarts.

As long as the WS uses MojoCompositorFrameSinkPrivate to tell the GPU what FrameSinkId the reference belongs to then if 3(b) happens 6(a) can't happen, so that is one less case to deal with.

There is the issue of what if the child submits a CF before the parent. I think this can only happen (at least for mus) on GPU restart but it needs to accounted for. The parent ServerWindow might not have a ServerWindowCompositorFrameSinkManager yet in this case. 

fsamuel noted we can create a SWCFSM for the parent and queue the ownership message. The only problem is we need to know what ServerWindow will be the parent CFS, as not at all ServerWindows have CFSs associated with them. I think it will always be one of the roots in the WindowTree for the immediate parent ServerWindow.

Also right now for mus FrameSinkId registration and GpuCompositorFrameSink creation is essentially the same (again for mus). Although if we send the parent FrameSinkId over MojoCompositorFrameSinkPrivate then we know the GpuCompositorFrameSink exists, so it's not necessary to have any further checks.

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

commit 85cbca558fe98eae0ddd61845ef5ebbadae19d4a
Author: kylechar <kylechar@chromium.org>
Date: Tue Feb 28 13:44:43 2017

Refactor how temporary references are stored.

This change is preparing to add an owner for temporary references. We
need to store the SurfaceId and owner FrameSinkId for each temporary
reference but also the order that temporary references from the
FrameSinkId were added.

To simplify this, a map is used to store the temporary reference and
data about it. This data is unused in this CL. We still store the order
in a separate map based on FrameSinkId.

Reduce public methods on SurfaceManager to simplify API. Some minor test
changes to work with the refactored SurfaceManager internals and to test
a case I noticed was missing.

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

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

[modify] https://crrev.com/85cbca558fe98eae0ddd61845ef5ebbadae19d4a/cc/surfaces/compositor_frame_sink_support_unittest.cc
[modify] https://crrev.com/85cbca558fe98eae0ddd61845ef5ebbadae19d4a/cc/surfaces/surface_manager.cc
[modify] https://crrev.com/85cbca558fe98eae0ddd61845ef5ebbadae19d4a/cc/surfaces/surface_manager.h
[modify] https://crrev.com/85cbca558fe98eae0ddd61845ef5ebbadae19d4a/cc/surfaces/surface_manager_ref_unittest.cc

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

commit 803cb96d3089cc214ee46062cf3759a13646b172
Author: kylechar <kylechar@chromium.org>
Date: Tue Feb 28 14:55:44 2017

Add temporary reference ownership to SurfaceManager.

We normally remove a temporary reference when the first surface
reference is added for a surface. If something happens to the parent
client, say it crashes, it may never add a surface reference. We would
be left with dangling temporary references and memory wouldn't get
freed.

Start adding a system to SurfaceManager where each temporary reference
is assigned an owner by a trusted process. This assignment will happen
from the mus-ws process (or browser process). The owner is the
FrameSinkId that is expected to embed the new Surface. If the owner
FrameSinkId is invalidated, all temporary references it owns are removed
and the Surfaces can cleaned up.

The mus-ws and IPC parts of this will be added in a followup CL.

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

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

[modify] https://crrev.com/803cb96d3089cc214ee46062cf3759a13646b172/cc/surfaces/surface_manager.cc
[modify] https://crrev.com/803cb96d3089cc214ee46062cf3759a13646b172/cc/surfaces/surface_manager.h
[modify] https://crrev.com/803cb96d3089cc214ee46062cf3759a13646b172/cc/surfaces/surface_manager_ref_unittest.cc

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

commit 5c97d3ea18174c41ad3ab15f0a9c5fe14717829d
Author: kylechar <kylechar@chromium.org>
Date: Thu Mar 02 21:54:53 2017

Implement temporary reference assignment with DisplayCompositor.

This CL implements the temporary reference assignment via
DisplayCompositor. When DisplayCompositorClient finds out a new surface
has been created, it tries to find the CompositorFrameSink that will
embed the new surface. This is done in mus-ws as it is both fully
trusted and has the full ServerWindow heirarchy.

If a parent is found an IPC is sent back to DisplayCompositor to assign
ownership of the temporary reference. If the CompositorFrameSink that
owns a temporary reference is invalidated, the temporary reference is
removed because the owner can't add a real reference after invalidation.

If no parent is found, for example if the ServerWindow no longer exists
or isn't parented to something visible, then an IPC is sent back to
DisplayCompositor to drop the temporary reference. In this case mus-ws
doesn't expect anything to embed the new surface.

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

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

[modify] https://crrev.com/5c97d3ea18174c41ad3ab15f0a9c5fe14717829d/cc/ipc/display_compositor.mojom
[modify] https://crrev.com/5c97d3ea18174c41ad3ab15f0a9c5fe14717829d/cc/ipc/mojo_compositor_frame_sink.mojom
[modify] https://crrev.com/5c97d3ea18174c41ad3ab15f0a9c5fe14717829d/cc/surfaces/compositor_frame_sink_support.cc
[modify] https://crrev.com/5c97d3ea18174c41ad3ab15f0a9c5fe14717829d/cc/surfaces/compositor_frame_sink_support.h
[modify] https://crrev.com/5c97d3ea18174c41ad3ab15f0a9c5fe14717829d/components/display_compositor/gpu_compositor_frame_sink.cc
[modify] https://crrev.com/5c97d3ea18174c41ad3ab15f0a9c5fe14717829d/components/display_compositor/gpu_compositor_frame_sink.h
[modify] https://crrev.com/5c97d3ea18174c41ad3ab15f0a9c5fe14717829d/components/display_compositor/gpu_root_compositor_frame_sink.cc
[modify] https://crrev.com/5c97d3ea18174c41ad3ab15f0a9c5fe14717829d/components/display_compositor/gpu_root_compositor_frame_sink.h
[modify] https://crrev.com/5c97d3ea18174c41ad3ab15f0a9c5fe14717829d/services/ui/surfaces/display_compositor.cc
[modify] https://crrev.com/5c97d3ea18174c41ad3ab15f0a9c5fe14717829d/services/ui/surfaces/display_compositor.h
[modify] https://crrev.com/5c97d3ea18174c41ad3ab15f0a9c5fe14717829d/services/ui/ws/server_window_compositor_frame_sink_manager.cc
[modify] https://crrev.com/5c97d3ea18174c41ad3ab15f0a9c5fe14717829d/services/ui/ws/server_window_compositor_frame_sink_manager.h
[modify] https://crrev.com/5c97d3ea18174c41ad3ab15f0a9c5fe14717829d/services/ui/ws/window_server.cc
[modify] https://crrev.com/5c97d3ea18174c41ad3ab15f0a9c5fe14717829d/services/ui/ws/window_server.h