Split base::ObserverList functionality to get a light-weight implementation (a SubscriberList) |
||
Issue descriptionIn cc::ElementAnimations we use ObserverList as a basic collection of items which supports subscription/un-subscription during iterations. 1) We don't use NOTIFY_EXISTING_ONLY. 2) We don't need WeakPtr 3) We don't use check_empty. 4) We don't do nested iterations. Having a separate implementation would help to introduce thread-safety checks like this: https://codereview.chromium.org/1602443003/
,
Nov 22 2016
SubscriberList<Foo> won't invalidate iterators if 'add/remove a subscriber' happens during the iteration. Semantically is should be similar to the case where you make a local copy of std::vector to iterate on.
,
Nov 22 2016
Our simple and critical use case: we notify the subscriber about OnAnimationEnd and the subscriber unsubscribes itself immediately.
,
Nov 24 2016
I think ObserverList has too many knobs, but I'm also not too enthusiastic about adding yet another observer list type. =/
,
Nov 24 2016
> SubscriberList<Foo> won't invalidate iterators if 'add/remove a subscriber'
> happens during the iteration. Semantically is should be similar to the case
> where you make a local copy of std::vector to iterate on.
Typically this is done with vector too by doing.
std::vector<Foo*> copy(subscribers_);
for (auto* f : copy) { f->Thing(); }
BeginFrameSource does this type of thing in cc for example.
,
Nov 24 2016
That's an extra heap allocation (and some implications on observer's lifetime)
,
Nov 29 2016
You're right, but is it a problem here? It's usually an O(1) type of thing per frame or whatever.
,
Nov 29 2016
Let's won't fix it. Feel free to reopen later. |
||
►
Sign in to add a comment |
||
Comment 1 by dcheng@chromium.org
, Nov 22 2016