diff --git a/modules/javafx.controls/src/main/java/com/sun/javafx/scene/control/LambdaMultiplePropertyChangeListenerHandler.java b/modules/javafx.controls/src/main/java/com/sun/javafx/scene/control/LambdaMultiplePropertyChangeListenerHandler.java index ceef60598ed..6776b81939f 100644 --- a/modules/javafx.controls/src/main/java/com/sun/javafx/scene/control/LambdaMultiplePropertyChangeListenerHandler.java +++ b/modules/javafx.controls/src/main/java/com/sun/javafx/scene/control/LambdaMultiplePropertyChangeListenerHandler.java @@ -1,5 +1,5 @@ /* - * Copyright (c) 2015, 2016, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 2015, 2021, 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 @@ -25,23 +25,62 @@ package com.sun.javafx.scene.control; -import javafx.beans.value.ChangeListener; -import javafx.beans.value.ObservableValue; -import javafx.beans.value.WeakChangeListener; - import java.util.HashMap; +import java.util.IdentityHashMap; import java.util.Map; import java.util.function.Consumer; +import javafx.beans.InvalidationListener; +import javafx.beans.Observable; +import javafx.beans.WeakInvalidationListener; +import javafx.beans.value.ChangeListener; +import javafx.beans.value.ObservableValue; +import javafx.beans.value.WeakChangeListener; +import javafx.collections.ListChangeListener; +import javafx.collections.ListChangeListener.Change; +import javafx.collections.ObservableList; +import javafx.collections.WeakListChangeListener; + +/** + * Handler to manage multiple listeners to multiple observables. It handles + * adding/removing listeners for common notification events (change, + * invalidation and list change) on particular instances of observables. The + * listeners are wrapped into their weak counterparts to minimize the potential of + * memory leaks. + *

+ * Clients can register consumers to be invoked on receiving notification. + * Un-/Registration api is separate per notification type and per observable. + * It is allowed to register multiple consumers per observable. They + * are executed in the order of registration. Note that unregistration + * of a given observable stops observing that observable (for the notification + * type of the unregistration) completely, that is none of the consumers + * previously registered with this handler will be executed after unregistering. + *

+ * Disposing removes all listeners added by this handler from all registered observables. + */ public final class LambdaMultiplePropertyChangeListenerHandler { +// FIXME JDK-8265401: name doesn't fit after widening to support more notification event types + @SuppressWarnings("rawtypes") + private static final Consumer EMPTY_CONSUMER = e -> {}; + + // support change listeners private final Map, Consumer>> propertyReferenceMap; private final ChangeListener propertyChangedListener; private final WeakChangeListener weakPropertyChangedListener; - private static final Consumer> EMPTY_CONSUMER = e -> {}; + // support invalidation listeners + private final Map> observableReferenceMap; + private final InvalidationListener invalidationListener; + private final WeakInvalidationListener weakInvalidationListener; + + // support list change listeners + private final Map, Consumer>> observableListReferenceMap; + private final ListChangeListener listChangeListener; + private final WeakListChangeListener weakListChangeListener; public LambdaMultiplePropertyChangeListenerHandler() { + // change listening support this.propertyReferenceMap = new HashMap<>(); this.propertyChangedListener = (observable, oldValue, newValue) -> { // because all consumers are chained, this calls each consumer for the given property @@ -49,16 +88,32 @@ public LambdaMultiplePropertyChangeListenerHandler() { propertyReferenceMap.getOrDefault(observable, EMPTY_CONSUMER).accept(observable); }; this.weakPropertyChangedListener = new WeakChangeListener<>(propertyChangedListener); + + // invalidation listening support + this.observableReferenceMap = new HashMap<>(); + this.invalidationListener = obs -> { + observableReferenceMap.getOrDefault(obs, EMPTY_CONSUMER).accept(obs); + }; + this.weakInvalidationListener = new WeakInvalidationListener(invalidationListener); + + // list change listening support + this.observableListReferenceMap = new IdentityHashMap<>(); + this.listChangeListener = change -> { + observableListReferenceMap.getOrDefault(change.getList(), EMPTY_CONSUMER).accept(change); + }; + this.weakListChangeListener = new WeakListChangeListener<>(listChangeListener); } /** - * Subclasses can invoke this method to register that we want to listen to - * property change events for the given property. + * Registers a consumer to be invoked on change notification from the given property. Does nothing + * if property or consumer is null. Consumers registered to the same property will be executed + * in the order they have been registered. * - * @param property + * @param property the property to observe for change notification + * @param consumer the consumer to be invoked on change notification from the property */ public final void registerChangeListener(ObservableValue property, Consumer> consumer) { - if (consumer == null) return; + if (property == null || consumer == null) return; // we only add a listener if the propertyReferenceMap does not contain the property // (that is, we've added a consumer to this specific property for the first @@ -70,17 +125,109 @@ public final void registerChangeListener(ObservableValue property, Consumer> unregisterChangeListeners(ObservableValue property) { + if (property == null) return null; property.removeListener(weakPropertyChangedListener); return propertyReferenceMap.remove(property); } + /** + * Registers a consumer to be invoked on invalidation notification from the given observable. + * Does nothing if observable or consumer is null. Consumers registered to the same observable will be executed + * in the order they have been registered. + * + * @param observable the observable to observe for invalidation notification + * @param consumer the consumer to be invoked on invalidation notification from the observable + * + */ + public final void registerInvalidationListener(Observable observable, Consumer consumer) { + if (observable == null || consumer == null) return; + if (!observableReferenceMap.containsKey(observable)) { + observable.addListener(weakInvalidationListener); + } + observableReferenceMap.merge(observable, consumer, Consumer::andThen); + } + + /** + * Stops observing the given observable for invalidation notification. + * Returns a single chained consumer consisting of all consumers registered with + * {@link #registerInvalidationListener(Observable, Consumer)} in the + * order they have been registered. + * + * @param observable the observable to stop observing for invalidation notification + * @return a single chained consumer consisting of all consumers registered for given observable + * or null if none has been registered or the observable is null + * + */ + public final Consumer unregisterInvalidationListeners(Observable observable) { + if (observable == null) return null; + observable.removeListener(weakInvalidationListener); + return observableReferenceMap.remove(observable); + } + + /** + * Registers a consumer to be invoked on list change notification from the given observable list. + * Does nothing if list or consumer is null. Consumers registered to the same observable list + * will be executed in the order they have been registered. + * + * @param list the observable list observe for list change notification + * @param consumer the consumer to be invoked on list change notification from the list + * + */ + public final void registerListChangeListener(ObservableList list, Consumer> consumer) { + if (list == null || consumer == null) return; + if (!observableListReferenceMap.containsKey(list)) { + list.addListener(weakListChangeListener); + } + observableListReferenceMap.merge(list, consumer, Consumer::andThen); + } + + /** + * Stops observing the given observable list for list change notification. + * Returns a single chained consumer consisting of all consumers registered with + * {@link #registerListChangeListener(ObservableList, Consumer)} in the order they have been registered. + * + * @param list the observable list to stop observing for list change notification + * @return a single chained consumer consisting of all consumers added for the given list + * or null if none has been registered or the list is null + */ + public final Consumer> unregisterListChangeListeners(ObservableList list) { + if (list == null) return null; + list.removeListener(weakListChangeListener); + return observableListReferenceMap.remove(list); + } + + + /** + * Stops observing all types of notification from all registered observables. + *

+ * Note: this handler is still usable after calling this method. + */ public void dispose() { - // unhook listeners + // unhook change listeners for (ObservableValue value : propertyReferenceMap.keySet()) { value.removeListener(weakPropertyChangedListener); } propertyReferenceMap.clear(); + // unhook invalidation listeners + for (Observable value : observableReferenceMap.keySet()) { + value.removeListener(weakInvalidationListener); + } + observableReferenceMap.clear(); + // unhook list change listeners + for (ObservableList list : observableListReferenceMap.keySet()) { + list.removeListener(weakListChangeListener); + } + observableListReferenceMap.clear(); } } diff --git a/modules/javafx.controls/src/main/java/javafx/scene/control/SkinBase.java b/modules/javafx.controls/src/main/java/javafx/scene/control/SkinBase.java index 4beb42d0082..2fbe2f8646c 100644 --- a/modules/javafx.controls/src/main/java/javafx/scene/control/SkinBase.java +++ b/modules/javafx.controls/src/main/java/javafx/scene/control/SkinBase.java @@ -1,5 +1,5 @@ /* - * Copyright (c) 2012, 2017, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 2012, 2021, 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 @@ -25,15 +25,18 @@ package javafx.scene.control; -import com.sun.javafx.scene.control.LambdaMultiplePropertyChangeListenerHandler; -import javafx.beans.value.ObservableValue; -import javafx.css.CssMetaData; -import javafx.css.PseudoClass; import java.util.Collections; import java.util.List; import java.util.function.Consumer; +import com.sun.javafx.scene.control.LambdaMultiplePropertyChangeListenerHandler; + +import javafx.beans.Observable; +import javafx.beans.value.ObservableValue; +import javafx.collections.ListChangeListener.Change; import javafx.collections.ObservableList; +import javafx.css.CssMetaData; +import javafx.css.PseudoClass; import javafx.css.Styleable; import javafx.event.EventHandler; import javafx.geometry.HPos; @@ -206,39 +209,112 @@ protected final void consumeMouseEvents(boolean value) { /** - * Subclasses can invoke this method to register that they want to listen to - * property change events for the given property. Registered {@link Consumer} instances - * will be executed in the order in which they are registered. - * @param property the property - * @param consumer the consumer + * Registers an operation to perform when the given {@code observable} sends a change event. + * Does nothing if either {@code observable} or {@code operation} are {@code null}. + * If multiple operations are registered on the same observable, they will be performed in the + * order in which they were registered. + * + * @param observable the observable to observe for change events, may be {@code null} + * @param operation the operation to perform when the observable sends a change event, + * may be {@code null} */ - protected final void registerChangeListener(ObservableValue property, Consumer> consumer) { + protected final void registerChangeListener(ObservableValue observable, Consumer> operation) { if (lambdaChangeListenerHandler == null) { lambdaChangeListenerHandler = new LambdaMultiplePropertyChangeListenerHandler(); } - lambdaChangeListenerHandler.registerChangeListener(property, consumer); + lambdaChangeListenerHandler.registerChangeListener(observable, operation); } /** - * Unregisters all change listeners that have been registered using {@link #registerChangeListener(ObservableValue, Consumer)} - * for the given property. The end result is that the given property is no longer observed by any of the change - * listeners, but it may still have additional listeners registered on it through means outside of - * {@link #registerChangeListener(ObservableValue, Consumer)}. + * Unregisters all operations that have been registered using + * {@link #registerChangeListener(ObservableValue, Consumer)} + * for the given {@code observable}. Does nothing if {@code observable} is {@code null}. * - * @param property The property for which all listeners should be removed. - * @return A single chained {@link Consumer} consisting of all {@link Consumer consumers} registered through - * {@link #registerChangeListener(ObservableValue, Consumer)}. If no consumers have been registered on this - * property, null will be returned. + * @param observable the observable for which the registered operations should be removed, + * may be {@code null} + * @return a composed consumer representing all previously registered operations, or + * {@code null} if none have been registered or the observable is {@code null} * @since 9 */ - protected final Consumer> unregisterChangeListeners(ObservableValue property) { + protected final Consumer> unregisterChangeListeners(ObservableValue observable) { if (lambdaChangeListenerHandler == null) { return null; } - return lambdaChangeListenerHandler.unregisterChangeListeners(property); + return lambdaChangeListenerHandler.unregisterChangeListeners(observable); } + /** + * Registers an operation to perform when the given {@code observable} sends an invalidation event. + * Does nothing if either {@code observable} or {@code operation} are {@code null}. + * If multiple operations are registered on the same observable, they will be performed in the + * order in which they were registered. + * + * @param observable the observable to observe for invalidation events, may be {@code null} + * @param operation the operation to perform when the observable sends an invalidation event, + * may be {@code null} + * @since 17 + */ + protected final void registerInvalidationListener(Observable observable, Consumer operation) { + if (lambdaChangeListenerHandler == null) { + lambdaChangeListenerHandler = new LambdaMultiplePropertyChangeListenerHandler(); + } + lambdaChangeListenerHandler.registerInvalidationListener(observable, operation); + } + /** + * Unregisters all operations that have been registered using + * {@link #registerInvalidationListener(Observable, Consumer)} + * for the given {@code observable}. Does nothing if {@code observable} is {@code null}. + * + * @param observable the observable for which the registered operations should be removed, + * may be {@code null} + * @return a composed consumer representing all previously registered operations, or + * {@code null} if none have been registered or the observable is {@code null} + * @since 17 + */ + protected final Consumer unregisterInvalidationListeners(Observable observable) { + if (lambdaChangeListenerHandler == null) { + return null; + } + return lambdaChangeListenerHandler.unregisterInvalidationListeners(observable); + } + + + /** + * Registers an operation to perform when the given {@code observableList} sends a list change event. + * Does nothing if either {@code observableList} or {@code operation} are {@code null}. + * If multiple operations are registered on the same observable list, they will be performed in the + * order in which they were registered. + * + * @param observableList the observableList to observe for list change events, may be {@code null} + * @param operation the operation to perform when the observableList sends a list change event, + * may be {@code null} + * @since 17 + */ + protected final void registerListChangeListener(ObservableList observableList, Consumer> operation) { + if (lambdaChangeListenerHandler == null) { + lambdaChangeListenerHandler = new LambdaMultiplePropertyChangeListenerHandler(); + } + lambdaChangeListenerHandler.registerListChangeListener(observableList, operation); + } + + /** + * Unregisters all operations that have been registered using + * {@link #registerListChangeListener(ObservableList, Consumer)} + * for the given {@code observableList}. Does nothing if {@code observableList} is {@code null}. + * + * @param observableList the observableList for which the registered operations should be removed, + * may be {@code null} + * @return a composed consumer representing all previously registered operations, or + * {@code null} if none have been registered or the observableList is {@code null} + * @since 17 + */ + protected final Consumer> unregisterListChangeListeners(ObservableList observableList) { + if (lambdaChangeListenerHandler == null) { + return null; + } + return lambdaChangeListenerHandler.unregisterListChangeListeners(observableList); + } /*************************************************************************** diff --git a/modules/javafx.controls/src/shims/java/javafx/scene/control/SkinBaseShim.java b/modules/javafx.controls/src/shims/java/javafx/scene/control/SkinBaseShim.java index d10ad96d361..b099293e77a 100644 --- a/modules/javafx.controls/src/shims/java/javafx/scene/control/SkinBaseShim.java +++ b/modules/javafx.controls/src/shims/java/javafx/scene/control/SkinBaseShim.java @@ -28,7 +28,7 @@ import java.util.List; import javafx.scene.Node; -public class SkinBaseShim extends SkinBase { +public class SkinBaseShim extends SkinBase { public SkinBaseShim(final C control) { super(control); diff --git a/modules/javafx.controls/src/test/java/test/com/sun/javafx/scene/control/LambdaMultipleListHandlerTest.java b/modules/javafx.controls/src/test/java/test/com/sun/javafx/scene/control/LambdaMultipleListHandlerTest.java new file mode 100644 index 00000000000..e49899abd4c --- /dev/null +++ b/modules/javafx.controls/src/test/java/test/com/sun/javafx/scene/control/LambdaMultipleListHandlerTest.java @@ -0,0 +1,256 @@ +/* + * Copyright (c) 2021, 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 + * under the terms of the GNU General Public License version 2 only, as + * published by the Free Software Foundation. Oracle designates this + * particular file as subject to the "Classpath" exception as provided + * by Oracle in the LICENSE file that accompanied this code. + * + * This code is distributed in the hope that it will be useful, but WITHOUT + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License + * version 2 for more details (a copy is included in the LICENSE file that + * accompanied this code). + * + * You should have received a copy of the GNU General Public License version + * 2 along with this work; if not, write to the Free Software Foundation, + * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA. + * + * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA + * or visit www.oracle.com if you need additional information or have any + * questions. + */ + +package test.com.sun.javafx.scene.control; + +import java.lang.ref.WeakReference; +import java.util.ArrayList; +import java.util.List; +import java.util.function.Consumer; + +import org.junit.Before; +import org.junit.Test; + +import com.sun.javafx.scene.control.LambdaMultiplePropertyChangeListenerHandler; + +import static org.junit.Assert.*; +import static test.com.sun.javafx.scene.control.infrastructure.ControlSkinFactory.*; + +import javafx.beans.property.ObjectProperty; +import javafx.beans.property.SimpleObjectProperty; +import javafx.collections.FXCollections; +import javafx.collections.ListChangeListener; +import javafx.collections.ListChangeListener.Change; +import javafx.collections.ObservableList; + +/** + * Test support of listChange listeners. + */ +public class LambdaMultipleListHandlerTest { + + private LambdaMultiplePropertyChangeListenerHandler handler; + private ObservableList items; + +//------------ unregister + + /** + * Single consumer for multiple lists: test that + * removing from one list doesn't effect listening to other list + */ + @Test + public void testUnregistersSingleConsumerMultipleLists() { + List> changes = new ArrayList<>(); + Consumer> consumer = change -> changes.add(change); + ObservableList otherList = FXCollections.observableArrayList("other"); + handler.registerListChangeListener(items, consumer); + handler.registerListChangeListener(otherList, consumer); + handler.unregisterListChangeListeners(otherList); + items.add("added"); + otherList.add("added other"); + assertEquals(1, changes.size()); + assertEquals(items, changes.get(0).getList()); + } + + /** + * Test that all consumers for a single list are removed + * and manually adding the removed consumer chain as listener + * has the same effect as when invoked via handler. + */ + @Test + public void testUnregistersMultipleConsumers() { + List> changes = new ArrayList<>(); + Consumer> consumer = change -> changes.add(change); + List> secondChanges = new ArrayList<>(); + Consumer> secondConsumer = change -> secondChanges.addAll(changes); + handler.registerListChangeListener(items, consumer); + handler.registerListChangeListener(items, secondConsumer); + // remove listener chain + Consumer> removedChain = handler.unregisterListChangeListeners(items); + items.add("added after removed"); + assertEquals("none of the removed listeners must be notified", + 0, changes.size() + secondChanges.size()); + // manually add the chained listener + items.addListener((ListChangeListener)(c -> removedChain.accept(c))); + items.add("added"); + assertEquals(1, changes.size()); + assertEquals(changes, secondChanges); + } + + @Test + public void testUnregistersSingleConsumer() { + List> changes = new ArrayList<>(); + Consumer> consumer = change -> changes.add(change); + ObservableList otherList = FXCollections.observableArrayList("other"); + handler.registerListChangeListener(items, consumer); + Consumer> removed = handler.unregisterListChangeListeners(items); + items.add("added"); + assertEquals(0, changes.size()); + assertSame(consumer, removed); + } + + /** + * Test unregisters not registered list. + */ + @Test + public void testUnregistersNotRegistered() { + assertNull(handler.unregisterListChangeListeners(items)); + } + + @Test + public void testUnregistersNull() { + assertNull(handler.unregisterListChangeListeners(null)); + } + + +//------------- register + + @Test + public void testRegisterConsumerToMultipleLists() { + List> changes = new ArrayList<>(); + Consumer> consumer = change -> changes.add(change); + ObservableList otherList = FXCollections.observableArrayList("other"); + handler.registerListChangeListener(items, consumer); + handler.registerListChangeListener(otherList, consumer); + items.add("added"); + otherList.add("added other"); + assertEquals(2, changes.size()); + assertEquals(items, changes.get(0).getList()); + assertEquals(otherList, changes.get(1).getList()); + } + + /** + * Test that multiple consumers to same observable are invoked in order + * of registration. + */ + @Test + public void testRegisterMultipleConsumerToSingleList() { + List> changes = new ArrayList<>(); + Consumer> consumer = change -> changes.add(change); + List> secondChanges = new ArrayList<>(); + Consumer> secondConsumer = change -> secondChanges.addAll(changes); + handler.registerListChangeListener(items, consumer); + handler.registerListChangeListener(items, secondConsumer); + items.add("added"); + assertEquals(1, changes.size()); + assertEquals(changes, secondChanges); + } + + @Test + public void testRegister() { + List> changes = new ArrayList<>(); + Consumer> consumer = change -> changes.add(change); + handler.registerListChangeListener(items, consumer); + String added = "added"; + items.add(added); + assertEquals(1, changes.size()); + Change change = changes.get(0); + change.next(); + assertTrue(change.wasAdded()); + assertTrue(change.getAddedSubList().contains(added)); + } + + @Test + public void testRegisterNullConsumer() { + handler.registerListChangeListener(items, null); + } + + @Test + public void testRegisterNullList() { + handler.registerListChangeListener(null, c -> {}); + } + +//--------- dispose + + @Test + public void testDispose() { + List> changes = new ArrayList<>(); + Consumer> consumer = change -> changes.add(change); + handler.registerListChangeListener(items, consumer); + handler.dispose(); + items.add("added"); + assertEquals("listener must not be invoked after dispose", 0, changes.size()); + handler.registerListChangeListener(items, consumer); + items.add("added"); + assertEquals("listener must be invoked when re-registered after dispose", 1, changes.size()); + } + + +//--------- test weak registration + + /** + * Test that handler is gc'ed and listener no longer notified. + */ + @Test + public void testRegisterMemoryLeak() { + List> changes = new ArrayList<>(); + Consumer> consumer = change -> changes.add(change); + LambdaMultiplePropertyChangeListenerHandler handler = new LambdaMultiplePropertyChangeListenerHandler(); + WeakReference ref = new WeakReference<>(handler); + handler.registerListChangeListener(items, consumer); + items.add("added"); + assertEquals(1, changes.size()); + handler = null; + attemptGC(ref); + assertNull("handler must be gc'ed", ref.get()); + items.add("another"); + assertEquals("listener must not be invoked after gc", 1, changes.size()); + } + + +//----------- setup and intial + + @Before + public void setup() { + handler = new LambdaMultiplePropertyChangeListenerHandler(); + items = FXCollections.observableArrayList("one", "two", "four"); + } + + /** + * Demonstrating why we need an invalidation listener for list-valued observables. + */ + @Test + public void testInvalidationOfListValuedObservable() { + String[] data = {"one", "two", "other"}; + ObservableList first = FXCollections.observableArrayList(data); + ObjectProperty> itemsProperty = new SimpleObjectProperty<>(first); + assertSame(first, itemsProperty.get()); + int[] invalidations = new int[] {0}; + int[] changes = new int[] {0}; + itemsProperty.addListener(obs -> invalidations[0]++); + itemsProperty.addListener((obs, ov, nv) -> changes[0]++); + itemsProperty.set(FXCollections.observableArrayList(data)); + // notifications when newList.equals(oldList) + assertEquals("changeListener not notified", 0, changes[0]); + assertEquals("invalidationListener notified", 1, invalidations[0]); + itemsProperty.get().add("added"); + // sanity: no notification on modifications to the list + assertEquals(0, changes[0]); + assertEquals(1, invalidations[0]); + // sanity: notification from both when !newList.equals(oldList) + itemsProperty.set(first); + assertEquals(1, changes[0]); + assertEquals(2, invalidations[0]); + } +} diff --git a/modules/javafx.controls/src/test/java/test/com/sun/javafx/scene/control/LambdaMultipleObservableHandlerTest.java b/modules/javafx.controls/src/test/java/test/com/sun/javafx/scene/control/LambdaMultipleObservableHandlerTest.java new file mode 100644 index 00000000000..19c075ac977 --- /dev/null +++ b/modules/javafx.controls/src/test/java/test/com/sun/javafx/scene/control/LambdaMultipleObservableHandlerTest.java @@ -0,0 +1,366 @@ +/* + * Copyright (c) 2021, 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 + * under the terms of the GNU General Public License version 2 only, as + * published by the Free Software Foundation. Oracle designates this + * particular file as subject to the "Classpath" exception as provided + * by Oracle in the LICENSE file that accompanied this code. + * + * This code is distributed in the hope that it will be useful, but WITHOUT + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License + * version 2 for more details (a copy is included in the LICENSE file that + * accompanied this code). + * + * You should have received a copy of the GNU General Public License version + * 2 along with this work; if not, write to the Free Software Foundation, + * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA. + * + * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA + * or visit www.oracle.com if you need additional information or have any + * questions. + */ + +package test.com.sun.javafx.scene.control; + +import java.lang.ref.WeakReference; +import java.util.Arrays; +import java.util.Collection; +import java.util.function.Consumer; + +import org.junit.Before; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.junit.runners.Parameterized; + +import com.sun.javafx.scene.control.LambdaMultiplePropertyChangeListenerHandler; + +import static org.junit.Assert.*; +import static test.com.sun.javafx.scene.control.infrastructure.ControlSkinFactory.*; + +import javafx.beans.Observable; +import javafx.beans.binding.Bindings; +import javafx.beans.binding.NumberBinding; +import javafx.beans.property.IntegerProperty; +import javafx.beans.property.SimpleIntegerProperty; +import javafx.beans.value.ObservableValue; + +/** + * Test LambdaMultiplePropertyChangeListenerHandler. + *

+ * + * This test is parameterized in testing change- or invalidationListener api. + */ +@RunWith(Parameterized.class) +public class LambdaMultipleObservableHandlerTest { + + private LambdaMultiplePropertyChangeListenerHandler handler; + private boolean useChangeListener; + +// -------------- unregister + + /** + * Single consumer for multiple observables: test that + * removing from one observable doesn't effect listening to other observable + */ + @Test + public void testUnregistersSingleConsumerMultipleObservables() { + IntegerProperty p = new SimpleIntegerProperty(); + IntegerProperty other = new SimpleIntegerProperty(); + int[] count = new int[] {0}; + Consumer> consumer = c -> count[0]++; + registerListener(p, consumer); + registerListener(other, consumer); + unregisterListeners(other); + p.set(100); + other.set(100); + assertEquals(1, count[0]); + } + + /** + * Test that all consumers for a single observable are removed + * and manually adding the removed consumer chain as listener + * has the same effect as when invoked via handler. + */ + @Test + public void testUnregistersMultipleConsumers() { + IntegerProperty p = new SimpleIntegerProperty(); + int[] action = new int[] {0}; + int actionValue = 10; + int[] secondAction = new int[] {0}; + // register multiple consumers + registerListener(p, c -> action[0] = actionValue); + registerListener(p, c -> secondAction[0] = action[0]); + // remove all + Consumer removedChain = unregisterListeners(p); + p.set(100); + assertEquals("none of the removed listeners must be notified", 0, action[0] + secondAction[0]); + + // manually add the chained consumers + addListener(p, removedChain); + p.set(200); + // assert effect of manually added chain is same + assertEquals("effect of removed consumer chain", actionValue, action[0]); + assertEquals("effect of removed consumer chain", action[0], secondAction[0]); + } + + @Test + public void testUnregistersSingleConsumer() { + IntegerProperty p = new SimpleIntegerProperty(); + int[] count = new int[] {0}; + Consumer consumer = c -> count[0]++; + registerListener(p, consumer); + Consumer removed = unregisterListeners(p); + p.set(100); + assertEquals(0, count[0]); + assertSame("single registered listener must be returned", consumer, removed); + } + + /** + * Test unregisters not registered observable. + */ + @Test + public void testUnregistersNotRegistered() { + IntegerProperty p = new SimpleIntegerProperty(); + assertNull(unregisterListeners(p)); + } + + @Test + public void testUnregistersNull() { + assertNull(unregisterListeners(null)); + } + + +//------------ register + + @Test + public void testRegisterConsumerToMultipleObservables() { + IntegerProperty p = new SimpleIntegerProperty(); + IntegerProperty other = new SimpleIntegerProperty(); + int[] count = new int[] {0}; + Consumer consumer = c -> count[0]++; + registerListener(p, consumer); + registerListener(other, consumer); + p.set(100); + other.set(100); + assertEquals(2, count[0]); + } + + /** + * Test that multiple consumers to same observable are invoked in order + * of registration. + */ + @Test + public void testRegisterMultipleConsumerToSingleObservable() { + IntegerProperty p = new SimpleIntegerProperty(); + int[] action = new int[] {0}; + int actionValue = 10; + int[] secondAction = new int[] {0}; + registerListener(p, c -> action[0] = actionValue); + registerListener(p, c -> secondAction[0] = action[0]); + p.set(100); + assertEquals(actionValue, action[0]); + assertEquals(action[0], secondAction[0]); + } + + @Test + public void testRegister() { + IntegerProperty p = new SimpleIntegerProperty(); + int[] count = new int[] {0}; + registerListener(p, c -> count[0]++); + p.set(100); + assertEquals(1, count[0]); + } + + @Test + public void testRegisterNullConsumer() { + IntegerProperty p = new SimpleIntegerProperty(); + registerListener(p, null); + } + + @Test + public void testRegisterNullObservable() { + registerListener(null, c -> {}); + } + +//--------- dispose + + @Test + public void testDispose() { + IntegerProperty p = new SimpleIntegerProperty(); + int[] count = new int[] {0}; + registerListener(p, c -> count[0]++); + handler.dispose(); + p.set(100); + assertEquals("listener must not be invoked after dispose", 0, count[0]); + // re-register + registerListener(p, c -> count[0]++); + p.set(200); + assertEquals("listener must be invoked when re-registered after dispose", 1, count[0]); + } + + +//--------- test weak registration + + /** + * Test that handler is gc'ed and listener no longer notified. + */ + @Test + public void testRegisterMemoryLeak() { + IntegerProperty p = new SimpleIntegerProperty(); + int[] count = new int[] {0}; + Consumer> consumer = c -> count[0]++; + LambdaMultiplePropertyChangeListenerHandler handler = new LambdaMultiplePropertyChangeListenerHandler(); + WeakReference ref = + new WeakReference<>(handler); + registerListener(handler, p, consumer); + p.setValue(100); + int notified = count[0]; + assertEquals("sanity: listener invoked", notified, count[0]); + assertNotNull(ref.get()); + handler = null; + attemptGC(ref); + assertNull("handler must be gc'ed", ref.get()); + p.setValue(200); + assertEquals("listener must not be invoked after gc", notified, count[0]); + } + + +//-------------- not-parameterized tests +// guard against cross-over effects for change/invalidationListener on same observable + + /** + * Register both invalidation/change listener on same property. + */ + @Test + public void testRegisterBoth() { + IntegerProperty p = new SimpleIntegerProperty(); + int[] count = new int[] {0}; + handler.registerChangeListener(p, c -> count[0]++); + handler.registerInvalidationListener(p, c -> count[0]++); + p.set(100); + assertEquals("both listener types must be invoked", 2, count[0]); + } + + /** + * Register both invalidation/change listener, remove change listener + */ + @Test + public void testRegisterBothRemoveChangeListener() { + IntegerProperty p = new SimpleIntegerProperty(); + int[] count = new int[] {0}; + handler.registerChangeListener(p, c -> count[0]++); + handler.registerInvalidationListener(p, c -> count[0]++); + handler.unregisterChangeListeners(p); + p.set(200); + assertEquals("", 1, count[0]); + } + + /** + * Register both invalidation/change listener, remove invalidationListener. + */ + @Test + public void testRegisterBothRemoveInvalidationListener() { + IntegerProperty p = new SimpleIntegerProperty(); + int[] count = new int[] {0}; + handler.registerChangeListener(p, c -> count[0]++); + handler.registerInvalidationListener(p, c -> count[0]++); + handler.unregisterInvalidationListeners(p); + p.set(200); + assertEquals("", 1, count[0]); + } + + /** + * Test that binding is invalid. + */ + @Test + public void testBindingInvalid() { + IntegerProperty num1 = new SimpleIntegerProperty(1); + IntegerProperty num2 = new SimpleIntegerProperty(2); + NumberBinding p = Bindings.add(num1,num2); + int[] count = new int[] {0}; + handler.registerChangeListener(p, c -> count[0]++); + handler.registerInvalidationListener(p, c -> count[0]++); + handler.unregisterChangeListeners(p); + num1.set(200); + assertEquals("sanity: received invalidation", 1, count[0]); + assertFalse("binding must not be valid", p.isValid()); + } + + +//----------------------- helpers to un/registration listeners + + /** + * Registers the consumer for notification from the observable, + * using the default handler. + * + */ + protected void registerListener(Observable p, Consumer consumer) { + registerListener(handler, p, consumer); + } + + /** + * Registers the consumer for notification from the observable, + * using the given handler. + */ + protected void registerListener(LambdaMultiplePropertyChangeListenerHandler handler, Observable p, Consumer consumer) { + if (useChangeListener) { + handler.registerChangeListener((ObservableValue) p, consumer); + } else { + handler.registerInvalidationListener(p, consumer); + } + } + + /** + * Unregisters listeners from observable, using default handler + */ + protected Consumer unregisterListeners(Observable p) { + return unregisterListeners(handler, p); + } + + /** + * Unregisters listeners from observable, using default handler + */ + protected Consumer unregisterListeners(LambdaMultiplePropertyChangeListenerHandler handler, Observable p) { + if (useChangeListener) { + return handler.unregisterChangeListeners((ObservableValue) p); + } + return handler.unregisterInvalidationListeners(p); + } + + protected void addListener(ObservableValue p, Consumer consumer) { + if (useChangeListener) { + p.addListener((obs, ov, nv) -> consumer.accept(obs)); + } else { + p.addListener(obs -> consumer.accept(obs)); + } + } + + +//-------------- parameters + + // Note: name property not supported before junit 4.11 + @Parameterized.Parameters //(name = "{index}: changeListener {0} ") + public static Collection data() { + Object[][] data = new Object[][] { + {true}, // test changeListener api + {false} // test invalidationListener api + }; + return Arrays.asList(data); + } + + public LambdaMultipleObservableHandlerTest(boolean useChangeListener) { + this.useChangeListener = useChangeListener; + } + + +//------------ setup and initial + + @Before + public void setup() { + this.handler = new LambdaMultiplePropertyChangeListenerHandler(); + } + +} diff --git a/modules/javafx.controls/src/test/java/test/javafx/scene/control/SkinBaseTest.java b/modules/javafx.controls/src/test/java/test/javafx/scene/control/SkinBaseTest.java index f8f6e6afbf5..8df05bdf786 100644 --- a/modules/javafx.controls/src/test/java/test/javafx/scene/control/SkinBaseTest.java +++ b/modules/javafx.controls/src/test/java/test/javafx/scene/control/SkinBaseTest.java @@ -1,5 +1,5 @@ /* - * Copyright (c) 2011, 2015, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 2011, 2021, 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 @@ -25,14 +25,25 @@ package test.javafx.scene.control; -import javafx.scene.control.Control; -import javafx.scene.control.SkinBaseShim; -import static org.junit.Assert.assertNull; -import static org.junit.Assert.assertSame; +import java.util.ArrayList; +import java.util.List; +import java.util.function.Consumer; import org.junit.Before; import org.junit.Test; +import static org.junit.Assert.*; + +import javafx.beans.Observable; +import javafx.beans.property.IntegerProperty; +import javafx.beans.property.SimpleIntegerProperty; +import javafx.beans.value.ObservableValue; +import javafx.collections.FXCollections; +import javafx.collections.ListChangeListener.Change; +import javafx.collections.ObservableList; +import javafx.scene.control.Control; +import javafx.scene.control.SkinBaseShim; + public class SkinBaseTest { private ControlStub c; private SkinBaseStub s; @@ -57,9 +68,97 @@ public class SkinBaseTest { assertNull(s.getSkinnable()); } - public static final class SkinBaseStub extends SkinBaseShim { +//-------------- testing listener registration +// Note: the behavior is fully tested in the handler test, here we only verify that the +// expected methods are actually used + + @Test + public void testChangeSupport() { + IntegerProperty p = new SimpleIntegerProperty(); + int[] count = new int[] {0}; + Consumer> consumer = c -> count[0]++; + s.addChangeListener(p, consumer); + p.set(200); + assertEquals("change listener must be notified", 1, count[0]); + Consumer> removed = s.removeChangeListeners(p); + p.set(100); + assertEquals("changeListener must not be notified", 1, count[0]); + assertSame(consumer, removed); + } + + @Test + public void testInvalidationSupport() { + IntegerProperty p = new SimpleIntegerProperty(); + int[] count = new int[] {0}; + Consumer consumer = c -> count[0]++; + s.addInvalidationListener(p, consumer); + p.set(200); + assertEquals("invalidation listener must be notified", 1, count[0]); + Consumer removed = s.removeInvalidationListeners(p); + p.set(100); + assertEquals("invalidation listener must not be notified", 1, count[0]); + assertSame(consumer, removed); + } + + @Test + public void testListChangeSupport() { + ObservableList list = FXCollections.observableArrayList("one"); + List> changes = new ArrayList<>(); + Consumer> consumer = c -> changes.add(c); + s.addListChangeListener(list, consumer); + list.add("added"); + assertEquals(1, changes.size()); + Consumer> removed = s.removeListChangeListeners(list); + list.add("another"); + assertEquals(1, changes.size()); + assertSame(consumer, removed); + } + + @Test + public void testRegisterNull() { + s.addChangeListener(null, null); + s.addInvalidationListener(null, null); + s.addListChangeListener(null, null); + } + + @Test + public void testUnregistersNull() { + assertNull(s.removeChangeListeners(null)); + assertNull(s.removeInvalidationListeners(null)); + assertNull(s.removeListChangeListeners(null)); + } + + public static class SkinBaseStub extends SkinBaseShim { public SkinBaseStub(C control) { super(control); } + + // FIXME: un/registerXXListener are final protected + // - how to access without adding a wrapper (with potential of introducing bugs)? + void addChangeListener(ObservableValue p, Consumer> consumer) { + registerChangeListener(p, consumer); + } + + Consumer> removeChangeListeners(ObservableValue p) { + return unregisterChangeListeners(p); + } + + void addInvalidationListener(Observable p, Consumer consumer) { + registerInvalidationListener(p, consumer); + } + + Consumer removeInvalidationListeners(Observable p) { + return unregisterInvalidationListeners(p); + } + + void addListChangeListener(ObservableList list, Consumer> consumer) { + registerListChangeListener(list, consumer); + } + + Consumer> removeListChangeListeners(ObservableList list) { + return unregisterListChangeListeners(list); + } + } + }