RFR: 8214699: Node.getPseudoClassStates must return the same instance on every call

[Jeanette Winzenburg]

On Fri, 19 Jun 2020 23:49:45 GMT, Kevin Rushforth <kcr at openjdk.org> wrote:

>> Node.getPseudoClassStates() returns a new UnmodifiableObservableSet of PseudoClassState on each call. So in order to
>> listen to any changes in this set, user must call the method Node.getPseudoClassStates() only once and keep a strong
>> reference to the returned UnmodifiableObservableSet.
>>  
>> So the fix is that the method Node.getPseudoClassStates() should return the same UnmodifiableObservableSet on every
>> call. As the returned set is an UnmodifiableObservableSet, it will not have any impact on it's usage.
>>  
>> Added a small unit test. and,
>> Altered(minor) a test which was modified in
>> past(https://github.com/openjdk/jfx/commit/0ac98695a1b11443c342fad4f009d6e03a052522)
>> (https://github.com/openjdk/jfx/commit/62323e0a9c5817b33daa262d6914eba0e8d274ff) along with this method and should be
>> updated in view of this change.
>
> modules/javafx.graphics/src/test/java/test/javafx/scene/NodeTest.java line 168:
> 
>> 167:         assertSame("getPseudoClassStates() should always return the same instance",
>> 168:                 node.getPseudoClassStates(), node.getPseudoClassStates());
>> 169:     }
> 
> To make this more clear, I recommend to store the expected value in a local variable and then call assert.

would suggest some more tests (overcautious maybe, be have seen horses vomit at unbelievable places ;)

Test that the returned set

- is indeed unmodifiable
- not gc'ed
- is a wrapper around the backing set

Test code:

    @Test(expected=UnsupportedOperationException.class)
    public void testPseudoClassesUnmodifiable() {
        Node node = new Rectangle();
        node.getPseudoClassStates().add(PseudoClass.getPseudoClass("dummy"));
    }

    @Test
    public void testPseudoClassesNotGCed() {
        Node node = new Rectangle();
        WeakReference<Set<?>> weakRef = new WeakReference<>(node.getPseudoClassStates());
        ControlSkinFactory.attemptGC(weakRef);
        assertNotNull("pseudoClassStates must not be gc'ed", weakRef.get());
    }
    
    // requires additional method in NodeShim
    @Test
    public void testUnmodifiablePseudoClassStatesEqualsBackingStates() {
        Rectangle node = new Rectangle();
        PseudoClass pseudo = PseudoClass.getPseudoClass("somepseudo");
        node.pseudoClassStateChanged(pseudo, true);
        assertEquals(1, node.getPseudoClassStates().size());
        assertEquals(NodeShim.pseudoClassStates(node).size(), node.getPseudoClassStates().size());
        assertTrue(NodeShim.pseudoClassStates(node).contains(pseudo));
        assertTrue(node.getPseudoClassStates().contains(pseudo));
    }

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

PR: https://git.openjdk.java.net/jfx/pull/253