Adds BeginFrameControl via DevTools.

Adds BeginFrameControl via DevTools.

This provides control over rendering, primarily for use in headless
chrome. See http://bit.ly/bfc-v1 for detailed requirements; to sum up:
- Normal BeginFrameSources are deactivated.
- DevTools initiates each BeginFrame (and may wait for it to be drawn).
- Essentially mimic a synchronous renderer, i.e. a BeginFrame causes a
  new main frame and leads to a successful commit of this frame to the
  browser display.
- Plan for future support of OOPIFs.

This first prototype approaches the problem as follows:
- We assume that every page has its own window and that we are on Aura.
- This allows us to replace the window's (i.e. the Aura display's)
  BeginFrameSource with a BFS that we control via DevTools.
- On DevTools' SendBeginFrame, we force the RenderView to redraw and
  the renderer compositor to produce a new main frame. We also ensure
  that this frame is successfully drawn in the BeginFrame issued by
  SendBeginFrame.
- We ensure that the DisplayScheduler waits for the commit of the
  renderer's new main frame before drawing in response to the
  BeginFrame. For this purpose, we track this frame using a latency
  info.

For OOPIF, we would need to force each RenderFrame to redraw and track
their frames in the browser, also via latency infos.

DO NOT SUBMIT: prototype only, multiple TODOs, missing tests.

BUG=646774
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_precise_blink_rel

[Eric Seckler, 2016-10-14 10:27:45]

Description was changed from

==========
Adds BeginFrameControl via DevTools.

This provides control over rendering, primarily for use in headless
chrome. See http://bit.ly/bfc-v1 for detailed requirements; to sum up:
- Normal BeginFrameSources are deactivated.
- DevTools initiates each BeginFrame (and may wait for it to be drawn).
- Essentially mimic a synchronous renderer, i.e. a BeginFrame causes a
  new main frame and leads to a successful commit of this frame to the
  browser display.
- Plan for future support of OOPIFs.

This first prototype approaches the problem as follows:
- We assume that every page has its own window and that we are on Aura.
- This allows us to replace the window's (i.e. the Aura display's)
  BeginFrameSource with a BFS that we control via DevTools.
- On DevTools' SendBeginFrame, force the RenderView to redraw and the
  renderer compositor to produce a new main frame. Also ensure that
  this frame is successfully drawn in the BeginFrame issued by
  SendBeginFrame.
- Ensure that the DisplayScheduler waits for commits of the renderer's
  new main frame before drawing as response to the BeginFrame. For this
  purpose, we track this frame using a latency info.

For OOPIF, we would need to force each RenderFrame to redraw and track
their frames in the browser, also via latency infos.

DO NOT SUBMIT: prototype only, multiple TODOs, missing tests.

BUG=646774
==========

to

==========
Adds BeginFrameControl via DevTools.

This provides control over rendering, primarily for use in headless
chrome. See http://bit.ly/bfc-v1 for detailed requirements; to sum up:
- Normal BeginFrameSources are deactivated.
- DevTools initiates each BeginFrame (and may wait for it to be drawn).
- Essentially mimic a synchronous renderer, i.e. a BeginFrame causes a
  new main frame and leads to a successful commit of this frame to the
  browser display.
- Plan for future support of OOPIFs.

This first prototype approaches the problem as follows:
- We assume that every page has its own window and that we are on Aura.
- This allows us to replace the window's (i.e. the Aura display's)
  BeginFrameSource with a BFS that we control via DevTools.
- On DevTools' SendBeginFrame, force the RenderView to redraw and the
  renderer compositor to produce a new main frame. Also ensure that
  this frame is successfully drawn in the BeginFrame issued by
  SendBeginFrame.
- Ensure that the DisplayScheduler waits for commits of the renderer's
  new main frame before drawing as response to the BeginFrame. For this
  purpose, we track this frame using a latency info.

For OOPIF, we would need to force each RenderFrame to redraw and track
their frames in the browser, also via latency infos.

DO NOT SUBMIT: prototype only, multiple TODOs, missing tests.

BUG=646774
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_precise_blink_rel
==========

[Eric Seckler, 2016-10-14 10:46:22]

Description was changed from

==========
Adds BeginFrameControl via DevTools.

This provides control over rendering, primarily for use in headless
chrome. See http://bit.ly/bfc-v1 for detailed requirements; to sum up:
- Normal BeginFrameSources are deactivated.
- DevTools initiates each BeginFrame (and may wait for it to be drawn).
- Essentially mimic a synchronous renderer, i.e. a BeginFrame causes a
  new main frame and leads to a successful commit of this frame to the
  browser display.
- Plan for future support of OOPIFs.

This first prototype approaches the problem as follows:
- We assume that every page has its own window and that we are on Aura.
- This allows us to replace the window's (i.e. the Aura display's)
  BeginFrameSource with a BFS that we control via DevTools.
- On DevTools' SendBeginFrame, force the RenderView to redraw and the
  renderer compositor to produce a new main frame. Also ensure that
  this frame is successfully drawn in the BeginFrame issued by
  SendBeginFrame.
- Ensure that the DisplayScheduler waits for commits of the renderer's
  new main frame before drawing as response to the BeginFrame. For this
  purpose, we track this frame using a latency info.

For OOPIF, we would need to force each RenderFrame to redraw and track
their frames in the browser, also via latency infos.

DO NOT SUBMIT: prototype only, multiple TODOs, missing tests.

BUG=646774
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_precise_blink_rel
==========

to

==========
Adds BeginFrameControl via DevTools.

This provides control over rendering, primarily for use in headless
chrome. See http://bit.ly/bfc-v1 for detailed requirements; to sum up:
- Normal BeginFrameSources are deactivated.
- DevTools initiates each BeginFrame (and may wait for it to be drawn).
- Essentially mimic a synchronous renderer, i.e. a BeginFrame causes a
  new main frame and leads to a successful commit of this frame to the
  browser display.
- Plan for future support of OOPIFs.

This first prototype approaches the problem as follows:
- We assume that every page has its own window and that we are on Aura.
- This allows us to replace the window's (i.e. the Aura display's)
  BeginFrameSource with a BFS that we control via DevTools.
- On DevTools' SendBeginFrame, force the RenderView to redraw and the
  renderer compositor to produce a new main frame. Also ensure that
  this frame is successfully drawn in the BeginFrame issued by
  SendBeginFrame.
- Ensure that the DisplayScheduler waits for the commit of the
  renderer's new main frame before drawing as response to the
  BeginFrame. For this purpose, we track this frame using a latency
  info.

For OOPIF, we would need to force each RenderFrame to redraw and track
their frames in the browser, also via latency infos.

DO NOT SUBMIT: prototype only, multiple TODOs, missing tests.

BUG=646774
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_precise_blink_rel
==========

[Eric Seckler, 2016-10-14 11:18:48]

Patchset #1 (id:1) has been deleted

[Eric Seckler, 2016-10-14 11:29:30]

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

[commit-bot: I haz the power, 2016-10-14 11:29:41]

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, 2016-10-14 11:33:53]

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

[commit-bot: I haz the power, 2016-10-14 11:33:54]

Dry run: Try jobs failed on following builders:
  android_clang_dbg_recipe on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_clan...)
  android_n5x_swarming_rel on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_n5x_...)
  cast_shell_android on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/cast_shell_a...)

[Eric Seckler, 2016-10-14 12:40:21]

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

[commit-bot: I haz the power, 2016-10-14 12:40:36]

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

[Eric Seckler, 2016-10-14 12:42:18]

Patchset #1 (id:20001) has been deleted

[commit-bot: I haz the power, 2016-10-14 12:44:56]

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

[commit-bot: I haz the power, 2016-10-14 12:44:57]

Dry run: Try jobs failed on following builders:
  android_clang_dbg_recipe on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_clan...)
  android_compile_dbg on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_comp...)
  linux_android_rel_ng on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/linux_androi...)

[Eric Seckler, 2016-10-14 12:56:05]

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

[commit-bot: I haz the power, 2016-10-14 12:56:22]

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, 2016-10-14 13:06:07]

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

[commit-bot: I haz the power, 2016-10-14 13:06:09]

Dry run: Try jobs failed on following builders:
  chromium_presubmit on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)

[Eric Seckler, 2016-10-14 14:33:02]

Description was changed from

==========
Adds BeginFrameControl via DevTools.

This provides control over rendering, primarily for use in headless
chrome. See http://bit.ly/bfc-v1 for detailed requirements; to sum up:
- Normal BeginFrameSources are deactivated.
- DevTools initiates each BeginFrame (and may wait for it to be drawn).
- Essentially mimic a synchronous renderer, i.e. a BeginFrame causes a
  new main frame and leads to a successful commit of this frame to the
  browser display.
- Plan for future support of OOPIFs.

This first prototype approaches the problem as follows:
- We assume that every page has its own window and that we are on Aura.
- This allows us to replace the window's (i.e. the Aura display's)
  BeginFrameSource with a BFS that we control via DevTools.
- On DevTools' SendBeginFrame, force the RenderView to redraw and the
  renderer compositor to produce a new main frame. Also ensure that
  this frame is successfully drawn in the BeginFrame issued by
  SendBeginFrame.
- Ensure that the DisplayScheduler waits for the commit of the
  renderer's new main frame before drawing as response to the
  BeginFrame. For this purpose, we track this frame using a latency
  info.

For OOPIF, we would need to force each RenderFrame to redraw and track
their frames in the browser, also via latency infos.

DO NOT SUBMIT: prototype only, multiple TODOs, missing tests.

BUG=646774
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_precise_blink_rel
==========

to

==========
Adds BeginFrameControl via DevTools.

This provides control over rendering, primarily for use in headless
chrome. See http://bit.ly/bfc-v1 for detailed requirements; to sum up:
- Normal BeginFrameSources are deactivated.
- DevTools initiates each BeginFrame (and may wait for it to be drawn).
- Essentially mimic a synchronous renderer, i.e. a BeginFrame causes a
  new main frame and leads to a successful commit of this frame to the
  browser display.
- Plan for future support of OOPIFs.

This first prototype approaches the problem as follows:
- We assume that every page has its own window and that we are on Aura.
- This allows us to replace the window's (i.e. the Aura display's)
  BeginFrameSource with a BFS that we control via DevTools.
- On DevTools' SendBeginFrame, we force the RenderView to redraw and
  the renderer compositor to produce a new main frame. We also ensure
  that this frame is successfully drawn in the BeginFrame issued by
  SendBeginFrame.
- We ensure that the DisplayScheduler waits for the commit of the
  renderer's new main frame before drawing in response to the
  BeginFrame. For this purpose, we track this frame using a latency
  info.

For OOPIF, we would need to force each RenderFrame to redraw and track
their frames in the browser, also via latency infos.

DO NOT SUBMIT: prototype only, multiple TODOs, missing tests.

BUG=646774
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_precise_blink_rel
==========

[Eric Seckler, 2016-10-14 14:37:25]

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

[commit-bot: I haz the power, 2016-10-14 14:37:39]

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

[Eric Seckler, 2016-10-14 14:37:40]

Patchset #1 (id:40001) has been deleted

[Eric Seckler, 2016-10-14 14:37:49]

Patchset #1 (id:60001) has been deleted

[Eric Seckler, 2016-10-14 14:39:26]

eseckler@chromium.org changed reviewers:
+ brianderson@chromium.org, danakj@chromium.org

[Eric Seckler, 2016-10-14 14:39:27]

Hi Brian and Dana,

This is a first attempt at making rendering controllable through DevTools for
headless chrome. It's in no way polished and actually feels rather invasive and
brittle in parts, which is why I wanted to get it out to you sooner rather than
later.

Maybe you have clever ideas for improvements or a completely different approach
that we haven't yet thought of? :)

Thanks for your advice!
Eric

[commit-bot: I haz the power, 2016-10-14 14:48:48]

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

[commit-bot: I haz the power, 2016-10-14 14:48:49]

Dry run: Try jobs failed on following builders:
  chromium_presubmit on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)

[danakj, 2016-10-14 21:07:50]

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

[danakj, 2016-10-14 21:07:51]

-danakj +enne

[danakj, 2016-10-14 21:08:04]

danakj@chromium.org changed reviewers:
- danakj@chromium.org

[enne (OOO), 2016-10-19 21:40:24]

At the top level, being able to switch out the BeginFrameSource that the
DisplayScheduler is using to something that ticks explicitly seems like the
right approach.

I'm assuming all the cc::Scheduler and cc::Proxy changes are all just a
performance optimization? There's a worry that the scheduler is doing
unnecessary work and so this is trying to force everything off.  Is this
something that you've observed or is this just a speculative worry?  This is
probably the scariest and most invasive part of this patch to me.  The
possibility of deadlock while waiting for something that won't ever commit (e.g.
aborted commits) seems really high.

At some point, you do want to draw even if you don't have a commit, correct?
Like, if some main thread takes like 10 seconds or something but will eventually
complete, you don't want to wait for that for your screenshot? If so, then it
sounds like you'll need to have some deadline.  I wonder if there's a way to
delay submitting the compositor frame from the renderer or from oopifs in a
slightly more elegant manner, possibly by just extending the frame deadline out
on the begin frame args to how long you'd like to wait.

I think that LatencyInfo in general already has ways to track when a particular
frame gets dropped along the way because there's no update or nothing visible
has changed.  Is it possible to just add LatencyInfo requests sent to all
renderers?  This is probably a question for brianderson, as I know he wanted to
track outstanding begin frames and acks at some point to throttle bad producers
or have other top level behavior.

[Eric Seckler, 2016-10-20 11:31:07]

Thanks for the feedback, Adrienne :)

On 2016/10/19 21:40:24, enne wrote:
> At the top level, being able to switch out the BeginFrameSource that the
> DisplayScheduler is using to something that ticks explicitly seems like the
> right approach.
> 
> I'm assuming all the cc::Scheduler and cc::Proxy changes are all just a
> performance optimization? There's a worry that the scheduler is doing
> unnecessary work and so this is trying to force everything off.  Is this
> something that you've observed or is this just a speculative worry?  This is
> probably the scariest and most invasive part of this patch to me.  The
> possibility of deadlock while waiting for something that won't ever commit
(e.g.
> aborted commits) seems really high.
> 
> At some point, you do want to draw even if you don't have a commit, correct?
> Like, if some main thread takes like 10 seconds or something but will
eventually
> complete, you don't want to wait for that for your screenshot? If so, then it
> sounds like you'll need to have some deadline.  I wonder if there's a way to
> delay submitting the compositor frame from the renderer or from oopifs in a
> slightly more elegant manner, possibly by just extending the frame deadline
out
> on the begin frame args to how long you'd like to wait.

I agree, in their current form, the changes to cc::Scheduler/Proxy feel rather
ugly. Their motivation isn't actually performance, though.

From a user-perspective (DevTools client), what we have is:
* Occasionally send a single BeginFrame. This should trigger a main frame in all
renderers (e.g. to ensure that requestAnimationFrame handlers are run).
* Wait for commit of the resulting browser frame, which includes the main frames
from the renderers. After receiving that, maybe eventually send another
BeginFrame.

That's essentially a synchronous BeginFrame behavior that always produces main
frames - and where we can't be sure that we will receive a "next" BeginFrame.

We force the renderer to produce a new main frame using ForceRedraw (in its main
thread) and wait for the LatencyInfo identifying this main frame in the browser.
RenderViewImpl::OnForceRedraw ensures that the next "fresh" main frame produced
by the main thread contains forced updates (but its commit may be aborted for
other reasons). That main frame will also contain the ForceRedraw's LatencyInfo.

cc::Scheduler::DelayDrawingUntilFreshMainFrame is an attempt to ensure that the
renderer commits this new main frame to the browser as a result of the single
BeginFrame we send, even if the main thread is very slow or aborts the
commit/draw, and even if the impl thread wants to draw as well (e.g. to service
some animations). Otherwise, it would be possible that the BeginFrame is
serviced
(1) by an immediate ImplFrame.
(2) by an older main frame that was previously committed and/or activated but
not drawn.
(3) without updates b/c BeginMainFrame or Draw was aborted.
As a result of any of these, the main frame would get stuck in the compositor
waiting for the next BeginFrame, which we won't send, and we'd wait forever for
the ForceRedraw's LatencyInfo.

For (3), I'd alternatively be happy to "abort" the BeginFrame in the compositor,
if added a way to let the browser know about this (so it can stop waiting).

Right now, our BeginFrames don't have any deadline. I think we'd like a deadline
eventually, but it would be a rather long one and more of an "abort (waiting
for?) any outstanding renderer frames and commit what you have received so far
in the browser", i.e. I don't think we would want an impl-thread frame from
outstanding renderers in such cases.

> I think that LatencyInfo in general already has ways to track when a
particular
> frame gets dropped along the way because there's no update or nothing visible
> has changed.  Is it possible to just add LatencyInfo requests sent to all
> renderers?  This is probably a question for brianderson, as I know he wanted
to
> track outstanding begin frames and acks at some point to throttle bad
producers
> or have other top level behavior.

I'm not aware that LatencyInfo can track dropped frames - but I know that there
are mechanisms that can be used to ensure that a LatencyInfo isn't lost if a
frame gets aborted/dropped (and is instead saved and attached to the next
successful frame). That's what's used for the screenshot / ForceRedraw
LatencyInfo.

When we talked to Brian a little while ago, he also mentioned that the "reverse
plumbing" of BeginFrame acks was in the backlog. If we had this, I think we
should be able to get rid of ForceRedraw and tracking LatencyInfos (that'd be
nice), but we'd still need to make cc::Scheduler produce and wait for a new main
frame before ack-ing the BeginFrame. It would probably make dealing with
aborted/dropped frames (and exceeded deadlines) nicer, as long as we can track
"good" and "bad" acks and which renderer sent which ack (so we can report this
back to the DevTools client).

[brianderson, 2016-10-26 20:27:41]

Sorry for the slow response. This was a lot to go over and I was out sick for a
bit.


High level thoughts:

1) There’s an assumption/requirement that the devtools client does not want
impl-side updates.
  * Why not? Will they want them in the future?
  * Allowing impl-side updates (even after a long deadline) would allow us to
behave much more like normal Chrome and would simplify the cc::Scheduler logic
and tests.

2) Will we be able to get away with always having Display wait for all the
Renderers to respond?
  * It would keep the implementation simpler. Having to discard the Renderer’s
old work and coordinate that with new BeginFrames sounds messy.
  * Is the content always under the user’s control? If so, always waiting for
Renderer frames might be a way to encourage responsive content. =P


Implementation details:

3) It looks like the cc::Scheduler modifications will trigger a normal
ACTION_DRAW_IF_POSSIBLE, but you want to trigger ACTION_DRAW_FORCED, which will
force a response even if there is no damage. ACTION_DRAW_FORCED will be okay to
get things off the ground, we'll probably want an explicit request/response to
be more efficient in followup work.

4) It would be nice to avoid abusing LatencyInfo. Instead of using the
latency_id to track an outstanding request, can you use the BeginFrameObserver*?
You’d have to change the relationships and interfaces between Display /
DisplayScheduler / and BeginFrameSource. A potential implementation could be:
    1) Adding a new SurfaceBeginFrameSource class that proxies the Display’s
DelayBasedBeginFrameSource and intercepts the Add/RemoveObserver calls to
replace DisplayScheduler’s child_surface_ids_to_expect_damage_from_ with
begin_frame_observers_to_expect_responses_from_.
    2) Implement SurfaceBeginFrameSource::DidFinishFrame and call it after every
frame submission to replace DisplayScheduler’s child_surface_ids_damaged_.
    3) Optionally add a begin_frame_id that’s added to BeginFrameArgs and is
taken as an argument in DidFinishFrame. (Not sure if this part is necessary.)

This would unify the logic that waits for outstanding frames from renderers for
regular Chrome and headless. It’s also something I’ve been wanting to do but
haven’t prioritized yet. If you want me to prioritize this instead of picking it
up yourself, please let me know!

5) We may want to reuse the cc::Proxy+Scheduler logic outside of headless.  For
example to force low latency mode during resize events. Can you implement this
feature as a new flag to BeginFrameArgs that explicitly indicates it wants a
main-thread update included?

[Sami, 2016-10-27 12:31:50]

On 2016/10/26 20:27:41, brianderson wrote:
> Sorry for the slow response. This was a lot to go over and I was out sick for
a
> bit.
> 
> 
> High level thoughts:
> 
> 1) There’s an assumption/requirement that the devtools client does not want
> impl-side updates.
>   * Why not? Will they want them in the future?
>   * Allowing impl-side updates (even after a long deadline) would allow us to
> behave much more like normal Chrome and would simplify the cc::Scheduler logic
> and tests.
> 
> 2) Will we be able to get away with always having Display wait for all the
> Renderers to respond?
>   * It would keep the implementation simpler. Having to discard the Renderer’s
> old work and coordinate that with new BeginFrames sounds messy.
>   * Is the content always under the user’s control? If so, always waiting for
> Renderer frames might be a way to encourage responsive content. =P
> 
> 
> Implementation details:
> 
> 3) It looks like the cc::Scheduler modifications will trigger a normal
> ACTION_DRAW_IF_POSSIBLE, but you want to trigger ACTION_DRAW_FORCED, which
will
> force a response even if there is no damage. ACTION_DRAW_FORCED will be okay
to
> get things off the ground, we'll probably want an explicit request/response to
> be more efficient in followup work.
> 
> 4) It would be nice to avoid abusing LatencyInfo. Instead of using the
> latency_id to track an outstanding request, can you use the
BeginFrameObserver*?
> You’d have to change the relationships and interfaces between Display /
> DisplayScheduler / and BeginFrameSource. A potential implementation could be:
>     1) Adding a new SurfaceBeginFrameSource class that proxies the Display’s
> DelayBasedBeginFrameSource and intercepts the Add/RemoveObserver calls to
> replace DisplayScheduler’s child_surface_ids_to_expect_damage_from_ with
> begin_frame_observers_to_expect_responses_from_.
>     2) Implement SurfaceBeginFrameSource::DidFinishFrame and call it after
every
> frame submission to replace DisplayScheduler’s child_surface_ids_damaged_.
>     3) Optionally add a begin_frame_id that’s added to BeginFrameArgs and is
> taken as an argument in DidFinishFrame. (Not sure if this part is necessary.)
> 
> This would unify the logic that waits for outstanding frames from renderers
for
> regular Chrome and headless. It’s also something I’ve been wanting to do but
> haven’t prioritized yet. If you want me to prioritize this instead of picking
it
> up yourself, please let me know!
> 
> 5) We may want to reuse the cc::Proxy+Scheduler logic outside of headless. 
For
> example to force low latency mode during resize events. Can you implement this
> feature as a new flag to BeginFrameArgs that explicitly indicates it wants a
> main-thread update included?

Thanks for the feedback. We had a chat with Eric and we now also feel that
framing this in the form of long deadlines rather than special casing forced
redraws is probably cleaner and more maintainable. Specifically what we had in
mind:

1. Add support for setting an infinite deadline. In practice this would mean
each pipeline stage waits forever for its children (including impl waiting for
main frames and display waiting all surfaces) and draws as soon as everyone is
done. (The Devtools client is free to use non-infinite deadlines too to get
Chrome's current behavior.)

2. Add BeginFrameAck/Nack messages which identify the BeginFrame which is being
produced. For efficiency we might want to bundle the Ack inside the
CompositorFrame RWHV message.

3. The Ack/Nack should also have a way to say if it includes the latest
BeginMainFrame or not. This is to detect cases where a BeginMainFrame was
already in flight when we sent our synthetic BeginFrame with an infinite
deadline. (The mismatch can be resolved by sending another BeginFrame.)

Re: timeouts, I think it's fine to wait forever if the deadline is infinite and
rely on a higher level watchdog for recovery.

How does this sound?

[Eric Seckler, 2016-11-07 16:40:03]

Following up on our chat last week, I've updated the design doc
(http://bit.ly/bfc-v1). It also includes a summary of compositor-related changes
- brianderson@/enne@ feel free to take a peek :)

As discussed, I'll look into realizing infinite deadlines + "commit when
children ready" without the additional nack path first (forcing main frame
commits) as a first step.

Thanks for your support, everyone! :)

[Eric Seckler, 2016-11-08 18:57:51]

On 2016/11/07 16:40:03, Eric Seckler wrote:
> Following up on our chat last week, I've updated the design doc
> (http://bit.ly/bfc-v1). It also includes a summary of compositor-related
changes
> - brianderson@/enne@ feel free to take a peek :)
> 
> As discussed, I'll look into realizing infinite deadlines + "commit when
> children ready" without the additional nack path first (forcing main frame
> commits) as a first step.
> 
> Thanks for your support, everyone! :)

Quickly summarizing the fruits of a chat with Sunny:
* OnDraw isn't really well suited for us b/c of its synchronous IPCs.
* But making BeginFrameControl a cmdline flag for cc/scheduler + using
VisualStateCallback for NACK of BeginFrames can simplify things: No need for
extra params to BeginFrameArgs or frame sequence numbers in CompositorFrame.

Resulting proposal:
* Instead of supporting infinite deadlines, add a new cc/scheduler cmdline flag
that
  1) disables impl-only draws and
  2) effectively enables infinite deadlines (like OnDraw)
* Add a bool to VisualStateCallback that is set when the renderer does not need
to draw. Use it to wait for all surface clients in the display scheduler.
* Replace the BeginFrameSources using the same cmdline flag at startup. This
means the DevTools commands can only be support when this flag is set.

Still need to figure out:
* Do we need a mechanism similar to VisualStateCallback for NACK of BeginFrames
on browser-side surfaces? (Is there any such thing as a browser-side surface in
headless? what about video? html5 canvas?)
* Are we forgetting something else? :)

[enne (OOO), 2016-11-10 19:46:40]

Re: visual state callback instead of sequence numbers.  As long as brianderson
is ok with this, I am.  It sounded like he wanted to be able to track
non-responding / slow renderers with frame sequence numbers, so it wasn't just
for headless reasons.  Would we add visual state callbacks on every frame, in
that case?

Re: other surfaces.  Yeah, there's been some talk about video going to surfaces.
 Offscreen canvas is definitely moving to surfaces.  It looks like visual state
callbacks are only on renderers themselves (which I think also covers OOPIFs?),
but won't help at all for other surfaces.  Doing something at the surface /
begin frame level will catch all cases.

Re: infinite deadlines.  I had been hoping that this would "just work" in the
scheduler instead of adding more flags and modes.  I'm honestly a little
concerned with how many scheduler modes we have.  Changing the scheduler is a
giant mess at this point imo, and I don't really know what the answer here is. 
I just see "add more modes" and am a bit hesitant.  I'm really surprised that a
command line flag was even an option.  Because this was devtools-related, I had
assumed that runtime only was the only option.

[Eric Seckler, 2016-11-11 17:00:08]

Thanks for the feedback, Adrienne :)

It sounds like for chrome's use cases, sequence numbers + infinite DL in
BeginFrames are closer to what we'll want eventually (and as you say, also
covers browser-side surfaces).

I think our main incentive for visual state callback + cmdline flag (e.g.
scheduler setting) would be that it seems simpler to achieve in the short term.
For headless, we'd be ok with this being a cmdline flag - it just means we need
to enable the DevTools commands only when this flag is set. At the moment, we
don't think it's necessary for us to dynamically switch in/out of that mode
(though, it would be a nice-to-have).

We'd propose to start with visual state callbacks + a scheduler setting
initially (for headless only) and replace this by BeginFrame NACK with sequence
numbers + BeginFrameArgs params later (for chrome + browser-side surfaces).
Though I'm not actually sure whether a scheduler setting is less work than the
additional BeginFrameArgs params for infinite DL / main frame. The former might
be simpler because it doesn't have to deal with switching between "normal" and
"infinite + main frame" BeginFrames. Let's see how it turns out when coded up?
:)

Brian, does that sound ok?

[Eric Seckler, 2016-11-22 15:58:20]

Here's an update -- for documentary purposes, no action required.

TLDR: We have a proof-of-concept (using approach from revised doc, NOT
VisualStateCallbacks). I'll tackle BeginFrame ACK/NACK and BFObserver-tracking
first and send a patch for that in the next few days.

It turns out that VisualStateCallbacks don't really solve our problems - They're
racy with the BeginFrame if not used in synchronous renderer mode, and adding a
flag to the VisualStateResponse message that specifies whether the frame was
successfully submitted or aborted is more complicated than we hoped.

For this reason, I built a prototype for the approach described in the revised
design doc (infinite deadlines), but without BeginFrame NACKs from the renderer
(instead forcing draws as Brian suggested). It adds an
"allow_latency_optimizations" flag to BeginFrameArgs, which ensures that the
BeginFrame (and BeginMainFrame) is not skipped in the compositor. At the moment,
because there are no NACKs from the renderer, the flag also forces a draw
(ACTION_DRAW_FORCED). The Compositor and the DisplayScheduler trigger an
immediate deadline when all children (BeginMainFrame / BeginFrameObservers) are
done. The prototype seems to work fine, but does have some limitations:
 1) Without sequence numbers, it's impossible to distinguish an ACK of the
current BeginFrame from an ACK of the previous BeginFrame. But this can only
really happen in the browser-side cc::Scheduler, which may trigger a
DidFinishFrame() for the previous frame during OnBeginFrame().
 2) If the deadline is exceeded, the DisplayScheduler may commit without the
CompositorFrame from the renderer, which causes the CompositorFrame to be stuck
in the browser. When the next BeginFrame occurs, the renderer then cannot draw
(because pending_swaps > 0), and no ACK occurs.

We think this would be fine for headless (where we use infinite deadlines and
don't care about the browser compositor). But: It already requires a lot of
BeginFrameObserver-tracking, and sequence numbers + NACKs would avoid a few
hacky bits. Thus, we think it makes sense for me to implement the ACK/NACK and
observer-tracking first. With that, we can do BeginFrameControl cleanly.

As first step, I plan to implement a patch that:
 - Adds sequence numbers for acknowledged frame and last "full" frame (that
included updates from all children).
 - Tracks down all BFOs and makes sure they call DidFinishFrame().
 - Adds a NACK IPC from renderer -> browser.
 - Tracks BFO responses in the browser.
 - Replaces DisplaySchedulers heuristics for "all surfaces damaged" with "all
active observers responded".

And a second patch that:
 - Makes cc::Scheduler commit ASAP after BeginMainFrame completes (or if there
are no updates).

Later, I'll add:
 - Support for replacing the BFS of a Display.
 - allow_latency_optimizations to BeginFrameArgs.
 - DevTools commands for BeginFrameControl.

[Dan Montz, 2017-05-02 23:31:02]

On 2016/11/22 15:58:20, Eric Seckler wrote:
> Here's an update -- for documentary purposes, no action required.
> 
> TLDR: We have a proof-of-concept (using approach from revised doc, NOT
> VisualStateCallbacks). I'll tackle BeginFrame ACK/NACK and BFObserver-tracking
> first and send a patch for that in the next few days.
> 
> It turns out that VisualStateCallbacks don't really solve our problems -
They're
> racy with the BeginFrame if not used in synchronous renderer mode, and adding
a
> flag to the VisualStateResponse message that specifies whether the frame was
> successfully submitted or aborted is more complicated than we hoped.
> 
> For this reason, I built a prototype for the approach described in the revised
> design doc (infinite deadlines), but without BeginFrame NACKs from the
renderer
> (instead forcing draws as Brian suggested). It adds an
> "allow_latency_optimizations" flag to BeginFrameArgs, which ensures that the
> BeginFrame (and BeginMainFrame) is not skipped in the compositor. At the
moment,
> because there are no NACKs from the renderer, the flag also forces a draw
> (ACTION_DRAW_FORCED). The Compositor and the DisplayScheduler trigger an
> immediate deadline when all children (BeginMainFrame / BeginFrameObservers)
are
> done. The prototype seems to work fine, but does have some limitations:
>  1) Without sequence numbers, it's impossible to distinguish an ACK of the
> current BeginFrame from an ACK of the previous BeginFrame. But this can only
> really happen in the browser-side cc::Scheduler, which may trigger a
> DidFinishFrame() for the previous frame during OnBeginFrame().
>  2) If the deadline is exceeded, the DisplayScheduler may commit without the
> CompositorFrame from the renderer, which causes the CompositorFrame to be
stuck
> in the browser. When the next BeginFrame occurs, the renderer then cannot draw
> (because pending_swaps > 0), and no ACK occurs.
> 
> We think this would be fine for headless (where we use infinite deadlines and
> don't care about the browser compositor). But: It already requires a lot of
> BeginFrameObserver-tracking, and sequence numbers + NACKs would avoid a few
> hacky bits. Thus, we think it makes sense for me to implement the ACK/NACK and
> observer-tracking first. With that, we can do BeginFrameControl cleanly.
> 
> As first step, I plan to implement a patch that:
>  - Adds sequence numbers for acknowledged frame and last "full" frame (that
> included updates from all children).
>  - Tracks down all BFOs and makes sure they call DidFinishFrame().
>  - Adds a NACK IPC from renderer -> browser.
>  - Tracks BFO responses in the browser.
>  - Replaces DisplaySchedulers heuristics for "all surfaces damaged" with "all
> active observers responded".
> 
> And a second patch that:
>  - Makes cc::Scheduler commit ASAP after BeginMainFrame completes (or if there
> are no updates).
> 
> Later, I'll add:
>  - Support for replacing the BFS of a Display.
>  - allow_latency_optimizations to BeginFrameArgs.
>  - DevTools commands for BeginFrameControl.


Are the DevTool commands for BeginFrameControl available now?  I see there
hasn't been an update in a over 5 months.

[Eric Seckler, 2017-05-03 07:23:07]

On 2017/05/02 23:31:02, dan.montz wrote:
> Are the DevTool commands for BeginFrameControl available now?  I see there
> hasn't been an update in a over 5 months.

We're not quite there yet, but actively working on it. Feel free to follow along
on the bug (https://crbug.com/646774) and its blockers.