diff --git a/modules/javafx.base/src/main/java/javafx/beans/binding/BooleanBinding.java b/modules/javafx.base/src/main/java/javafx/beans/binding/BooleanBinding.java index f573c59b88..aca5ac9155 100644 --- a/modules/javafx.base/src/main/java/javafx/beans/binding/BooleanBinding.java +++ b/modules/javafx.base/src/main/java/javafx/beans/binding/BooleanBinding.java @@ -67,6 +67,13 @@ public BooleanBinding() { private boolean value; private boolean valid = false; + + /** + * Invalidation listener used for observing dependencies. This + * is never cleared once created as there is no way to determine + * when all dependencies that were previously bound were removed + * in one or more calls to {@link #unbind(Observable...)}. + */ private BindingHelperObserver observer; private ExpressionHelper helper = null; @@ -119,7 +126,6 @@ protected final void unbind(Observable... dependencies) { for (final Observable dep : dependencies) { dep.removeListener(observer); } - observer = null; } } diff --git a/modules/javafx.base/src/main/java/javafx/beans/binding/DoubleBinding.java b/modules/javafx.base/src/main/java/javafx/beans/binding/DoubleBinding.java index e1ef8e3d48..8b06a90e1c 100644 --- a/modules/javafx.base/src/main/java/javafx/beans/binding/DoubleBinding.java +++ b/modules/javafx.base/src/main/java/javafx/beans/binding/DoubleBinding.java @@ -113,6 +113,13 @@ private double value; private boolean valid; + + /** + * Invalidation listener used for observing dependencies. This + * is never cleared once created as there is no way to determine + * when all dependencies that were previously bound were removed + * in one or more calls to {@link #unbind(Observable...)}. + */ private BindingHelperObserver observer; private ExpressionHelper helper = null; @@ -165,7 +172,6 @@ protected final void unbind(Observable... dependencies) { for (final Observable dep : dependencies) { dep.removeListener(observer); } - observer = null; } } diff --git a/modules/javafx.base/src/main/java/javafx/beans/binding/FloatBinding.java b/modules/javafx.base/src/main/java/javafx/beans/binding/FloatBinding.java index 52c47b4c2c..5309a558ec 100644 --- a/modules/javafx.base/src/main/java/javafx/beans/binding/FloatBinding.java +++ b/modules/javafx.base/src/main/java/javafx/beans/binding/FloatBinding.java @@ -63,6 +63,13 @@ private float value; private boolean valid; + + /** + * Invalidation listener used for observing dependencies. This + * is never cleared once created as there is no way to determine + * when all dependencies that were previously bound were removed + * in one or more calls to {@link #unbind(Observable...)}. + */ private BindingHelperObserver observer; private ExpressionHelper helper = null; @@ -115,7 +122,6 @@ protected final void unbind(Observable... dependencies) { for (final Observable dep : dependencies) { dep.removeListener(observer); } - observer = null; } } diff --git a/modules/javafx.base/src/main/java/javafx/beans/binding/IntegerBinding.java b/modules/javafx.base/src/main/java/javafx/beans/binding/IntegerBinding.java index fec04f64a3..f064dabcf9 100644 --- a/modules/javafx.base/src/main/java/javafx/beans/binding/IntegerBinding.java +++ b/modules/javafx.base/src/main/java/javafx/beans/binding/IntegerBinding.java @@ -63,6 +63,13 @@ private int value; private boolean valid = false; + + /** + * Invalidation listener used for observing dependencies. This + * is never cleared once created as there is no way to determine + * when all dependencies that were previously bound were removed + * in one or more calls to {@link #unbind(Observable...)}. + */ private BindingHelperObserver observer; private ExpressionHelper helper = null; @@ -115,7 +122,6 @@ protected final void unbind(Observable... dependencies) { for (final Observable dep : dependencies) { dep.removeListener(observer); } - observer = null; } } diff --git a/modules/javafx.base/src/main/java/javafx/beans/binding/ListBinding.java b/modules/javafx.base/src/main/java/javafx/beans/binding/ListBinding.java index 17b671b0b9..6c4a86daaa 100644 --- a/modules/javafx.base/src/main/java/javafx/beans/binding/ListBinding.java +++ b/modules/javafx.base/src/main/java/javafx/beans/binding/ListBinding.java @@ -75,6 +75,13 @@ public void onChanged(Change change) { private ObservableList value; private boolean valid = false; + + /** + * Invalidation listener used for observing dependencies. This + * is never cleared once created as there is no way to determine + * when all dependencies that were previously bound were removed + * in one or more calls to {@link #unbind(Observable...)}. + */ private BindingHelperObserver observer; private ListExpressionHelper helper = null; @@ -203,7 +210,6 @@ protected final void unbind(Observable... dependencies) { dep.removeListener(observer); } } - observer = null; } } diff --git a/modules/javafx.base/src/main/java/javafx/beans/binding/LongBinding.java b/modules/javafx.base/src/main/java/javafx/beans/binding/LongBinding.java index 1e803b20da..f6e8d68b54 100644 --- a/modules/javafx.base/src/main/java/javafx/beans/binding/LongBinding.java +++ b/modules/javafx.base/src/main/java/javafx/beans/binding/LongBinding.java @@ -63,6 +63,13 @@ private long value; private boolean valid = false; + + /** + * Invalidation listener used for observing dependencies. This + * is never cleared once created as there is no way to determine + * when all dependencies that were previously bound were removed + * in one or more calls to {@link #unbind(Observable...)}. + */ private BindingHelperObserver observer; private ExpressionHelper helper = null; @@ -115,7 +122,6 @@ protected final void unbind(Observable... dependencies) { for (final Observable dep : dependencies) { dep.removeListener(observer); } - observer = null; } } diff --git a/modules/javafx.base/src/main/java/javafx/beans/binding/ObjectBinding.java b/modules/javafx.base/src/main/java/javafx/beans/binding/ObjectBinding.java index 92a0721b3f..74e112f9bb 100644 --- a/modules/javafx.base/src/main/java/javafx/beans/binding/ObjectBinding.java +++ b/modules/javafx.base/src/main/java/javafx/beans/binding/ObjectBinding.java @@ -63,6 +63,13 @@ private T value; private boolean valid = false; + + /** + * Invalidation listener used for observing dependencies. This + * is never cleared once created as there is no way to determine + * when all dependencies that were previously bound were removed + * in one or more calls to {@link #unbind(Observable...)}. + */ private BindingHelperObserver observer; private ExpressionHelper helper = null; @@ -115,7 +122,6 @@ protected final void unbind(Observable... dependencies) { for (final Observable dep : dependencies) { dep.removeListener(observer); } - observer = null; } } diff --git a/modules/javafx.base/src/main/java/javafx/beans/binding/StringBinding.java b/modules/javafx.base/src/main/java/javafx/beans/binding/StringBinding.java index d32e3c376a..99ea9eec0c 100644 --- a/modules/javafx.base/src/main/java/javafx/beans/binding/StringBinding.java +++ b/modules/javafx.base/src/main/java/javafx/beans/binding/StringBinding.java @@ -62,6 +62,13 @@ private String value; private boolean valid = false; + + /** + * Invalidation listener used for observing dependencies. This + * is never cleared once created as there is no way to determine + * when all dependencies that were previously bound were removed + * in one or more calls to {@link #unbind(Observable...)}. + */ private BindingHelperObserver observer; private ExpressionHelper helper = null; @@ -114,7 +121,6 @@ protected final void unbind(Observable... dependencies) { for (final Observable dep : dependencies) { dep.removeListener(observer); } - observer = null; } } diff --git a/modules/javafx.base/src/test/java/test/javafx/binding/GenericBindingTest.java b/modules/javafx.base/src/test/java/test/javafx/binding/GenericBindingTest.java index 7c0f273b5c..1c8cc122d8 100644 --- a/modules/javafx.base/src/test/java/test/javafx/binding/GenericBindingTest.java +++ b/modules/javafx.base/src/test/java/test/javafx/binding/GenericBindingTest.java @@ -38,10 +38,13 @@ import org.junit.runner.RunWith; import org.junit.runners.Parameterized; +import java.lang.reflect.Constructor; import java.util.Arrays; import java.util.Collection; import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertTrue; import static org.junit.Assert.fail; @RunWith(Parameterized.class) @@ -49,32 +52,33 @@ private static final Object UNDEFINED = null; - private final ObservableStub dependency1; - private final ObservableStub dependency2; - private final BindingMock binding0; - private final BindingMock binding1; - private final BindingMock binding2; + private final ObservableStub dependency1 = new ObservableStub(); + private final ObservableStub dependency2 = new ObservableStub(); private final T value1; private final T value2; + private final Constructor> bindingMockClassConstructor; + + private BindingMock binding0; + private BindingMock binding1; + private BindingMock binding2; private InvalidationListenerMock invalidationListener; private ChangeListenerMock changeListener; public GenericBindingTest( T value1, T value2, - ObservableStub dependency1, - ObservableStub dependency2, - BindingMock binding0, BindingMock binding1, BindingMock binding2) { + Class> bindingMockClass) throws Exception { this.value1 = value1; this.value2 = value2; - this.dependency1 = dependency1; - this.dependency2 = dependency2; - this.binding0 = binding0; - this.binding1 = binding1; - this.binding2 = binding2; + this.bindingMockClassConstructor = bindingMockClass.getConstructor(Observable[].class); } @Before - public void setUp() { + public void setUp() throws Exception { + // Recreate bindings as they may have been altered by one of the tests + binding0 = bindingMockClassConstructor.newInstance((Object)new Observable[] {}); + binding1 = bindingMockClassConstructor.newInstance((Object)new Observable[] {dependency1}); + binding2 = bindingMockClassConstructor.newInstance((Object)new Observable[] {dependency1, dependency2}); + invalidationListener = new InvalidationListenerMock(); changeListener = new ChangeListenerMock(UNDEFINED); binding0.setValue(value2); @@ -292,67 +296,86 @@ public void testTwoDependencies() { assertEquals(true, binding2.isValid()); } + @Test + public void testUnbindDependencies() { + // Start by making binding valid: + binding2.getValue(); + assertTrue(binding2.isValid()); + + // Changing dependency1 should cause binding to become invalid: + dependency1.fireValueChangedEvent(); + assertFalse(binding2.isValid()); + + // Make valid again: + binding2.getValue(); + assertTrue(binding2.isValid()); + + // Changing dependency2 should cause binding to become invalid: + dependency2.fireValueChangedEvent(); + assertFalse(binding2.isValid()); + + // Make valid again: + binding2.getValue(); + assertTrue(binding2.isValid()); + + // Remove dependency1: + binding2.publicUnbind(dependency1); + + // Check that binding2 is no longer affected by changes in dependency1: + dependency1.fireValueChangedEvent(); + assertTrue(binding2.isValid()); + + // But still affected by changes in dependency2: + dependency2.fireValueChangedEvent(); + assertFalse(binding2.isValid()); + + // Make valid again: + binding2.getValue(); + assertTrue(binding2.isValid()); + + // Remove dependency2: + binding2.publicUnbind(dependency2); + + // Check that binding2 is no longer affected by changes in dependency2: + dependency2.fireValueChangedEvent(); + assertTrue(binding2.isValid()); // Fixed by 8243115 + } + @Parameterized.Parameters public static Collection parameters() { - final ObservableStub dependency1 = new ObservableStub(); - final ObservableStub dependency2 = new ObservableStub(); return Arrays.asList(new Object[][] { { Float.MIN_VALUE, Float.MAX_VALUE, - dependency1, dependency2, - new FloatBindingImpl(), - new FloatBindingImpl(dependency1), - new FloatBindingImpl(dependency1, dependency2), + FloatBindingImpl.class }, { Double.MIN_VALUE, Double.MAX_VALUE, - dependency1, dependency2, - new DoubleBindingImpl(), - new DoubleBindingImpl(dependency1), - new DoubleBindingImpl(dependency1, dependency2), + DoubleBindingImpl.class }, { Long.MIN_VALUE, Long.MAX_VALUE, - dependency1, dependency2, - new LongBindingImpl(), - new LongBindingImpl(dependency1), - new LongBindingImpl(dependency1, dependency2), + LongBindingImpl.class }, { Integer.MIN_VALUE, Integer.MAX_VALUE, - dependency1, dependency2, - new IntegerBindingImpl(), - new IntegerBindingImpl(dependency1), - new IntegerBindingImpl(dependency1, dependency2), + IntegerBindingImpl.class }, { true, false, - dependency1, dependency2, - new BooleanBindingImpl(), - new BooleanBindingImpl(dependency1), - new BooleanBindingImpl(dependency1, dependency2), + BooleanBindingImpl.class }, { "Hello World", "Goodbye", - dependency1, dependency2, - new StringBindingImpl(), - new StringBindingImpl(dependency1), - new StringBindingImpl(dependency1, dependency2), + StringBindingImpl.class }, { - new Object(), new Object(), - dependency1, dependency2, - new ObjectBindingImpl(), - new ObjectBindingImpl(dependency1), - new ObjectBindingImpl(dependency1, dependency2), + new Object(), new Object(), + ObjectBindingImpl.class }, { - FXCollections.observableArrayList(), FXCollections.observableArrayList(), - dependency1, dependency2, - new ListBindingImpl(), - new ListBindingImpl(dependency1), - new ListBindingImpl(dependency1, dependency2), - }, + FXCollections.observableArrayList(), FXCollections.observableArrayList(), + ListBindingImpl.class + } }); } @@ -369,6 +392,7 @@ public Object getValue() { int getComputeValueCounter(); void reset(); void setValue(T value); + void publicUnbind(Observable... observables); } private static class DoubleBindingImpl extends DoubleBinding implements BindingMock { @@ -401,6 +425,10 @@ public double computeValue() { fail("Should not reach here"); return null; } + + public void publicUnbind(Observable... observables) { + super.unbind(observables); + } } private static class FloatBindingImpl extends FloatBinding implements BindingMock { @@ -433,6 +461,10 @@ public float computeValue() { fail("Should not reach here"); return null; } + + public void publicUnbind(Observable... observables) { + super.unbind(observables); + } } private static class LongBindingImpl extends LongBinding implements BindingMock { @@ -465,6 +497,10 @@ public long computeValue() { fail("Should not reach here"); return null; } + + public void publicUnbind(Observable... observables) { + super.unbind(observables); + } } private static class IntegerBindingImpl extends IntegerBinding implements BindingMock { @@ -497,6 +533,10 @@ public int computeValue() { fail("Should not reach here"); return null; } + + public void publicUnbind(Observable... observables) { + super.unbind(observables); + } } private static class BooleanBindingImpl extends BooleanBinding implements BindingMock { @@ -529,6 +569,10 @@ public boolean computeValue() { fail("Should not reach here"); return null; } + + public void publicUnbind(Observable... observables) { + super.unbind(observables); + } } private static class ObjectBindingImpl extends ObjectBinding implements BindingMock { @@ -561,6 +605,10 @@ public Object computeValue() { fail("Should not reach here"); return null; } + + public void publicUnbind(Observable... observables) { + super.unbind(observables); + } } private static class StringBindingImpl extends StringBinding implements BindingMock { @@ -593,6 +641,10 @@ public String computeValue() { fail("Should not reach here"); return null; } + + public void publicUnbind(Observable... observables) { + super.unbind(observables); + } } private static class ListBindingImpl extends ListBinding implements BindingMock> { @@ -625,6 +677,10 @@ public ListBindingImpl(Observable... dep) { fail("Should not reach here"); return null; } + + public void publicUnbind(Observable... observables) { + super.unbind(observables); + } }