compositor: Add rAF-like functionality for the UI.

compositor: Add rAF-like functionality for the UI.

Introduce CompositorAnimationObserver. An observer is called at every
animation step in the compositor, until the observer is removed from
the compositor's observer list.

Update LayerAnimatorCollection to become a CompositorAnimationObserver,
and use this task-based mechanism for driving the layer animations.

BUG=398117
R=piman@chromium.org

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

[sadrul, 2014-07-31 01:02:18]

Hi! Does this look like a reasonable change? The motivation for this CL is to
allow doing flings in the ui (e.g. flinging multiple windows,
http://crbug.com/398117), where the fling would animate more than one layer at
the same time. It would also allow doing things that don't necessarily operate
on a layer (e.g. scrolling a views::View in views::ScrollAnimator).

[piman, 2014-07-31 01:17:03]

https://codereview.chromium.org/427103002/diff/60001/ui/compositor/animation_...
File ui/compositor/animation_frame_task.h (right):

https://codereview.chromium.org/427103002/diff/60001/ui/compositor/animation_...
ui/compositor/animation_frame_task.h:55: virtual
scoped_ptr<ScopedAnimationFrameTask> RequestAnimationFrameTask(
I would prefer if ScopedAnimationFrameTask was not part of the API.
Instead you can make RemoveAnimationFrameTask part of the API, and have
ScopedAnimationFrameTask explicitly call it. No need for closures any more,
simpler code overall, I think.

https://codereview.chromium.org/427103002/diff/60001/ui/compositor/compositor.cc
File ui/compositor/compositor.cc (right):

https://codereview.chromium.org/427103002/diff/60001/ui/compositor/compositor...
ui/compositor/compositor.cc:302: CHECK(rafs_.count(task));
nit: DCHECK

https://codereview.chromium.org/427103002/diff/60001/ui/compositor/compositor...
ui/compositor/compositor.cc:307: }
maybe it's naive, but why wouldn't the standard Observer pattern work here?
Replace RequestAnimationFrameTask and RemoveAnimationFrameTask by an
Add/RemoveAnimationObserver (which would add to an ObserverList, and possibly
call SetNeedsAnimate), do a FOR_ALL_OBSERVERS here, and SetNeedsAnimate iff
there is any observer left.

[sadrul, 2014-07-31 23:48:13]

I have changed to using an observer-list instead.

I feel like it's easy to forget to remove an observer, which is often the source
of difficult-to-trace bugs. But we use observers in a lot of places; I suppose
this won't be any worse. PTAL.

[piman, 2014-08-01 01:34:46]

On 2014/07/31 23:48:13, sadrul wrote:
> I have changed to using an observer-list instead.
> 
> I feel like it's easy to forget to remove an observer, which is often the
source
> of difficult-to-trace bugs. But we use observers in a lot of places; I suppose
> this won't be any worse. PTAL.

LGTM.
Note, you can still have a Scoped thing that adds/remove observers, it's just
easier to use if it doesn't leak into the API.

[sadrul, 2014-08-01 16:04:44]

Committed patchset #5 manually as 287011 (presubmit successful).