8323615: PopupControl.skin.setSkin(Skin) fails to call dispose() on d…

…iscarded Skin

Reviewed-by: angorya, mstrauss

modules/javafx.controls/src/main/java/javafx/scene/control/PopupControl.java

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2010, 2022, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2010, 2024, 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
@@ -213,18 +213,6 @@ public PopupControl() {
         // a reference to the old value.
         private Skin<?> oldValue;
 
-        @Override
-        public void set(Skin<?> v) {
-
-            if (v == null
-                    ? oldValue == null
-                    : oldValue != null && v.getClass().equals(oldValue.getClass()))
-                return;
-
-            super.set(v);
-
-        }
-
         @Override protected void invalidated() {
             Skin<?> skin = get();
 

modules/javafx.controls/src/test/java/test/javafx/scene/control/PopupControlTest.java

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2010, 2022, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2010, 2024, 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
@@ -37,7 +37,10 @@
 import org.junit.Before;
 import org.junit.Test;
 
+import java.lang.ref.WeakReference;
+
 import static org.junit.Assert.*;
+import static test.com.sun.javafx.scene.control.infrastructure.ControlSkinFactory.attemptGC;
 
 /**
  *
@@ -650,4 +653,29 @@ private static final class PopupControlSkin<C extends PopupControl> implements S
 
     //TODO: test computePref____ methods
 
+    /**
+     * Set a skin -> set another instance of the same skin (class).
+     */
+    @Test
+    public void testMemoryLeakSameSkinClass() {
+        popup.setSkin(new PopupControlSkin<>());
+        Skin<?> skin = popup.getSkin();
+        popup.setSkin(new PopupControlSkin<>());
+
+        WeakReference<?> weakRef = new WeakReference<>(skin);
+        skin = null;
+        attemptGC(weakRef);
+        assertNull("Unused Skin must be gc'ed", weakRef.get());
+    }
+
+    @Test
+    public void testSetSkinOfSameClass() {
+        popup.setSkin(new PopupControlSkin<>());
+        Skin<?> oldSkin = popup.getSkin();
+        popup.setSkin(new PopupControlSkin<>());
+        Skin<?> newSkin = popup.getSkin();
+
+        assertNotEquals("New skin was not set", oldSkin, newSkin);
+    }
+
 }