[Rev 01] RFR: 8236840: Memory leak when switching ButtonSkin

[Jeanette Winzenburg]

On Sat, 28 Mar 2020 06:42:06 GMT, Ambarish Rapte <arapte at openjdk.org> wrote:

>> ButtonSkin adds a `ChangeListener` to `Control.sceneProperty()` which results in leaking the `ButtonSkin` itself when
>> the `Button`'s skin is changed to a new `ButtonSkin`.  Using a `WeakChangeListener` instead of `ChangeListener` solves
>> the issue.
>> Please take a look.
>
> Ambarish Rapte has updated the pull request incrementally with one additional commit since the last revision:
> 
>   Changes for Kevin's review comment

Changes requested by fastegal (Author).

modules/javafx.controls/src/main/java/javafx/scene/control/skin/ButtonSkin.java line 101:

> 100:     };
> 101:     WeakChangeListener<Scene> weakChangeListener = new WeakChangeListener<>(sceneChangeListener);
> 102:

a loose naming convention for weak listeners seems to be to prepend _weak_ to the name of the strong listener, which
would be
    
    WeakChangeListener<Scene> weakSceneChangeListener = new WeakChangeListener<>(sceneChangeListener);

modules/javafx.controls/src/main/java/javafx/scene/control/skin/ButtonSkin.java line 178:

> 177:     @Override public void dispose() {
> 178:         getSkinnable().sceneProperty().removeListener(weakChangeListener);
> 179:         super.dispose();

+1! Actually, looks like that manual remove is really needed - otherwise we get an NPE when removing the default button
from its parent after the skin has been switched:

    @Test
    public void testDefaultButtonSwitchSkinAndRemove() {
        Button button = new Button();
        button.setDefaultButton(true);
        Group root = new Group(button);
        Scene scene = new Scene(root);
        Stage stage = new Stage();
        stage.setScene(scene);
        stage.show();
        
        button.setSkin(new ButtonSkin1(button));
        root.getChildren().remove(button);
    }

Note: to see this NPE as failing test (vs. its printout on sysout), we need to re-wire the uncaughtExceptionHandler,
see ComboBoxTest setup/cleanup for an example.

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

PR: https://git.openjdk.java.net/jfx/pull/147