On Wed, 8 Feb 2023 19:07:51 GMT, Andy Goryachev <ango...@openjdk.org> wrote:
> two questions:
>
> 1. should the timeline object be set to null here? the reference will
> get overwritten in seriesRemoved():414, there is probably no need to keep
> this object in memory
>
Yes timeline can be made null here. Updated the code.
> 2. is it possible that `seriesBeingRemovedIsAdded()` will be invoked
> twice with a different `series` argument? In other words, are we going to
> face a situation when a `timeline` from unrelated series will not get
> `stop`'ped?
>
>
> (these question apply to other charts as well)
Since `seriesBeingRemovedIsAdded()` is called from the event handler which gets
invoked when the series changes and further there is a check on `setToRemove`
boolean, I believe the method will not be called with different `series` and
cause unwanted `series` timeline to be stopped.
> tests/system/src/test/java/test/javafx/scene/control/XYChartExceptionOnAddingRemovedSeriesTest.java
> line 315:
>
>> 313: stage.show();
>> 314:
>> 315: Thread.currentThread().setUncaughtExceptionHandler((t2, e)
>> -> {
>
> I think it's better to set this handler in a pre-test method annotated with
> @Before, and clean up in @After - see, for instance, how it's done in
> BehaviorCleanupTest.
Updated code to set the handler in function annotated with @BeforeClass and
cleaned up in @AfterClass.
-------------
PR: https://git.openjdk.org/jfx/pull/1015
