RFR: 4938801: The popup does not go when the component is removed [v6]

[Jeremy Wood]

On Wed, 30 Jul 2025 03:44:37 GMT, Prasanta Sadhukhan <psadhukhan at openjdk.org> wrote:

>> src/java.desktop/share/classes/javax/swing/JPopupMenu.java line 1427:
>> 
>>> 1425:             indexCounter++;
>>> 1426:         }
>>> 1427:     }
>> 
>> This may be a naive question: is the lambda we use for `propListener` Serializable?
>> 
>> (If so: great. If not: won't we get inconsistent results if we're working with a deserialized copy of a JPopupMenu? The original will have a non-null `propListener` field, and the copy will have a null `propListener`? Or am I missing something?)
>
> It is transient so  its value should not be included in the default serialization process in my opinion

I'm fine with `propListener` being transient ... as long as when we call readObject() we still make a point to initialize `propListener`.

Here's a unit test that demonstrates what I'm worried about:

import javax.swing.*;
import java.beans.PropertyChangeListener;
import java.io.*;

public class JPopupMenuSerializationTest {
    static class JPopupMenuSubclass extends JPopupMenu implements Serializable {
        private transient PropertyChangeListener propListener =
                (e) -> {
                    if (e.getOldValue() != null
                            && e.getNewValue() == null
                            && isVisible()) {
                        setVisible(false);
                    }
                };

        @Serial
        private void writeObject(ObjectOutputStream s) throws IOException {
            Object serializable = propListener instanceof Serializable ?
                    (Serializable) propListener : null;
            System.out.println("Serialized: " + serializable);
            s.writeObject(serializable);
        }

        @Serial
        private void readObject(ObjectInputStream s)
                throws IOException, ClassNotFoundException {
            PropertyChangeListener l = (PropertyChangeListener) s.readObject();
            System.out.println("Deserialized: " + l);
            if (l != null) {
                propListener = l;
            }
            System.out.println("propListener: " + propListener);
        }
    }

    public static void main(String[] args) throws Exception {
        JPopupMenuSerializationTest test = new JPopupMenuSerializationTest();
        test.testSerialization();
    }

    public void testSerialization() throws Exception {
        JPopupMenuSubclass popupMenu = new JPopupMenuSubclass();
        byte[] data = serialize(popupMenu);
        JPopupMenuSubclass copy = deserialize(data);

        assertTrue("original.propListener should exist", popupMenu.propListener != null);

        // currently this fails, and IMO it should pass:
        assertTrue("copy.propListener should exist",  copy.propListener != null);
    }

    private static void assertTrue(String errorMsg, boolean b) {
        if (!b)
            throw new Error(errorMsg);
    }

    private static byte[] serialize(JPopupMenuSubclass popupMenu) throws IOException {
        try (ByteArrayOutputStream byteOut = new ByteArrayOutputStream()) {
            try (ObjectOutputStream objOut = new ObjectOutputStream(byteOut)) {
                objOut.writeObject(popupMenu);
            }
            return byteOut.toByteArray();
        }
    }

    private static JPopupMenuSubclass deserialize(byte[] data) throws IOException, ClassNotFoundException {
        try (ByteArrayInputStream byteIn = new ByteArrayInputStream(data)) {
            try (ObjectInputStream objIn = new ObjectInputStream(byteIn)) {
                return (JPopupMenuSubclass) objIn.readObject();
            }
        }
    }
}


Currently on my machine this outputs:

Serialized: null
Deserialized: null
propListener: null
Exception in thread "main" java.lang.Error: copy.propListener should exist
	at JPopupMenuSerializationTest.assertTrue(JPopupMenuSerializationTest.java:54)
	at JPopupMenuSerializationTest.testSerialization(JPopupMenuSerializationTest.java:49)
	at JPopupMenuSerializationTest.main(JPopupMenuSerializationTest.java:38)


This means (I think?) a deserialized copy is going to act different than the original, right? (I'm not an expert on serializing UI components, so maybe I'm missing something...?)

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

PR Review Comment: https://git.openjdk.org/jdk/pull/26407#discussion_r2241513779