400230: [Property View] StringEditor.dispose() is never executed and causes memory problems

https://bugs.eclipse.org/bugs/show_bug.cgi?id=400230

7 files changed, 33 insertions, 31 deletions

diff --git a/plugins/infra/widget/org.eclipse.papyrus.infra.widgets/src/org/eclipse/papyrus/infra/widgets/editors/AbstractEditor.java b/plugins/infra/widget/org.eclipse.papyrus.infra.widgets/src/org/eclipse/papyrus/infra/widgets/editors/AbstractEditor.java
index ef26fe81227..4526632711c 100644
--- a/plugins/infra/widget/org.eclipse.papyrus.infra.widgets/src/org/eclipse/papyrus/infra/widgets/editors/AbstractEditor.java
+++ b/plugins/infra/widget/org.eclipse.papyrus.infra.widgets/src/org/eclipse/papyrus/infra/widgets/editors/AbstractEditor.java
@@ -18,6 +18,8 @@ import org.eclipse.core.databinding.Binding;
 import org.eclipse.core.databinding.DataBindingContext;

 import org.eclipse.core.databinding.conversion.IConverter;

 import org.eclipse.swt.SWT;

+import org.eclipse.swt.events.DisposeEvent;

+import org.eclipse.swt.events.DisposeListener;

 import org.eclipse.swt.events.FocusEvent;

 import org.eclipse.swt.events.FocusListener;

 import org.eclipse.swt.layout.GridData;

@@ -40,7 +42,7 @@ import org.eclipse.ui.views.properties.tabbed.TabbedPropertySheetWidgetFactory;
 //Thus, it is not possible to add custom listeners on the editors

 //We should forward the listeners to the encapsulated (this.addListener(int, Listener) -> getMainWidget().addListener(int, Listener))

 //Problem: some widgets have more than one "main widget" (e.g. EnumRadio).

-public abstract class AbstractEditor extends Composite {

+public abstract class AbstractEditor extends Composite implements DisposeListener {

 

 	/**

 	 * The label for this editor. May be null.

@@ -134,6 +136,7 @@ public abstract class AbstractEditor extends Composite {
 		if(label != null) {

 			createLabel(label);

 		}

+		parent.addDisposeListener(this);

 	}

 

 	/**

@@ -355,4 +358,9 @@ public abstract class AbstractEditor extends Composite {
 

 	@Override

 	public abstract void setToolTipText(String text);

+

+	public void widgetDisposed(DisposeEvent e) {

+		dispose();

+	}

+

 }

diff --git a/plugins/infra/widget/org.eclipse.papyrus.infra.widgets/src/org/eclipse/papyrus/infra/widgets/editors/CompactMultipleValueEditor.java b/plugins/infra/widget/org.eclipse.papyrus.infra.widgets/src/org/eclipse/papyrus/infra/widgets/editors/CompactMultipleValueEditor.java
index bda2d889c89..44c5728e5cb 100644
--- a/plugins/infra/widget/org.eclipse.papyrus.infra.widgets/src/org/eclipse/papyrus/infra/widgets/editors/CompactMultipleValueEditor.java
+++ b/plugins/infra/widget/org.eclipse.papyrus.infra.widgets/src/org/eclipse/papyrus/infra/widgets/editors/CompactMultipleValueEditor.java
@@ -162,7 +162,6 @@ public class CompactMultipleValueEditor extends AbstractListEditor implements IC
 	protected void doBinding() {

 		// We don't do a real Databinding in this case

 		modelProperty.addChangeListener(this);

-		getParent().addDisposeListener(this);

 		handleChange(null);

 	}

 

@@ -218,9 +217,12 @@ public class CompactMultipleValueEditor extends AbstractListEditor implements IC
 	/**

 	 * {@inheritDoc}

 	 */

-	public void widgetDisposed(org.eclipse.swt.events.DisposeEvent e) {

-		dispose();

-		modelProperty.removeChangeListener(this);

+	@Override

+	public void dispose() {

+		if(modelProperty != null) {

+			modelProperty.removeChangeListener(this);

+		}

+		super.dispose();

 	}

 

 	/**

diff --git a/plugins/infra/widget/org.eclipse.papyrus.infra.widgets/src/org/eclipse/papyrus/infra/widgets/editors/IntegerMask.java b/plugins/infra/widget/org.eclipse.papyrus.infra.widgets/src/org/eclipse/papyrus/infra/widgets/editors/IntegerMask.java
index f9ad94a5103..a86aa1e45f0 100644
--- a/plugins/infra/widget/org.eclipse.papyrus.infra.widgets/src/org/eclipse/papyrus/infra/widgets/editors/IntegerMask.java
+++ b/plugins/infra/widget/org.eclipse.papyrus.infra.widgets/src/org/eclipse/papyrus/infra/widgets/editors/IntegerMask.java
@@ -20,7 +20,6 @@ import org.eclipse.core.databinding.observable.IChangeListener;
 import org.eclipse.papyrus.infra.widgets.Activator;

 import org.eclipse.papyrus.infra.widgets.messages.Messages;

 import org.eclipse.swt.SWT;

-import org.eclipse.swt.events.DisposeEvent;

 import org.eclipse.swt.events.DisposeListener;

 import org.eclipse.swt.events.SelectionEvent;

 import org.eclipse.swt.events.SelectionListener;

@@ -209,8 +208,4 @@ public class IntegerMask extends AbstractValueEditor implements SelectionListene
 		refreshCheckboxes();

 	}

 

-	public void widgetDisposed(final DisposeEvent e) {

-		dispose();

-	}

-

 }

diff --git a/plugins/infra/widget/org.eclipse.papyrus.infra.widgets/src/org/eclipse/papyrus/infra/widgets/editors/MultipleStringFileEditor.java b/plugins/infra/widget/org.eclipse.papyrus.infra.widgets/src/org/eclipse/papyrus/infra/widgets/editors/MultipleStringFileEditor.java
index c55f76c6562..067debd74bd 100644
--- a/plugins/infra/widget/org.eclipse.papyrus.infra.widgets/src/org/eclipse/papyrus/infra/widgets/editors/MultipleStringFileEditor.java
+++ b/plugins/infra/widget/org.eclipse.papyrus.infra.widgets/src/org/eclipse/papyrus/infra/widgets/editors/MultipleStringFileEditor.java
@@ -1,3 +1,14 @@
+/*****************************************************************************

+ * Copyright (c) 2013 CEA LIST.

+ * 

+ * All rights reserved. This program and the accompanying materials

+ * are made available under the terms of the Eclipse Public License v1.0

+ * which accompanies this distribution, and is available at

+ * http://www.eclipse.org/legal/epl-v10.html

+ *

+ * Contributors:

+ *  Camille Letavernier (CEA LIST) camille.letavernier@cea.fr - Initial API and implementation

+ *****************************************************************************/

 package org.eclipse.papyrus.infra.widgets.editors;

 

 import java.io.File;

diff --git a/plugins/infra/widget/org.eclipse.papyrus.infra.widgets/src/org/eclipse/papyrus/infra/widgets/editors/MultipleValueEditor.java b/plugins/infra/widget/org.eclipse.papyrus.infra.widgets/src/org/eclipse/papyrus/infra/widgets/editors/MultipleValueEditor.java
index 22312525cab..5a5411df9e2 100644
--- a/plugins/infra/widget/org.eclipse.papyrus.infra.widgets/src/org/eclipse/papyrus/infra/widgets/editors/MultipleValueEditor.java
+++ b/plugins/infra/widget/org.eclipse.papyrus.infra.widgets/src/org/eclipse/papyrus/infra/widgets/editors/MultipleValueEditor.java
@@ -320,7 +320,6 @@ public class MultipleValueEditor extends AbstractListEditor implements Selection
 		//We don't do a real Databinding in this case

 		treeViewer.setInput(modelProperty);

 		modelProperty.addChangeListener(this);

-		getParent().addDisposeListener(this);

 	}

 

 	/**

@@ -572,12 +571,12 @@ public class MultipleValueEditor extends AbstractListEditor implements Selection
 		treeViewer.refresh();

 	}

 

-	/**

-	 * {@inheritDoc}

-	 */

-	public void widgetDisposed(org.eclipse.swt.events.DisposeEvent e) {

-		dispose();

-		modelProperty.removeChangeListener(this);

+	@Override

+	public void dispose() {

+		if(modelProperty != null) {

+			modelProperty.removeChangeListener(this);

+		}

+		super.dispose();

 	}

 

 	/**

diff --git a/plugins/infra/widget/org.eclipse.papyrus.infra.widgets/src/org/eclipse/papyrus/infra/widgets/editors/ReferenceDialog.java b/plugins/infra/widget/org.eclipse.papyrus.infra.widgets/src/org/eclipse/papyrus/infra/widgets/editors/ReferenceDialog.java
index 7725c8394f7..315819b315f 100644
--- a/plugins/infra/widget/org.eclipse.papyrus.infra.widgets/src/org/eclipse/papyrus/infra/widgets/editors/ReferenceDialog.java
+++ b/plugins/infra/widget/org.eclipse.papyrus.infra.widgets/src/org/eclipse/papyrus/infra/widgets/editors/ReferenceDialog.java
@@ -29,7 +29,6 @@ import org.eclipse.papyrus.infra.widgets.providers.IAdaptableContentProvider;
 import org.eclipse.papyrus.infra.widgets.providers.IStaticContentProvider;

 import org.eclipse.swt.SWT;

 import org.eclipse.swt.custom.CLabel;

-import org.eclipse.swt.events.DisposeEvent;

 import org.eclipse.swt.events.MouseEvent;

 import org.eclipse.swt.events.MouseListener;

 import org.eclipse.swt.events.SelectionEvent;

@@ -345,10 +344,6 @@ public class ReferenceDialog extends AbstractValueEditor implements SelectionLis
 		dialog.setInitialElementSelections(initialValues);

 	}

 

-	public void widgetDisposed(DisposeEvent e) {

-		dispose();

-	}

-

 	@Override

 	public void setModelObservable(IObservableValue modelProperty) {

 		setWidgetObservable(new ReferenceDialogObservableValue(this, this.currentValueLabel, modelProperty, labelProvider));

diff --git a/plugins/infra/widget/org.eclipse.papyrus.infra.widgets/src/org/eclipse/papyrus/infra/widgets/editors/StringLabel.java b/plugins/infra/widget/org.eclipse.papyrus.infra.widgets/src/org/eclipse/papyrus/infra/widgets/editors/StringLabel.java
index 3e1a383a63c..ccdf266b377 100644
--- a/plugins/infra/widget/org.eclipse.papyrus.infra.widgets/src/org/eclipse/papyrus/infra/widgets/editors/StringLabel.java
+++ b/plugins/infra/widget/org.eclipse.papyrus.infra.widgets/src/org/eclipse/papyrus/infra/widgets/editors/StringLabel.java
@@ -17,8 +17,6 @@ import org.eclipse.jface.viewers.ILabelProvider;
 import org.eclipse.jface.viewers.LabelProvider;

 import org.eclipse.papyrus.infra.widgets.Activator;

 import org.eclipse.swt.custom.CLabel;

-import org.eclipse.swt.events.DisposeEvent;

-import org.eclipse.swt.events.DisposeListener;

 import org.eclipse.swt.graphics.Image;

 import org.eclipse.swt.widgets.Composite;

 

@@ -81,13 +79,7 @@ public class StringLabel extends AbstractValueEditor implements IChangeListener
 	public void doBinding() {

 		//We don't do a real databinding here

 		this.modelProperty.addChangeListener(this);

-		valueLabel.addDisposeListener(new DisposeListener() {

-

-			public void widgetDisposed(DisposeEvent e) {

-				dispose();

-			}

-

-		});

+		valueLabel.addDisposeListener(this);

 		updateLabel();

 	}