Add/remove surface references via MojoCompositorFrameSink.

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

Committed: https://crrev.com/2255ff5852f6d1ab187601fc18231f97409d3031
Cr-Commit-Position: refs/heads/master@{#436705}

[kylechar, 2016-11-30 17:16:48]

Description was changed from

==========
Add/remove surface references via MojoCompositorFrameSink.

WIP.

BUG=659227
==========

to

==========
Add/remove surface references via MojoCompositorFrameSink.

WIP.

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

[Fady Samuel, 2016-12-04 23:30:12]

fsamuel@chromium.org changed reviewers:
+ fsamuel@chromium.org

[Fady Samuel, 2016-12-04 23:30:13]

https://codereview.chromium.org/2541683004/diff/1/cc/ipc/display_compositor.m...
File cc/ipc/display_compositor.mojom (right):

https://codereview.chromium.org/2541683004/diff/1/cc/ipc/display_compositor.m...
cc/ipc/display_compositor.mojom:41: OnRootSurfaceId(cc.mojom.SurfaceId
surface_id);
OnDisplayCompositorCreated?

"OnRootSurfaceId" doesn't tell me what just happened. We should always have a
root surface ID available when we need it, right? Could you please add a comment
explaining why that's the case?

https://codereview.chromium.org/2541683004/diff/20001/services/ui/surfaces/gp...
File services/ui/surfaces/gpu_compositor_frame_sink.cc (right):

https://codereview.chromium.org/2541683004/diff/20001/services/ui/surfaces/gp...
services/ui/surfaces/gpu_compositor_frame_sink.cc:103:
display_compositor_->AddSurfaceReference(ref);
I'd love to see this added to CompositorFrameSinkSupport. I think we might need
a display compositor support to make that happen.

If we do that then we can start looking into transitioning ui::Compositor away
from SurfaceSequences *I think*.

[kylechar, 2016-12-05 20:26:45]

https://codereview.chromium.org/2541683004/diff/1/cc/ipc/display_compositor.m...
File cc/ipc/display_compositor.mojom (right):

https://codereview.chromium.org/2541683004/diff/1/cc/ipc/display_compositor.m...
cc/ipc/display_compositor.mojom:41: OnRootSurfaceId(cc.mojom.SurfaceId
surface_id);
On 2016/12/04 23:30:12, Fady Samuel wrote:
> OnDisplayCompositorCreated?
> 
> "OnRootSurfaceId" doesn't tell me what just happened. We should always have a
> root surface ID available when we need it, right? Could you please add a
comment
> explaining why that's the case?

Done.

https://codereview.chromium.org/2541683004/diff/20001/services/ui/surfaces/gp...
File services/ui/surfaces/gpu_compositor_frame_sink.cc (right):

https://codereview.chromium.org/2541683004/diff/20001/services/ui/surfaces/gp...
services/ui/surfaces/gpu_compositor_frame_sink.cc:103:
display_compositor_->AddSurfaceReference(ref);
On 2016/12/04 23:30:12, Fady Samuel wrote:
> I'd love to see this added to CompositorFrameSinkSupport. I think we might
need
> a display compositor support to make that happen.
> 
> If we do that then we can start looking into transitioning ui::Compositor away
> from SurfaceSequences *I think*.

CompositorFrameSinkSupport doesn't have access to DisplayCompositor, is that
what you mean?

[kylechar, 2016-12-06 14:31:06]

kylechar@chromium.org changed reviewers:
+ junov@chromium.org, sky@chromium.org, tsepez@chromium.org

[kylechar, 2016-12-06 14:31:06]

fsamuel: services/ui/surfaces/*
sky: services/ui/ws/*
tsepez: *.mojom
junov: chrome/browser/renderer_host/offscreen_canvas*

[kylechar, 2016-12-06 14:41:06]

Description was changed from

==========
Add/remove surface references via MojoCompositorFrameSink.

WIP.

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

to

==========
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.

Adding reference counting to the MojoCompositorFrameSink interface is
the simplest way to achieve this synchronization. Remove the messages
from mojom::DisplayCompositor and add 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 is the starting point for
surface references and 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 it's SurfaceId
is unguessable.

BUG=659227
==========

[kylechar, 2016-12-06 15:04:00]

Description was changed from

==========
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.

Adding reference counting to the MojoCompositorFrameSink interface is
the simplest way to achieve this synchronization. Remove the messages
from mojom::DisplayCompositor and add 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 is the starting point for
surface references and 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 it's SurfaceId
is unguessable.

BUG=659227
==========

to

==========
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
==========

[kylechar, 2016-12-06 15:53:00]

Description was changed from

==========
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
==========

to

==========
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
==========

[sky, 2016-12-06 17:35:22]

LGTM

[Fady Samuel, 2016-12-06 17:56:10]

lgtm

[Tom Sepez, 2016-12-06 17:56:20]

mojom lgtm

[kylechar, 2016-12-06 19:23:53]

The CQ bit was checked by kylechar@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-12-06 19:24:41]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-12-06 20:20:07]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-12-06 20:20:08]

Dry run: This issue passed the CQ dry run.

[kylechar, 2016-12-06 20:26:27]

Description was changed from

==========
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
==========

to

==========
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
==========

[kylechar, 2016-12-06 20:27:14]

Thanks! TBR junov.

[kylechar, 2016-12-06 20:27:28]

The CQ bit was checked by kylechar@chromium.org

[commit-bot: I haz the power, 2016-12-06 20:28:11]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-12-06 20:42:39]

CQ is committing da patch.

Bot data: {"patchset_id": 60001, "attempt_start_ts": 1481056048559860,
"parent_rev": "2740f1aa7d7bfc843c34b564bc08f2d0f0e6dc78", "commit_rev":
"b23a014d1a21fa256f4c6b3e85f4d2044270b929"}

[commit-bot: I haz the power, 2016-12-06 20:43:04]

Description was changed from

==========
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
==========

to

==========
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
==========

[commit-bot: I haz the power, 2016-12-06 20:43:05]

Committed patchset #4 (id:60001)

[commit-bot: I haz the power, 2016-12-06 20:46:40]

Description was changed from

==========
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
==========

to

==========
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

Committed: https://crrev.com/2255ff5852f6d1ab187601fc18231f97409d3031
Cr-Commit-Position: refs/heads/master@{#436705}
==========

[commit-bot: I haz the power, 2016-12-06 20:46:41]

Patchset 4 (id:??) landed as
https://crrev.com/2255ff5852f6d1ab187601fc18231f97409d3031
Cr-Commit-Position: refs/heads/master@{#436705}