Remove SendBeginFramesToChildren plumbing

Remove SendBeginFramesToChildren plumbing

This patch implements the browser half of the Browser<->RenderWidget
begin frame plumbing.  It reuses the existing ipc mechanisms to
send begin frames to the renderer and receive "needs begin frame"
messages in return.

Before this patch, the ui::Compositor would send "begin frames for
children" all the way from cc::Scheduler -> cc::SingleThreadProxy ->
cc::LayerTreeHost -> ui::Compositor -> ui::BeginFrameObserverProxy ->
content::RenderWidgetHostViewAura.

This patch uses the SurfaceFactoryClient::SetBeginFrameSource routing to
go SurfaceManager -> DelegatedFrameHost -> RenderWidgetHostViewAura
instead.  RenderWidgetHostViewAura just observes the BeginFrameSource
directly and sends begin frame messages to the renderer when the begin
frame source ticks.

Because this path is only meaningful on the renderer side when
--enable-begin-frame-scheduling is on, this patch does not remove the
vsync paths and the compositor vsync manager quite yet.  That will need
to be done as a follow up after this is turned on.

This depends on https://codereview.chromium.org/1821863002

CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel

Committed: https://crrev.com/6d9b09303ab06012cfe16f4714e6beaa14eb9eb0
Cr-Commit-Position: refs/heads/master@{#388118}

[enne (OOO), 2016-03-31 22:13:12]

Description was changed from

==========
Remove SendBeginFramesToChildren plumbing

This patch implements the browser half of the Browser<->RenderWidget
begin frame plumbing.  It reuses the existing ipc mechanisms to
send begin frames to the renderer and receive "needs begin frame"
messages in return.

Before this patch, the ui::Compositor would send "begin frames for
children" all the way from cc::Scheduler -> cc::SingleThreadProxy ->
cc::LayerTreeHost -> ui::Compositor -> ui::BeginFrameObserverProxy ->
content::RenderWidgetHostViewAura.

This patch uses the SurfaceFactoryClient::SetBeginFrameSource routing to
go SurfaceManager -> DelegatedFrameHost -> RenderWidgetHostViewAura
instead.  RenderWidgetHostViewAura just observes the BeginFrameSource
directly and sends begin frame messages to the renderer when the begin
frame source ticks.

Because this path is only meaningful on the renderer side when
--enable-begin-frame-scheduling is on, this patch does not remove the
vsync paths and the compositor vsync manager quite yet.  That will need
to be done as a follow up after this is turned on.
==========

to

==========
Remove SendBeginFramesToChildren plumbing

This patch implements the browser half of the Browser<->RenderWidget
begin frame plumbing.  It reuses the existing ipc mechanisms to
send begin frames to the renderer and receive "needs begin frame"
messages in return.

Before this patch, the ui::Compositor would send "begin frames for
children" all the way from cc::Scheduler -> cc::SingleThreadProxy ->
cc::LayerTreeHost -> ui::Compositor -> ui::BeginFrameObserverProxy ->
content::RenderWidgetHostViewAura.

This patch uses the SurfaceFactoryClient::SetBeginFrameSource routing to
go SurfaceManager -> DelegatedFrameHost -> RenderWidgetHostViewAura
instead.  RenderWidgetHostViewAura just observes the BeginFrameSource
directly and sends begin frame messages to the renderer when the begin
frame source ticks.

Because this path is only meaningful on the renderer side when
--enable-begin-frame-scheduling is on, this patch does not remove the
vsync paths and the compositor vsync manager quite yet.  That will need
to be done as a follow up after this is turned on.
CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel
==========

[enne (OOO), 2016-04-07 18:44:30]

Description was changed from

==========
Remove SendBeginFramesToChildren plumbing

This patch implements the browser half of the Browser<->RenderWidget
begin frame plumbing.  It reuses the existing ipc mechanisms to
send begin frames to the renderer and receive "needs begin frame"
messages in return.

Before this patch, the ui::Compositor would send "begin frames for
children" all the way from cc::Scheduler -> cc::SingleThreadProxy ->
cc::LayerTreeHost -> ui::Compositor -> ui::BeginFrameObserverProxy ->
content::RenderWidgetHostViewAura.

This patch uses the SurfaceFactoryClient::SetBeginFrameSource routing to
go SurfaceManager -> DelegatedFrameHost -> RenderWidgetHostViewAura
instead.  RenderWidgetHostViewAura just observes the BeginFrameSource
directly and sends begin frame messages to the renderer when the begin
frame source ticks.

Because this path is only meaningful on the renderer side when
--enable-begin-frame-scheduling is on, this patch does not remove the
vsync paths and the compositor vsync manager quite yet.  That will need
to be done as a follow up after this is turned on.
CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel
==========

to

==========
Remove SendBeginFramesToChildren plumbing

This patch implements the browser half of the Browser<->RenderWidget
begin frame plumbing.  It reuses the existing ipc mechanisms to
send begin frames to the renderer and receive "needs begin frame"
messages in return.

Before this patch, the ui::Compositor would send "begin frames for
children" all the way from cc::Scheduler -> cc::SingleThreadProxy ->
cc::LayerTreeHost -> ui::Compositor -> ui::BeginFrameObserverProxy ->
content::RenderWidgetHostViewAura.

This patch uses the SurfaceFactoryClient::SetBeginFrameSource routing to
go SurfaceManager -> DelegatedFrameHost -> RenderWidgetHostViewAura
instead.  RenderWidgetHostViewAura just observes the BeginFrameSource
directly and sends begin frame messages to the renderer when the begin
frame source ticks.

Because this path is only meaningful on the renderer side when
--enable-begin-frame-scheduling is on, this patch does not remove the
vsync paths and the compositor vsync manager quite yet.  That will need
to be done as a follow up after this is turned on.

CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel
==========

[enne (OOO), 2016-04-07 18:44:30]

enne@chromium.org changed reviewers:
+ danakj@chromium.org, skyostil@chromium.org, sunnyps@chromium.org

[enne (OOO), 2016-04-07 18:44:54]

Description was changed from

==========
Remove SendBeginFramesToChildren plumbing

This patch implements the browser half of the Browser<->RenderWidget
begin frame plumbing.  It reuses the existing ipc mechanisms to
send begin frames to the renderer and receive "needs begin frame"
messages in return.

Before this patch, the ui::Compositor would send "begin frames for
children" all the way from cc::Scheduler -> cc::SingleThreadProxy ->
cc::LayerTreeHost -> ui::Compositor -> ui::BeginFrameObserverProxy ->
content::RenderWidgetHostViewAura.

This patch uses the SurfaceFactoryClient::SetBeginFrameSource routing to
go SurfaceManager -> DelegatedFrameHost -> RenderWidgetHostViewAura
instead.  RenderWidgetHostViewAura just observes the BeginFrameSource
directly and sends begin frame messages to the renderer when the begin
frame source ticks.

Because this path is only meaningful on the renderer side when
--enable-begin-frame-scheduling is on, this patch does not remove the
vsync paths and the compositor vsync manager quite yet.  That will need
to be done as a follow up after this is turned on.

CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel
==========

to

==========
Remove SendBeginFramesToChildren plumbing

This patch implements the browser half of the Browser<->RenderWidget
begin frame plumbing.  It reuses the existing ipc mechanisms to
send begin frames to the renderer and receive "needs begin frame"
messages in return.

Before this patch, the ui::Compositor would send "begin frames for
children" all the way from cc::Scheduler -> cc::SingleThreadProxy ->
cc::LayerTreeHost -> ui::Compositor -> ui::BeginFrameObserverProxy ->
content::RenderWidgetHostViewAura.

This patch uses the SurfaceFactoryClient::SetBeginFrameSource routing to
go SurfaceManager -> DelegatedFrameHost -> RenderWidgetHostViewAura
instead.  RenderWidgetHostViewAura just observes the BeginFrameSource
directly and sends begin frame messages to the renderer when the begin
frame source ticks.

Because this path is only meaningful on the renderer side when
--enable-begin-frame-scheduling is on, this patch does not remove the
vsync paths and the compositor vsync manager quite yet.  That will need
to be done as a follow up after this is turned on.

This depends on https://codereview.chromium.org/1821863002

CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel
==========

[enne (OOO), 2016-04-07 18:47:20]

This patch won't do anything without --enable-begin-frame-scheduling turned on. 
I think I need to make that flag work for oopif renderers before I can try
turning it on by default in RenderWidget for non-Android platforms.

[enne (OOO), 2016-04-07 18:47:49]

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

[no sievers, 2016-04-07 20:35:35]

content lgtm

https://codereview.chromium.org/1841083007/diff/20001/content/browser/rendere...
File content/browser/renderer_host/render_widget_host_view_aura.cc (right):

https://codereview.chromium.org/1841083007/diff/20001/content/browser/rendere...
content/browser/renderer_host/render_widget_host_view_aura.cc:667:
observing_begin_frame_source_ = needs_begin_frames;
nit: I'm wondering if calling it |needs_begin_frames_| rather than
|observing_begin_frame_source_| is a bit less confusing, since it might actually
be |true| when we aren't actually observing in the case where there was no BFS.

[danakj, 2016-04-07 22:18:55]

The LGTM's overfloweth.

[sunnyps, 2016-04-07 23:57:54]

LGTM. Thanks for getting rid of this.

https://codereview.chromium.org/1841083007/diff/20001/content/browser/rendere...
File content/browser/renderer_host/render_widget_host_view_aura.cc (right):

https://codereview.chromium.org/1841083007/diff/20001/content/browser/rendere...
content/browser/renderer_host/render_widget_host_view_aura.cc:677:
delegated_frame_host_->SetVSyncParameters(args.frame_time, args.interval);
nit: Does this belong in DelegatedFrameHost::OnBeginFrame?

[enne (OOO), 2016-04-14 18:12:16]

https://codereview.chromium.org/1841083007/diff/20001/content/browser/rendere...
File content/browser/renderer_host/render_widget_host_view_aura.cc (right):

https://codereview.chromium.org/1841083007/diff/20001/content/browser/rendere...
content/browser/renderer_host/render_widget_host_view_aura.cc:677:
delegated_frame_host_->SetVSyncParameters(args.frame_time, args.interval);
On 2016/04/07 at 23:57:54, sunnyps wrote:
> nit: Does this belong in DelegatedFrameHost::OnBeginFrame?

Not unless you think the DelegatedFrameHost should also be observing the
BeginFrameSource just for this?

[enne (OOO), 2016-04-19 00:53:20]

On 2016/04/14 at 18:12:16, enne wrote:
>
https://codereview.chromium.org/1841083007/diff/20001/content/browser/rendere...
> File content/browser/renderer_host/render_widget_host_view_aura.cc (right):
> 
>
https://codereview.chromium.org/1841083007/diff/20001/content/browser/rendere...
> content/browser/renderer_host/render_widget_host_view_aura.cc:677:
delegated_frame_host_->SetVSyncParameters(args.frame_time, args.interval);
> On 2016/04/07 at 23:57:54, sunnyps wrote:
> > nit: Does this belong in DelegatedFrameHost::OnBeginFrame?
> 
> Not unless you think the DelegatedFrameHost should also be observing the
BeginFrameSource just for this?

I'm just going to commit this.  If you want some refactoring follow-up, I'd be
happy to chat.  :)

[enne (OOO), 2016-04-19 00:53:23]

The CQ bit was checked by enne@chromium.org

[enne (OOO), 2016-04-19 00:53:23]

The patchset sent to the CQ was uploaded after l-g-t-m from danakj@chromium.org,
sunnyps@chromium.org, sievers@chromium.org
Link to the patchset: https://codereview.chromium.org/1841083007/#ps170001
(title: "Rebase")

[commit-bot: I haz the power, 2016-04-19 00:53:39]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1841083007/170001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1841083007/170001

[sunnyps, 2016-04-19 01:00:22]

https://codereview.chromium.org/1841083007/diff/20001/content/browser/rendere...
File content/browser/renderer_host/render_widget_host_view_aura.cc (right):

https://codereview.chromium.org/1841083007/diff/20001/content/browser/rendere...
content/browser/renderer_host/render_widget_host_view_aura.cc:677:
delegated_frame_host_->SetVSyncParameters(args.frame_time, args.interval);
On 2016/04/14 18:12:16, enne wrote:
> On 2016/04/07 at 23:57:54, sunnyps wrote:
> > nit: Does this belong in DelegatedFrameHost::OnBeginFrame?
> 
> Not unless you think the DelegatedFrameHost should also be observing the
> BeginFrameSource just for this?

Sorry for the late reply. I confused this with OnSetNeedsBeginFrames.

[commit-bot: I haz the power, 2016-04-19 01:52:24]

Committed patchset #10 (id:170001)

[commit-bot: I haz the power, 2016-04-19 01:53:24]

Description was changed from

==========
Remove SendBeginFramesToChildren plumbing

This patch implements the browser half of the Browser<->RenderWidget
begin frame plumbing.  It reuses the existing ipc mechanisms to
send begin frames to the renderer and receive "needs begin frame"
messages in return.

Before this patch, the ui::Compositor would send "begin frames for
children" all the way from cc::Scheduler -> cc::SingleThreadProxy ->
cc::LayerTreeHost -> ui::Compositor -> ui::BeginFrameObserverProxy ->
content::RenderWidgetHostViewAura.

This patch uses the SurfaceFactoryClient::SetBeginFrameSource routing to
go SurfaceManager -> DelegatedFrameHost -> RenderWidgetHostViewAura
instead.  RenderWidgetHostViewAura just observes the BeginFrameSource
directly and sends begin frame messages to the renderer when the begin
frame source ticks.

Because this path is only meaningful on the renderer side when
--enable-begin-frame-scheduling is on, this patch does not remove the
vsync paths and the compositor vsync manager quite yet.  That will need
to be done as a follow up after this is turned on.

This depends on https://codereview.chromium.org/1821863002

CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel
==========

to

==========
Remove SendBeginFramesToChildren plumbing

This patch implements the browser half of the Browser<->RenderWidget
begin frame plumbing.  It reuses the existing ipc mechanisms to
send begin frames to the renderer and receive "needs begin frame"
messages in return.

Before this patch, the ui::Compositor would send "begin frames for
children" all the way from cc::Scheduler -> cc::SingleThreadProxy ->
cc::LayerTreeHost -> ui::Compositor -> ui::BeginFrameObserverProxy ->
content::RenderWidgetHostViewAura.

This patch uses the SurfaceFactoryClient::SetBeginFrameSource routing to
go SurfaceManager -> DelegatedFrameHost -> RenderWidgetHostViewAura
instead.  RenderWidgetHostViewAura just observes the BeginFrameSource
directly and sends begin frame messages to the renderer when the begin
frame source ticks.

Because this path is only meaningful on the renderer side when
--enable-begin-frame-scheduling is on, this patch does not remove the
vsync paths and the compositor vsync manager quite yet.  That will need
to be done as a follow up after this is turned on.

This depends on https://codereview.chromium.org/1821863002

CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel

Committed: https://crrev.com/6d9b09303ab06012cfe16f4714e6beaa14eb9eb0
Cr-Commit-Position: refs/heads/master@{#388118}
==========

[commit-bot: I haz the power, 2016-04-19 01:53:25]

Patchset 10 (id:??) landed as
https://crrev.com/6d9b09303ab06012cfe16f4714e6beaa14eb9eb0
Cr-Commit-Position: refs/heads/master@{#388118}