Pull parts of TopLevelDisplayClient into DisplayCompositor

Pull parts of TopLevelDisplayClient into DisplayCompositor

This CL starts to expose the beginnings of the API DisplayCompositor
should have:

SubmitCompositorFrame
RequestCopyOfOutput

TODO:

Hittest

TopLevelDisplayClient becomes a very thin implementation of
cc::DisplayClient and starts to resemble OnscreenDisplayClient.

BUG=610937, 610935
Committed: https://crrev.com/4ae9b136bbf91f081db79870a0ceee7d88b78464
Cr-Commit-Position: refs/heads/master@{#393731}

[Fady Samuel, 2016-05-12 19:41:22]

Description was changed from

==========
Pull parts of TopLevelDisplayClient into DisplayCompositor

BUG=
==========

to

==========
Pull parts of TopLevelDisplayClient into DisplayCompositor

This CL starts to expose the beginnings of the API DisplayCompositor
should have:

SubmitCompositorFrame
RequestCopyOfOutput

TODO:
Resize
Hittest

BUG=610937, 610935
==========

[Fady Samuel, 2016-05-12 19:42:05]

Description was changed from

==========
Pull parts of TopLevelDisplayClient into DisplayCompositor

This CL starts to expose the beginnings of the API DisplayCompositor
should have:

SubmitCompositorFrame
RequestCopyOfOutput

TODO:
Resize
Hittest

BUG=610937, 610935
==========

to

==========
Pull parts of TopLevelDisplayClient into DisplayCompositor

This CL starts to expose the beginnings of the API DisplayCompositor
should have:

SubmitCompositorFrame
RequestCopyOfOutput

TODO:

Resize
Hittest

TopLevelDisplayClient becomes a very thin implementation of cc::DisplayClient
and starts to resemble OnscreenDisplayClient.

BUG=610937, 610935
==========

[Fady Samuel, 2016-05-12 19:42:26]

Description was changed from

==========
Pull parts of TopLevelDisplayClient into DisplayCompositor

This CL starts to expose the beginnings of the API DisplayCompositor
should have:

SubmitCompositorFrame
RequestCopyOfOutput

TODO:

Resize
Hittest

TopLevelDisplayClient becomes a very thin implementation of cc::DisplayClient
and starts to resemble OnscreenDisplayClient.

BUG=610937, 610935
==========

to

==========
Pull parts of TopLevelDisplayClient into DisplayCompositor

This CL starts to expose the beginnings of the API DisplayCompositor
should have:

SubmitCompositorFrame
RequestCopyOfOutput

TODO:

Resize
Hittest

TopLevelDisplayClient becomes a very thin implementation of 
cc::DisplayClient and starts to resemble OnscreenDisplayClient.

BUG=610937, 610935
==========

[Fady Samuel, 2016-05-12 19:42:46]

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

[Fady Samuel, 2016-05-12 22:01:21]

Description was changed from

==========
Pull parts of TopLevelDisplayClient into DisplayCompositor

This CL starts to expose the beginnings of the API DisplayCompositor
should have:

SubmitCompositorFrame
RequestCopyOfOutput

TODO:

Resize
Hittest

TopLevelDisplayClient becomes a very thin implementation of 
cc::DisplayClient and starts to resemble OnscreenDisplayClient.

BUG=610937, 610935
==========

to

==========
Pull parts of TopLevelDisplayClient into DisplayCompositor

This CL starts to expose the beginnings of the API DisplayCompositor
should have:

SubmitCompositorFrame
RequestCopyOfOutput

TODO:

Hittest

TopLevelDisplayClient becomes a very thin implementation of 
cc::DisplayClient and starts to resemble OnscreenDisplayClient.

BUG=610937, 610935
==========

[rjkroege, 2016-05-12 23:03:45]

I have lots of questions. Please add comments.

https://codereview.chromium.org/1976663003/diff/40001/components/mus/surfaces...
File components/mus/surfaces/display_compositor.h (right):

https://codereview.chromium.org/1976663003/diff/40001/components/mus/surfaces...
components/mus/surfaces/display_compositor.h:19: // TODO(fsamuel): This should
become a mojo interface for the mus-gpu split.
in the sequencing of this work, when do we mojo-i-fy it?

https://codereview.chromium.org/1976663003/diff/40001/components/mus/surfaces...
components/mus/surfaces/display_compositor.h:21: // The DisplayCompositor
receives CompositorFrames from all sources,
this implies that DisplayCompositor is responsible for kicking off the
composition on vblank. Yes? So SubmitCompositorFrame is async.

https://codereview.chromium.org/1976663003/diff/40001/components/mus/surfaces...
components/mus/surfaces/display_compositor.h:24: class DisplayCompositor :
public cc::SurfaceFactoryClient {
how will these have their ownership in musgpu tracked?

And... how does a musws reconnect to a DisplayCompositor after the musgpu has
experienced a sad.

https://codereview.chromium.org/1976663003/diff/40001/components/mus/surfaces...
components/mus/surfaces/display_compositor.h:32: void
SubmitCompositorFrame(std::unique_ptr<cc::CompositorFrame> frame,
Please indicate when the callback gets invoked.

https://codereview.chromium.org/1976663003/diff/40001/components/mus/surfaces...
components/mus/surfaces/display_compositor.h:35: // TODO(fsamuel): This is used
for surface hittesting and should not be
per our previous discussion, musws needs to maintain a window <-> surface
mapping. How does the musws get knowledge of the surfaces sufficient for using
the hit testing API?

https://codereview.chromium.org/1976663003/diff/40001/components/mus/surfaces...
components/mus/surfaces/display_compositor.h:39: void RequestCopyOfOutput(
what is this for?

[Fady Samuel, 2016-05-12 23:13:45]

PTAL Rob!

https://codereview.chromium.org/1976663003/diff/40001/components/mus/surfaces...
File components/mus/surfaces/display_compositor.h (right):

https://codereview.chromium.org/1976663003/diff/40001/components/mus/surfaces...
components/mus/surfaces/display_compositor.h:19: // TODO(fsamuel): This should
become a mojo interface for the mus-gpu split.
On 2016/05/12 23:03:44, rjkroege wrote:
> in the sequencing of this work, when do we mojo-i-fy it?

Once mus/ws no longer directly refers to cc, then as a last step I'll make it a
mojo interface and use that instead.

https://codereview.chromium.org/1976663003/diff/40001/components/mus/surfaces...
components/mus/surfaces/display_compositor.h:21: // The DisplayCompositor
receives CompositorFrames from all sources,
On 2016/05/12 23:03:44, rjkroege wrote:
> this implies that DisplayCompositor is responsible for kicking off the
> composition on vblank. Yes? So SubmitCompositorFrame is async.

Yes, SubmitCompositorFrame is async.

https://codereview.chromium.org/1976663003/diff/40001/components/mus/surfaces...
components/mus/surfaces/display_compositor.h:24: class DisplayCompositor :
public cc::SurfaceFactoryClient {
On 2016/05/12 23:03:44, rjkroege wrote:
> how will these have their ownership in musgpu tracked?
> 
> And... how does a musws reconnect to a DisplayCompositor after the musgpu has
> experienced a sad.

mus-ws connects to mojo:musgpu, requests a DisplayCompositorFactory interface,
and from that, a DisplayCompositor.

https://codereview.chromium.org/1976663003/diff/40001/components/mus/surfaces...
components/mus/surfaces/display_compositor.h:32: void
SubmitCompositorFrame(std::unique_ptr<cc::CompositorFrame> frame,
On 2016/05/12 23:03:44, rjkroege wrote:
> Please indicate when the callback gets invoked.

Done.

https://codereview.chromium.org/1976663003/diff/40001/components/mus/surfaces...
components/mus/surfaces/display_compositor.h:35: // TODO(fsamuel): This is used
for surface hittesting and should not be
On 2016/05/12 23:03:44, rjkroege wrote:
> per our previous discussion, musws needs to maintain a window <-> surface
> mapping. How does the musws get knowledge of the surfaces sufficient for using
> the hit testing API?

See here: https://bugs.chromium.org/p/chromium/issues/detail?id=610937

The DisplayCompositor embedder specifies IDs for "CompositorFrameSinks".

https://codereview.chromium.org/1976663003/diff/40001/components/mus/surfaces...
components/mus/surfaces/display_compositor.h:39: void RequestCopyOfOutput(
On 2016/05/12 23:03:44, rjkroege wrote:
> what is this for?

Readback for screenshots.

[rjkroege, 2016-05-12 23:26:37]

https://codereview.chromium.org/1976663003/diff/60001/components/mus/surfaces...
File components/mus/surfaces/display_compositor.cc (right):

https://codereview.chromium.org/1976663003/diff/60001/components/mus/surfaces...
components/mus/surfaces/display_compositor.cc:92:
factory_.SubmitCompositorFrame(surface_id_, std::move(frame),
why do you need to wrap the Callback?

https://codereview.chromium.org/1976663003/diff/60001/components/mus/surfaces...
File components/mus/surfaces/display_compositor.h (right):

https://codereview.chromium.org/1976663003/diff/60001/components/mus/surfaces...
components/mus/surfaces/display_compositor.h:32: // DisplayCompositor embedders
submits a CompositorFrame when content on the
submits -> submit

https://codereview.chromium.org/1976663003/diff/60001/components/mus/surfaces...
components/mus/surfaces/display_compositor.h:37: const base::Closure& callback);
should be a DrawCallback type.

https://codereview.chromium.org/1976663003/diff/60001/components/mus/surfaces...
components/mus/surfaces/display_compositor.h:43: void RequestCopyOfOutput(
what is this for?

[Fady Samuel, 2016-05-13 18:43:41]

PTAL Rob!

https://codereview.chromium.org/1976663003/diff/60001/components/mus/surfaces...
File components/mus/surfaces/display_compositor.cc (right):

https://codereview.chromium.org/1976663003/diff/60001/components/mus/surfaces...
components/mus/surfaces/display_compositor.cc:92:
factory_.SubmitCompositorFrame(surface_id_, std::move(frame),
On 2016/05/12 23:26:36, rjkroege wrote:
> why do you need to wrap the Callback?
Removed.

https://codereview.chromium.org/1976663003/diff/60001/components/mus/surfaces...
File components/mus/surfaces/display_compositor.h (right):

https://codereview.chromium.org/1976663003/diff/60001/components/mus/surfaces...
components/mus/surfaces/display_compositor.h:32: // DisplayCompositor embedders
submits a CompositorFrame when content on the
On 2016/05/12 23:26:36, rjkroege wrote:
> submits -> submit

Done.

https://codereview.chromium.org/1976663003/diff/60001/components/mus/surfaces...
components/mus/surfaces/display_compositor.h:37: const base::Closure& callback);
On 2016/05/12 23:26:36, rjkroege wrote:
> should be a DrawCallback type.

Done.

https://codereview.chromium.org/1976663003/diff/60001/components/mus/surfaces...
components/mus/surfaces/display_compositor.h:43: void RequestCopyOfOutput(
On 2016/05/12 23:26:36, rjkroege wrote:
> what is this for?

This is used for taking screenshots. I added a short description. I don't want
to make it a mus+ash specific description since it's a generic API.

[rjkroege, 2016-05-13 20:39:35]

lgtm with nit

https://codereview.chromium.org/1976663003/diff/80001/components/mus/surfaces...
File components/mus/surfaces/display_compositor.h (right):

https://codereview.chromium.org/1976663003/diff/80001/components/mus/surfaces...
components/mus/surfaces/display_compositor.h:45: // This requests the display
CompositorFrame be renderer and given to the
"be renderer"? "be rendered" maybe?

[Fady Samuel, 2016-05-13 22:45:26]

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

[Fady Samuel, 2016-05-13 22:45:27]

+sky@ for components/mus/ws

https://codereview.chromium.org/1976663003/diff/80001/components/mus/surfaces...
File components/mus/surfaces/display_compositor.h (right):

https://codereview.chromium.org/1976663003/diff/80001/components/mus/surfaces...
components/mus/surfaces/display_compositor.h:45: // This requests the display
CompositorFrame be renderer and given to the
On 2016/05/13 20:39:35, rjkroege wrote:
> "be renderer"? "be rendered" maybe?

Done.

[sky, 2016-05-13 23:50:36]

LGTM

[Fady Samuel, 2016-05-13 23:50:59]

The CQ bit was checked by fsamuel@chromium.org

[Fady Samuel, 2016-05-13 23:50:59]

The patchset sent to the CQ was uploaded after l-g-t-m from
rjkroege@chromium.org
Link to the patchset: https://codereview.chromium.org/1976663003/#ps100001
(title: "Fixed typo")

[commit-bot: I haz the power, 2016-05-13 23:51:37]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1976663003/100001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1976663003/100001

[commit-bot: I haz the power, 2016-05-14 00:27:52]

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

[commit-bot: I haz the power, 2016-05-14 00:27:53]

Try jobs failed on following builders:
  win8_chromium_ng on tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win8_chromium_ng/...)

[Fady Samuel, 2016-05-14 00:59:58]

The CQ bit was checked by fsamuel@chromium.org

[Fady Samuel, 2016-05-14 00:59:58]

The patchset sent to the CQ was uploaded after l-g-t-m from
rjkroege@chromium.org, sky@chromium.org
Link to the patchset: https://codereview.chromium.org/1976663003/#ps120001
(title: "Fix ID allocation")

[commit-bot: I haz the power, 2016-05-14 01:00:25]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1976663003/120001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1976663003/120001

[Fady Samuel, 2016-05-14 01:50:22]

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

[commit-bot: I haz the power, 2016-05-14 01:50:38]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1976663003/140001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1976663003/140001

[commit-bot: I haz the power, 2016-05-14 02:52:21]

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

[commit-bot: I haz the power, 2016-05-14 02:52:22]

Dry run: This issue passed the CQ dry run.

[Fady Samuel, 2016-05-14 02:56:07]

The CQ bit was checked by fsamuel@chromium.org

[Fady Samuel, 2016-05-14 02:56:08]

The patchset sent to the CQ was uploaded after l-g-t-m from
rjkroege@chromium.org, sky@chromium.org
Link to the patchset: https://codereview.chromium.org/1976663003/#ps140001
(title: "RegisterSurfaceIdNamespace")

[commit-bot: I haz the power, 2016-05-14 02:56:27]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1976663003/140001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1976663003/140001

[commit-bot: I haz the power, 2016-05-14 03:00:17]

Description was changed from

==========
Pull parts of TopLevelDisplayClient into DisplayCompositor

This CL starts to expose the beginnings of the API DisplayCompositor
should have:

SubmitCompositorFrame
RequestCopyOfOutput

TODO:

Hittest

TopLevelDisplayClient becomes a very thin implementation of 
cc::DisplayClient and starts to resemble OnscreenDisplayClient.

BUG=610937, 610935
==========

to

==========
Pull parts of TopLevelDisplayClient into DisplayCompositor

This CL starts to expose the beginnings of the API DisplayCompositor
should have:

SubmitCompositorFrame
RequestCopyOfOutput

TODO:

Hittest

TopLevelDisplayClient becomes a very thin implementation of 
cc::DisplayClient and starts to resemble OnscreenDisplayClient.

BUG=610937, 610935
==========

[commit-bot: I haz the power, 2016-05-14 03:00:18]

Committed patchset #8 (id:140001)

[commit-bot: I haz the power, 2016-05-14 03:01:35]

Description was changed from

==========
Pull parts of TopLevelDisplayClient into DisplayCompositor

This CL starts to expose the beginnings of the API DisplayCompositor
should have:

SubmitCompositorFrame
RequestCopyOfOutput

TODO:

Hittest

TopLevelDisplayClient becomes a very thin implementation of 
cc::DisplayClient and starts to resemble OnscreenDisplayClient.

BUG=610937, 610935
==========

to

==========
Pull parts of TopLevelDisplayClient into DisplayCompositor

This CL starts to expose the beginnings of the API DisplayCompositor
should have:

SubmitCompositorFrame
RequestCopyOfOutput

TODO:

Hittest

TopLevelDisplayClient becomes a very thin implementation of
cc::DisplayClient and starts to resemble OnscreenDisplayClient.

BUG=610937, 610935

Committed: https://crrev.com/4ae9b136bbf91f081db79870a0ceee7d88b78464
Cr-Commit-Position: refs/heads/master@{#393731}
==========

[commit-bot: I haz the power, 2016-05-14 03:01:36]

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