Bug 531792: [Tips] API cleanupI20180327-2000I20180327-0805

* Replaced listeners with property change support

Change-Id: Idf3769c16f3e1fb70971ab25cfacc4fb4db36a25
Signed-off-by: Wim Jongman <wim.jongman@remainsoftware.com>

7 files changed, 68 insertions, 174 deletions

diff --git a/org.eclipse.tips.core/src/org/eclipse/tips/core/TipManager.java b/org.eclipse.tips.core/src/org/eclipse/tips/core/TipManager.java
index 2101f160c..c09da82a8 100644
--- a/org.eclipse.tips.core/src/org/eclipse/tips/core/TipManager.java
+++ b/org.eclipse.tips.core/src/org/eclipse/tips/core/TipManager.java
@@ -10,6 +10,8 @@
  *****************************************************************************/
 package org.eclipse.tips.core;
 
+import java.beans.PropertyChangeEvent;
+import java.beans.PropertyChangeSupport;
 import java.util.ArrayList;
 import java.util.Collections;
 import java.util.HashMap;
@@ -32,8 +34,8 @@ public abstract class TipManager implements ITipManager {
 	private Map<Integer, List<String>> fProviderPrio = new TreeMap<>();
 	protected boolean fOpen;
 	private boolean fServeReadTips = false;
-	private TipProviderListenerManager fListenerManager = new TipProviderListenerManager();
 	private boolean fIsDiposed;
+	private PropertyChangeSupport fChangeSupport = new PropertyChangeSupport(this);
 
 	/**
 	 * Instantiates a new TipManager.
@@ -44,8 +46,7 @@ public abstract class TipManager implements ITipManager {
 	/**
 	 * Gets the provider with the specified ID.
 	 *
-	 * @param providerID
-	 *            the id of the provider to fetch
+	 * @param providerID the id of the provider to fetch
 	 * @return the provider with the specified ID or null if no such provider
 	 *         exists.
 	 * @see TipProvider#getID()
@@ -57,12 +58,14 @@ public abstract class TipManager implements ITipManager {
 
 	/**
 	 * Binds the passed provider to this manager. Implementations should override,
-	 * call super and the asynchronously call the
+	 * call super, and then asynchronously call the
 	 * {@link TipProvider#loadNewTips(org.eclipse.core.runtime.IProgressMonitor)}
 	 * method.
+	 * 
+	 * This manager then starts listening to the a {@link TipProvider#PROP_READY}
+	 * property change event and resends it through its own change support.
 	 *
-	 * @param provider
-	 *            the {@link TipProvider} to register.
+	 * @param provider the {@link TipProvider} to register.
 	 *
 	 * @return this
 	 */
@@ -72,11 +75,20 @@ public abstract class TipManager implements ITipManager {
 		log(LogUtil.info("Registering provider: " + provider.getID() + " : " + provider.getDescription()));
 		provider.setManager(this);
 		addToMaps(provider, new Integer(getPriority(provider)));
-		provider.getListenerManager().addProviderListener(
-				myProvider -> getListenerManager().notifyListeners(TipProviderListener.EVENT_READY, myProvider));
+		provider.getChangeSupport().addPropertyChangeListener(event -> {
+			if (event.getPropertyName().equals(TipProvider.PROP_READY)) {
+				PropertyChangeEvent newEvent = new PropertyChangeEvent(this, event.getPropertyName(), null, provider);
+				newEvent.setPropagationId(event.getPropagationId());
+				getChangeSupport().firePropertyChange(newEvent);
+			}
+		});
 		return this;
 	}
 
+	public PropertyChangeSupport getChangeSupport() {
+		return fChangeSupport;
+	}
+
 	private void checkDisposed() {
 		if (isDisposed()) {
 			throw new RuntimeException("This TipManager is disposed.");
@@ -89,8 +101,7 @@ public abstract class TipManager implements ITipManager {
 	 * {@link TipProvider#getExpression()} was purposed to aid in the calculation of
 	 * the priority.
 	 *
-	 * @param provider
-	 *            the provider
+	 * @param provider the provider
 	 * @return the priority, lower is higher, never negative.
 	 */
 	public abstract int getPriority(TipProvider provider);
@@ -163,8 +174,8 @@ public abstract class TipManager implements ITipManager {
 	/**
 	 * Determines if the Tips framework must run at startup.
 	 *
-	 * @param shouldRun
-	 *            true if the tips should be displayed at startup, false otherwise.
+	 * @param shouldRun true if the tips should be displayed at startup, false
+	 *                  otherwise.
 	 *
 	 * @return this
 	 *
@@ -175,11 +186,10 @@ public abstract class TipManager implements ITipManager {
 	/**
 	 * Opens the Tip of the Day dialog.
 	 *
-	 * @param startUp
-	 *            When called from a startup situation, true must be passed for
-	 *            <code>pStartup</code>. If in a manual starting situation, false
-	 *            must be passed. This enables the manager to decide to skip opening
-	 *            the dialog at startup (e.g., no new tip items).
+	 * @param startUp When called from a startup situation, true must be passed for
+	 *                <code>pStartup</code>. If in a manual starting situation,
+	 *                false must be passed. This enables the manager to decide to
+	 *                skip opening the dialog at startup (e.g., no new tip items).
 	 *
 	 * @return this
 	 *
@@ -220,8 +230,7 @@ public abstract class TipManager implements ITipManager {
 	 * Indicates whether read tips must be served or not. Subclasses could override,
 	 * to save the state somewhere, but must call super.
 	 *
-	 * @param serveRead
-	 *            true of read tips may be served by the {@link TipProvider}s
+	 * @param serveRead true of read tips may be served by the {@link TipProvider}s
 	 * @return this
 	 * @see TipManager#mustServeReadTips()
 	 */
@@ -243,16 +252,6 @@ public abstract class TipManager implements ITipManager {
 		return fServeReadTips;
 	}
 
-	/**
-	 * Gets the listener manager so that interested parties can subscribe to the
-	 * events of this provider.
-	 *
-	 * @return the listener manager, never null.
-	 */
-	public TipProviderListenerManager getListenerManager() {
-		return fListenerManager;
-	}
-
 	@Override
 	public boolean isDisposed() {
 		return fIsDiposed;
diff --git a/org.eclipse.tips.core/src/org/eclipse/tips/core/TipProvider.java b/org.eclipse.tips.core/src/org/eclipse/tips/core/TipProvider.java
index fde1da7b3..4bc72b118 100644
--- a/org.eclipse.tips.core/src/org/eclipse/tips/core/TipProvider.java
+++ b/org.eclipse.tips.core/src/org/eclipse/tips/core/TipProvider.java
@@ -10,6 +10,7 @@
  *****************************************************************************/
 package org.eclipse.tips.core;
 
+import java.beans.PropertyChangeSupport;
 import java.util.ArrayList;
 import java.util.Collections;
 import java.util.Comparator;
@@ -47,12 +48,17 @@ import org.eclipse.tips.core.internal.FinalTip;
  */
 public abstract class TipProvider {
 
+	/**
+	 * Ready property.
+	 */
+	public static final String PROP_READY = "PR";
+
 	private ITipManager fTipManager;
 	private int fTipIndex;
 	protected List<Tip> fTips = new ArrayList<>();
 	private Tip fCurrentTip;
 	private boolean fReady;
-	private TipProviderListenerManager fListenerManager = new TipProviderListenerManager();
+	private PropertyChangeSupport fChangeSupport = new PropertyChangeSupport(this);
 	private Tip fFinalTip = new FinalTip(getID());
 	private String fExpression;
 
@@ -85,9 +91,9 @@ public abstract class TipProvider {
 	public abstract String getID();
 
 	/**
-	 * The image used by the UI for low resolution 
+	 * The image used by the UI for low resolution
 	 *
-	 * @return a 48x48 {@link TipImage} 
+	 * @return a 48x48 {@link TipImage}
 	 */
 	public abstract TipImage getImage();
 
@@ -105,8 +111,7 @@ public abstract class TipProvider {
 	 * Subclasses may override (calling super(false) to fetch the list) if they want
 	 * to serve or sort the list of tips in a different way.
 	 *
-	 * @param filter
-	 *            false or true, see description above.
+	 * @param filter false or true, see description above.
 	 * @return an unmodifiable list of tips.
 	 */
 	public synchronized List<Tip> getTips(boolean filter) {
@@ -205,8 +210,7 @@ public abstract class TipProvider {
 	 * served from the constructor (i.e. by calling {@link #setTips(List)}), making
 	 * them available immediately
 	 *
-	 * @param monitor
-	 *            The monitor to report back progress.
+	 * @param monitor The monitor to report back progress.
 	 * @return the status in case you want to report problems.
 	 * @see TipProvider#setTips(List)
 	 * @see TipProvider#isReady()
@@ -226,8 +230,7 @@ public abstract class TipProvider {
 	 * will call this method. Subclasses may override but must not forget to call
 	 * super in order to save the {@link ITipManager}.
 	 *
-	 * @param tipManager
-	 *            the {@link ITipManager}
+	 * @param tipManager the {@link ITipManager}
 	 * @return this
 	 */
 	public synchronized TipProvider setManager(ITipManager tipManager) {
@@ -241,20 +244,19 @@ public abstract class TipProvider {
 	 * constructor of the {@link TipProvider} but may also be called from the
 	 * asynchronous {@link #loadNewTips(IProgressMonitor)} method.
 	 *
-	 * @param tips
-	 *            a list of {@link Tip} objects
+	 * @param tips a list of {@link Tip} objects
 	 * @return this
 	 * @see #addTips(List)
 	 * @see #isReady()
 	 * @see #loadNewTips(IProgressMonitor)
 	 */
 	public TipProvider setTips(List<Tip> tips) {
-		if(getManager().isDisposed()) {
+		if (getManager().isDisposed()) {
 			return this;
 		}
 		doSetTips(tips, true);
 		fReady = true;
-		fListenerManager.notifyListeners(TipProviderListener.EVENT_READY, this);
+		fChangeSupport.firePropertyChange(PROP_READY, false, true);
 		return this;
 	}
 
@@ -264,8 +266,7 @@ public abstract class TipProvider {
 	 * constructor of the {@link TipProvider} but may also be called from the
 	 * asynchronous {@link #loadNewTips(IProgressMonitor)} method.
 	 *
-	 * @param tips
-	 *            a list of {@link Tip} objects
+	 * @param tips a list of {@link Tip} objects
 	 * @return this
 	 * @see #setTips(List)
 	 * @see #isReady()
@@ -274,7 +275,7 @@ public abstract class TipProvider {
 	public TipProvider addTips(List<Tip> tips) {
 		doSetTips(tips, false);
 		fReady = true;
-		fListenerManager.notifyListeners(TipProviderListener.EVENT_READY, this);
+		fChangeSupport.firePropertyChange(PROP_READY, false, true);
 		return this;
 	}
 
@@ -286,13 +287,13 @@ public abstract class TipProvider {
 	}
 
 	/**
-	 * Gets the listener manager so that interested parties can subscribe to the
-	 * events of this provider.
+	 * Gets the change support so that interested parties can subscribe to the
+	 * property change events of this provider.
 	 *
-	 * @return the {@link TipProviderListenerManager}
+	 * @return the {@link PropertyChangeSupport}
 	 */
-	public TipProviderListenerManager getListenerManager() {
-		return fListenerManager;
+	public PropertyChangeSupport getChangeSupport() {
+		return fChangeSupport;
 	}
 
 	/**
@@ -321,8 +322,7 @@ public abstract class TipProvider {
 	/**
 	 * Sets the expression to determine the priority of the provider.
 	 *
-	 * @param expression
-	 *            the expression, may be null.
+	 * @param expression the expression, may be null.
 	 *
 	 * @see #getExpression()
 	 */
diff --git a/org.eclipse.tips.core/src/org/eclipse/tips/core/TipProviderListener.java b/org.eclipse.tips.core/src/org/eclipse/tips/core/TipProviderListener.java
deleted file mode 100644
index f9d4b59d3..000000000
--- a/org.eclipse.tips.core/src/org/eclipse/tips/core/TipProviderListener.java
+++ /dev/null
@@ -1,34 +0,0 @@
-/*******************************************************************************
- * Copyright (c) 2018 Remain Software
- * 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:
- *     wim.jongman@remainsoftware.com - initial API and implementation
- *******************************************************************************/
-package org.eclipse.tips.core;
-
-/**
- * Provides notifications on TipProvider events.
- *
- */
-@FunctionalInterface
-public interface TipProviderListener {
-
-	/**
-	 * Event ready. The TipProvider is ready to serve tips.
-	 */
-	public static final int EVENT_READY = 1;
-
-	/**
-	 * When the subject is ready to serve tips (e.g. fetched new tips from
-	 * somewhere) it will The provider is ready to serve tips. The default
-	 * implementation does nothing, subclasses may override.
-	 *
-	 * @param provider
-	 *            the provider.
-	 */
-	public void providerReady(TipProvider provider);
-}
\ No newline at end of file
diff --git a/org.eclipse.tips.core/src/org/eclipse/tips/core/TipProviderListenerManager.java b/org.eclipse.tips.core/src/org/eclipse/tips/core/TipProviderListenerManager.java
deleted file mode 100644
index 985cb0ecd..000000000
--- a/org.eclipse.tips.core/src/org/eclipse/tips/core/TipProviderListenerManager.java
+++ /dev/null
@@ -1,67 +0,0 @@
-/****************************************************************************
- * Copyright (c) 2017, 2018 Remain Software
- * 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:
- *    Wim Jongman <wim.jongman@remainsoftware.com> - initial API and implementation
- *****************************************************************************/
-package org.eclipse.tips.core;
-
-import java.util.ArrayList;
-import java.util.List;
-
-/**
- * Class to manage provider listeners.
- */
-public class TipProviderListenerManager {
-
-	protected List<TipProviderListener> fListeners = new ArrayList<>();
-
-	/**
-	 * Notifies all listeners that an event occurred.
-	 *
-	 * @param event
-	 *            the event
-	 * @param provider
-	 *            the provider
-	 */
-	public void notifyListeners(int event, TipProvider provider) {
-		synchronized (fListeners) {
-			for (TipProviderListener tipProviderListener : fListeners) {
-				switch (event) {
-				case TipProviderListener.EVENT_READY:
-					tipProviderListener.providerReady(provider);
-					break;
-				}
-			}
-		}
-	}
-
-	/**
-	 * Adds a listener to the list of listeners.
-	 *
-	 * @param pProviderListener
-	 *            the listener to be notified
-	 * @return this
-	 */
-	public TipProviderListenerManager addProviderListener(TipProviderListener pProviderListener) {
-		fListeners.remove(pProviderListener);
-		fListeners.add(pProviderListener);
-		return this;
-	}
-
-	/**
-	 * Removes a listener from the list of listeners.
-	 *
-	 * @param pProviderListener
-	 *            the listener to remove
-	 * @return this
-	 */
-	public TipProviderListenerManager removeProviderListener(TipProviderListener pProviderListener) {
-		fListeners.remove(pProviderListener);
-		return this;
-	}
-}
\ No newline at end of file
diff --git a/org.eclipse.tips.ide/src/org/eclipse/tips/ide/internal/IDETipManager.java b/org.eclipse.tips.ide/src/org/eclipse/tips/ide/internal/IDETipManager.java
index b4652412d..a8ee1e26e 100644
--- a/org.eclipse.tips.ide/src/org/eclipse/tips/ide/internal/IDETipManager.java
+++ b/org.eclipse.tips.ide/src/org/eclipse/tips/ide/internal/IDETipManager.java
@@ -87,9 +87,9 @@ public class IDETipManager extends DefaultTipManager {
 
 	@Override
 	public TipManager open(boolean startUp) {
-		// if (isOpen()) {
-		// return this;
-		// }
+		 if (isOpen()) {
+		 return this;
+		 }
 		if (!fSourceProviderAdded) {
 			IEvaluationService evaluationService = PlatformUI.getWorkbench().getService(IEvaluationService.class);
 			evaluationService.addSourceProvider(fSourceProvider);
diff --git a/org.eclipse.tips.ide/src/org/eclipse/tips/ide/internal/TipsHandler.java b/org.eclipse.tips.ide/src/org/eclipse/tips/ide/internal/TipsHandler.java
index 901823467..68bd6c94c 100644
--- a/org.eclipse.tips.ide/src/org/eclipse/tips/ide/internal/TipsHandler.java
+++ b/org.eclipse.tips.ide/src/org/eclipse/tips/ide/internal/TipsHandler.java
@@ -22,7 +22,9 @@ public class TipsHandler extends AbstractHandler {
 
 	@Override
 	public Object execute(ExecutionEvent event) throws ExecutionException {
-		Startup.loadProviders();
+		if (!IDETipManager.getInstance().isOpen()) {
+			Startup.loadProviders();
+		}
 		IDETipManager.getInstance().open(false);
 		return null;
 	}
diff --git a/org.eclipse.tips.ui/src/org/eclipse/tips/ui/internal/Slider.java b/org.eclipse.tips.ui/src/org/eclipse/tips/ui/internal/Slider.java
index a1ecb3eb7..c5c7d0aa5 100644
--- a/org.eclipse.tips.ui/src/org/eclipse/tips/ui/internal/Slider.java
+++ b/org.eclipse.tips.ui/src/org/eclipse/tips/ui/internal/Slider.java
@@ -10,6 +10,7 @@
  *******************************************************************************/
 package org.eclipse.tips.ui.internal;
 
+import java.beans.PropertyChangeListener;
 import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.HashMap;
@@ -30,7 +31,6 @@ import org.eclipse.swt.widgets.Control;
 import org.eclipse.tips.core.TipImage;
 import org.eclipse.tips.core.TipManager;
 import org.eclipse.tips.core.TipProvider;
-import org.eclipse.tips.core.TipProviderListener;
 import org.eclipse.tips.core.internal.LogUtil;
 import org.eclipse.tips.ui.internal.util.ImageUtil;
 import org.eclipse.tips.ui.internal.util.ResourceManager;
@@ -47,18 +47,16 @@ public class Slider extends Composite {
 	private TipManager fTipManager;
 	private Button fLeftButton;
 	private Button fRightButton;
-	private TipProviderListener fProviderListener;
 	private Composite fSelectedProviderButton;
 	private HashMap<String, Image> fProviderImageCache = new HashMap<>();
 	private int fIconSize = 48;
+	private PropertyChangeListener fPropertyChangeListener;
 
 	/**
 	 * Constructor for the Slider widget.
 	 *
-	 * @param parent
-	 *            the parent
-	 * @param style
-	 *            the SWT style bits
+	 * @param parent the parent
+	 * @param style  the SWT style bits
 	 */
 	public Slider(Composite parent, int style) {
 		super(parent, style);
@@ -104,7 +102,7 @@ public class Slider extends Composite {
 
 	private void setupDisposeListener() {
 		addListener(SWT.Dispose, event -> {
-			fTipManager.getListenerManager().removeProviderListener(fProviderListener);
+			fTipManager.getChangeSupport().removePropertyChangeListener(fPropertyChangeListener);
 			fProviderImageCache.values().forEach(img -> {
 				if (!img.isDisposed()) {
 					img.dispose();
@@ -114,7 +112,7 @@ public class Slider extends Composite {
 	}
 
 	private void setupProviderListener() {
-		fProviderListener = provider -> getDisplay().asyncExec(() -> load());
+		fPropertyChangeListener = provider -> getDisplay().asyncExec(() -> load());
 	}
 
 	private static Image getImage(String icon) {
@@ -230,13 +228,12 @@ public class Slider extends Composite {
 	/**
 	 * Sets the {@link TipManager}.
 	 *
-	 * @param tipManager
-	 *            the {@link TipManager}
+	 * @param tipManager the {@link TipManager}
 	 * @return this
 	 */
 	public Slider setTipManager(TipManager tipManager) {
 		fTipManager = tipManager;
-		fTipManager.getListenerManager().addProviderListener(fProviderListener);
+		fTipManager.getChangeSupport().addPropertyChangeListener(TipProvider.PROP_READY, fPropertyChangeListener);
 		fIconSize = 48;
 		load();
 		return this;
@@ -282,8 +279,7 @@ public class Slider extends Composite {
 	/**
 	 * Sets the passed provider as the selected provider in the slider.
 	 *
-	 * @param provider
-	 *            the new provider for the slider.
+	 * @param provider the new provider for the slider.
 	 *
 	 * @return this
 	 */
@@ -297,8 +293,7 @@ public class Slider extends Composite {
 	 * Adds the listener to the list of listeners to be called when an event with a
 	 * {@link TipProvider} occurs.
 	 *
-	 * @param listener
-	 *            the {@link ProviderSelectionListener}
+	 * @param listener the {@link ProviderSelectionListener}
 	 * @return this
 	 */
 	public Slider addTipProviderListener(ProviderSelectionListener listener) {
@@ -309,8 +304,7 @@ public class Slider extends Composite {
 	/**
 	 * Removes the listener from the list.
 	 *
-	 * @param listener
-	 *            the {@link ProviderSelectionListener}
+	 * @param listener the {@link ProviderSelectionListener}
 	 * @return this
 	 */
 	public Slider removeTipProviderListener(ProviderSelectionListener listener) {