New API: Animation/Timeline improvement

[John Hendrikx]

Hi list,

I've noticed that Animations and Timelines are often a source of leaks, 
and their clean-up is often either non-existent or incorrect.  The 
reason they cause leaks easily is because a running animation or 
timeline is globally referred from a singleton PrimaryTimer.  The 
animation or timeline then refers to properties or event handlers which 
refer to controls (which refer to parents and the entire scene).

For example:

- ScrollBarBehavior uses a Timeline, but neglects to clean it up.  If it 
was running at the time a Scene is detached from a Window, and that 
Scene is left to go out of scope, it won't because Timeline refers it; 
this can happen if the behavior never receives a key released event.

- ScrollBarBehavior has no dispose method overridden, so swapping Skins 
while the animation is running will leave a Timeline active (it uses 
Animation.INDEFINITE)

- SpinnerBehavior has flawed clean up; it attaches a Scene listener and 
disables its timeline when the scene changed, but the scene doesn't have 
to change for it to go out of scope as a whole... Result is that if you 
have a spinner timeline running, and you close the entire window (no 
Scene change happens), the entire Scene will still be referred.  It also 
uses an indefinite cycle count.  It also lacks a dispose method, so 
swapping Skins at a bad moment can also leave a reference.

I think these mistakes are common, and far too easy to make.  The most 
common use cases for animations revolve around modifying properties on 
visible controls, and although animations can be used for purposes other 
than animating UI controls, this is extremely rare.  So it is safe to 
say that in 99% of cases you want the animation to stop once a some Node 
is no longer showing. For both the mentioned buggy behaviors above, this 
would be perfect.  A spinner stops spinning when no longer showing, and 
a scroll bar stops scrolling when no longer showing.  It is also likely 
to apply for many other uses of timelines and animations.

I therefore want to propose a new API, either on Node or Animation (or 
both):

     /**
      * Creates a new timeline which is stopped automatically when this Node
      * is no longer showing. Stopping timelines is essential as they 
may refer
      * nodes even after they are no longer used anywhere, preventing 
them from
      * being garbage collected.
      */
     Node.createTimeline();  // and variants with the various Timeline 
constructors

And/or:

     /**
      * Links this Animation to the given Node, and stops the animation
      * automatically when the Node is no longer showing. Stopping 
animations
      * is essential as they may refer nodes even after they are no 
longer used
      * anywhere, preventing them from being garbage collected.
      */
     void stopWhenHidden(Node node);

The above API for Animation could also be provided through another 
constructor, which takes a Node which will it be linked to.

Alternatives:

- Be a lot more diligent about cleaning up animations and timelines 
(essentially maintain the status quo which has led to above bugs)
- Use this lengthy code fragment below:

     Timeline timeline = new Timeline();

     someNode.sceneProperty()
         .when(timeline.statusProperty().map(status -> status != 
Status.STOPPED))
         .flatMap(Scene::windowProperty)
         .flatMap(Window::showingProperty)
         .orElse(false)
         .subscribe(showing -> {
             if (!showing) timeline.stop();
         });

The `when` line ensures that the opposite problem (Nodes forever 
referencing Timelines) doesn't occur if you are creating a new Timeline 
for each use (not recommended, but nonetheless a common occurrence).

--John