single quotes

Is there some reason the BuildContext can't serve as the ticker provider?

To make that work (without requiring every widget to also have a custom element) we'd have to bake knowledge about Tickers into framework.dart, which I would be very reluctant to do.

Maybe we should get this from the tester? That will make it less likely for people to copy/pasta this into production code.

This was an excellent idea. Done.

As a side-effect it also allows the tester to verify that all these AnimationControllers are disposed as well.

Should enabled by @required ?

sure, why not. done.

You're inconsistent about whether you're using early return in these two error handlers.

This seems like a very dangerous default.

Yeah. Removed. I've actually removed the schedulerTickerProvider entirely.

This field seems like a bug factory. It's easy to use and your code is only wrong in that it burns battery. Maybe move it to flutter_test, which would service almost all of the clients.

agreed. removed

Won't this drop a frame? The oldController.value is the value we had at the last tick but calling forward will cause the first frame of the new control (i.e., next frame) to have that value as well.

Instead, I think we need some way of changing the ticker provider for a running animation, presumably by handing the timebase off between the tickers so they don't repeat a frame.

I'm pretty sure that forward() will cause this frame to have the value of the from, and the next frame will have advanced.

But I agree that it's cleaner to build the animation controller from the old one, so let me explore that.

I think you forgot to actually store the new value in _tickerProvider.

oops fixed

The problem with this approach is that these animations are exposed publicly by this class. Clients won't expect them to change. Specifically, how will they know to move their child animations onto the new instances?

We could solve that by introducing proxy animations to hide the identity change, but I think it would be better to teach AnimationController how to switch ticker providers, especially because of the frame-repeating problem mentioned in another comment.

done as suggested

Maybe assert that?

Right now there's no way to tell -- the ticker provider isn't kept around to compare against.

All the parameters could be enclosed with [].

added hyperlinks where appropriate

Same here.

flutter before meta?

fixed

Would 'supply' be a better fit than 'vend'? (I may be missing the nuance here, though.)

I think both are fine.

Maybe The type of the provided ticker depends on the implementing class.?

I think the text I have now is more grammatically correct. "implementing class" is a bit of a weird construction.

But more to the point, the text here isn't talking about the object type per se, but about the kind of behavior it may have. So for example, you could have a ticker provider return the same type as another, but with different behaviour. Case in point, most of the ticker providers essentially provide Ticker objects, but they don't all act the same.

Maybe it would be useful to add [some of] this explanation to the docs?

Also, enabled should probably not be null.

asserted

'should' is really not explicit enough here.

what should it say instead?

What I was trying to say is that it does not explain why they should or should not be enabled.

'used ... to enable and disable' is a bit confusing since this method only returns a bool. Maybe 'use ... when enabling or disabling'?

tweaked

'vending' vs 'supplying'?

they are both fine. "vending" implies more of a transactional operation which is why i used that.

'This' -> 'That'

why that?

I guess 'That' in this particular case underlines the separation of the two ideas. To me, 'This' brings the two closer than they naturally are, but this may be personal bias.

Does the base class not have @mustCallSuper? If not, should we introduce it here?

It's complicated. The base class is actually a mixin. I tried adding @mustCallSuper the other day but it ends up being a bit of a mess, iirc.

Oh, looks like I cleaned the mess up, too, and we do have it now. La la la.

Yeah, clearly we need a @mustCallSuper somewhere.

Always one step ahead, I see.

Bad indent.

@protected if this is for subclasses.

Why didn't we use resync here?

oops, I forgot this one.

fixed

Should we null out _startTime as well?

I intentionally didn't so that the class would end up in a bogus state after dispose(). I can add a comment to that effect.

added:

    // We intentionally don't null out _startTime. This means that if start()                                                                                                                                                                             
    // was ever called, the object is now in a bogus state. This weakly helps                                                                                                                                                                             
    // catch cases of use-after-dispose.                                                                                                                                                                                                                  

Should this be called muted to match the property on Ticker?

I don't have a strong opinion here.

I guess I'll change it to muted for consistency.

Actually calling this "muted" here leads to a weird case where you set things to false if they're on, which is weird. I'm not sure what to do.

What's the point of the temporary variable?

removed

I'd use a temporary to avoid making this of query so many times.

moved it from the other dependenciesChanged. :-)

Nit: ContinuousVSync?

I was going to make this change but actually that name makes it sound like it fires continually, which is a bit weird...