WillDraw happens every time and the ack is meant to unblock the next frame and only happens once, also.

IIRC in an earlier IRC discussion, rjkroege@ and piman@ felt that maybe DidReceiveCompositorFrameAck can go away entirely now that we have unified BeginFrame.

Right now begin frames are always sent to clients, so they have to throttle themselves, and they do so with an ack. If the begin frame was only sent to clients that had been drawn at least once, then we wouldn't need to throttle there and wouldn't need acks.

danakj@: Ohh I see. You're saying we shouldn't issue a BeginFrame to a client that hasn't yet sent out a corresponding CompositorFrame? We don't do that right now?

*a CompositorFrame corresponding to the previous BeginFrame, I should say.

We would have to send 1 BeginFrame to a client, then wait for it to submit a frame, wait for it to be drawn once (the DrawCallback today), then we could send another BeginFrame.

But this would require clients to remember theyd be asked for a beginframe instead, and sometimes they get lost during startup apparently. So this isn't a trivial change, but ya if we did that then we wouldn't need the ack to throttle, and we'd get globally coordinated throttling.

+brianderson@, eseckler@: How does this fit in with BeginFrameAck work? I'm trying to wrap my head around the BeginFrame protocol. Why isn't a CompositorFrame the ACK to a BeginFrame? Clients should only request Beginframes if they have an update and they ACK those BeginFrames with a CompositorFrame, is that right? Perhaps there's a race there any sometimes a client doesn't have an update anymore, and then we need another IPC for a client to say no update?

I think I've confused myself with all of this. Please clarify.

@Fady regarding BeginFrameAcks: If a client produces a CompositorFrame as response to a BeginFrame, the CompositorFrame will serve as the BeginFrameAck. The BeginFrameAck will be included in the CompositorFrame's metadata in such a case.

Right now, there are situations where a client wants to receive a BeginFrame but can't or doesn't need to draw. For example, the cc::Scheduler asks for "proactive" BeginFrames, in the hope that it will have some updates for the next BeginFrame, but might not actually have any. Sometimes, you also don't know in advance whether some pending change actually results in visible damage during drawing.

In such cases, you don't end up with a CompositorFrame. To acknowledge that we're done with the BeginFrame, the client will send a BeginFrameAck through a separate IPC (aka BeginFrameDidNotDraw) through its CompositorFrameSink. This way, the DisplayScheduler will know that it doesn't need to wait for a CompositorFrame from said client.

From what I gather, getting rid of CompositorFrameAcks doesn't change this, since a client might still end up without needing to draw or a draw without damages after a BeginFrame was received. AFAIK, the CompositorFrameAcks currently prevent you from submitting a new CompositorFrame while the parent hasn't drawn an older one yet.

What I understand from Dana's suggestions is that the parent would just not send BeginFrames to the child while a prior CompositorFrame is still in the parent's pipe. Not sure if relevant, but could this increase latency b/c we end up blocking the pipe "too far up"? Or is that irrelevant b/c the parent should be drawing at the same frame rate?

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

commit ce75f9388f73820bc5bb31401bcc75520f467a3f
Author: staraz <staraz@chromium.org>
Date: Wed Mar 29 16:17:12 2017

Remove CompositorObserver::OnCompositingEnded()

OnCompistingEnded() is only used in tests. By replacing usages of
OnCompositingEnded() with OnCompositingStarted(), we are able to remove
the call to OnCompositingEnded() in
ui::Compositor::DidReceiveCompositorFrameAck() without making the tests less
effective.

This is the first step in simplifying CompositorFrameAcks.

BUG= 671202 

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

[modify] https://crrev.com/ce75f9388f73820bc5bb31401bcc75520f467a3f/ash/mus/non_client_frame_controller_unittest.cc
[modify] https://crrev.com/ce75f9388f73820bc5bb31401bcc75520f467a3f/content/browser/renderer_host/browser_compositor_view_mac.mm
[modify] https://crrev.com/ce75f9388f73820bc5bb31401bcc75520f467a3f/content/browser/renderer_host/delegated_frame_host.cc
[modify] https://crrev.com/ce75f9388f73820bc5bb31401bcc75520f467a3f/content/browser/renderer_host/delegated_frame_host.h
[modify] https://crrev.com/ce75f9388f73820bc5bb31401bcc75520f467a3f/ui/compositor/compositor.cc
[modify] https://crrev.com/ce75f9388f73820bc5bb31401bcc75520f467a3f/ui/compositor/compositor.h
[modify] https://crrev.com/ce75f9388f73820bc5bb31401bcc75520f467a3f/ui/compositor/compositor_observer.h
[modify] https://crrev.com/ce75f9388f73820bc5bb31401bcc75520f467a3f/ui/compositor/compositor_unittest.cc
[modify] https://crrev.com/ce75f9388f73820bc5bb31401bcc75520f467a3f/ui/compositor/layer.cc
[modify] https://crrev.com/ce75f9388f73820bc5bb31401bcc75520f467a3f/ui/compositor/layer_unittest.cc
[modify] https://crrev.com/ce75f9388f73820bc5bb31401bcc75520f467a3f/ui/compositor/test/draw_waiter_for_test.cc
[modify] https://crrev.com/ce75f9388f73820bc5bb31401bcc75520f467a3f/ui/compositor/test/draw_waiter_for_test.h
[modify] https://crrev.com/ce75f9388f73820bc5bb31401bcc75520f467a3f/ui/snapshot/snapshot_aura_unittest.cc
[modify] https://crrev.com/ce75f9388f73820bc5bb31401bcc75520f467a3f/ui/views/controls/scroll_view_unittest.cc
[modify] https://crrev.com/ce75f9388f73820bc5bb31401bcc75520f467a3f/ui/views/view_unittest.cc

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

commit 09b06cad7bc4b38fa64142b16a0c6adcbcc875b4
Author: staraz <staraz@chromium.org>
Date: Thu Apr 06 23:27:21 2017

[exo]CompositorFrameSinkHolder calls WillDraw in DidReceiveCompositorFrameAck

CommitSurfaceHierarchy() moves pending_frame_callbacks_ into
frame_callbacks_ before calling UpdateSurface() so that the ack doesn't
arrive too early.

exo::Surface is the only class which needs
MojoCompositorFrameSinkClient::WillDrawSurface(). With this change, we
can remove WillDrawSurface from the mojo interface and reduce IPC overhead.

BUG= 671202 ,  709076 

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

[modify] https://crrev.com/09b06cad7bc4b38fa64142b16a0c6adcbcc875b4/components/exo/compositor_frame_sink_holder.cc
[modify] https://crrev.com/09b06cad7bc4b38fa64142b16a0c6adcbcc875b4/components/exo/surface.cc
[modify] https://crrev.com/09b06cad7bc4b38fa64142b16a0c6adcbcc875b4/components/exo/surface.h
[modify] https://crrev.com/09b06cad7bc4b38fa64142b16a0c6adcbcc875b4/components/exo/surface_unittest.cc

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

commit 514a2755c94d9333fb54925d3ca7a818cd40189e
Author: staraz <staraz@chromium.org>
Date: Mon Apr 10 21:37:52 2017

Remove MojoCompositorFrameSink::WillDrawSurface

WillDrawSurface is no longer being used on the client side. Removing it would
reduce the number of IPCs we send.

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

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

[modify] https://crrev.com/514a2755c94d9333fb54925d3ca7a818cd40189e/cc/ipc/mojo_compositor_frame_sink.mojom
[modify] https://crrev.com/514a2755c94d9333fb54925d3ca7a818cd40189e/components/exo/compositor_frame_sink.cc
[modify] https://crrev.com/514a2755c94d9333fb54925d3ca7a818cd40189e/components/exo/compositor_frame_sink.h
[modify] https://crrev.com/514a2755c94d9333fb54925d3ca7a818cd40189e/components/exo/compositor_frame_sink_holder.cc
[modify] https://crrev.com/514a2755c94d9333fb54925d3ca7a818cd40189e/components/exo/compositor_frame_sink_holder.h
[modify] https://crrev.com/514a2755c94d9333fb54925d3ca7a818cd40189e/components/viz/frame_sinks/gpu_compositor_frame_sink.cc
[modify] https://crrev.com/514a2755c94d9333fb54925d3ca7a818cd40189e/components/viz/frame_sinks/gpu_root_compositor_frame_sink.cc
[modify] https://crrev.com/514a2755c94d9333fb54925d3ca7a818cd40189e/content/browser/renderer_host/offscreen_canvas_compositor_frame_sink.cc
[modify] https://crrev.com/514a2755c94d9333fb54925d3ca7a818cd40189e/services/ui/public/cpp/client_compositor_frame_sink.cc
[modify] https://crrev.com/514a2755c94d9333fb54925d3ca7a818cd40189e/services/ui/public/cpp/client_compositor_frame_sink.h
[modify] https://crrev.com/514a2755c94d9333fb54925d3ca7a818cd40189e/services/ui/ws/display_client_compositor_frame_sink.cc
[modify] https://crrev.com/514a2755c94d9333fb54925d3ca7a818cd40189e/services/ui/ws/display_client_compositor_frame_sink.h
[modify] https://crrev.com/514a2755c94d9333fb54925d3ca7a818cd40189e/third_party/WebKit/Source/platform/graphics/OffscreenCanvasFrameDispatcherImpl.cpp
[modify] https://crrev.com/514a2755c94d9333fb54925d3ca7a818cd40189e/third_party/WebKit/Source/platform/graphics/OffscreenCanvasFrameDispatcherImpl.h

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

commit b14cc10ccf83caad627156403239fc6758c2dbbb
Author: staraz <staraz@chromium.org>
Date: Thu Apr 13 18:06:39 2017

Revert of Remove CompositorObserver::OnCompositingEnded() (patchset #5 id:80001 of https://codereview.chromium.org/2776973004/ )

Reason for revert:
Cause LayerWithRealCompositorTest.ReportMetrics to
be flaky on Windows.

We don't need this change anyway now that
MojoCompositorFrameSinkClient::WillDrawSurface() is
removed.

Original issue's description:
> Remove CompositorObserver::OnCompositingEnded()
>
> OnCompistingEnded() is only used in tests. By replacing usages of
> OnCompositingEnded() with OnCompositingStarted(), we are able to remove
> the call to OnCompositingEnded() in
> ui::Compositor::DidReceiveCompositorFrameAck() without making the tests less
> effective.
>
> This is the first step in simplifying CompositorFrameAcks.
>
> BUG= 671202 
>
> Review-Url: https://codereview.chromium.org/2776973004
> Cr-Commit-Position: refs/heads/master@{#460409}
> Committed: https://chromium.googlesource.com/chromium/src/+/ce75f9388f73820bc5bb31401bcc75520f467a3f

TBR=fsamuel@chromium.org,danakj@chromium.org,varkha@chromium.org,sky@chromium.org,asvitkine@chromium.org
# Not skipping CQ checks because original CL landed more than 1 days ago.
BUG= 671202 ,  709080 

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

[modify] https://crrev.com/b14cc10ccf83caad627156403239fc6758c2dbbb/ash/mus/non_client_frame_controller_unittest.cc
[modify] https://crrev.com/b14cc10ccf83caad627156403239fc6758c2dbbb/content/browser/renderer_host/browser_compositor_view_mac.mm
[modify] https://crrev.com/b14cc10ccf83caad627156403239fc6758c2dbbb/content/browser/renderer_host/delegated_frame_host.cc
[modify] https://crrev.com/b14cc10ccf83caad627156403239fc6758c2dbbb/content/browser/renderer_host/delegated_frame_host.h
[modify] https://crrev.com/b14cc10ccf83caad627156403239fc6758c2dbbb/ui/compositor/compositor.cc
[modify] https://crrev.com/b14cc10ccf83caad627156403239fc6758c2dbbb/ui/compositor/compositor_observer.h
[modify] https://crrev.com/b14cc10ccf83caad627156403239fc6758c2dbbb/ui/compositor/compositor_unittest.cc
[modify] https://crrev.com/b14cc10ccf83caad627156403239fc6758c2dbbb/ui/compositor/layer_unittest.cc
[modify] https://crrev.com/b14cc10ccf83caad627156403239fc6758c2dbbb/ui/compositor/test/draw_waiter_for_test.cc
[modify] https://crrev.com/b14cc10ccf83caad627156403239fc6758c2dbbb/ui/compositor/test/draw_waiter_for_test.h
[modify] https://crrev.com/b14cc10ccf83caad627156403239fc6758c2dbbb/ui/snapshot/snapshot_aura_unittest.cc
[modify] https://crrev.com/b14cc10ccf83caad627156403239fc6758c2dbbb/ui/views/controls/scroll_view_unittest.cc
[modify] https://crrev.com/b14cc10ccf83caad627156403239fc6758c2dbbb/ui/views/view_unittest.cc