Just to clarify the deadlines that the DisplayScheduler uses:
 (a) a late deadline scheduled for next BeginFrame time (args.frame_time + args.interval)
 (b) a normal deadline scheduled for about 2/3 of args.interval (original args.deadline - DefaultEstimatedParentDrawTime)
 (c) a slightly earlier deadline scheduled for about 1/3 of args.interval (original args.deadline - 2 * DefaultEstimatedParentDrawTime)
 (d) an immediate deadline

At the moment, these deadlines are used as follows:
 (a) if no damage exists yet or we can't draw (swap throttled, no output surface, locked resources, ..)
 (b) as soon as any one surface has reported damage (i.e. activated a CompositorFrame)
 (c) when all active child surfaces[1] have reported damage, and we're also expecting root surface damage [2], but haven't seen any yet. This is primarily to compensate for inaccurate root-surface-damage heuristics.
 (d) as soon as all active child surfaces have reported damage and the root surface has reported damage if we are expecting it to.

[1] heuristic: child surfaces that had damage during the last BeginFrame.
[2] heuristic: we're expecting root surface damage if a resize happened or the root surface had damage during the last BeginFrame.

With BeginFrameAcks, we're hoping to remove the prior-BeginFrame-damage-based heuristics and replace them with BeginFrameAcks from the damage producers. Ideally, we'd no longer need deadline (c); and (d) would become:

 (d) as soon as all current BeginFrameObservers have acknowledged.

This change depends on all observes acknowledging "no damage for this BeginFrame" when there isn't any.

What we'd need to think about for surface synchronization is at what point during the BeginFrame to report no-damage if a surface has a pending but not-activated CompositorFrame. As Fady mentioned, if we defer acking the BeginFrame until the CompositorFrame is activated, we would prevent an immediate deadline (d) from triggering during the BeginFrame, forcing us to use a late (a) or normal (b) deadline.

My guess is that that's actually what we want, since we want to give the CompositorFrame-blocking dependencies a chance to resolve during the same BeginFrame. If there was other (activated) damage on other Surfaces, we'd still use a normal deadline (b).

Are there cases where we know that it's unlikely that the blockers are resolved during the same BeginFrame? If so, we could acknowledge no-damage for the blocked Surface earlier.

One idea: Maybe we could acknowledge no-damage for blocked Surfaces/CFSSupports as soon as all CFFSSupports are either blocked or have acknowledged the same BeginFrame? That should capture cases where a Surface is blocked on a CompositorFrame producer that already finished the same BeginFrame, I think.

Hi, please set Available/Feature on this type of tracking issue so we don't have to go through them in our triage.

Assigning to eseckler@ since he's actively thinking about this.

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

commit e356c64f253923983248656d304d8aa471aad5f0
Author: eseckler <eseckler@chromium.org>
Date: Fri May 26 12:59:36 2017

[cc] Plumb BeginFrameAcks through SurfaceManager to DisplayScheduler.

Plumbs the BeginFrameAcks from DidNotProduceFrame and
SubmitCompositorFrame through new SurfaceObserver methods from
CompositorFrameSinks to the DisplayScheduler.

Also replaces the expected surface damage heuristics previously used
by DisplayScheduler with tracking BeginFrame(Ack)s and surface damage.

This is work towards unified BeginFrame acknowledgments, see:
Tracking bug:  https://crbug.com/697086 
Design doc: http://bit.ly/beginframeacks

BUG= 697086 , 646774,  678119 ,  703079 ,  707513 
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_trusty_blink_rel;master.tryserver.chromium.linux:linux_site_isolation

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

[modify] https://crrev.com/e356c64f253923983248656d304d8aa471aad5f0/cc/BUILD.gn
[modify] https://crrev.com/e356c64f253923983248656d304d8aa471aad5f0/cc/surfaces/compositor_frame_sink_support.cc
[modify] https://crrev.com/e356c64f253923983248656d304d8aa471aad5f0/cc/surfaces/compositor_frame_sink_support_unittest.cc
[modify] https://crrev.com/e356c64f253923983248656d304d8aa471aad5f0/cc/surfaces/direct_compositor_frame_sink_unittest.cc
[modify] https://crrev.com/e356c64f253923983248656d304d8aa471aad5f0/cc/surfaces/display.cc
[modify] https://crrev.com/e356c64f253923983248656d304d8aa471aad5f0/cc/surfaces/display.h
[modify] https://crrev.com/e356c64f253923983248656d304d8aa471aad5f0/cc/surfaces/display_scheduler.cc
[modify] https://crrev.com/e356c64f253923983248656d304d8aa471aad5f0/cc/surfaces/display_scheduler.h
[modify] https://crrev.com/e356c64f253923983248656d304d8aa471aad5f0/cc/surfaces/display_scheduler_unittest.cc
[modify] https://crrev.com/e356c64f253923983248656d304d8aa471aad5f0/cc/surfaces/display_unittest.cc
[modify] https://crrev.com/e356c64f253923983248656d304d8aa471aad5f0/cc/surfaces/surface.cc
[modify] https://crrev.com/e356c64f253923983248656d304d8aa471aad5f0/cc/surfaces/surface.h
[modify] https://crrev.com/e356c64f253923983248656d304d8aa471aad5f0/cc/surfaces/surface_manager.cc
[modify] https://crrev.com/e356c64f253923983248656d304d8aa471aad5f0/cc/surfaces/surface_manager.h
[modify] https://crrev.com/e356c64f253923983248656d304d8aa471aad5f0/cc/surfaces/surface_observer.h
[modify] https://crrev.com/e356c64f253923983248656d304d8aa471aad5f0/cc/surfaces/surface_synchronization_unittest.cc
[add] https://crrev.com/e356c64f253923983248656d304d8aa471aad5f0/cc/test/fake_surface_observer.cc
[add] https://crrev.com/e356c64f253923983248656d304d8aa471aad5f0/cc/test/fake_surface_observer.h
[modify] https://crrev.com/e356c64f253923983248656d304d8aa471aad5f0/components/viz/frame_sinks/mojo_frame_sink_manager.cc
[modify] https://crrev.com/e356c64f253923983248656d304d8aa471aad5f0/components/viz/frame_sinks/mojo_frame_sink_manager.h
[modify] https://crrev.com/e356c64f253923983248656d304d8aa471aad5f0/content/browser/frame_host/render_widget_host_view_child_frame_unittest.cc
[modify] https://crrev.com/e356c64f253923983248656d304d8aa471aad5f0/content/browser/renderer_host/render_widget_host_view_aura_unittest.cc

I'm marking this as FIXED. Please reopen if you disagree.

I think we need to think about this again for the --run-all-compositing-stages-before-draw mode. Currently, we extend the DisplayScheduler's BeginFrame deadline for surfaces with pending CompositorFrames. For this mode, that means blocking indefinitely.

However, AFAIU, pending CompositorFrames may only activate in future BeginFrames, even in --run-all-.. mode. For examples, because a surface dependency's frame sink is CompositorFrameAck-throttled on an older surface.

Thus, I think we'll have to consider surfaces with pending CompositorFrames as ready in DisplayScheduler. (We were already thinking about this before in https://codereview.chromium.org/2854163003/.)

As it turns out, in full-pipeline mode, pending CompositorFrames are almost guaranteed to activate in the same BeginFrame - and so considering them as blocking is actually beneficial for the full-pipeline mode. See also: https://docs.google.com/document/d/1PppegrpXhOzKKAuNlP6XOEnviXFGUiX2hop00Cxcv4o/edit#heading=h.96vuopjnyc7s .