private boolean valueWasModifiedByDataSourceDuringCommit;
+ /**
+ * Whether this field currently listens to Property events.
+ */
+ private boolean isListening = false;
+
/* Component basics */
/*
// Saves the old value
final Object oldValue = value;
- // Stops listening the old data source changes
- if (dataSource != null
- && Property.ValueChangeNotifier.class
- .isAssignableFrom(dataSource.getClass())) {
- ((Property.ValueChangeNotifier) dataSource).removeListener(this);
- }
- if (dataSource != null
- && Property.ReadOnlyStatusChangeNotifier.class
- .isAssignableFrom(dataSource.getClass())) {
- ((Property.ReadOnlyStatusChangeNotifier) dataSource)
- .removeListener(this);
- }
+ // Stop listening to the old data source
+ removePropertyListeners();
// Sets the new data source
dataSource = newDataSource;
modified = true;
}
- // Listens the new data source if possible
- if (dataSource instanceof Property.ValueChangeNotifier) {
- ((Property.ValueChangeNotifier) dataSource).addListener(this);
- }
- if (dataSource instanceof Property.ReadOnlyStatusChangeNotifier) {
- ((Property.ReadOnlyStatusChangeNotifier) dataSource)
- .addListener(this);
- }
+ // Listen to new data source if possible
+ addPropertyListeners();
// Copy the validators from the data source
if (dataSource instanceof Validatable) {
if (actionManager != null) {
actionManager.setViewer(getWindow());
}
+ // No-op if listeners already registered
+ addPropertyListeners();
}
@Override
if (actionManager != null) {
actionManager.setViewer((Window) null);
}
+ // Stop listening to data source events on detach to avoid a potential
+ // memory leak. See #6155.
+ removePropertyListeners();
}
/**
focusable.focus();
}
}
+
+ private void addPropertyListeners() {
+ if (!isListening) {
+ if (dataSource instanceof Property.ValueChangeNotifier) {
+ ((Property.ValueChangeNotifier) dataSource).addListener(this);
+ }
+ if (dataSource instanceof Property.ReadOnlyStatusChangeNotifier) {
+ ((Property.ReadOnlyStatusChangeNotifier) dataSource)
+ .addListener(this);
+ }
+ isListening = true;
+ }
+ }
+
+ private void removePropertyListeners() {
+ if (isListening) {
+ if (dataSource instanceof Property.ValueChangeNotifier) {
+ ((Property.ValueChangeNotifier) dataSource)
+ .removeListener(this);
+ }
+ if (dataSource instanceof Property.ReadOnlyStatusChangeNotifier) {
+ ((Property.ReadOnlyStatusChangeNotifier) dataSource)
+ .removeListener(this);
+ }
+ isListening = false;
+ }
+ }
}
\ No newline at end of file
--- /dev/null
+package com.vaadin.tests.server.component.abstractfield;
+
+import static org.junit.Assert.assertEquals;
+
+import com.vaadin.data.Property;
+import com.vaadin.data.util.AbstractProperty;
+import com.vaadin.ui.AbstractField;
+
+import org.junit.Test;
+
+public class RemoveListenersOnDetach {
+
+ int numValueChanges = 0;
+ int numReadOnlyChanges = 0;
+
+ AbstractField field = new AbstractField() {
+ @Override
+ public Class<?> getType() {
+ return null;
+ }
+
+ @Override
+ public void valueChange(Property.ValueChangeEvent event) {
+ super.valueChange(event);
+ numValueChanges++;
+ }
+
+ @Override
+ public void readOnlyStatusChange(
+ Property.ReadOnlyStatusChangeEvent event) {
+ super.readOnlyStatusChange(event);
+ numReadOnlyChanges++;
+ }
+ };
+
+ Property property = new AbstractProperty() {
+ public Object getValue() {
+ return null;
+ }
+
+ public void setValue(Object newValue) throws ReadOnlyException,
+ ConversionException {
+ fireValueChange();
+ }
+
+ public Class<?> getType() {
+ return null;
+ }
+ };
+
+ @Test
+ public void testAttachDetach() {
+ field.setPropertyDataSource(property);
+
+ property.setValue(null);
+ property.setReadOnly(true);
+ assertEquals(1, numValueChanges);
+ assertEquals(1, numReadOnlyChanges);
+
+ field.attach();
+ property.setValue(null);
+ property.setReadOnly(false);
+ assertEquals(2, numValueChanges);
+ assertEquals(2, numReadOnlyChanges);
+
+ field.detach();
+ property.setValue(null);
+ property.setReadOnly(true);
+ assertEquals(2, numValueChanges);
+ assertEquals(2, numReadOnlyChanges);
+ }
+}