changeset 11301:ab32008d11a2

8209938: Default and Cancel button cause memory leak Reviewed-by: kcr, jvos, nlisker
author arapte
date Tue, 16 Jul 2019 01:20:09 +0530
parents 4c6d39fc63ef
children b6c8cd273e58 c91a28ed4845
files modules/javafx.controls/src/main/java/javafx/scene/control/skin/ButtonSkin.java modules/javafx.controls/src/test/java/test/javafx/scene/control/ButtonTest.java
diffstat 2 files changed, 72 insertions(+), 8 deletions(-) [+]
line wrap: on
line diff
--- a/modules/javafx.controls/src/main/java/javafx/scene/control/skin/ButtonSkin.java	Thu Jul 11 15:37:25 2019 +0530
+++ b/modules/javafx.controls/src/main/java/javafx/scene/control/skin/ButtonSkin.java	Tue Jul 16 01:20:09 2019 +0530
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2010, 2016, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2010, 2019, Oracle and/or its affiliates. All rights reserved.
  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
  *
  * This code is free software; you can redistribute it and/or modify it
@@ -124,6 +124,24 @@
                 }
             }
         });
+        control.sceneProperty().addListener((ov, oldScene, newScene) -> {
+            if (oldScene != null) {
+                if (getSkinnable().isDefaultButton()) {
+                    setDefaultButton(oldScene, false);
+                }
+                if (getSkinnable().isCancelButton()) {
+                    setCancelButton(oldScene, false);
+                }
+            }
+            if (newScene != null) {
+                if (getSkinnable().isDefaultButton()) {
+                    setDefaultButton(newScene, true);
+                }
+                if (getSkinnable().isCancelButton()) {
+                    setCancelButton(newScene, true);
+                }
+            }
+        });
 
         // set visuals
         if (getSkinnable().isDefaultButton()) {
@@ -168,14 +186,17 @@
      *                                                                         *
      **************************************************************************/
 
-    private void setDefaultButton(boolean value) {
-        Scene scene = getSkinnable().getScene();
+    private void setDefaultButton(boolean isDefault) {
+        setDefaultButton(getSkinnable().getScene(), isDefault);
+    }
+
+    private void setDefaultButton(Scene scene, boolean isDefault) {
         if (scene != null) {
             KeyCode acceleratorCode = KeyCode.ENTER;
             defaultAcceleratorKeyCodeCombination = new KeyCodeCombination(acceleratorCode);
 
             Runnable oldDefault = scene.getAccelerators().get(defaultAcceleratorKeyCodeCombination);
-            if (!value) {
+            if (!isDefault) {
                 /**
                  * first check of there's a default button already
                  */
@@ -195,14 +216,17 @@
         }
     }
 
-    private void setCancelButton(boolean value) {
-        Scene scene = getSkinnable().getScene();
+    private void setCancelButton(boolean isCancel) {
+        setCancelButton(getSkinnable().getScene(), isCancel);
+    }
+
+    private void setCancelButton(Scene scene, boolean isCancel) {
         if (scene != null) {
             KeyCode acceleratorCode = KeyCode.ESCAPE;
             cancelAcceleratorKeyCodeCombination = new KeyCodeCombination(acceleratorCode);
 
             Runnable oldCancel = scene.getAccelerators().get(cancelAcceleratorKeyCodeCombination);
-            if (!value) {
+            if (!isCancel) {
                 /**
                  * first check of there's a cancel button already
                  */
--- a/modules/javafx.controls/src/test/java/test/javafx/scene/control/ButtonTest.java	Thu Jul 11 15:37:25 2019 +0530
+++ b/modules/javafx.controls/src/test/java/test/javafx/scene/control/ButtonTest.java	Tue Jul 16 01:20:09 2019 +0530
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2010, 2015, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2010, 2019, Oracle and/or its affiliates. All rights reserved.
  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
  *
  * This code is free software; you can redistribute it and/or modify it
@@ -251,6 +251,26 @@
         tk.firePulse();
     }
 
+    // Test for JDK-8209938
+    @Test public void defaultButtonSceneAccelerators() {
+        assertEquals("Scene.getAccelerators() should contain no accelerators.",
+            0, scene.getAccelerators().size());
+
+        HBox btnParent = new HBox();
+        btnParent.getChildren().add(btn);
+        root.getChildren().add(btnParent);
+        btn.setDefaultButton(true);
+        show();
+        assertEquals("Scene.getAccelerators() should contain one accelerator" +
+            " for Default button.", 1, scene.getAccelerators().size());
+
+        root.getChildren().remove(btnParent);
+        assertEquals("Default button accelerator should be removed from" +
+            " Scene.getAccelerators().", 0, scene.getAccelerators().size());
+
+        tk.firePulse();
+    }
+
     /*********************************************************************
      * Tests for the cancelButton state                                 *
      ********************************************************************/
@@ -306,6 +326,26 @@
         assertPseudoClassDoesNotExist(btn, "cancel");
     }
 
+    // Test for JDK-8209938
+    @Test public void cancelButtonSceneAccelerators() {
+        assertEquals("Scene.getAccelerators() should contain no accelerators.",
+            0, scene.getAccelerators().size());
+
+        HBox btnParent = new HBox();
+        btnParent.getChildren().add(btn);
+        root.getChildren().add(btnParent);
+        btn.setCancelButton(true);
+        show();
+        assertEquals("Scene.getAccelerators() should contain one accelerator" +
+            " for Cancel button.", 1, scene.getAccelerators().size());
+
+        root.getChildren().remove(btnParent);
+        assertEquals("Cancel button accelerator should be removed from" +
+            " Scene.getAccelerators().", 0, scene.getAccelerators().size());
+
+        tk.firePulse();
+    }
+
     @Ignore("impl_cssSet API removed")
     @Test public void cannotSpecifyCancelButtonViaCSS() {
 //        btn.impl_cssSet("-fx-cancel-button", true);