fix mac frame-rate regression for non-threaded GPU compositing

fix mac frame-rate regression for non-threaded GPU compositing

This change looks weird - sorry about that. On Mac, setNeedsDisplay-driven drawRect calls appear to be implemented with a software timer that is slightly slower than vsync. So in order to render aligned with Vsync we have to either use CVDisplayLink or call CGLFlushDrawable immediately when new frames arrive at the browser RWHVMac. CVDisplayLink adds a frame of latency, so we don't want to use that for rendering. So, we call CGLFlushDrawable immediately.

Calling CGLFlushDrawable is fine except when the window becomes non-visible, during which undocumented behavior kicks in: CGLFlushDrawable returns immediately, no longer throttled to vsync. So, we have the unfortunate task of manually throttling CGLFlushDrawable so that chrome doesn't spin while the window is hidden.

The implementation of manual throttling is to use CVDisplayLink to compute vsync frame counts, and compare that to the flush (swap) counts. If the swap count gets more than frames ahead of vsync, we sleep for 1 frame interval to simulate blocking on vsync.

NOTE: this code path is only for the non-threaded accelerated compositing path. It will not be triggered for the threaded compositing path because we won't render frames at faster than vsync unless there's a bug in the compositor thread scheduling logic.

BUG=139416
Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=150056

[vangelis, 2012-08-01 00:53:12]

https://chromiumcodereview.appspot.com/10828104/diff/2001/content/browser/ren...
File content/browser/renderer_host/compositing_iosurface_mac.mm (right):

https://chromiumcodereview.appspot.com/10828104/diff/2001/content/browser/ren...
content/browser/renderer_host/compositing_iosurface_mac.mm:575: swap_count =
++swap_count_;
Should these two values be reset somewhere on a successful frame?  What happens
if we have very heavy content and swaps come at a slow rate (say once a second)?
In that case, vsync_count would get way ahead of swap_count

[jbates, 2012-08-01 01:00:47]

https://chromiumcodereview.appspot.com/10828104/diff/2001/content/browser/ren...
File content/browser/renderer_host/compositing_iosurface_mac.mm (right):

https://chromiumcodereview.appspot.com/10828104/diff/2001/content/browser/ren...
content/browser/renderer_host/compositing_iosurface_mac.mm:575: swap_count =
++swap_count_;
On 2012/08/01 00:53:12, vangelis wrote:
> Should these two values be reset somewhere on a successful frame? 

They get reset when the 1 second idle timer fires. I think that is enough, in
combination with the vsync_count cap.

 What happens
> if we have very heavy content and swaps come at a slow rate (say once a
second)?
> In that case, vsync_count would get way ahead of swap_count

vsync_count is capped at swap_count for that reason (in the DisplayLinkTick
method).

[vangelis, 2012-08-01 01:33:01]

https://chromiumcodereview.appspot.com/10828104/diff/2001/content/browser/ren...
File content/browser/renderer_host/compositing_iosurface_mac.mm (right):

https://chromiumcodereview.appspot.com/10828104/diff/2001/content/browser/ren...
content/browser/renderer_host/compositing_iosurface_mac.mm:575: swap_count =
++swap_count_;
On 2012/08/01 01:00:47, jbates wrote:
> On 2012/08/01 00:53:12, vangelis wrote:
> > Should these two values be reset somewhere on a successful frame? 
> 
> They get reset when the 1 second idle timer fires. I think that is enough, in
> combination with the vsync_count cap.
> 
>  What happens
> > if we have very heavy content and swaps come at a slow rate (say once a
> second)?
> > In that case, vsync_count would get way ahead of swap_count
> 
> vsync_count is capped at swap_count for that reason (in the DisplayLinkTick
> method).

Oh, I see.  That makes sense.

[vangelis, 2012-08-02 19:56:31]

LGTM from me.  Nico, do you want to take a look at the more mac-specific
details?

[Nico, 2012-08-02 20:07:02]

On 2012/08/02 19:56:31, vangelis wrote:
> LGTM from me.  Nico, do you want to take a look at the more mac-specific
> details?

I stopped understanding how all the painting stuff works a while ago :-(

[jbates, 2012-08-02 20:08:39]

sky: content/browser/OWNERS

[sky, 2012-08-03 16:29:05]

LGTM

[commit-bot: I haz the power, 2012-08-03 16:45:52]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/jbates@chromium.org/10828104/2001

[commit-bot: I haz the power, 2012-08-03 20:30:50]

Try job failure for 10828104-2001 (retry) on win_rel for step
"interactive_ui_tests" (clobber build).
It's a second try, previously, step "compile" failed.
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=win_rel&nu...

[commit-bot: I haz the power, 2012-08-05 20:20:56]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/jbates@chromium.org/10828104/2001

[commit-bot: I haz the power, 2012-08-05 21:58:10]

Change committed as 150056