Mac: Fix janky YouTube animations

Mac: Fix janky YouTube animations

Based on experimentation, it appears that calling setNeedsDisplay or
displayIfNeeded on a CAOpenGLLayer which has the isAsynchronous
property set, will result in the animation getting screwed up. In
particular, calls to canDrawInCGLContext stop occurring.

This was working in M35 and M36 because, entirely accidentally, we
were calling setAsynchronous:NO before most instances of
setNeedsDisplay and displayIfNeeded. In r272469, these calls were
removed, and resulting in the janky animation.

Add calls to setAsynchronous:NO before all instances of calling
displayIfNeeded, and almost all instances of calling setNeedsDisplay.

BUG=395827
TEST=manual, automated tests in progress

Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=284916

[ccameron, 2014-07-22 06:40:42]

This is the issue I was mentioning earlier.

It look like toggling isAsynchronous for a CAOpenGLLayer is only
quasi-supported. Considering that it has gone through M35 and M36 with this
workaround, and had no issues, I think this is safe (I'd be a bit more worried
if I'd come up with this de novo here).

https://codereview.chromium.org/408153002/diff/1/content/browser/renderer_hos...
File content/browser/renderer_host/compositing_iosurface_layer_mac.h (right):

https://codereview.chromium.org/408153002/diff/1/content/browser/renderer_hos...
content/browser/renderer_host/compositing_iosurface_layer_mac.h:55: // ensure
that the frame be acked.
I renamed these methods to be a bit more explicit.

https://codereview.chromium.org/408153002/diff/1/content/browser/renderer_hos...
File content/browser/renderer_host/render_widget_host_view_mac.mm (right):

https://codereview.chromium.org/408153002/diff/1/content/browser/renderer_hos...
content/browser/renderer_host/render_widget_host_view_mac.mm:1781: void
RenderWidgetHostViewMac::AcceleratedSurfaceSuspend() {
I grabbed this while in the neighborhood. It made reasoning about what state is
allocated much easier (in practice, non-delegated rendering, which is the only
place this is used, is likely to remain dead code and eventually be eliminated
entirely).

[Ken Russell (switch to Gerrit), 2014-07-22 19:52:05]

LGTM if this has been tested; let's work on getting an automated test in place
ASAP.

https://codereview.chromium.org/408153002/diff/1/content/browser/renderer_hos...
File content/browser/renderer_host/compositing_iosurface_layer_mac.mm (right):

https://codereview.chromium.org/408153002/diff/1/content/browser/renderer_hos...
content/browser/renderer_host/compositing_iosurface_layer_mac.mm:114: [layer_
setAsynchronous:NO];
I don't know the various state transitions done to this layer; who sets it back
to asynchronous? If we're in the middle of a scroll (is the layer in async mode
then?) and for some reason we have to display a frame immediately, will the
scroll be aborted abruptly?

[ccameron, 2014-07-23 07:50:34]

Thanks!

Checking CQ bit, but this won't resolve until lkgr gets past r284856 (crossing
fingers that it might).

https://codereview.chromium.org/408153002/diff/1/content/browser/renderer_hos...
File content/browser/renderer_host/compositing_iosurface_layer_mac.mm (right):

https://codereview.chromium.org/408153002/diff/1/content/browser/renderer_hos...
content/browser/renderer_host/compositing_iosurface_layer_mac.mm:114: [layer_
setAsynchronous:NO];
On 2014/07/22 19:52:04, Ken Russell wrote:
> I don't know the various state transitions done to this layer; who sets it
back
> to asynchronous? If we're in the middle of a scroll (is the layer in async
mode
> then?) and for some reason we have to display a frame immediately, will the
> scroll be aborted abruptly?

It gets re-set to asynchronous as soon as a new frame from the renderer comes
in. The asynchronous mode is a way of making animation smooth -- the synchronous
mode works fine in terms of making forward progress, but can be janky. In these
situations (where we're forcing a display), we're prioritizing getting a frame
up over smoothness (it may be that we'll want to push this prioritization down
to the renderer too, but that's for another day).

[ccameron, 2014-07-23 07:50:38]

The CQ bit was checked by ccameron@chromium.org

[commit-bot: I haz the power, 2014-07-23 07:51:46]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/ccameron@chromium.org/408153002/1

[commit-bot: I haz the power, 2014-07-23 14:18:18]

Change committed as 284916