Bug 519934 - [Generic Editor] allow stateful extensionsI20170724-2000

Currently, the way extension points and registry are implemented and
used make that the same extension (ie same PresentationReconciler) is
used for all instances of the Generic Editor that match the
content-type.
As all known extensions so far manage to be stateless and as most APIs
are designed to not need the class to keep state, this wasn't much of an
issue.
However, the introduction of reconciler and some use-cases related to it
have highlighted that some classes need to be stateful, and that there
is nothing that  enforce them be be stateless. So we need the Generic
Editor to assume by default that classes are stateful, and to provide a
distinct instance of the extension for each editor instance.


Change-Id: Ibfabd88685820df81fe5c53dbe754bb34e2b521e
Signed-off-by: Mickael Istria <mistria@redhat.com>

6 files changed, 87 insertions, 113 deletions

diff --git a/org.eclipse.ui.genericeditor/src/org/eclipse/ui/internal/genericeditor/AutoEditStrategyRegistry.java b/org.eclipse.ui.genericeditor/src/org/eclipse/ui/internal/genericeditor/AutoEditStrategyRegistry.java
index 40769998a13..349f455d7ba 100644
--- a/org.eclipse.ui.genericeditor/src/org/eclipse/ui/internal/genericeditor/AutoEditStrategyRegistry.java
+++ b/org.eclipse.ui.genericeditor/src/org/eclipse/ui/internal/genericeditor/AutoEditStrategyRegistry.java
@@ -48,7 +48,6 @@ public class AutoEditStrategyRegistry {
 
 		private IConfigurationElement extension;
 		private IContentType targetContentType;
-		private IAutoEditStrategy strategy;
 
 		public AutoEditStrategyExtension(IConfigurationElement extension) throws Exception {
 			this.extension = extension;
@@ -56,15 +55,13 @@ public class AutoEditStrategyRegistry {
 					.getContentType(extension.getAttribute(CONTENT_TYPE_ATTRIBUTE));
 		}
 
-		public IAutoEditStrategy getStrategy() {
-			if (this.strategy == null) {
-				try {
-					this.strategy = (IAutoEditStrategy) extension.createExecutableExtension(CLASS_ATTRIBUTE);
-				} catch (CoreException e) {
-					e.printStackTrace();
-				}
+		public IAutoEditStrategy createStrategy() {
+			try {
+				return (IAutoEditStrategy) extension.createExecutableExtension(CLASS_ATTRIBUTE);
+			} catch (CoreException e) {
+				GenericEditorPlugin.getDefault().getLog().log(new Status(IStatus.ERROR, GenericEditorPlugin.BUNDLE_ID, e.getMessage(), e));
+				return null;
 			}
-			return strategy;
 		}
 
 		IConfigurationElement getConfigurationElement() {
@@ -102,7 +99,10 @@ public class AutoEditStrategyRegistry {
 		List<IAutoEditStrategy> res = new ArrayList<>();
 		for (AutoEditStrategyExtension ext : this.extensions.values()) {
 			if (contentTypes.contains(ext.targetContentType)) {
-				res.add(ext.getStrategy());
+				IAutoEditStrategy strategy = ext.createStrategy();
+				if (strategy != null) {
+					res.add(strategy);
+				}
 			}
 		}
 		return res;
diff --git a/org.eclipse.ui.genericeditor/src/org/eclipse/ui/internal/genericeditor/CompositeTextHover.java b/org.eclipse.ui.genericeditor/src/org/eclipse/ui/internal/genericeditor/CompositeTextHover.java
index cefebe6375c..89944fb780b 100644
--- a/org.eclipse.ui.genericeditor/src/org/eclipse/ui/internal/genericeditor/CompositeTextHover.java
+++ b/org.eclipse.ui.genericeditor/src/org/eclipse/ui/internal/genericeditor/CompositeTextHover.java
@@ -1,5 +1,5 @@
 /*******************************************************************************
- * Copyright (c) 2016 Red Hat Inc. and others.
+ * Copyright (c) 2016-2017 Red Hat Inc. and others.
  * 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
@@ -19,7 +19,6 @@ import org.eclipse.jface.text.ITextHover;
 import org.eclipse.jface.text.ITextHoverExtension;
 import org.eclipse.jface.text.ITextHoverExtension2;
 import org.eclipse.jface.text.ITextViewer;
-import org.eclipse.ui.internal.genericeditor.TextHoverRegistry.TextHoverExtension;
 
 /**
  * A text hover that delegates its operations to children
@@ -29,23 +28,22 @@ import org.eclipse.ui.internal.genericeditor.TextHoverRegistry.TextHoverExtensio
  */
 public class CompositeTextHover implements ITextHover, ITextHoverExtension, ITextHoverExtension2 {
 
-	private List<TextHoverExtension> hoversToConsider;
-	private TextHoverExtension currentHover = null;
+	private List<ITextHover> hoversToConsider;
+	private ITextHover currentHover = null;
 
-	public CompositeTextHover(List<TextHoverExtension> hoversToConsider) {
+	public CompositeTextHover(List<ITextHover> hoversToConsider) {
 		Assert.isNotNull(hoversToConsider);
 		this.hoversToConsider = hoversToConsider;
 	}
 
 	@Override
 	public Object getHoverInfo2(ITextViewer textViewer, IRegion hoverRegion) {
-		for (TextHoverExtension hover : this.hoversToConsider) {
-			ITextHover delegate = hover.getDelegate();
+		for (ITextHover hover : this.hoversToConsider) {
 			Object res = null;
-			if (delegate instanceof ITextHoverExtension2) {
-				res = ((ITextHoverExtension2)delegate).getHoverInfo2(textViewer, hoverRegion);
+			if (hover instanceof ITextHoverExtension2) {
+				res = ((ITextHoverExtension2)hover).getHoverInfo2(textViewer, hoverRegion);
 			} else {
-				res = delegate.getHoverInfo(textViewer, hoverRegion);
+				res = hover.getHoverInfo(textViewer, hoverRegion);
 			}
 			if (res != null) {
 				currentHover = hover;
@@ -57,8 +55,8 @@ public class CompositeTextHover implements ITextHover, ITextHoverExtension, ITex
 
 	@Override
 	public IInformationControlCreator getHoverControlCreator() {
-		if (this.currentHover != null) {
-			ITextHover hover = this.currentHover.getDelegate();
+		ITextHover hover = this.currentHover;
+		if (hover != null) {
 			if (hover instanceof ITextHoverExtension) {
 				return ((ITextHoverExtension)hover).getHoverControlCreator();
 			}
@@ -68,9 +66,8 @@ public class CompositeTextHover implements ITextHover, ITextHoverExtension, ITex
 
 	@Override
 	public String getHoverInfo(ITextViewer textViewer, IRegion hoverRegion) {
-		for (TextHoverExtension hover : this.hoversToConsider) {
-			ITextHover delegate = hover.getDelegate();
-			String res = delegate.getHoverInfo(textViewer, hoverRegion);
+		for (ITextHover hover : this.hoversToConsider) {
+			String res = hover.getHoverInfo(textViewer, hoverRegion);
 			if (res != null) {
 				currentHover = hover;
 				return res;
@@ -81,9 +78,8 @@ public class CompositeTextHover implements ITextHover, ITextHoverExtension, ITex
 
 	@Override
 	public IRegion getHoverRegion(ITextViewer textViewer, int offset) {
-		for (TextHoverExtension hover : this.hoversToConsider) {
-			ITextHover delegate = hover.getDelegate();
-			IRegion res = delegate.getHoverRegion(textViewer, offset);
+		for (ITextHover hover : this.hoversToConsider) {
+			IRegion res = hover.getHoverRegion(textViewer, offset);
 			if (res != null) {
 				return res;
 			}
diff --git a/org.eclipse.ui.genericeditor/src/org/eclipse/ui/internal/genericeditor/ContentAssistProcessorRegistry.java b/org.eclipse.ui.genericeditor/src/org/eclipse/ui/internal/genericeditor/ContentAssistProcessorRegistry.java
index 1ce07367e52..0da557bfef7 100644
--- a/org.eclipse.ui.genericeditor/src/org/eclipse/ui/internal/genericeditor/ContentAssistProcessorRegistry.java
+++ b/org.eclipse.ui.genericeditor/src/org/eclipse/ui/internal/genericeditor/ContentAssistProcessorRegistry.java
@@ -1,5 +1,5 @@
 /*******************************************************************************
- * Copyright (c) 2016 Red Hat Inc. and others.
+ * Copyright (c) 2016-2017 Red Hat Inc. and others.
  * 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
@@ -45,37 +45,14 @@ import org.eclipse.ui.PlatformUI;
 public class ContentAssistProcessorRegistry {
 
 	private static final String EXTENSION_POINT_ID = GenericEditorPlugin.BUNDLE_ID + ".contentAssistProcessors"; //$NON-NLS-1$
-	
-	/**
-	 * This class wraps and proxies an {@link IContentAssistProcessor} provided through extensions
-	 * and loads it lazily when it can contribute to the editor, then delegates all operations to
-	 * actual processor.
-	 * When the contribution cannot contribute to the editor, this wrapper will return neutral values
-	 * that don't affect editor behavior.
-	 */
-	private static class ContentAssistProcessorExtension implements IContentAssistProcessor {
-		private static final String CONTENT_TYPE_ATTRIBUTE = "contentType"; //$NON-NLS-1$
-		private static final String CLASS_ATTRIBUTE = "class"; //$NON-NLS-1$
 
-		private IConfigurationElement extension;
+	static class ContentAssistProcessorDelegate implements IContentAssistProcessor {
+		private final IContentAssistProcessor delegate;
 		private IContentType targetContentType;
 
-		private IContentAssistProcessor delegate;
-
-		private ContentAssistProcessorExtension(IConfigurationElement element) throws Exception {
-			this.extension = element;
-			this.targetContentType = Platform.getContentTypeManager().getContentType(element.getAttribute(CONTENT_TYPE_ATTRIBUTE));
-		}
-
-		private IContentAssistProcessor getDelegate() {
-			if (this.delegate == null) {
-				try {
-					this.delegate = (IContentAssistProcessor) extension.createExecutableExtension(CLASS_ATTRIBUTE);
-				} catch (CoreException e) {
-					e.printStackTrace();
-				}
-			}
-			return delegate;
+		public ContentAssistProcessorDelegate(IContentAssistProcessor delegate, IContentType targetContentType) {
+			this.delegate = delegate;
+			this.targetContentType = targetContentType;
 		}
 
 		/**
@@ -104,7 +81,7 @@ public class ContentAssistProcessorRegistry {
 		@Override
 		public ICompletionProposal[] computeCompletionProposals(ITextViewer viewer, int offset) {
 			if (isActive(viewer)) {
-				return getDelegate().computeCompletionProposals(viewer, offset);
+				return delegate.computeCompletionProposals(viewer, offset);
 			}
 			return new ICompletionProposal[0];
 		}
@@ -112,7 +89,7 @@ public class ContentAssistProcessorRegistry {
 		@Override
 		public IContextInformation[] computeContextInformation(ITextViewer viewer, int offset) {
 			if (isActive(viewer)) {
-				return getDelegate().computeContextInformation(viewer, offset);
+				return delegate.computeContextInformation(viewer, offset);
 			}
 			return new IContextInformation[0];
 		}
@@ -120,7 +97,7 @@ public class ContentAssistProcessorRegistry {
 		@Override
 		public char[] getCompletionProposalAutoActivationCharacters() {
 			if (isActive(null)) {
-				return getDelegate().getCompletionProposalAutoActivationCharacters();
+				return delegate.getCompletionProposalAutoActivationCharacters();
 			}
 			return null;
 		}
@@ -128,7 +105,7 @@ public class ContentAssistProcessorRegistry {
 		@Override
 		public char[] getContextInformationAutoActivationCharacters() {
 			if (isActive(null)) {
-				return getDelegate().getContextInformationAutoActivationCharacters();
+				return delegate.getContextInformationAutoActivationCharacters();
 			}
 			return null;
 		}
@@ -136,7 +113,7 @@ public class ContentAssistProcessorRegistry {
 		@Override
 		public String getErrorMessage() {
 			if (isActive(null)) {
-				return getDelegate().getErrorMessage();
+				return delegate.getErrorMessage();
 			}
 			return null;
 		}
@@ -144,7 +121,39 @@ public class ContentAssistProcessorRegistry {
 		@Override
 		public IContextInformationValidator getContextInformationValidator() {
 			if (isActive(null)) {
-				return getDelegate().getContextInformationValidator();
+				return delegate.getContextInformationValidator();
+			}
+			return null;
+		}
+	}
+
+	/**
+	 * This class wraps and proxies an {@link IContentAssistProcessor} provided through extensions
+	 * and loads it lazily when it can contribute to the editor, then delegates all operations to
+	 * actual processor.
+	 * When the contribution cannot contribute to the editor, this wrapper will return neutral values
+	 * that don't affect editor behavior.
+	 */
+	private static class ContentAssistProcessorExtension {
+		private static final String CONTENT_TYPE_ATTRIBUTE = "contentType"; //$NON-NLS-1$
+		private static final String CLASS_ATTRIBUTE = "class"; //$NON-NLS-1$
+
+		private IConfigurationElement extension;
+		private IContentType targetContentType;
+
+		private ContentAssistProcessorExtension(IConfigurationElement element) throws Exception {
+			this.extension = element;
+			this.targetContentType = Platform.getContentTypeManager().getContentType(element.getAttribute(CONTENT_TYPE_ATTRIBUTE));
+		}
+
+		public ContentAssistProcessorDelegate createDelegate() {
+			try {
+				IContentAssistProcessor delegate = (IContentAssistProcessor) extension.createExecutableExtension(CLASS_ATTRIBUTE);
+				if (delegate != null) {
+					return new ContentAssistProcessorDelegate(delegate, targetContentType);
+				}
+			} catch (CoreException e) {
+				GenericEditorPlugin.getDefault().getLog().log(new Status(IStatus.ERROR, GenericEditorPlugin.BUNDLE_ID, e.getMessage(), e));
 			}
 			return null;
 		}
@@ -179,7 +188,7 @@ public class ContentAssistProcessorRegistry {
 		List<IContentAssistProcessor> res = new ArrayList<>();
 		for (ContentAssistProcessorExtension ext : this.extensions.values()) {
 			if (contentTypes.contains(ext.targetContentType)) {
-				res.add(ext);
+				res.add(ext.createDelegate());
 			}
 		}
 		return res;
diff --git a/org.eclipse.ui.genericeditor/src/org/eclipse/ui/internal/genericeditor/ExtensionBasedTextEditor.java b/org.eclipse.ui.genericeditor/src/org/eclipse/ui/internal/genericeditor/ExtensionBasedTextEditor.java
index dbdc3b840ef..e0b1253c715 100644
--- a/org.eclipse.ui.genericeditor/src/org/eclipse/ui/internal/genericeditor/ExtensionBasedTextEditor.java
+++ b/org.eclipse.ui.genericeditor/src/org/eclipse/ui/internal/genericeditor/ExtensionBasedTextEditor.java
@@ -1,5 +1,5 @@
 /*******************************************************************************
- * Copyright (c) 2000, 2016 Red Hat Inc. and others.
+ * Copyright (c) 2000, 2017 Red Hat Inc. and others.
  * 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
@@ -70,4 +70,5 @@ public class ExtensionBasedTextEditor extends TextEditor {
 		new ProjectionSupport(viewer,getAnnotationAccess(),getSharedColors()).install();
 		viewer.doOperation(ProjectionViewer.TOGGLE);
 	}
+
 }
diff --git a/org.eclipse.ui.genericeditor/src/org/eclipse/ui/internal/genericeditor/PresentationReconcilerRegistry.java b/org.eclipse.ui.genericeditor/src/org/eclipse/ui/internal/genericeditor/PresentationReconcilerRegistry.java
index b973b816cf2..b1692332f9b 100644
--- a/org.eclipse.ui.genericeditor/src/org/eclipse/ui/internal/genericeditor/PresentationReconcilerRegistry.java
+++ b/org.eclipse.ui.genericeditor/src/org/eclipse/ui/internal/genericeditor/PresentationReconcilerRegistry.java
@@ -1,5 +1,5 @@
 /*******************************************************************************
- * Copyright (c) 2016 Red Hat Inc. and others.
+ * Copyright (c) 2016-2017 Red Hat Inc. and others.
  * 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
@@ -25,10 +25,7 @@ import org.eclipse.core.runtime.IStatus;
 import org.eclipse.core.runtime.Platform;
 import org.eclipse.core.runtime.Status;
 import org.eclipse.core.runtime.content.IContentType;
-import org.eclipse.jface.text.ITextViewer;
-import org.eclipse.jface.text.presentation.IPresentationDamager;
 import org.eclipse.jface.text.presentation.IPresentationReconciler;
-import org.eclipse.jface.text.presentation.IPresentationRepairer;
 import org.eclipse.jface.text.source.ISourceViewer;
 
 /**
@@ -46,52 +43,26 @@ public class PresentationReconcilerRegistry {
 	 * and loads it lazily when it can contribute to the editor, then delegates all operations to
 	 * actual reconcilier.
 	 */
-	private static class PresentationReconcilerExtension implements IPresentationReconciler {
+	private static class PresentationReconcilerExtension {
 		private static final String CLASS_ATTRIBUTE = "class"; //$NON-NLS-1$
 		private static final String CONTENT_TYPE_ATTRIBUTE = "contentType"; //$NON-NLS-1$
 
 		private IConfigurationElement extension;
 		private IContentType targetContentType;
 
-		private IPresentationReconciler delegate;
-
 		private PresentationReconcilerExtension(IConfigurationElement element) throws Exception {
 			this.extension = element;
 			this.targetContentType = Platform.getContentTypeManager().getContentType(element.getAttribute(CONTENT_TYPE_ATTRIBUTE));
 		}
 
-		private IPresentationReconciler getDelegate() {
-			if (this.delegate == null) {
-				try {
-					this.delegate = (IPresentationReconciler) extension.createExecutableExtension(CLASS_ATTRIBUTE);
-				} catch (CoreException e) {
-					e.printStackTrace();
-				}
+		public IPresentationReconciler createDelegate() {
+			try {
+				return (IPresentationReconciler) extension.createExecutableExtension(CLASS_ATTRIBUTE);
+			} catch (CoreException e) {
+				GenericEditorPlugin.getDefault().getLog().log(new Status(IStatus.ERROR, GenericEditorPlugin.BUNDLE_ID, e.getMessage(), e));
 			}
-			return delegate;
-		}
-
-		@Override
-		public void install(ITextViewer viewer) {
-			getDelegate().install(viewer);
-		}
-
-		@Override
-		public void uninstall() {
-			getDelegate().uninstall();
+			return null;
 		}
-
-		@Override
-		public IPresentationDamager getDamager(String contentType) {
-			return getDelegate().getDamager(contentType);
-
-		}
-
-		@Override
-		public IPresentationRepairer getRepairer(String contentType) {
-			return getDelegate().getRepairer(contentType);
-		}
-
 	}
 	private Map<IConfigurationElement, PresentationReconcilerExtension> extensions = new HashMap<>();
 	private boolean outOfSync = true;
@@ -122,7 +93,7 @@ public class PresentationReconcilerRegistry {
 		List<IPresentationReconciler> res = new ArrayList<>();
 		for (PresentationReconcilerExtension ext : this.extensions.values()) {
 			if (contentTypes.contains(ext.targetContentType)) {
-				res.add(ext);
+				res.add(ext.createDelegate());
 			}
 		}
 		return res;
diff --git a/org.eclipse.ui.genericeditor/src/org/eclipse/ui/internal/genericeditor/TextHoverRegistry.java b/org.eclipse.ui.genericeditor/src/org/eclipse/ui/internal/genericeditor/TextHoverRegistry.java
index d6e83720ed6..0e572876762 100644
--- a/org.eclipse.ui.genericeditor/src/org/eclipse/ui/internal/genericeditor/TextHoverRegistry.java
+++ b/org.eclipse.ui.genericeditor/src/org/eclipse/ui/internal/genericeditor/TextHoverRegistry.java
@@ -56,7 +56,6 @@ public final class TextHoverRegistry {
 
 		private IConfigurationElement extension;
 		private IContentType targetContentType;
-		private ITextHover delegate;
 		private String id;
 		private String isBefore;
 		private String isAfter;
@@ -69,15 +68,13 @@ public final class TextHoverRegistry {
 			this.isAfter = extension.getAttribute(IS_AFTER_ATTRIBUTE);
 		}
 
-		public ITextHover getDelegate() {
-			if (this.delegate == null) {
-				try {
-					this.delegate = (ITextHover) extension.createExecutableExtension(CLASS_ATTRIBUTE);
-				} catch (CoreException e) {
-					e.printStackTrace();
-				}
+		public ITextHover createDelegate() {
+			try {
+				return (ITextHover) extension.createExecutableExtension(CLASS_ATTRIBUTE);
+			} catch (CoreException e) {
+				GenericEditorPlugin.getDefault().getLog().log(new Status(IStatus.ERROR, GenericEditorPlugin.BUNDLE_ID, e.getMessage(), e));
 			}
-			return delegate;
+			return null;
 		}
 
 		public String getId() {
@@ -120,7 +117,7 @@ public final class TextHoverRegistry {
 			}
 		}
 		if (!hoversToConsider.isEmpty()) {
-			return new CompositeTextHover(hoversToConsider);
+			return new CompositeTextHover(hoversToConsider.stream().map(TextHoverExtension::createDelegate).collect(Collectors.toList()));
 		}
 		return null;
 	}