8264677: MemoryLeak: Progressindicator leaks, when treeShowing is false

Reviewed-by: kcr, arapte

modules/javafx.controls/src/main/java/javafx/scene/control/skin/ProgressIndicatorSkin.java

@@ -742,7 +742,12 @@ private void rebuildTimeline() {
                 }
 
                 ((Timeline)indeterminateTransition).getKeyFrames().setAll(keyFrames);
-                indeterminateTransition.playFromStart();
+
+                if (NodeHelper.isTreeShowing(control)) {
+                    indeterminateTransition.playFromStart();
+                } else {
+                    indeterminateTransition.jumpTo(Duration.ZERO);
+                }
             } else {
                 if (indeterminateTransition != null) {
                     indeterminateTransition.stop();

tests/system/src/test/java/test/javafx/scene/control/ProgressIndicatorLeakTest.java

@@ -28,6 +28,7 @@
 import javafx.application.Application;
 import javafx.application.Platform;
 import javafx.scene.Node;
+import javafx.scene.Group;
 import javafx.scene.Scene;
 import javafx.scene.control.ProgressIndicator;
 import javafx.scene.control.skin.ProgressIndicatorSkin;
@@ -38,57 +39,92 @@
 import java.util.LinkedList;
 import java.util.concurrent.CountDownLatch;
 import java.util.concurrent.TimeUnit;
+import java.util.concurrent.atomic.AtomicReference;
 
 import junit.framework.Assert;
 import org.junit.AfterClass;
 import org.junit.BeforeClass;
 import org.junit.Test;
+import static org.junit.Assert.assertTrue;
+import static test.util.Util.TIMEOUT;
+import test.util.Util;
 import test.util.memory.JMemoryBuddy;
 
 public class ProgressIndicatorLeakTest {
 
-    static CountDownLatch startupLatch;
-    static WeakReference<Node> detIndicator = null;
-    static Stage stage = null;
+    @BeforeClass
+    public static void initFX() throws Exception {
+        CountDownLatch startupLatch = new CountDownLatch(1);
+        Platform.setImplicitExit(false);
+        Platform.startup(startupLatch::countDown);
+        Assert.assertTrue("Timeout waiting for FX runtime to start",
+                startupLatch.await(TIMEOUT, TimeUnit.MILLISECONDS));
+    }
 
-    public static class TestApp extends Application {
+    @Test
+    public void leakDeterminationIndicator() throws Exception {
+        JMemoryBuddy.memoryTest((checker) -> {
+            CountDownLatch showingLatch = new CountDownLatch(1);
+            Util.runAndWait(() -> {
+                Stage stage = new Stage();
+                ProgressIndicator indicator = new ProgressIndicator(-1);
+                indicator.setSkin(new ProgressIndicatorSkin(indicator));
+                Scene scene = new Scene(indicator);
+                stage.setScene(scene);
+                indicator.setProgress(1.0);
+                Assert.assertEquals("size is wrong", 1, indicator.getChildrenUnmodifiable().size());
+                Node detIndicator = indicator.getChildrenUnmodifiable().get(0);
+                indicator.setProgress(-1.0);
+                indicator.setProgress(1.0);
+                checker.assertCollectable(detIndicator);
+                stage.setOnShown(l -> {
+                    Platform.runLater(() -> showingLatch.countDown());
+                });
+                stage.show();
+            });
+            try {
+                Assert.assertTrue("Timeout waiting for setOnShown", showingLatch.await(15, TimeUnit.SECONDS));
+            } catch (InterruptedException e) {
+                throw new RuntimeException(e);
+            }
+        });
+    }
 
-        @Override
-        public void start(Stage primaryStage) throws Exception {
-            ProgressIndicator indicator = new ProgressIndicator(-1);
-            indicator.setSkin(new ProgressIndicatorSkin(indicator));
-            Scene scene = new Scene(indicator);
-            primaryStage.setScene(scene);
-            stage = primaryStage;
-            indicator.setProgress(1.0);
-            Assert.assertEquals("size is wrong", 1, indicator.getChildrenUnmodifiable().size());
-            detIndicator = new WeakReference<Node>(indicator.getChildrenUnmodifiable().get(0));
-            indicator.setProgress(-1.0);
-            indicator.setProgress(1.0);
+    @Test
+    public void stageLeakWhenTreeNotShowing() throws Exception {
+        JMemoryBuddy.memoryTest((checker) -> {
+            CountDownLatch showingLatch = new CountDownLatch(1);
+            AtomicReference<Stage> stage = new AtomicReference<>();
 
-            primaryStage.setOnShown(l -> {
-                Platform.runLater(() -> startupLatch.countDown());
+            Util.runAndWait(() -> {
+                stage.set(new Stage());
+                Group root = new Group();
+                root.setVisible(false);
+                root.getChildren().add(new ProgressIndicator());
+                stage.get().setScene(new Scene(root));
+                stage.get().setOnShown(l -> {
+                    Platform.runLater(() -> showingLatch.countDown());
+                });
+                stage.get().show();
             });
-            primaryStage.show();
-        }
-    }
 
-    @BeforeClass
-    public static void initFX() throws Exception {
-        startupLatch = new CountDownLatch(1);
-        new Thread(() -> Application.launch(TestApp.class, (String[]) null)).start();
-        Assert.assertTrue("Timeout waiting for FX runtime to start", startupLatch.await(15, TimeUnit.SECONDS));
-    }
+            try {
+                assertTrue("Timeout waiting test stage", showingLatch.await(15, TimeUnit.SECONDS));
+            } catch (InterruptedException e) {
+                throw new RuntimeException(e);
+            }
 
-    @Test
-    public void memoryTest() throws Exception {
-        JMemoryBuddy.assertCollectable(detIndicator);
+            Util.runAndWait(() -> {
+                stage.get().close();
+            });
+
+            checker.assertCollectable(stage.get());
+        });
     }
 
     @AfterClass
     public static void teardownOnce() {
         Platform.runLater(() -> {
-            stage.hide();
             Platform.exit();
         });
     }