GpuDisplayCompositorFrameSink => GpuRootCompositorFrameSink

GpuDisplayCompositorFrameSink => GpuRootCompositorFrameSink

GpuDisplayCompositorFrameSink, and GpuOffscreenCompositorFrameSink are 
a bit of a misnomer. Both are "onscreen" in the sense that their 
content will be displayed on screen at some point. This CL renames 
GpuDisplayCompositorFrameSink to GpuRootCompositorFrameSink, and the 
child CompositorFrameSinks, GpuCompositorFrameSink.

GpuRootCompositorFrameSink != a derived class of GpuCompositorFrameSink 
though because the Root holds an associated interface to 
MojoCompositorFrameSink.

Finally, this CL also updates display_compositor.mojom to match the new
naming.

BUG=693569
TBR=tsepez@chromium.org for rename in display_compositor.mojom
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_trusty_blink_rel

Review-Url: https://codereview.chromium.org/2710703005
Cr-Commit-Position: refs/heads/master@{#452725}
Committed: https://chromium.googlesource.com/chromium/src/+/77b4ea5b3c861be061f8f498d42ad9f63c55e82f

[Fady Samuel, 2017-02-22 23:07:09]

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

[commit-bot: I haz the power, 2017-02-22 23:07:51]

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

[Fady Samuel, 2017-02-22 23:16:18]

Description was changed from

==========
WIP: GpuRootCompositorFrameSink

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

to

==========
WIP: GpuRootCompositorFrameSink

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

[Fady Samuel, 2017-02-22 23:18:47]

Description was changed from

==========
WIP: GpuRootCompositorFrameSink

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

to

==========
WIP: GpuRootCompositorFrameSink

GpuDisplayCompositorFrameSink, and GpuOffscreenCompositorFrameSink are 
a bit of a misnomer. Both are "onscreen" in the sense that their 
content will be displayed on screen at some point. This CL renames
GpuDisplayCompositorFrameSink to GpuRootCompositorFrameSink, and the child
CompositorFrameSinks, GpuCompositorFrameSink.

GpuRootCompositorFrameSink != a derived class of GpuCompositorFrameSink 
though because the Root holds an associated interface to 
MojoCompositorFrameSink.

Finally, this CL also updates display_compositor.mojom to match the new
naming.

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

[Fady Samuel, 2017-02-22 23:19:09]

Description was changed from

==========
WIP: GpuRootCompositorFrameSink

GpuDisplayCompositorFrameSink, and GpuOffscreenCompositorFrameSink are 
a bit of a misnomer. Both are "onscreen" in the sense that their 
content will be displayed on screen at some point. This CL renames
GpuDisplayCompositorFrameSink to GpuRootCompositorFrameSink, and the child
CompositorFrameSinks, GpuCompositorFrameSink.

GpuRootCompositorFrameSink != a derived class of GpuCompositorFrameSink 
though because the Root holds an associated interface to 
MojoCompositorFrameSink.

Finally, this CL also updates display_compositor.mojom to match the new
naming.

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

to

==========
WIP: GpuRootCompositorFrameSink

GpuDisplayCompositorFrameSink, and GpuOffscreenCompositorFrameSink are 
a bit of a misnomer. Both are "onscreen" in the sense that their 
content will be displayed on screen at some point. This CL renames 
GpuDisplayCompositorFrameSink to GpuRootCompositorFrameSink, and the 
child CompositorFrameSinks, GpuCompositorFrameSink.

GpuRootCompositorFrameSink != a derived class of GpuCompositorFrameSink 
though because the Root holds an associated interface to 
MojoCompositorFrameSink.

Finally, this CL also updates display_compositor.mojom to match the new
naming.

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

[Fady Samuel, 2017-02-22 23:19:15]

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

[Fady Samuel, 2017-02-22 23:20:00]

Description was changed from

==========
WIP: GpuRootCompositorFrameSink

GpuDisplayCompositorFrameSink, and GpuOffscreenCompositorFrameSink are 
a bit of a misnomer. Both are "onscreen" in the sense that their 
content will be displayed on screen at some point. This CL renames 
GpuDisplayCompositorFrameSink to GpuRootCompositorFrameSink, and the 
child CompositorFrameSinks, GpuCompositorFrameSink.

GpuRootCompositorFrameSink != a derived class of GpuCompositorFrameSink 
though because the Root holds an associated interface to 
MojoCompositorFrameSink.

Finally, this CL also updates display_compositor.mojom to match the new
naming.

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

to

==========
GpuDisplayCompositorFrameSink => GpuRootCompositorFrameSink

GpuDisplayCompositorFrameSink, and GpuOffscreenCompositorFrameSink are 
a bit of a misnomer. Both are "onscreen" in the sense that their 
content will be displayed on screen at some point. This CL renames 
GpuDisplayCompositorFrameSink to GpuRootCompositorFrameSink, and the 
child CompositorFrameSinks, GpuCompositorFrameSink.

GpuRootCompositorFrameSink != a derived class of GpuCompositorFrameSink 
though because the Root holds an associated interface to 
MojoCompositorFrameSink.

Finally, this CL also updates display_compositor.mojom to match the new
naming.

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

[Fady Samuel, 2017-02-22 23:20:10]

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

[commit-bot: I haz the power, 2017-02-22 23:20:47]

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, 2017-02-23 01:01:11]

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

[commit-bot: I haz the power, 2017-02-23 01:01:12]

Dry run: This issue passed the CQ dry run.

[danakj, 2017-02-23 22:13:50]

https://codereview.chromium.org/2710703005/diff/40001/cc/ipc/display_composit...
File cc/ipc/display_compositor.mojom (right):

https://codereview.chromium.org/2710703005/diff/40001/cc/ipc/display_composit...
cc/ipc/display_compositor.mojom:53: // This CompositorFrameSink only submits
frames to a buffer instead
This comment seems like it needs fixing, saying its to a buffer instead of a
display? It's not a root and has to be parented by something to appear, rather?

https://codereview.chromium.org/2710703005/diff/40001/components/display_comp...
File components/display_compositor/gpu_compositor_frame_sink.h (right):

https://codereview.chromium.org/2710703005/diff/40001/components/display_comp...
components/display_compositor/gpu_compositor_frame_sink.h:31:
cc::SurfaceManager* surface_manager,
Does this class need to know about SurfaceManager* and FrameSinkRequest? Passing
in a CompositorFrameSinkSupport means this class only knows the minimum types it
needs to and doesn't introduce lifetime concerns and sudden additional
dependencies. It gives better encapsulations. I think this is regressing that.

https://codereview.chromium.org/2710703005/diff/40001/components/display_comp...
components/display_compositor/gpu_compositor_frame_sink.h:55: protected:
Remove

https://codereview.chromium.org/2710703005/diff/40001/components/display_comp...
File components/display_compositor/gpu_root_compositor_frame_sink.h (right):

https://codereview.chromium.org/2710703005/diff/40001/components/display_comp...
components/display_compositor/gpu_root_compositor_frame_sink.h:8: #include
"cc/surfaces/display.h"
forward declare?

https://codereview.chromium.org/2710703005/diff/40001/components/display_comp...
components/display_compositor/gpu_root_compositor_frame_sink.h:10: #include
"components/display_compositor/gpu_compositor_frame_sink.h"
needed?

https://codereview.chromium.org/2710703005/diff/40001/components/display_comp...
components/display_compositor/gpu_root_compositor_frame_sink.h:11: #include
"components/display_compositor/gpu_compositor_frame_sink_delegate.h"
forward declare?

https://codereview.chromium.org/2710703005/diff/40001/components/display_comp...
components/display_compositor/gpu_root_compositor_frame_sink.h:28:
std::unique_ptr<cc::BeginFrameSource> begin_frame_source,
forward declare the types this uses

https://codereview.chromium.org/2710703005/diff/40001/components/display_comp...
components/display_compositor/gpu_root_compositor_frame_sink.h:81: // it created
with non-null gpu::SurfaceHandle. In the window server, the
was created with a

https://codereview.chromium.org/2710703005/diff/40001/components/display_comp...
components/display_compositor/gpu_root_compositor_frame_sink.h:82: // display
root window's CompositorFrameSink will have a valid
This is the RootCFS doesn't it always have a display?

[danakj, 2017-02-23 22:14:01]

LGTM once that stuff is addressed

[Fady Samuel, 2017-02-23 23:14:57]

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

[Fady Samuel, 2017-02-23 23:14:58]

+sky@ for services/ui
+tsepeze for display_compositor.mojom

https://codereview.chromium.org/2710703005/diff/40001/cc/ipc/display_composit...
File cc/ipc/display_compositor.mojom (right):

https://codereview.chromium.org/2710703005/diff/40001/cc/ipc/display_composit...
cc/ipc/display_compositor.mojom:53: // This CompositorFrameSink only submits
frames to a buffer instead
On 2017/02/23 22:13:44, danakj wrote:
> This comment seems like it needs fixing, saying its to a buffer instead of a
> display? It's not a root and has to be parented by something to appear,
rather?

Done.

https://codereview.chromium.org/2710703005/diff/40001/components/display_comp...
File components/display_compositor/gpu_compositor_frame_sink.h (right):

https://codereview.chromium.org/2710703005/diff/40001/components/display_comp...
components/display_compositor/gpu_compositor_frame_sink.h:31:
cc::SurfaceManager* surface_manager,
On 2017/02/23 22:13:44, danakj wrote:
> Does this class need to know about SurfaceManager* and FrameSinkRequest?
Passing
> in a CompositorFrameSinkSupport means this class only knows the minimum types
it
> needs to and doesn't introduce lifetime concerns and sudden additional
> dependencies. It gives better encapsulations. I think this is regressing that.

Agreed but GpuCompositorFrameSink is a CompositorFrameSinkSupportClient and so I
can't avoid that unless I add a BindToClient or something on
CompositorFrameSinkSupport. That'll touch code all over the place, so I'll leave
that to a subsequent CL.

https://codereview.chromium.org/2710703005/diff/40001/components/display_comp...
components/display_compositor/gpu_compositor_frame_sink.h:55: protected:
On 2017/02/23 22:13:44, danakj wrote:
> Remove

Done.

https://codereview.chromium.org/2710703005/diff/40001/components/display_comp...
File components/display_compositor/gpu_root_compositor_frame_sink.h (right):

https://codereview.chromium.org/2710703005/diff/40001/components/display_comp...
components/display_compositor/gpu_root_compositor_frame_sink.h:8: #include
"cc/surfaces/display.h"
On 2017/02/23 22:13:46, danakj wrote:
> forward declare?

Done.

https://codereview.chromium.org/2710703005/diff/40001/components/display_comp...
components/display_compositor/gpu_root_compositor_frame_sink.h:10: #include
"components/display_compositor/gpu_compositor_frame_sink.h"
On 2017/02/23 22:13:47, danakj wrote:
> needed?

Not anymore, removed.

https://codereview.chromium.org/2710703005/diff/40001/components/display_comp...
components/display_compositor/gpu_root_compositor_frame_sink.h:11: #include
"components/display_compositor/gpu_compositor_frame_sink_delegate.h"
On 2017/02/23 22:13:48, danakj wrote:
> forward declare?

Done.

https://codereview.chromium.org/2710703005/diff/40001/components/display_comp...
components/display_compositor/gpu_root_compositor_frame_sink.h:28:
std::unique_ptr<cc::BeginFrameSource> begin_frame_source,
On 2017/02/23 22:13:45, danakj wrote:
> forward declare the types this uses

Done.

https://codereview.chromium.org/2710703005/diff/40001/components/display_comp...
components/display_compositor/gpu_root_compositor_frame_sink.h:81: // it created
with non-null gpu::SurfaceHandle. In the window server, the
On 2017/02/23 22:13:46, danakj wrote:
> was created with a 

Done.

https://codereview.chromium.org/2710703005/diff/40001/components/display_comp...
components/display_compositor/gpu_root_compositor_frame_sink.h:82: // display
root window's CompositorFrameSink will have a valid
On 2017/02/23 22:13:46, danakj wrote:
> This is the RootCFS doesn't it always have a display?

Yea, I removed the second sentence to avoid the confusion.

[danakj, 2017-02-23 23:17:33]

https://codereview.chromium.org/2710703005/diff/40001/components/display_comp...
File components/display_compositor/gpu_compositor_frame_sink.h (right):

https://codereview.chromium.org/2710703005/diff/40001/components/display_comp...
components/display_compositor/gpu_compositor_frame_sink.h:31:
cc::SurfaceManager* surface_manager,
On 2017/02/23 23:14:58, Fady Samuel wrote:
> On 2017/02/23 22:13:44, danakj wrote:
> > Does this class need to know about SurfaceManager* and FrameSinkRequest?
> Passing
> > in a CompositorFrameSinkSupport means this class only knows the minimum
types
> it
> > needs to and doesn't introduce lifetime concerns and sudden additional
> > dependencies. It gives better encapsulations. I think this is regressing
that.
> 
> Agreed but GpuCompositorFrameSink is a CompositorFrameSinkSupportClient and so
I
> can't avoid that unless I add a BindToClient or something on
> CompositorFrameSinkSupport. That'll touch code all over the place, so I'll
leave
> that to a subsequent CL.

Ah I see, then there's a good reason at least, thanks.

[Fady Samuel, 2017-02-23 23:42:55]

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

[commit-bot: I haz the power, 2017-02-23 23:43:25]

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

[sky, 2017-02-24 01:03:42]

LGTM

[Fady Samuel, 2017-02-24 01:22:35]

Description was changed from

==========
GpuDisplayCompositorFrameSink => GpuRootCompositorFrameSink

GpuDisplayCompositorFrameSink, and GpuOffscreenCompositorFrameSink are 
a bit of a misnomer. Both are "onscreen" in the sense that their 
content will be displayed on screen at some point. This CL renames 
GpuDisplayCompositorFrameSink to GpuRootCompositorFrameSink, and the 
child CompositorFrameSinks, GpuCompositorFrameSink.

GpuRootCompositorFrameSink != a derived class of GpuCompositorFrameSink 
though because the Root holds an associated interface to 
MojoCompositorFrameSink.

Finally, this CL also updates display_compositor.mojom to match the new
naming.

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

to

==========
GpuDisplayCompositorFrameSink => GpuRootCompositorFrameSink

GpuDisplayCompositorFrameSink, and GpuOffscreenCompositorFrameSink are 
a bit of a misnomer. Both are "onscreen" in the sense that their 
content will be displayed on screen at some point. This CL renames 
GpuDisplayCompositorFrameSink to GpuRootCompositorFrameSink, and the 
child CompositorFrameSinks, GpuCompositorFrameSink.

GpuRootCompositorFrameSink != a derived class of GpuCompositorFrameSink 
though because the Root holds an associated interface to 
MojoCompositorFrameSink.

Finally, this CL also updates display_compositor.mojom to match the new
naming.

BUG=693569
TBR=tsepez@chromium.org for rename in display_compositor.mojom
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_trusty_blink_rel
==========

[Fady Samuel, 2017-02-24 01:22:54]

No IPCs are being added or removed so I'm TBR'ing tsepez@.

[commit-bot: I haz the power, 2017-02-24 02:37:28]

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

[commit-bot: I haz the power, 2017-02-24 02:37:29]

Dry run: This issue passed the CQ dry run.

[Fady Samuel, 2017-02-24 02:44:38]

The CQ bit was checked by fsamuel@chromium.org

[Fady Samuel, 2017-02-24 02:44:38]

The patchset sent to the CQ was uploaded after l-g-t-m from danakj@chromium.org
Link to the patchset: https://codereview.chromium.org/2710703005/#ps60001
(title: "Addressed Dana's comments")

[commit-bot: I haz the power, 2017-02-24 02:45:02]

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, 2017-02-24 02:51:47]

CQ is committing da patch.

Bot data: {"patchset_id": 60001, "attempt_start_ts": 1487904278283740,
"parent_rev": "36fa76db42e1543599371dd2d5cb2c32bcee39f2", "commit_rev":
"77b4ea5b3c861be061f8f498d42ad9f63c55e82f"}

[commit-bot: I haz the power, 2017-02-24 02:52:28]

Description was changed from

==========
GpuDisplayCompositorFrameSink => GpuRootCompositorFrameSink

GpuDisplayCompositorFrameSink, and GpuOffscreenCompositorFrameSink are 
a bit of a misnomer. Both are "onscreen" in the sense that their 
content will be displayed on screen at some point. This CL renames 
GpuDisplayCompositorFrameSink to GpuRootCompositorFrameSink, and the 
child CompositorFrameSinks, GpuCompositorFrameSink.

GpuRootCompositorFrameSink != a derived class of GpuCompositorFrameSink 
though because the Root holds an associated interface to 
MojoCompositorFrameSink.

Finally, this CL also updates display_compositor.mojom to match the new
naming.

BUG=693569
TBR=tsepez@chromium.org for rename in display_compositor.mojom
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_trusty_blink_rel
==========

to

==========
GpuDisplayCompositorFrameSink => GpuRootCompositorFrameSink

GpuDisplayCompositorFrameSink, and GpuOffscreenCompositorFrameSink are 
a bit of a misnomer. Both are "onscreen" in the sense that their 
content will be displayed on screen at some point. This CL renames 
GpuDisplayCompositorFrameSink to GpuRootCompositorFrameSink, and the 
child CompositorFrameSinks, GpuCompositorFrameSink.

GpuRootCompositorFrameSink != a derived class of GpuCompositorFrameSink 
though because the Root holds an associated interface to 
MojoCompositorFrameSink.

Finally, this CL also updates display_compositor.mojom to match the new
naming.

BUG=693569
TBR=tsepez@chromium.org for rename in display_compositor.mojom
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_trusty_blink_rel

Review-Url: https://codereview.chromium.org/2710703005
Cr-Commit-Position: refs/heads/master@{#452725}
Committed:
https://chromium.googlesource.com/chromium/src/+/77b4ea5b3c861be061f8f498d42a...
==========

[commit-bot: I haz the power, 2017-02-24 02:52:29]

Committed patchset #4 (id:60001) as
https://chromium.googlesource.com/chromium/src/+/77b4ea5b3c861be061f8f498d42a...