Maybe DirectCompositorFrameSink can be split up into two pieces. A "client-side" piece and a "service-side" piece. The client side object can speak directly to the service-side object for now (until we are confident about performance) but the service-side object should implement MojoCompositorFrameSink for consistency.

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

commit 6ef62be33d4c2fcc79b6046e8b1a6fd9a1c50093
Author: staraz <staraz@chromium.org>
Date: Fri Feb 03 04:50:00 2017

Removed cc::CompositorFrameSink::client_thread_checker_

RendererCompositorFrameSink holds its own thread checker similar to what
WindowCompositorFrameSink and DirectCompositorFrameSink do. This way,
cc::CompositorFrameSink::client_thread_check_ is no longer needed and we have
one less field in the API class.

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

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

[modify] https://crrev.com/6ef62be33d4c2fcc79b6046e8b1a6fd9a1c50093/cc/output/compositor_frame_sink.cc
[modify] https://crrev.com/6ef62be33d4c2fcc79b6046e8b1a6fd9a1c50093/cc/output/compositor_frame_sink.h
[modify] https://crrev.com/6ef62be33d4c2fcc79b6046e8b1a6fd9a1c50093/content/renderer/gpu/renderer_compositor_frame_sink.cc
[modify] https://crrev.com/6ef62be33d4c2fcc79b6046e8b1a6fd9a1c50093/content/renderer/gpu/renderer_compositor_frame_sink.h

This is a high priority task, but I realized I wasn't clear WHY it's a high priority task. Here's a long-winded explanation:

Currently we have multiple display compositor in Mus+Ash, one that lives in the Mus process and one that lives in the browser process. --use-mus-in-renderer does *not* address that. We still have two instances of SurfaceManager, two instances of SurfaceAggregator, two OutputSurfaces, etc. In particular, a key deliverable of a unified display compositor in Mus+Ash is MusBrowserCompositorOutputSurface goes away.

The overhead here is exo, browser UI, offscreen canvas feed their CompositorFrames into the browser display compositor, and at the final stage of that display compositor, MusBrowserCompositorOutputSurface constructs a CompositorFrame referring to the browser's "front" buffer and passes it along to Mus for compositing.

Ultimately, the browser clients should submit CompositorFrames directly to display compositor in Mus, completely avoiding the use of SurfaceManager in the browser process to maximize opportunities for surface draw occlusion in SurfaceAggregator, once implemented. We've identified that overdraw as being one of the key risks to a shippable Mus+Ash in September 2016.

piman@ raised the concern about the added cost of an IPC. In particular, up to this point, browser UI submitted CompositorFrames directly to the display compositor in the browser process, through DirectCompositorFrameSink. Preliminary studies done by samans@ and weiliangc@ suggest that CompositorFrames can be expensive, taking up about 1 ms of a frame's budget on low end devices on average. danakj@ suggests that moving the display compositor to the GPU process gives us a clean compositor thread that minimizes queuing delay of messages and may gain back some of the overheads we've measured so far.

We would like to test the cost of UI CompositorFrames over IPC in isolation from the rest of Mus+Ash changes. Splitting DirectCompositorFrameSink takes us in that direction. Once we create a CompositorFrameSinkSupport and ultimately a mojo IPC boundary (perhaps behind a flag) we can test the real world overhead of a CompositorFrame IPC in UI performance/memory/battery life. If we are confident in its performance, then we can turn on that code path by default knowing that that will not be a major pain point in Mus+Ash.

> In particular, a key deliverable of a unified display compositor in Mus+Ash
> is MusBrowserCompositorOutputSurface goes away.

afaict, we are not using MusBrowserCompositorOutputSurface currently (with --use-mus-in-renderer turned on by default), and the browser UI submits the CF directly to mus (through ui::WindowCompositorFrameSink).

Interesting, I forgot that the browser UI has its own ContextFactory in Mus+Ash. Presumably that means we're not using GpuProcessTransportFactory (at least for UI at the moment) so we should get performance similar to what we'd get at the end state of a unified display compositor with --use-mus-in-renderer. We still need to unify these code paths for offscreen canvas, exo and OOPIFs.

To address piman@'s concerns, I'd like test the cost of ui::Compositor's CompositorFrameSink over IPC in the wild very soon in isolation from the rest of Mus+Ash though.

The end state of this refactor is DirectCompositorFrameSink is diced into two pieces: one that looks like WindowCompositorFrameSink, and one that looks like GpuOffscreenCompositorFrameSink. Then we actually use WindowCompositorFrameSInk and GpuOffscreenCompositorFrameSink, add a bunch of traces, in there, and we can measure before AND after performance with MojoCompositorFrameSink in isolation.

The patch to use CompositorFrameSinkSupport looks just about ready to land: https://codereview.chromium.org/2612083002/

I should add that we observed a 20% performance regression in exo when we introduced mojo IPC so it's not immediately obvious to me what the performance impact will be to use IPC from browser UI.

> I should add that we observed a 20% performance regression in exo when we
> introduced mojo IPC ...

What did we do to get back the perf for exo?

#10: We stopped using IPC and performance was back up :(

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

commit aa231112e50ded728119bba18dfd4d93d596f00f
Author: staraz <staraz@chromium.org>
Date: Tue Feb 07 17:53:24 2017

Add AddChildFrameSink() and RemoveChildFrameSink() To WindowAndroidCompositor

DelegatedFrameHostAndroid calls WindowAndroidCompositor::AddChildFrameSink()
and RemoveChildFrameSink() when it needs to attach to/detach from a compositor.

CompositorImpl manages an unordered set of pending FrameSinkIds that emulates
the message queue in mojo on desktop Chrome.

In the future, the FrameSinkId will be invalidated during gpu restart. This
change ensures that SurfaceManager::RegisterHierarchy() only gets called when
the root window has a valid FrameSinkId.

BUG= 673543 

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

[modify] https://crrev.com/aa231112e50ded728119bba18dfd4d93d596f00f/content/browser/renderer_host/compositor_impl_android.cc
[modify] https://crrev.com/aa231112e50ded728119bba18dfd4d93d596f00f/content/browser/renderer_host/compositor_impl_android.h
[modify] https://crrev.com/aa231112e50ded728119bba18dfd4d93d596f00f/content/browser/renderer_host/render_widget_host_view_android.cc
[modify] https://crrev.com/aa231112e50ded728119bba18dfd4d93d596f00f/ui/android/delegated_frame_host_android.cc
[modify] https://crrev.com/aa231112e50ded728119bba18dfd4d93d596f00f/ui/android/delegated_frame_host_android.h
[modify] https://crrev.com/aa231112e50ded728119bba18dfd4d93d596f00f/ui/android/window_android_compositor.h

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

commit c405416053b99aa4d6f98f3126608c660773f6b4
Author: staraz <staraz@chromium.org>
Date: Tue Feb 07 22:05:31 2017

DirectCompositorFrameSink Uses CompositorFrameSinkSupport

CompositorFrameSinkSupport is the common service-side implementation of
CompositorFrameSink. In an effort to evaluate mojo performance
independently of refactoring existing code, CompositorFrameSinkSupport
provides the same public interface as MojoCompositorFrameSink but does
not depend on mojo.

This CL takes us in the direction of splitting DirectCompositorFrameSink
into two pieces: a client side component and a service side component.

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

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

[modify] https://crrev.com/c405416053b99aa4d6f98f3126608c660773f6b4/cc/surfaces/compositor_frame_sink_support.cc
[modify] https://crrev.com/c405416053b99aa4d6f98f3126608c660773f6b4/cc/surfaces/compositor_frame_sink_support.h
[modify] https://crrev.com/c405416053b99aa4d6f98f3126608c660773f6b4/cc/surfaces/direct_compositor_frame_sink.cc
[modify] https://crrev.com/c405416053b99aa4d6f98f3126608c660773f6b4/cc/surfaces/direct_compositor_frame_sink.h
[modify] https://crrev.com/c405416053b99aa4d6f98f3126608c660773f6b4/components/display_compositor/gpu_compositor_frame_sink.cc
[modify] https://crrev.com/c405416053b99aa4d6f98f3126608c660773f6b4/components/display_compositor/gpu_compositor_frame_sink.h
[modify] https://crrev.com/c405416053b99aa4d6f98f3126608c660773f6b4/components/display_compositor/gpu_display_compositor_frame_sink.cc
[modify] https://crrev.com/c405416053b99aa4d6f98f3126608c660773f6b4/components/display_compositor/gpu_display_compositor_frame_sink.h
[modify] https://crrev.com/c405416053b99aa4d6f98f3126608c660773f6b4/components/display_compositor/gpu_offscreen_compositor_frame_sink.cc
[modify] https://crrev.com/c405416053b99aa4d6f98f3126608c660773f6b4/components/exo/compositor_frame_sink.cc
[modify] https://crrev.com/c405416053b99aa4d6f98f3126608c660773f6b4/content/browser/renderer_host/offscreen_canvas_compositor_frame_sink.cc

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

commit 5dc5c67c91f37b85db7333304b5bec57c42a3810
Author: jam <jam@chromium.org>
Date: Wed Feb 08 23:19:23 2017

Revert of DirectCompositorFrameSink Uses CompositorFrameSinkSupport (patchset #39 id:830001 of https://codereview.chromium.org/2612083002/ )

Reason for revert:
Causing lots of flake with Android tests
https://bugs.chromium.org/p/chromium/issues/detail?id=676070.

Original issue's description:
> DirectCompositorFrameSink Uses CompositorFrameSinkSupport
>
> CompositorFrameSinkSupport is the common service-side implementation of
> CompositorFrameSink. In an effort to evaluate mojo performance
> independently of refactoring existing code, CompositorFrameSinkSupport
> provides the same public interface as MojoCompositorFrameSink but does
> not depend on mojo.
>
> This CL takes us in the direction of splitting DirectCompositorFrameSink
> into two pieces: a client side component and a service side component.
>
> BUG= 673543 
> CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_trusty_blink_rel
>
> Review-Url: https://codereview.chromium.org/2612083002
> Cr-Commit-Position: refs/heads/master@{#448731}
> Committed: https://chromium.googlesource.com/chromium/src/+/c405416053b99aa4d6f98f3126608c660773f6b4

TBR=reveman@chromium.org,danakj@chromium.org,fsamuel@chromium.org,jbauman@chromium.org,xlai@chromium.org,boliu@chromium.org,staraz@chromium.org
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG= 673543 

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

[modify] https://crrev.com/5dc5c67c91f37b85db7333304b5bec57c42a3810/cc/surfaces/compositor_frame_sink_support.cc
[modify] https://crrev.com/5dc5c67c91f37b85db7333304b5bec57c42a3810/cc/surfaces/compositor_frame_sink_support.h
[modify] https://crrev.com/5dc5c67c91f37b85db7333304b5bec57c42a3810/cc/surfaces/direct_compositor_frame_sink.cc
[modify] https://crrev.com/5dc5c67c91f37b85db7333304b5bec57c42a3810/cc/surfaces/direct_compositor_frame_sink.h
[modify] https://crrev.com/5dc5c67c91f37b85db7333304b5bec57c42a3810/components/display_compositor/gpu_compositor_frame_sink.cc
[modify] https://crrev.com/5dc5c67c91f37b85db7333304b5bec57c42a3810/components/display_compositor/gpu_compositor_frame_sink.h
[modify] https://crrev.com/5dc5c67c91f37b85db7333304b5bec57c42a3810/components/display_compositor/gpu_display_compositor_frame_sink.cc
[modify] https://crrev.com/5dc5c67c91f37b85db7333304b5bec57c42a3810/components/display_compositor/gpu_display_compositor_frame_sink.h
[modify] https://crrev.com/5dc5c67c91f37b85db7333304b5bec57c42a3810/components/display_compositor/gpu_offscreen_compositor_frame_sink.cc
[modify] https://crrev.com/5dc5c67c91f37b85db7333304b5bec57c42a3810/components/exo/compositor_frame_sink.cc
[modify] https://crrev.com/5dc5c67c91f37b85db7333304b5bec57c42a3810/content/browser/renderer_host/offscreen_canvas_compositor_frame_sink.cc

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

commit f4a164c08e371b3d9948a078c5866b702eef6fd1
Author: staraz <staraz@chromium.org>
Date: Tue Feb 14 21:10:33 2017

Prepare CompositorFrameSinkSupport To Be Used By DirectCompositorFrameSink

Add handles_frame_sink_id_invalidation and need_sync_points flags to
CompositorFrameSinkSupport and its usages.

This CL is a subset of changes in 2612083002. It contains all the changes to
CompositorFrameSinkSupport that are needed for it to be used by
DirectCompositorFrameSink.

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

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

[modify] https://crrev.com/f4a164c08e371b3d9948a078c5866b702eef6fd1/android_webview/browser/surfaces_instance.cc
[modify] https://crrev.com/f4a164c08e371b3d9948a078c5866b702eef6fd1/cc/surfaces/compositor_frame_sink_support.cc
[modify] https://crrev.com/f4a164c08e371b3d9948a078c5866b702eef6fd1/cc/surfaces/compositor_frame_sink_support.h
[modify] https://crrev.com/f4a164c08e371b3d9948a078c5866b702eef6fd1/cc/surfaces/compositor_frame_sink_support_unittest.cc
[modify] https://crrev.com/f4a164c08e371b3d9948a078c5866b702eef6fd1/components/display_compositor/gpu_display_compositor_frame_sink.cc
[modify] https://crrev.com/f4a164c08e371b3d9948a078c5866b702eef6fd1/components/display_compositor/gpu_offscreen_compositor_frame_sink.cc
[modify] https://crrev.com/f4a164c08e371b3d9948a078c5866b702eef6fd1/components/exo/compositor_frame_sink.cc
[modify] https://crrev.com/f4a164c08e371b3d9948a078c5866b702eef6fd1/content/browser/renderer_host/offscreen_canvas_compositor_frame_sink.cc
[modify] https://crrev.com/f4a164c08e371b3d9948a078c5866b702eef6fd1/content/renderer/android/synchronous_compositor_frame_sink.cc

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

commit 9d4e3df6e621c6bd80dfaee7008b3e33d0c4fe79
Author: staraz <staraz@chromium.org>
Date: Sat Feb 18 15:16:39 2017

DirectCompositorFrameSink Uses CompositorFrameSinkSupport

CompositorFrameSinkSupport is the common service-side implementation of
CompositorFrameSink. In an effort to evaluate mojo performance
independently of refactoring existing code, CompositorFrameSinkSupport
provides the same public interface as MojoCompositorFrameSink but does
not depend on mojo.

This CL takes us in the direction of splitting DirectCompositorFrameSink
into two pieces: a client side component and a service side component.

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

Review-Url: https://codereview.chromium.org/2612083002
Cr-Original-Commit-Position: refs/heads/master@{#448731}
Committed: https://chromium.googlesource.com/chromium/src/+/c405416053b99aa4d6f98f3126608c660773f6b4
Review-Url: https://codereview.chromium.org/2612083002
Cr-Commit-Position: refs/heads/master@{#451462}

[modify] https://crrev.com/9d4e3df6e621c6bd80dfaee7008b3e33d0c4fe79/cc/surfaces/compositor_frame_sink_support.cc
[modify] https://crrev.com/9d4e3df6e621c6bd80dfaee7008b3e33d0c4fe79/cc/surfaces/compositor_frame_sink_support.h
[modify] https://crrev.com/9d4e3df6e621c6bd80dfaee7008b3e33d0c4fe79/cc/surfaces/direct_compositor_frame_sink.cc
[modify] https://crrev.com/9d4e3df6e621c6bd80dfaee7008b3e33d0c4fe79/cc/surfaces/direct_compositor_frame_sink.h