8220722: ProgressBarSkin: adds strong listener to control's width pro…

…perty Reviewed-by: aghaisas, arapte
openjdk · Dec 9, 2019 · 71ca899 · 71ca899
1 parent 07af89a
commit 71ca899
Show file tree

Hide file tree

Showing 2 changed files with 93 additions and 5 deletions.
diff --git a/modules/javafx.controls/src/main/java/javafx/scene/control/skin/ProgressBarSkin.java b/modules/javafx.controls/src/main/java/javafx/scene/control/skin/ProgressBarSkin.java
@@ -98,7 +98,7 @@ public ProgressBarSkin(ProgressBar control) {
 
         barWidth = ((int) (control.getWidth() - snappedLeftInset() - snappedRightInset()) * 2 * Math.min(1, Math.max(0, control.getProgress()))) / 2.0F;
 
-        control.widthProperty().addListener(observable -> updateProgress());
+        registerChangeListener(control.widthProperty(), o -> updateProgress());
 
         initialize();
         getSkinnable().requestLayout();

diff --git a/...les/javafx.controls/src/test/java/test/javafx/scene/control/skin/ProgressBarSkinTest.java b/...les/javafx.controls/src/test/java/test/javafx/scene/control/skin/ProgressBarSkinTest.java
@@ -25,27 +25,115 @@
 
 package test.javafx.scene.control.skin;
 
-import static org.junit.Assert.assertEquals;
+import java.lang.ref.WeakReference;
+
+import org.junit.After;
+import org.junit.Before;
+import org.junit.Test;
+
+import com.sun.javafx.tk.Toolkit;
+
+import static org.junit.Assert.*;
 
 import javafx.beans.value.ObservableValue;
+import javafx.scene.Scene;
 import javafx.scene.control.ProgressBar;
+import javafx.scene.control.Skin;
 import javafx.scene.control.skin.ProgressBarSkin;
-
-import org.junit.Before;
-import org.junit.Test;
+import javafx.scene.layout.Region;
+import javafx.scene.layout.StackPane;
+import javafx.stage.Stage;
+import test.com.sun.javafx.pgstub.StubToolkit;
 
 /**
  */
 public class ProgressBarSkinTest {
     private ProgressBar progressbar;
     private ProgressBarSkinMock skin;
+    private Scene scene;
+    private Stage stage;
+    private StackPane root;
 
     @Before public void setup() {
         progressbar = new ProgressBar();
         skin = new ProgressBarSkinMock(progressbar);
         progressbar.setSkin(skin);
     }
 
+    /**
+     * Helper method to init the stage only if really needed.
+     */
+    private void initStage() {
+        //This step is not needed (Just to make sure StubToolkit is loaded into VM)
+        Toolkit tk = (StubToolkit)Toolkit.getToolkit();
+        root = new StackPane();
+        scene = new Scene(root);
+        stage = new Stage();
+        stage.setScene(scene);
+    }
+
+    @After
+    public void cleanup() {
+        if (stage != null) {
+            stage.hide();
+        }
+    }
+
+    /**
+     * Test that inner bar width is in sync with its progressbar's width.
+     */
+    @Test
+    public void testWidthListener() {
+        initStage();
+        // set determinate
+        double progress = .5;
+        progressbar.setProgress(progress);
+        // make it resizable
+        progressbar.setMaxWidth(2000);
+        root.getChildren().setAll(progressbar);
+        double stageSize = 300;
+        stage.setWidth(stageSize);
+        stage.setHeight(stageSize);
+        stage.show();
+        // fire to force layout
+        Toolkit.getToolkit().firePulse();
+
+        assertEquals("progressbar fills root", root.getWidth(),
+                progressbar.getWidth(), 0.5);
+        Region innerBar = (Region) progressbar.lookup(".bar");
+        assertEquals("inner bar width updated",
+                progressbar.getWidth() * progress, innerBar.getWidth(), 0.5);
+    }
+
+    WeakReference<Skin<?>> weakSkinRef;
+
+    @Test
+    public void testWidthListenerGC() {
+        ProgressBar progressbar = new ProgressBar();
+        progressbar.setSkin(new ProgressBarSkin(progressbar));
+        weakSkinRef = new WeakReference<>(progressbar.getSkin());
+        progressbar.setSkin(null);
+        attemptGC(10);
+        assertNull("skin must be gc'ed", weakSkinRef.get());
+    }
+
+    private void attemptGC(int n) {
+        // Attempt gc n times
+        for (int i = 0; i < n; i++) {
+            System.gc();
+            System.runFinalization();
+
+            if (weakSkinRef.get() == null) {
+                break;
+            }
+            try {
+                Thread.sleep(500);
+            } catch (InterruptedException e) {
+               System.err.println("InterruptedException occurred during Thread.sleep()");
+            }
+        }
+    }
+
     @Test public void maxWidthTracksPreferred() {
         progressbar.setPrefWidth(500);
         assertEquals(500, progressbar.maxWidth(-1), 0);