RFR: 8276056: Control.skin.setSkin(Skin) fails to call dispose() on discarded Skin

[Michael Strauß]

On Wed, 29 Jun 2022 13:35:15 GMT, Marius Hanl <mhanl at openjdk.org> wrote:

> For some reason the `skinProperty` did not allow to set a new skin which is the same class as the previous one.
> This leads to multiple issues:
> 1. When creating a new skin (same class as previous), the skin will likely install some children and listener but is then rejected when setting it due to the `skinProperty` class constraint
> -> Control is in a weird state as the current skin was not disposed since it is still set, although we already and 'set' a new skin
> 2. A skin of the same class can behave differently, so it is not really valid to reject a skin just because it is the same class as the previous
> -> Just imagine we have the following skin class
> 
> class MyCustomButtonSkin extends ButtonSkin {
>     public MyCustomButtonSkin(Button button, boolean someFlag) { super(button); ... }
> }
> 
> Now if we would change the skin of the Button two times like this:
> 
> button.setSkin(new MyCustomButtonSkin(button, true));
> button.setSkin(new MyCustomButtonSkin(button, false));
> 
> The second time the skin will be rejected as it is the same class, although I may changed the skin behaviour, in this case demonstrated by a boolean flag for showing purposes.

modules/javafx.controls/src/main/java/javafx/scene/control/Control.java line 241:

> 239: 
> 240:         @Override
> 241:         //This code is basically a kind of optimization that prevents a Skin that is equal but not instance equal.

Why was this code here in the first place, considering that switching skins is surely not something that would logically be done repeatedly? Are there performance implications with this change?

-------------

PR: https://git.openjdk.org/jfx/pull/806