diff options
author | Christian W. Damus | 2014-11-07 21:48:07 +0000 |
---|---|---|
committer | Christian W. Damus | 2014-11-07 22:06:18 +0000 |
commit | e6903132f559c8b3f73892be56b57aad53c9b31e (patch) | |
tree | 75112ce7cbb6b24306e4415af3a7ad759546a122 | |
parent | 9206abd34dcdc0f28de66dab5d5cd0bf0edc179e (diff) | |
download | org.eclipse.papyrus-e6903132f559c8b3f73892be56b57aad53c9b31e.tar.gz org.eclipse.papyrus-e6903132f559c8b3f73892be56b57aad53c9b31e.tar.xz org.eclipse.papyrus-e6903132f559c8b3f73892be56b57aad53c9b31e.zip |
450536: [Performance] New memory leaks in Mars M3
https://bugs.eclipse.org/bugs/show_bug.cgi?id=450536
Fix two leaks of models in the Model Explorer view:
- the UndoRedoActionProvider was not disposing its undo/redo action handlers, thus leaking them as listeners on the operation history
- the ModelExplorerPageBookView was not disposing property-sheet pages (with their own undo/redo handlers) until it, itself, was entirely disposed (not when one of its pages is disposed for closing of a model)
3 files changed, 64 insertions, 83 deletions
diff --git a/plugins/views/modelexplorer/org.eclipse.papyrus.views.modelexplorer/src/org/eclipse/papyrus/views/modelexplorer/ModelExplorerPage.java b/plugins/views/modelexplorer/org.eclipse.papyrus.views.modelexplorer/src/org/eclipse/papyrus/views/modelexplorer/ModelExplorerPage.java index d1498dd1e64..819c8c6c5cf 100644 --- a/plugins/views/modelexplorer/org.eclipse.papyrus.views.modelexplorer/src/org/eclipse/papyrus/views/modelexplorer/ModelExplorerPage.java +++ b/plugins/views/modelexplorer/org.eclipse.papyrus.views.modelexplorer/src/org/eclipse/papyrus/views/modelexplorer/ModelExplorerPage.java @@ -1,5 +1,5 @@ /*****************************************************************************
- * Copyright (c) 2010, 2014 LIFL, CEA LIST, and others.
+ * Copyright (c) 2010, 2014 LIFL, CEA LIST, Christian W. Damus, and others.
*
*
* All rights reserved. This program and the accompanying materials
@@ -10,6 +10,7 @@ * Contributors:
* Cedric Dumoulin (LIFL) cedric.dumoulin@lifl.fr - Initial API and implementation
* Christian W. Damus (CEA) - bug 434635
+ * Christian W. Damus - bug 450536
*
*****************************************************************************/
@@ -21,6 +22,8 @@ import org.eclipse.swt.widgets.Composite; import org.eclipse.swt.widgets.Control;
import org.eclipse.ui.IViewPart;
import org.eclipse.ui.IWorkbenchPart;
+import org.eclipse.ui.views.properties.IPropertySheetPage;
+import org.eclipse.ui.views.properties.tabbed.ITabbedPropertySheetPageContributor;
/**
@@ -33,12 +36,25 @@ public class ModelExplorerPage extends ViewPartPage { private SharedModelExplorerState state;
- /**
- * Constructor.
- *
- * @param part
- */
- public ModelExplorerPage() {
+ private final ITabbedPropertySheetPageContributor propertySheetContributor;
+ private IPropertySheetPage propertySheet;
+
+ public ModelExplorerPage(ITabbedPropertySheetPageContributor propertySheetContributor) {
+ super();
+
+ this.propertySheetContributor = propertySheetContributor;
+ }
+
+ @Override
+ public void dispose() {
+ try {
+ if (propertySheet != null) {
+ propertySheet.dispose();
+ }
+ } finally {
+ propertySheet = null;
+ super.dispose();
+ }
}
/**
@@ -83,4 +99,24 @@ public class ModelExplorerPage extends ViewPartPage { void setSharedState(SharedModelExplorerState state) {
this.state = state;
}
+
+ @Override
+ public Object getAdapter(@SuppressWarnings("rawtypes") Class adapter) {
+ Object result;
+
+ if (adapter == IPropertySheetPage.class) {
+ result = getPropertySheetPage();
+ } else {
+ result = super.getAdapter(adapter);
+ }
+
+ return result;
+ }
+
+ private IPropertySheetPage getPropertySheetPage() {
+ if (propertySheet == null) {
+ propertySheet = new ModelExplorerPropertySheetPage(propertySheetContributor);
+ }
+ return propertySheet;
+ }
}
diff --git a/plugins/views/modelexplorer/org.eclipse.papyrus.views.modelexplorer/src/org/eclipse/papyrus/views/modelexplorer/ModelExplorerPageBookView.java b/plugins/views/modelexplorer/org.eclipse.papyrus.views.modelexplorer/src/org/eclipse/papyrus/views/modelexplorer/ModelExplorerPageBookView.java index 5fa29922d98..3c06078c1ef 100644 --- a/plugins/views/modelexplorer/org.eclipse.papyrus.views.modelexplorer/src/org/eclipse/papyrus/views/modelexplorer/ModelExplorerPageBookView.java +++ b/plugins/views/modelexplorer/org.eclipse.papyrus.views.modelexplorer/src/org/eclipse/papyrus/views/modelexplorer/ModelExplorerPageBookView.java @@ -1,5 +1,5 @@ /***************************************************************************** - * Copyright (c) 2010, 2014 LIFL, CEA LIST, and others. + * Copyright (c) 2010, 2014 LIFL, CEA LIST, Christian W. Damus, and others. * * * All rights reserved. This program and the accompanying materials @@ -12,14 +12,12 @@ * Ansgar Radermacher (CEA) ansgar.radermacher@cea.fr - Added support for IGotoMarker * Christian W. Damus (CEA) - bug 434635 * Gabriel Pascual (ALL4TEC) gabriel.pascual@all4tec.net - Bug 431117 + * Christian W. Damus - bug 450536 * *****************************************************************************/ package org.eclipse.papyrus.views.modelexplorer; -import java.util.LinkedList; -import java.util.List; - import org.eclipse.core.resources.IMarker; import org.eclipse.emf.common.util.URI; import org.eclipse.emf.ecore.EObject; @@ -35,7 +33,6 @@ import org.eclipse.ui.IWorkbenchPart; import org.eclipse.ui.PartInitException; import org.eclipse.ui.ide.IGotoMarker; import org.eclipse.ui.navigator.CommonViewer; -import org.eclipse.ui.views.properties.IPropertySheetPage; import org.eclipse.ui.views.properties.tabbed.ITabbedPropertySheetPageContributor; import com.google.common.collect.Lists; @@ -54,9 +51,6 @@ public class ModelExplorerPageBookView extends MultiViewPageBookView implements private final SharedModelExplorerState state = new SharedModelExplorerState(); - /** The property sheet pages. */ - private List<IPropertySheetPage> propertiesSheetPages = new LinkedList<IPropertySheetPage>(); - @Override public void init(IViewSite site, IMemento memento) throws PartInitException { super.init(site, memento); @@ -66,15 +60,12 @@ public class ModelExplorerPageBookView extends MultiViewPageBookView implements } } - /** - * {@inheritDoc} - */ @Override protected PageRec doCreatePage(IWorkbenchPart part) { // part is of type IMultiDiagramEditor (because of isImportant() ) - ModelExplorerPage page = new ModelExplorerPage(); + ModelExplorerPage page = new ModelExplorerPage(this); page.setSharedState(state); // Init the page, and so the View @@ -83,29 +74,9 @@ public class ModelExplorerPageBookView extends MultiViewPageBookView implements return new PageRec(part, page); } - /** - * Retrieves the {@link IPropertySheetPage} that his Model Explorer uses. - * - * @return - */ - private IPropertySheetPage getPropertySheetPage() { - IPropertySheetPage propertySheetPage = new ModelExplorerPropertySheetPage(this); - propertiesSheetPages.add(propertySheetPage); - return propertySheetPage; - } - - /** - * {@inheritDoc} - */ @Override public Object getAdapter(@SuppressWarnings("rawtypes") Class adapter) { - if (IPropertySheetPage.class == adapter) { - // Do not test if tabbedPropertySheetPage is null before calling new - // this is managed by Eclipse which only call current method when necessary - return getPropertySheetPage(); - } - if (adapter == ITreeViewerSorting.class) { return new DefaultTreeViewerSorting() { @@ -127,17 +98,11 @@ public class ModelExplorerPageBookView extends MultiViewPageBookView implements return super.getAdapter(adapter); } - /** - * {@inheritDoc} - */ public String getContributorId() { // return Activator.PLUGIN_ID; return "TreeOutlinePage"; //$NON-NLS-1$ } - /** - * {@inheritDoc} - */ public void gotoMarker(IMarker marker) { String uriAttribute = marker.getAttribute(EValidator.URI_ATTRIBUTE, null); if (uriAttribute != null) { @@ -163,17 +128,4 @@ public class ModelExplorerPageBookView extends MultiViewPageBookView implements super.saveState(memento); } - - /** - * @see org.eclipse.ui.part.PageBookView#dispose() - * - */ - @Override - public void dispose() { - for (IPropertySheetPage page : propertiesSheetPages) { - page.dispose(); - } - propertiesSheetPages.clear(); - super.dispose(); - } } diff --git a/plugins/views/modelexplorer/org.eclipse.papyrus.views.modelexplorer/src/org/eclipse/papyrus/views/modelexplorer/actionprovider/UndoRedoActionProvider.java b/plugins/views/modelexplorer/org.eclipse.papyrus.views.modelexplorer/src/org/eclipse/papyrus/views/modelexplorer/actionprovider/UndoRedoActionProvider.java index 8d70cf8f5c4..b968376d08a 100644 --- a/plugins/views/modelexplorer/org.eclipse.papyrus.views.modelexplorer/src/org/eclipse/papyrus/views/modelexplorer/actionprovider/UndoRedoActionProvider.java +++ b/plugins/views/modelexplorer/org.eclipse.papyrus.views.modelexplorer/src/org/eclipse/papyrus/views/modelexplorer/actionprovider/UndoRedoActionProvider.java @@ -1,5 +1,5 @@ /***************************************************************************** - * Copyright (c) 2014 CEA LIST and others. + * Copyright (c) 2014 CEA LIST, Christian W. Damus, and others. * * All rights reserved. This program and the accompanying materials * are made available under the terms of the Eclipse Public License v1.0 @@ -8,6 +8,7 @@ * * Contributors: * Gabriel Pascual (ALL4TEC) gabriel.pascual@all4tec.net - Initial API and implementation + * Christian W. Damus - bug 450536 * *****************************************************************************/ @@ -21,7 +22,6 @@ import org.eclipse.emf.common.command.CommandStack; import org.eclipse.emf.common.command.CommandStackListener; import org.eclipse.emf.edit.domain.EditingDomain; import org.eclipse.emf.edit.domain.IEditingDomainProvider; -import org.eclipse.jface.action.IAction; import org.eclipse.ui.IActionBars; import org.eclipse.ui.IWorkbenchPartSite; import org.eclipse.ui.actions.ActionFactory; @@ -29,7 +29,6 @@ import org.eclipse.ui.navigator.CommonNavigator; import org.eclipse.ui.navigator.ICommonActionExtensionSite; import org.eclipse.ui.navigator.ICommonViewerSite; import org.eclipse.ui.navigator.ICommonViewerWorkbenchSite; -import org.eclipse.ui.operations.OperationHistoryActionHandler; import org.eclipse.ui.operations.RedoActionHandler; import org.eclipse.ui.operations.UndoActionHandler; @@ -43,10 +42,10 @@ public class UndoRedoActionProvider extends AbstractCommonActionProvider impleme /** The undo action. */ - IAction undoAction = null; + private UndoActionHandler undoAction = null; /** The redo action. */ - private IAction redoAction = null; + private RedoActionHandler redoAction = null; /** Listened command stack. */ private CommandStack commandStack; @@ -57,8 +56,6 @@ public class UndoRedoActionProvider extends AbstractCommonActionProvider impleme */ public UndoRedoActionProvider() { super(); - - } /** @@ -81,11 +78,6 @@ public class UndoRedoActionProvider extends AbstractCommonActionProvider impleme return commandStack; } - /** - * @see org.eclipse.ui.navigator.CommonActionProvider#init(org.eclipse.ui.navigator.ICommonActionExtensionSite) - * - * @param aSite - */ @Override public void init(ICommonActionExtensionSite aSite) { super.init(aSite); @@ -119,11 +111,6 @@ public class UndoRedoActionProvider extends AbstractCommonActionProvider impleme return context; } - /** - * @see org.eclipse.ui.actions.ActionGroup#fillActionBars(org.eclipse.ui.IActionBars) - * - * @param actionBars - */ @Override public void fillActionBars(IActionBars actionBars) { @@ -143,26 +130,32 @@ public class UndoRedoActionProvider extends AbstractCommonActionProvider impleme public void commandStackChanged(EventObject event) { // Refresh Undo handler after Command stack state has changed - if (undoAction instanceof OperationHistoryActionHandler) { - ((OperationHistoryActionHandler) undoAction).update(); + if (undoAction != null) { + undoAction.update(); } // Refresh Redo handler after Command stack state has changed - if (redoAction instanceof OperationHistoryActionHandler) { - ((OperationHistoryActionHandler) redoAction).update(); + if (redoAction != null) { + redoAction.update(); } } - /** - * @see org.eclipse.ui.actions.ActionGroup#dispose() - * - */ @Override public void dispose() { // Detach listener of command stack getCommandStack().removeCommandStackListener(this); + commandStack = null; + + if (undoAction != null) { + undoAction.dispose(); + undoAction = null; + } + if (redoAction != null) { + redoAction.dispose(); + redoAction = null; + } super.dispose(); } |