8267505: {List,Set,Map}PropertyBase::bind should check against identity

Reviewed-by: mhanl, kcr

Author: Jose Pereda

modules/javafx.base/src/main/java/javafx/beans/property/ListPropertyBase.java

@@ -270,7 +270,7 @@ public void bind(final ObservableValue<? extends ObservableList<E>> newObservabl
             throw new NullPointerException("Cannot bind to null");
         }
 
-        if (!newObservable.equals(observable)) {
+        if (newObservable != observable) {
             unbind();
             observable = newObservable;
             if (listener == null) {

modules/javafx.base/src/main/java/javafx/beans/property/MapPropertyBase.java

@@ -271,7 +271,7 @@ public void bind(final ObservableValue<? extends ObservableMap<K, V>> newObserva
         if (newObservable == null) {
             throw new NullPointerException("Cannot bind to null");
         }
-        if (!newObservable.equals(observable)) {
+        if (newObservable != observable) {
             unbind();
             observable = newObservable;
             if (listener == null) {

modules/javafx.base/src/main/java/javafx/beans/property/SetPropertyBase.java

@@ -272,7 +272,7 @@ public void bind(final ObservableValue<? extends ObservableSet<E>> newObservable
             throw new NullPointerException("Cannot bind to null");
         }
 
-        if (!newObservable.equals(this.observable)) {
+        if (newObservable != this.observable) {
             unbind();
             observable = newObservable;
             if (listener == null) {

modules/javafx.base/src/test/java/test/javafx/beans/property/ListPropertyBaseTest.java

@@ -627,6 +627,43 @@ public void testRebind() {
         invalidationListener.check(null, 0);
     }
 
+    @Test
+    public void testRebind_Identity() {
+        final ListProperty<Object> v1 = new SimpleListProperty<>(FXCollections.observableArrayList());
+        final ListProperty<Object> v2 = new SimpleListProperty<>(FXCollections.observableArrayList());
+        attachListChangeListener();
+
+        // bind
+        property.bind(v1);
+        property.check(1);
+        listChangeListener.check1AddRemove(property, EMPTY_LIST, 0, 0);
+        listChangeListener.clear();
+
+        // rebind to same
+        property.bind(v1);
+        property.check(0);
+        listChangeListener.check0();
+
+        // rebind to other, without explicitly unbinding
+        property.bind(v2);
+        property.check(1);
+        listChangeListener.check1AddRemove(property, EMPTY_LIST, 0, 0);
+        listChangeListener.clear();
+
+        v2.add("One");
+        listChangeListener.check1AddRemove(property, EMPTY_LIST, 0, 1);
+        listChangeListener.clear();
+
+        v2.add("Two");
+        listChangeListener.check1AddRemove(property, EMPTY_LIST, 1, 2);
+        listChangeListener.clear();
+
+        property.check(4);
+        assertTrue(property.isBound());
+        assertEquals(2, property.toArray().length);
+        assertEquals("ListProperty [bound, value: [One, Two]]", property.toString());
+    }
+
     @Test
     public void testUnbind() {
         attachInvalidationListener();

modules/javafx.base/src/test/java/test/javafx/beans/property/MapPropertyBaseTest.java

@@ -650,6 +650,41 @@ public void testRebind() {
         invalidationListener.check(null, 0);
     }
 
+    @Test
+    public void testRebind_Identity() {
+        final MapProperty<Object, Object> v1 = new SimpleMapProperty<>(FXCollections.observableHashMap());
+        final MapProperty<Object, Object> v2 = new SimpleMapProperty<>(FXCollections.observableHashMap());
+        attachMapChangeListener();
+
+        // bind
+        property.bind(v1);
+        property.check(1);
+        mapChangeListener.clear();
+
+        // rebind to same
+        property.bind(v1);
+        property.check(0);
+        mapChangeListener.check0();
+
+        // rebind to other, without explicitly unbinding
+        property.bind(v2);
+        property.check(1);
+        mapChangeListener.clear();
+
+        v2.put("One", "1");
+        mapChangeListener.assertAdded(MockMapObserver.Tuple.tup("One", "1"));
+        mapChangeListener.clear();
+
+        v2.put("Two", "2");
+        mapChangeListener.assertAdded(MockMapObserver.Tuple.tup("Two", "2"));
+        mapChangeListener.clear();
+
+        property.check(4);
+        assertTrue(property.isBound());
+        assertEquals(2, property.size());
+        assertEquals("MapProperty [bound, value: {One=1, Two=2}]", property.toString());
+    }
+
     @Test
     public void testUnbind() {
         attachInvalidationListener();

modules/javafx.base/src/test/java/test/javafx/beans/property/SetPropertyBaseTest.java

@@ -587,6 +587,41 @@ public void testRebind() {
         invalidationListener.check(null, 0);
     }
 
+    @Test
+    public void testRebind_Identity() {
+        final SetProperty<Object> v1 = new SimpleSetProperty<>(FXCollections.observableSet());
+        final SetProperty<Object> v2 = new SimpleSetProperty<>(FXCollections.observableSet());
+        attachSetChangeListener();
+
+        // bind
+        property.bind(v1);
+        property.check(1);
+        setChangeListener.clear();
+
+        // rebind to same
+        property.bind(v1);
+        property.check(0);
+        setChangeListener.check0();
+
+        // rebind to other, without explicitly unbinding
+        property.bind(v2);
+        property.check(1);
+        setChangeListener.clear();
+
+        v2.add("One");
+        setChangeListener.assertAdded(Tuple.tup("One"));
+        setChangeListener.clear();
+
+        v2.add("Two");
+        setChangeListener.assertAdded(Tuple.tup("Two"));
+        setChangeListener.clear();
+
+        property.check(4);
+        assertTrue(property.isBound());
+        assertEquals(2, property.toArray().length);
+        assertEquals("SetProperty [bound, value: [Two, One]]", property.toString());
+    }
+
     @Test
     public void testUnbind() {
         attachInvalidationListener();