Maintain global lists of animation controllers

Maintain global lists of animation controllers

This cl is an analog to https://codereview.chromium.org/11348256/, but rather
than storing the collections of animation controllers on the layer tree hosts,
we instead use global collections.

BUG=162111

[jamesr, 2012-12-05 02:25:49]

https://codereview.chromium.org/11443004/diff/1015/cc/animation_registrar.cc
File cc/animation_registrar.cc (right):

https://codereview.chromium.org/11443004/diff/1015/cc/animation_registrar.cc#...
cc/animation_registrar.cc:29: static AnimationRegistrarMap* s_registrar_map = 0;
in some situations we'll need to have multiple compositor instances driven from
different threads in the same process.  This map by itself is definitely not
threadsafe.

[nduca, 2012-12-05 02:45:02]

Seems like the other approach is better then. I was mainly just saying this:
have lth own a list of all animation controllers, even ones that aren't active.
have it also have a list of active animation controllers. we'd not do much with
the all-animation-controller list but it'd be damn useful during debugging, and
it also makes it a lot cleaner to grok whats going on in animation controller.
to a person quickly reading the code, its SUPER EASY to read
"registerController/unregisterController" calls during construction/movement
between trees, and super easy to read m_lth->didBecomeActive/didBecomeInactive
calls--- its obvious whats happening. You could even make
registerController/unregisterController be nops in release builds. The point is,
a simpel obvious API can be written but then optimized behind the covers to
encode the implicit "wel all you really care about is which ones are active or
maybe active" stuff. And, the simple and obvious api is easier to read. Thats a
really good thing imo.

</soapbox>

[Ian Vollick, 2012-12-05 15:08:22]

On 2012/12/05 02:25:49, jamesr wrote:
> https://codereview.chromium.org/11443004/diff/1015/cc/animation_registrar.cc
> File cc/animation_registrar.cc (right):
> 
>
https://codereview.chromium.org/11443004/diff/1015/cc/animation_registrar.cc#...
> cc/animation_registrar.cc:29: static AnimationRegistrarMap* s_registrar_map =
0;
> in some situations we'll need to have multiple compositor instances driven
from
> different threads in the same process.  This map by itself is definitely not
> threadsafe.

Yikes, it totally isn't! I don't know what I was thinking there. To give this
approach a fair shake, I think I've fixed the thread safety issue. Currently
I've got one list per process, but we could have a map from compositor id to
list to support multiple compositors per process.

[Ian Vollick, 2012-12-05 15:09:38]

On 2012/12/05 02:45:02, nduca wrote:
> Seems like the other approach is better then. I was mainly just saying this:
> have lth own a list of all animation controllers, even ones that aren't
active.
> have it also have a list of active animation controllers. we'd not do much
with
> the all-animation-controller list but it'd be damn useful during debugging,
and
> it also makes it a lot cleaner to grok whats going on in animation controller.
> to a person quickly reading the code, its SUPER EASY to read
> "registerController/unregisterController" calls during construction/movement
> between trees, and super easy to read m_lth->didBecomeActive/didBecomeInactive
> calls--- its obvious whats happening. You could even make
> registerController/unregisterController be nops in release builds. The point
is,
> a simpel obvious API can be written but then optimized behind the covers to
> encode the implicit "wel all you really care about is which ones are active or
> maybe active" stuff. And, the simple and obvious api is easier to read. Thats
a
> really good thing imo.
> 
> </soapbox>

Kk. Totally agree about the usefulness during debugging. The current patch has
both registration and activation handled by the AnimationRegistrar. wdyt?

[jamesr, 2012-12-05 19:19:29]

This isn't any more threadsafe than the last patch when there are multiple users
of the compositor from different 'main' threads, unless I'm missing something. 
Do you really need statics?

https://codereview.chromium.org/11443004/diff/6002/cc/animation_registrar.h
File cc/animation_registrar.h (right):

https://codereview.chromium.org/11443004/diff/6002/cc/animation_registrar.h#n...
cc/animation_registrar.h:12: class LayerAnimationController;
no indent here