Set BrowserCompositorOutputSurface's vsync parameters on mac

Set BrowserCompositorOutputSurface's vsync parameters on mac

Mac routes vsync parameters differently than other platforms.  Other
platforms go directly through BrowserCompositorOutputSurface, which then
routes it to the vsync manager (without begin frame scheduling) or just
to the root BeginFrameSource for that BrowserCompositorOutputsurface
(with begin frame scheduling).

Mac is different in that the RenderWidgetHostViewMac gets the vsync info
from the DisplayLink, which forwards it to the BrowserCompositorMac.
This would then forward that to the ui::Compositor's vsync manager which
would then be used to tick the browser and then indirectly would forward
to the begin frame source for the BrowserCompositorOutputSurface to tick
the display.

--enable-begin-frame-scheduling turns off the vsync manager, which
previously left the browser and then display compositors to tick at
default intervals and timebases on Mac, leading to power regressions.
OOPS.

This patch adds an additional route where the BrowserCompositorMac
forwards vsync info to the BrowserCompositorOutputSurface's
BeginFrameSource via ui::Compositor's ui::ContextFactory (aka
GpuProcessTransportFactory).  As the BrowserCompositorOutputSurface's
begin frame source is the source of all frames when using
--enable-begin-frame-scheduling, this fixes the regression.

Once --enable-begin-frame-scheduling becomes the default, all of the
vsync info and vsync manager plumbing can be removed.

With this patch, power usage on one test laptop for h264 video when
using --enable-begin-frame-scheduling goes from 2.00W -> 2.05W instead
of from 2.0W -> 2.3W.

R=ccameron@chromium.org

Committed: https://crrev.com/bd7d5c4d81610751aca50b1c599460c434f29be2
Cr-Commit-Position: refs/heads/master@{#407632}

[enne (OOO), 2016-07-21 20:55:40]

Description was changed from

==========
Set BrowserCompositorOutputSurface's vsync parameters on mac

Mac routes its vsync parameters differently on other platforms.  Other platforms
go through BrowserCompositorOutputSurface, which then routes it to the
vsync manager (without begin frame scheduling) or just to the root
BeginFrameSource
for that BrowserCompositorOutputsurface (with begin frame scheduling).

Mac is different in that the RenderWidgetHostViewMac gets the vsync info from
the DisplayLink, which forwards it to the BrowserCompositorMac.  This would
then forward that to the ui::Compositor's vsync manager which would then be
used to tick the browser and then indirectly would forward to the begin frame
source for the BrowserCompositorOutputSurface to tick the display.

--enable-begin-frame-scheduling turns off the vsync manager, which previously
left the browser and then display compositors to tick at default intervals
and timebases on Mac, leading to power regressions.  OOPS.

This patch adds an additional route where the BrowserCompositorMac forwards
vsync info to the BrowserCompositorOutputSurface's BeginFrameSource via
ui::Compositor's ui::ContextFactory (aka GpuProcessTransportFactory).  As
the BrowserCompositorOutputSurface's begin frame source is the source of all
frames when using --enable-begin-frame-scheduling, this fixes the regression.

Once --enable-begin-frame-scheduling becomes the default, all of the vsync info
and vsync manager plumbing can be removed.

With this patch, power usage on one test laptop for h264 video when using
--enable-begin-frame-scheduling goes from 2.00W -> 2.05W instead of from 2.0W
-> 2.2W.

R=ccameron@chromium.org
==========

to

==========
Set BrowserCompositorOutputSurface's vsync parameters on mac

Mac routes its vsync parameters differently than other platforms.  Other
platforms
go through BrowserCompositorOutputSurface, which then routes it to the
vsync manager (without begin frame scheduling) or just to the root
BeginFrameSource
for that BrowserCompositorOutputsurface (with begin frame scheduling).

Mac is different in that the RenderWidgetHostViewMac gets the vsync info from
the DisplayLink, which forwards it to the BrowserCompositorMac.  This would
then forward that to the ui::Compositor's vsync manager which would then be
used to tick the browser and then indirectly would forward to the begin frame
source for the BrowserCompositorOutputSurface to tick the display.

--enable-begin-frame-scheduling turns off the vsync manager, which previously
left the browser and then display compositors to tick at default intervals
and timebases on Mac, leading to power regressions.  OOPS.

This patch adds an additional route where the BrowserCompositorMac forwards
vsync info to the BrowserCompositorOutputSurface's BeginFrameSource via
ui::Compositor's ui::ContextFactory (aka GpuProcessTransportFactory).  As
the BrowserCompositorOutputSurface's begin frame source is the source of all
frames when using --enable-begin-frame-scheduling, this fixes the regression.

Once --enable-begin-frame-scheduling becomes the default, all of the vsync info
and vsync manager plumbing can be removed.

With this patch, power usage on one test laptop for h264 video when using
--enable-begin-frame-scheduling goes from 2.00W -> 2.05W instead of from 2.0W
-> 2.2W.

R=ccameron@chromium.org
==========

[enne (OOO), 2016-07-21 20:55:48]

Description was changed from

==========
Set BrowserCompositorOutputSurface's vsync parameters on mac

Mac routes its vsync parameters differently than other platforms.  Other
platforms
go through BrowserCompositorOutputSurface, which then routes it to the
vsync manager (without begin frame scheduling) or just to the root
BeginFrameSource
for that BrowserCompositorOutputsurface (with begin frame scheduling).

Mac is different in that the RenderWidgetHostViewMac gets the vsync info from
the DisplayLink, which forwards it to the BrowserCompositorMac.  This would
then forward that to the ui::Compositor's vsync manager which would then be
used to tick the browser and then indirectly would forward to the begin frame
source for the BrowserCompositorOutputSurface to tick the display.

--enable-begin-frame-scheduling turns off the vsync manager, which previously
left the browser and then display compositors to tick at default intervals
and timebases on Mac, leading to power regressions.  OOPS.

This patch adds an additional route where the BrowserCompositorMac forwards
vsync info to the BrowserCompositorOutputSurface's BeginFrameSource via
ui::Compositor's ui::ContextFactory (aka GpuProcessTransportFactory).  As
the BrowserCompositorOutputSurface's begin frame source is the source of all
frames when using --enable-begin-frame-scheduling, this fixes the regression.

Once --enable-begin-frame-scheduling becomes the default, all of the vsync info
and vsync manager plumbing can be removed.

With this patch, power usage on one test laptop for h264 video when using
--enable-begin-frame-scheduling goes from 2.00W -> 2.05W instead of from 2.0W
-> 2.2W.

R=ccameron@chromium.org
==========

to

==========
Set BrowserCompositorOutputSurface's vsync parameters on mac

Mac routes its vsync parameters differently than other platforms.  Other
platforms
go directly through BrowserCompositorOutputSurface, which then routes it to the
vsync manager (without begin frame scheduling) or just to the root
BeginFrameSource
for that BrowserCompositorOutputsurface (with begin frame scheduling).

Mac is different in that the RenderWidgetHostViewMac gets the vsync info from
the DisplayLink, which forwards it to the BrowserCompositorMac.  This would
then forward that to the ui::Compositor's vsync manager which would then be
used to tick the browser and then indirectly would forward to the begin frame
source for the BrowserCompositorOutputSurface to tick the display.

--enable-begin-frame-scheduling turns off the vsync manager, which previously
left the browser and then display compositors to tick at default intervals
and timebases on Mac, leading to power regressions.  OOPS.

This patch adds an additional route where the BrowserCompositorMac forwards
vsync info to the BrowserCompositorOutputSurface's BeginFrameSource via
ui::Compositor's ui::ContextFactory (aka GpuProcessTransportFactory).  As
the BrowserCompositorOutputSurface's begin frame source is the source of all
frames when using --enable-begin-frame-scheduling, this fixes the regression.

Once --enable-begin-frame-scheduling becomes the default, all of the vsync info
and vsync manager plumbing can be removed.

With this patch, power usage on one test laptop for h264 video when using
--enable-begin-frame-scheduling goes from 2.00W -> 2.05W instead of from 2.0W
-> 2.2W.

R=ccameron@chromium.org
==========

[enne (OOO), 2016-07-21 20:56:46]

Description was changed from

==========
Set BrowserCompositorOutputSurface's vsync parameters on mac

Mac routes its vsync parameters differently than other platforms.  Other
platforms
go directly through BrowserCompositorOutputSurface, which then routes it to the
vsync manager (without begin frame scheduling) or just to the root
BeginFrameSource
for that BrowserCompositorOutputsurface (with begin frame scheduling).

Mac is different in that the RenderWidgetHostViewMac gets the vsync info from
the DisplayLink, which forwards it to the BrowserCompositorMac.  This would
then forward that to the ui::Compositor's vsync manager which would then be
used to tick the browser and then indirectly would forward to the begin frame
source for the BrowserCompositorOutputSurface to tick the display.

--enable-begin-frame-scheduling turns off the vsync manager, which previously
left the browser and then display compositors to tick at default intervals
and timebases on Mac, leading to power regressions.  OOPS.

This patch adds an additional route where the BrowserCompositorMac forwards
vsync info to the BrowserCompositorOutputSurface's BeginFrameSource via
ui::Compositor's ui::ContextFactory (aka GpuProcessTransportFactory).  As
the BrowserCompositorOutputSurface's begin frame source is the source of all
frames when using --enable-begin-frame-scheduling, this fixes the regression.

Once --enable-begin-frame-scheduling becomes the default, all of the vsync info
and vsync manager plumbing can be removed.

With this patch, power usage on one test laptop for h264 video when using
--enable-begin-frame-scheduling goes from 2.00W -> 2.05W instead of from 2.0W
-> 2.2W.

R=ccameron@chromium.org
==========

to

==========
Set BrowserCompositorOutputSurface's vsync parameters on mac

Mac routes vsync parameters differently than other platforms.  Other
platforms go directly through BrowserCompositorOutputSurface, which then
routes it to the vsync manager (without begin frame scheduling) or just
to the root BeginFrameSource for that BrowserCompositorOutputsurface
(with begin frame scheduling).

Mac is different in that the RenderWidgetHostViewMac gets the vsync info
from the DisplayLink, which forwards it to the BrowserCompositorMac.
This would then forward that to the ui::Compositor's vsync manager which
would then be used to tick the browser and then indirectly would forward
to the begin frame source for the BrowserCompositorOutputSurface to tick
the display.

--enable-begin-frame-scheduling turns off the vsync manager, which
previously left the browser and then display compositors to tick at
default intervals and timebases on Mac, leading to power regressions.
OOPS.

This patch adds an additional route where the BrowserCompositorMac
forwards vsync info to the BrowserCompositorOutputSurface's
BeginFrameSource via ui::Compositor's ui::ContextFactory (aka
GpuProcessTransportFactory).  As the BrowserCompositorOutputSurface's
begin frame source is the source of all frames when using
--enable-begin-frame-scheduling, this fixes the regression.

Once --enable-begin-frame-scheduling becomes the default, all of the
vsync info and vsync manager plumbing can be removed.

With this patch, power usage on one test laptop for h264 video when
using --enable-begin-frame-scheduling goes from 2.00W -> 2.05W instead
of from 2.0W -> 2.2W.

R=ccameron@chromium.org
==========

[enne (OOO), 2016-07-21 20:58:42]

I'm not sure if there's some way to move DisplayLink somewhere closer to
Display/BrowserCompositorOutputSurface so Mac can behave more like other
platforms, but this seems like not super unreasonable plumbing.

[enne (OOO), 2016-07-21 21:08:49]

Description was changed from

==========
Set BrowserCompositorOutputSurface's vsync parameters on mac

Mac routes vsync parameters differently than other platforms.  Other
platforms go directly through BrowserCompositorOutputSurface, which then
routes it to the vsync manager (without begin frame scheduling) or just
to the root BeginFrameSource for that BrowserCompositorOutputsurface
(with begin frame scheduling).

Mac is different in that the RenderWidgetHostViewMac gets the vsync info
from the DisplayLink, which forwards it to the BrowserCompositorMac.
This would then forward that to the ui::Compositor's vsync manager which
would then be used to tick the browser and then indirectly would forward
to the begin frame source for the BrowserCompositorOutputSurface to tick
the display.

--enable-begin-frame-scheduling turns off the vsync manager, which
previously left the browser and then display compositors to tick at
default intervals and timebases on Mac, leading to power regressions.
OOPS.

This patch adds an additional route where the BrowserCompositorMac
forwards vsync info to the BrowserCompositorOutputSurface's
BeginFrameSource via ui::Compositor's ui::ContextFactory (aka
GpuProcessTransportFactory).  As the BrowserCompositorOutputSurface's
begin frame source is the source of all frames when using
--enable-begin-frame-scheduling, this fixes the regression.

Once --enable-begin-frame-scheduling becomes the default, all of the
vsync info and vsync manager plumbing can be removed.

With this patch, power usage on one test laptop for h264 video when
using --enable-begin-frame-scheduling goes from 2.00W -> 2.05W instead
of from 2.0W -> 2.2W.

R=ccameron@chromium.org
==========

to

==========
Set BrowserCompositorOutputSurface's vsync parameters on mac

Mac routes vsync parameters differently than other platforms.  Other
platforms go directly through BrowserCompositorOutputSurface, which then
routes it to the vsync manager (without begin frame scheduling) or just
to the root BeginFrameSource for that BrowserCompositorOutputsurface
(with begin frame scheduling).

Mac is different in that the RenderWidgetHostViewMac gets the vsync info
from the DisplayLink, which forwards it to the BrowserCompositorMac.
This would then forward that to the ui::Compositor's vsync manager which
would then be used to tick the browser and then indirectly would forward
to the begin frame source for the BrowserCompositorOutputSurface to tick
the display.

--enable-begin-frame-scheduling turns off the vsync manager, which
previously left the browser and then display compositors to tick at
default intervals and timebases on Mac, leading to power regressions.
OOPS.

This patch adds an additional route where the BrowserCompositorMac
forwards vsync info to the BrowserCompositorOutputSurface's
BeginFrameSource via ui::Compositor's ui::ContextFactory (aka
GpuProcessTransportFactory).  As the BrowserCompositorOutputSurface's
begin frame source is the source of all frames when using
--enable-begin-frame-scheduling, this fixes the regression.

Once --enable-begin-frame-scheduling becomes the default, all of the
vsync info and vsync manager plumbing can be removed.

With this patch, power usage on one test laptop for h264 video when
using --enable-begin-frame-scheduling goes from 2.00W -> 2.05W instead
of from 2.0W -> 2.3W.

R=ccameron@chromium.org
==========

[enne (OOO), 2016-07-21 22:26:24]

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

[commit-bot: I haz the power, 2016-07-21 22:27:00]

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-07-21 23:47:50]

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

[commit-bot: I haz the power, 2016-07-21 23:47:51]

Dry run: This issue passed the CQ dry run.

[ccameron, 2016-07-22 21:37:33]

On 2016/07/21 20:58:42, enne wrote:
> I'm not sure if there's some way to move DisplayLink somewhere closer to
> Display/BrowserCompositorOutputSurface so Mac can behave more like other
> platforms, but this seems like not super unreasonable plumbing.

Sorry for the delay, seeing if I can fit this into other refactorings!

[ccameron, 2016-07-25 17:30:12]

https://codereview.chromium.org/2170053002/diff/1/content/browser/renderer_ho...
File content/browser/renderer_host/browser_compositor_view_mac.mm (right):

https://codereview.chromium.org/2170053002/diff/1/content/browser/renderer_ho...
content/browser/renderer_host/browser_compositor_view_mac.mm:295: }
Would it make sense to just do:
    if (begin_frame_source_)
      begin_frame_source_->OnUpdateVSyncParameters(timebase, interval);
Or is |begin_frame_source_| in this context different than in GPTF?

This gets called at every swap (the arguments only change every few seconds, so
if too-frequent-calls is a problem, we can elide redundant ones here).

[enne (OOO), 2016-07-25 19:07:30]

Technically, at the moment, this is the same BFS, although it's just a
BeginFrameSource and not fully known as a SyntheticBeginFrameSource.  This BFS
comes indirectly from the SurfaceManager which comes from the Display.  There's
also some hope in the future that we would be able to throttle renderers or to
play games to catch up on latency.  So, trying to set the parameters here would
be breaking an abstraction that I was hoping to keep.

Mac is the odd platform out here, in that every renderer has its own browser
compositor / ui::Compositor / cc::Display.  Therefore, any BFS is going to be
1:1 the whole way through that pipeline.  Other platforms share the
ui::Compositor for multiple renderers.  It's also possible that a renderer could
appear on multiple displays, which doesn't seem like it is true for Mac.

[ccameron, 2016-07-25 21:34:25]

On 2016/07/25 19:07:30, enne wrote:
> Technically, at the moment, this is the same BFS, although it's just a
> BeginFrameSource and not fully known as a SyntheticBeginFrameSource.  This BFS
> comes indirectly from the SurfaceManager which comes from the Display. 
There's
> also some hope in the future that we would be able to throttle renderers or to
> play games to catch up on latency.  So, trying to set the parameters here
would
> be breaking an abstraction that I was hoping to keep.
> 
> Mac is the odd platform out here, in that every renderer has its own browser
> compositor / ui::Compositor / cc::Display.  Therefore, any BFS is going to be
> 1:1 the whole way through that pipeline.  Other platforms share the
> ui::Compositor for multiple renderers.  It's also possible that a renderer
could
> appear on multiple displays, which doesn't seem like it is true for Mac.

This lgtm. Once unified begin frame lands, and we stop supporting
non-unified-begin-frame, then I can move the DisplayLink around to make this
simpler.

[enne (OOO), 2016-07-25 22:18:48]

The CQ bit was checked by enne@chromium.org

[commit-bot: I haz the power, 2016-07-25 22:19:09]

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-07-25 22:25:58]

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

[commit-bot: I haz the power, 2016-07-25 22:26:00]

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...)

[enne (OOO), 2016-07-25 22:28:37]

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

[enne (OOO), 2016-07-25 22:29:00]

+derat for ash/ OWNERS

[Daniel Erat, 2016-07-25 23:11:31]

lgtm for ash/

[enne (OOO), 2016-07-25 23:29:23]

The CQ bit was checked by enne@chromium.org

[commit-bot: I haz the power, 2016-07-25 23:30:04]

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-07-25 23:35:16]

Committed patchset #1 (id:1)

[commit-bot: I haz the power, 2016-07-25 23:37:59]

Description was changed from

==========
Set BrowserCompositorOutputSurface's vsync parameters on mac

Mac routes vsync parameters differently than other platforms.  Other
platforms go directly through BrowserCompositorOutputSurface, which then
routes it to the vsync manager (without begin frame scheduling) or just
to the root BeginFrameSource for that BrowserCompositorOutputsurface
(with begin frame scheduling).

Mac is different in that the RenderWidgetHostViewMac gets the vsync info
from the DisplayLink, which forwards it to the BrowserCompositorMac.
This would then forward that to the ui::Compositor's vsync manager which
would then be used to tick the browser and then indirectly would forward
to the begin frame source for the BrowserCompositorOutputSurface to tick
the display.

--enable-begin-frame-scheduling turns off the vsync manager, which
previously left the browser and then display compositors to tick at
default intervals and timebases on Mac, leading to power regressions.
OOPS.

This patch adds an additional route where the BrowserCompositorMac
forwards vsync info to the BrowserCompositorOutputSurface's
BeginFrameSource via ui::Compositor's ui::ContextFactory (aka
GpuProcessTransportFactory).  As the BrowserCompositorOutputSurface's
begin frame source is the source of all frames when using
--enable-begin-frame-scheduling, this fixes the regression.

Once --enable-begin-frame-scheduling becomes the default, all of the
vsync info and vsync manager plumbing can be removed.

With this patch, power usage on one test laptop for h264 video when
using --enable-begin-frame-scheduling goes from 2.00W -> 2.05W instead
of from 2.0W -> 2.3W.

R=ccameron@chromium.org
==========

to

==========
Set BrowserCompositorOutputSurface's vsync parameters on mac

Mac routes vsync parameters differently than other platforms.  Other
platforms go directly through BrowserCompositorOutputSurface, which then
routes it to the vsync manager (without begin frame scheduling) or just
to the root BeginFrameSource for that BrowserCompositorOutputsurface
(with begin frame scheduling).

Mac is different in that the RenderWidgetHostViewMac gets the vsync info
from the DisplayLink, which forwards it to the BrowserCompositorMac.
This would then forward that to the ui::Compositor's vsync manager which
would then be used to tick the browser and then indirectly would forward
to the begin frame source for the BrowserCompositorOutputSurface to tick
the display.

--enable-begin-frame-scheduling turns off the vsync manager, which
previously left the browser and then display compositors to tick at
default intervals and timebases on Mac, leading to power regressions.
OOPS.

This patch adds an additional route where the BrowserCompositorMac
forwards vsync info to the BrowserCompositorOutputSurface's
BeginFrameSource via ui::Compositor's ui::ContextFactory (aka
GpuProcessTransportFactory).  As the BrowserCompositorOutputSurface's
begin frame source is the source of all frames when using
--enable-begin-frame-scheduling, this fixes the regression.

Once --enable-begin-frame-scheduling becomes the default, all of the
vsync info and vsync manager plumbing can be removed.

With this patch, power usage on one test laptop for h264 video when
using --enable-begin-frame-scheduling goes from 2.00W -> 2.05W instead
of from 2.0W -> 2.3W.

R=ccameron@chromium.org

Committed: https://crrev.com/bd7d5c4d81610751aca50b1c599460c434f29be2
Cr-Commit-Position: refs/heads/master@{#407632}
==========

[commit-bot: I haz the power, 2016-07-25 23:38:00]

Patchset 1 (id:??) landed as
https://crrev.com/bd7d5c4d81610751aca50b1c599460c434f29be2
Cr-Commit-Position: refs/heads/master@{#407632}