GpuCompositorFrameSink implements cc::mojom::DisplayPrivate

cc/ipc/display_compositor.mojom

 // Copyright 2016 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 module cc.mojom;
 
 import "cc/ipc/frame_sink_id.mojom";
 import "cc/ipc/mojo_compositor_frame_sink.mojom";
 import "cc/ipc/surface_id.mojom";
 import "cc/ipc/surface_sequence.mojom";
 import "gpu/ipc/common/surface_handle.mojom";
+import "mojo/common/time.mojom";
 import "ui/gfx/geometry/mojo/geometry.mojom";
+import "ui/gfx/mojo/color_space.mojom";
 
 // The DisplayCompositor interface is a privileged interface that allows
 // the display compositor host (browser or window server) to create
 // CompositorFrameSinks. Clients acquire a CompositorFrameSink connection
 // through the display compositor host. Clients request a
 // MojoCompositorFrameSink interface, and implement a
 // MojoCompositorFrameSinkClient interface. The display compositor host
 // holds one or more root CompositorFrameSinks that are tied to a valid
 // |surface_handle|. All other CompositorFrameSinks are offscreen. FrameSinkIds
 // are fixed for a given client and are determined ahead of time. Thus, a client
 // will typically simply request a CompositorFrameSink from the display
 // compositor host which will forward the request to the display compositor.
 interface DisplayCompositor {
   CreateDisplayCompositorFrameSink(

[kylechar, 2016/12/16 17:15:12]

Comments to disambiguate CreateDisplayCompositorFrameSink() vs
CreateOffscreenCompositorFrameSink() would be helpful.

[Alex Z., 2016/12/19 19:27:47]

Done.

       cc.mojom.FrameSinkId frame_sink_id,
       gpu.mojom.SurfaceHandle widget,
       cc.mojom.MojoCompositorFrameSink& compositor_frame_sink,
       cc.mojom.MojoCompositorFrameSinkPrivate& compositor_frame_sink_private,
-      cc.mojom.MojoCompositorFrameSinkClient compositor_frame_sink_client);
+      cc.mojom.MojoCompositorFrameSinkClient compositor_frame_sink_client,
+      DisplayPrivate& display_private);
 
   CreateOffscreenCompositorFrameSink(
       cc.mojom.FrameSinkId frame_sink_id,
       cc.mojom.MojoCompositorFrameSink& compositor_frame_sink,
       cc.mojom.MojoCompositorFrameSinkPrivate& compositor_frame_sink_private,
       cc.mojom.MojoCompositorFrameSinkClient compositor_frame_sink_client);
 };
 
 // The DisplayCompositorClient interface is implemented by the Display
 // Compositor Host, a stable, and privileged peer service to the display
 // compositor. The display compositor host is either the browser process in
 // Chrome or the window server process.
 interface DisplayCompositorClient {
   // Called by the display compostor when it is created. Provides the top level
   // root surface id that should reference display roots. This will always be
   // the first message sent.
   OnDisplayCompositorCreated(cc.mojom.SurfaceId root_surface_id);
 
   // Called by the display compositor immediately upon receiving a
   // CompositorFrame with a new SurfaceId for the first time.
   OnSurfaceCreated(cc.mojom.SurfaceId surface_id,
                    gfx.mojom.Size frame_size,
                    float device_scale_factor);
 };
+
+// The DisplayPrivate is implemented by GpuCompositorFrameSink. It is used so
+// that ContextFactory can talk to cc::Display without knowing about
+// cc::Display.

[Fady Samuel, 2016/12/16 17:18:18]

nit: I would not reference GpuCompositorFrameSink here, nor would I reference
ContextFactory because those are in different layers of the system. Maybe simply
say // See cc/surfaces/display.h

Almost move this above the DisplayCompositor interface. This should be near the
top of the file. That's simply easier for readability.
CreateDisplayCompositorFrameSink uses this interface, and so it would be nice to
encounter this interface first in the file.

[Alex Z., 2016/12/19 19:27:47]

Done.

+interface DisplayPrivate {

[kylechar, 2016/12/16 17:08:32]

Is it DisplayPrivate just to disambiguate it from other things named Display?

+  SetDisplayVisible(bool visible);
+  ResizeDisplay(gfx.mojom.Size size);

[sadrul, 2016/12/16 17:12:05]

Can you add a comment to mention whether |size| is in DIP or physical pixel
space?

[danakj, 2016/12/16 17:13:41]

Or name it size_in_dip size_in_physical_pixels or the like

[Alex Z., 2016/12/19 19:27:47]

Done.

+  SetDisplayColorSpace(gfx.mojom.ColorSpace color_space);
+  SetOutputIsSecure(bool secure);
+};