RFR: 8242621: TabPane: Memory leak when switching skin [v3]
Ambarish Rapte
arapte at openjdk.java.net
Wed Jan 6 12:16:01 UTC 2021
On Tue, 5 Jan 2021 16:03:42 GMT, Jeanette Winzenburg <fastegal at openjdk.org> wrote:
>> Ambarish Rapte has updated the pull request incrementally with one additional commit since the last revision:
>>
>> Review update
>
> modules/javafx.controls/src/test/java/test/javafx/scene/control/skin/SkinCleanupTest.java line 275:
>
>> 273: assertEquals(3, tabPane.getChildrenUnmodifiable().size());
>> 274: }
>> 275:
>
> hmm .. looks a bit unusual (could be me, though :)
>
> - why showControl? just install/replace the skin should be enough?
> - the outcome of testing an illegal state (after instantiating the skin and before really setting it as the control's skin at which time we have a skin : control relation of 2:1) is unspecified. F.i. there are skins that replace all children in the constructor (which may or may not be a good idea) that would fail a test similar to this.
>
> How about
>
> TabPane tabPane ..
> installDefaultSkin(tabPane);
> int children = tabPane.getxxChildren().size();
> replaceSkin(tabPane);
> assertEquals(children, tabPane.getxxChildren().size());
>
> If we decide to go without showing, the other tests should be changed as well :)
1. `showControl` is not required. It is now changed to `installDefaultSkin` in all tests.
2. Test did verify an illegal state, Intention was to keep a note of that illegal state. I have changed it as you suggested.
-------------
PR: https://git.openjdk.java.net/jfx/pull/318
More information about the openjfx-dev
mailing list