Bug 531792: [Tips] API cleanup

* Moved TipManager to internal
* Fixed all arising issues
* Added save read tips to preferences

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

20 files changed, 257 insertions, 153 deletions

diff --git a/org.eclipse.tips.core/src/org/eclipse/tips/core/ITipManager.java b/org.eclipse.tips.core/src/org/eclipse/tips/core/ITipManager.java
index a722914b5..52fbe3434 100644
--- a/org.eclipse.tips.core/src/org/eclipse/tips/core/ITipManager.java
+++ b/org.eclipse.tips.core/src/org/eclipse/tips/core/ITipManager.java
@@ -11,9 +11,10 @@
 package org.eclipse.tips.core;
 
 import org.eclipse.core.runtime.IStatus;
+import org.eclipse.tips.core.internal.TipManager;
 
 /**
- * The model maintained by the TipManager.
+ * The ITipManager interface.
  *
  */
 public interface ITipManager {
@@ -29,8 +30,7 @@ public interface ITipManager {
 	/**
 	 * Consults TipManager to determine the Tip's read status.
 	 *
-	 * @param tip
-	 *            the tip to query for its read status
+	 * @param tip the tip to query for its read status
 	 * @return true if the tip is read, false otherwise.
 	 */
 	public abstract boolean isRead(Tip tip);
@@ -38,8 +38,7 @@ public interface ITipManager {
 	/**
 	 * Instructs the TipManager to mark this tip as read.
 	 *
-	 * @param tip
-	 *            the tip to set as read.
+	 * @param tip the tip to set as read.
 	 * @return this
 	 */
 	public abstract ITipManager setAsRead(Tip tip);
@@ -47,8 +46,7 @@ public interface ITipManager {
 	/**
 	 * Central place of logging for the Tip Framework.
 	 *
-	 * @param status
-	 *            the {@link IStatus} which may not be null
+	 * @param status the {@link IStatus} which may not be null
 	 * @return this
 	 */
 	public ITipManager log(IStatus status);
@@ -59,16 +57,29 @@ public interface ITipManager {
 	 * {@link TipProvider#loadNewTips(org.eclipse.core.runtime.IProgressMonitor)}
 	 * method.
 	 *
-	 * @param provider
-	 *            the {@link TipProvider} to register which may not be null.
+	 * @param provider the {@link TipProvider} to register which may not be null.
 	 * @return this
 	 */
 	public ITipManager register(TipProvider provider);
+	
+	/**
+	 * Opens the Tip of the Day dialog.
+	 *
+	 * @param startUp When called from a startup situation, true must be passed for
+	 *                <code>startup</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
+	 *
+	 * @see #isOpen()
+	 */
+	public ITipManager open(boolean startUp);
 
 	/**
-	 * Returns the disposed stated.
+	 * Returns the open state.
 	 *
-	 * @return true if this manager is disposed, false otherwise.
+	 * @return true if this manager is open, false otherwise.
 	 */
-	public boolean isDisposed();
+	public boolean isOpen();
 }
\ No newline at end of file
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 4bc72b118..e5f364716 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
@@ -15,6 +15,7 @@ import java.util.ArrayList;
 import java.util.Collections;
 import java.util.Comparator;
 import java.util.List;
+import java.util.function.Predicate;
 import java.util.stream.Collectors;
 
 import org.eclipse.core.runtime.IProgressMonitor;
@@ -55,7 +56,7 @@ public abstract class TipProvider {
 
 	private ITipManager fTipManager;
 	private int fTipIndex;
-	protected List<Tip> fTips = new ArrayList<>();
+	private List<Tip> fTips = new ArrayList<>();
 	private Tip fCurrentTip;
 	private boolean fReady;
 	private PropertyChangeSupport fChangeSupport = new PropertyChangeSupport(this);
@@ -63,6 +64,22 @@ public abstract class TipProvider {
 	private String fExpression;
 
 	/**
+	 * A predicate that tests if a tip is valid based on its read state and the
+	 * requirement to only serve read tips. Subclasses may replace this predicate if
+	 * they want to add some additional tests.
+	 */
+	private Predicate<Tip> fUnreadTipPredicate = new Predicate<Tip>() {
+
+		@Override
+		public boolean test(Tip pTip) {
+			if (getManager().mustServeReadTips()) {
+				return true;
+			}
+			return !getManager().isRead(pTip);
+		}
+	};
+
+	/**
 	 * The zero argument constructor must be able to instantiate the TipProvider.
 	 * This method may also be used to quickly set the available tips by calling the
 	 * {@link #setTips(List)} method. The constructor may not be used to load tips
@@ -98,34 +115,6 @@ public abstract class TipProvider {
 	public abstract TipImage getImage();
 
 	/**
-	 * Get a list of tips. The default implementation returns tips based on the
-	 * following conditions: <br>
-	 * <dl>
-	 * <dt><code>pFilter</code> is false</dt>
-	 * <dd>Return all read and unread tips.</dd>
-	 * <dt><code>pFilter</code> is true</dt>
-	 * <dd>Return read and unread tips if the tipManager may serve unread tips,
-	 * otherwise return only unread tips.</dd>
-	 * </dl>
-	 * <p>
-	 * 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.
-	 * @return an unmodifiable list of tips.
-	 */
-	public synchronized List<Tip> getTips(boolean filter) {
-		if (filter) {
-			return Collections.unmodifiableList(fTips //
-					.stream() //
-					.filter(tip -> getManager().mustServeReadTips() || !getManager().isRead(tip)) //
-					.sorted(Comparator.comparing(Tip::getCreationDate).reversed()) //
-					.collect(Collectors.toList()));
-		}
-		return Collections.unmodifiableList(fTips);
-	}
-
-	/**
 	 * @return the {@link Tip} that was last returned by {@link #getNextTip()} or
 	 *         {@link #getPreviousTip()}
 	 */
@@ -146,12 +135,11 @@ public abstract class TipProvider {
 	 * @see #getCurrentTip()
 	 */
 	public synchronized Tip getNextTip() {
-		boolean unreadOnly = !getManager().mustServeReadTips();
-		List<Tip> list = getTips(unreadOnly);
+		List<Tip> list = getTips(fUnreadTipPredicate);
 		if (list.isEmpty()) {
 			return setCurrentTip(fFinalTip);
 		}
-		if (!unreadOnly && fCurrentTip != null) {
+		if (getManager().mustServeReadTips() && fCurrentTip != null) {
 			fTipIndex++;
 		} else if (fCurrentTip != null && getManager().isRead(fCurrentTip)) {
 			fTipIndex++;
@@ -168,7 +156,7 @@ public abstract class TipProvider {
 	 * @see #getCurrentTip()
 	 */
 	public Tip getPreviousTip() {
-		List<Tip> list = getTips(!getManager().mustServeReadTips());
+		List<Tip> list = getTips(fUnreadTipPredicate);
 		if (list.isEmpty()) {
 			return setCurrentTip(fFinalTip);
 		}
@@ -239,6 +227,35 @@ public abstract class TipProvider {
 	}
 
 	/**
+	 * A convenience method to get the list of tips based on the read status of the
+	 * tip and the requirement to serve unread or all tips.
+	 * 
+	 * @return the list of tips based on the description above
+	 * @see ITipManager#mustServeReadTips()
+	 * @see ITipManager#isRead(Tip)
+	 */
+	public List<Tip> getTips() {
+		return getTips(fUnreadTipPredicate);
+	}
+
+	/**
+	 * Get a list of tips filtered by the passed predicate.
+	 *
+	 * @param a {@link Predicate} targeting a Tip object or null to return all tips
+	 * @return an unmodifiable list of tips.
+	 */
+	public synchronized List<Tip> getTips(Predicate<Tip> predicate) {
+		if (predicate != null) {
+			return Collections.unmodifiableList(fTips //
+					.stream() //
+					.filter(tip -> predicate.test(tip)) //
+					.sorted(Comparator.comparing(Tip::getCreationDate).reversed()) //
+					.collect(Collectors.toList()));
+		}
+		return Collections.unmodifiableList(fTips);
+	}
+
+	/**
 	 * Sets the tips for this provider, replacing the current set of tips, and sets
 	 * the <code>ready</code> flag to true. This method is typically called from the
 	 * constructor of the {@link TipProvider} but may also be called from the
@@ -251,7 +268,7 @@ public abstract class TipProvider {
 	 * @see #loadNewTips(IProgressMonitor)
 	 */
 	public TipProvider setTips(List<Tip> tips) {
-		if (getManager().isDisposed()) {
+		if (!getManager().isOpen()) {
 			return this;
 		}
 		doSetTips(tips, true);
diff --git a/org.eclipse.tips.core/src/org/eclipse/tips/core/TipManager.java b/org.eclipse.tips.core/src/org/eclipse/tips/core/internal/TipManager.java
index c09da82a8..4d2a8ace4 100644
--- a/org.eclipse.tips.core/src/org/eclipse/tips/core/TipManager.java
+++ b/org.eclipse.tips.core/src/org/eclipse/tips/core/internal/TipManager.java
@@ -8,7 +8,7 @@
  * Contributors:
  *    Wim Jongman <wim.jongman@remainsoftware.com> - initial API and implementation
  *****************************************************************************/
-package org.eclipse.tips.core;
+package org.eclipse.tips.core.internal;
 
 import java.beans.PropertyChangeEvent;
 import java.beans.PropertyChangeSupport;
@@ -19,7 +19,8 @@ import java.util.List;
 import java.util.Map;
 import java.util.TreeMap;
 
-import org.eclipse.tips.core.internal.LogUtil;
+import org.eclipse.tips.core.ITipManager;
+import org.eclipse.tips.core.TipProvider;
 
 /**
  * An abstract implementation of ITipManager with additional control API. While
@@ -32,7 +33,7 @@ public abstract class TipManager implements ITipManager {
 
 	private Map<String, TipProvider> fProviders = new HashMap<>();
 	private Map<Integer, List<String>> fProviderPrio = new TreeMap<>();
-	protected boolean fOpen;
+	private boolean fOpen;
 	private boolean fServeReadTips = false;
 	private boolean fIsDiposed;
 	private PropertyChangeSupport fChangeSupport = new PropertyChangeSupport(this);
@@ -74,7 +75,7 @@ public abstract class TipManager implements ITipManager {
 		checkDisposed();
 		log(LogUtil.info("Registering provider: " + provider.getID() + " : " + provider.getDescription()));
 		provider.setManager(this);
-		addToMaps(provider, new Integer(getPriority(provider)));
+		addToMaps(provider, Integer.valueOf(getPriority(provider)));
 		provider.getChangeSupport().addPropertyChangeListener(event -> {
 			if (event.getPropertyName().equals(TipProvider.PROP_READY)) {
 				PropertyChangeEvent newEvent = new PropertyChangeEvent(this, event.getPropertyName(), null, provider);
@@ -184,20 +185,6 @@ public abstract class TipManager implements ITipManager {
 	public abstract TipManager setRunAtStartup(boolean shouldRun);
 
 	/**
-	 * 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).
-	 *
-	 * @return this
-	 *
-	 * @see #isOpen()
-	 */
-	public abstract TipManager open(boolean startUp);
-
-	/**
 	 * The default implementation disposes of this manager and all the TipProviders
 	 * when the dialog is disposed. Subclasses may override but must call super.
 	 */
@@ -219,13 +206,20 @@ public abstract class TipManager implements ITipManager {
 	}
 
 	/**
-	 * @return returns true if the tips are currently being displayed in some way.
+	 * @return true if this manager is currently open.
 	 */
+	@Override
 	public boolean isOpen() {
-		checkDisposed();
+		// checkDisposed();
+		if (isDisposed())
+			return false;
 		return fOpen;
 	}
 
+	protected void setOpen(boolean open) {
+		fOpen = open;
+	}
+
 	/**
 	 * Indicates whether read tips must be served or not. Subclasses could override,
 	 * to save the state somewhere, but must call super.
@@ -252,7 +246,6 @@ public abstract class TipManager implements ITipManager {
 		return fServeReadTips;
 	}
 
-	@Override
 	public boolean isDisposed() {
 		return fIsDiposed;
 	}
diff --git a/org.eclipse.tips.core/src/org/eclipse/tips/core/package-info.java b/org.eclipse.tips.core/src/org/eclipse/tips/core/package-info.java
index f63aef9af..f98e446bd 100644
--- a/org.eclipse.tips.core/src/org/eclipse/tips/core/package-info.java
+++ b/org.eclipse.tips.core/src/org/eclipse/tips/core/package-info.java
@@ -14,7 +14,7 @@
  * IDE you may use the tips extension point to register a new
  * {@link org.eclipse.tips.core.TipProvider}.If you want to run the Tip
  * framework outside of the IDE (e.g. in an RCP application) then you should
- * also implement your own {@link org.eclipse.tips.core.TipManager} and start
+ * also implement your own {@link org.eclipse.tips.core.internal.TipManager} and start
  * that with some action or at startup of your application.
  */
 package org.eclipse.tips.core;
\ No newline at end of file
diff --git a/org.eclipse.tips.examples/src/org/eclipse/tips/examples/swttip/SwtTipImpl.java b/org.eclipse.tips.examples/src/org/eclipse/tips/examples/swttip/SwtTipImpl.java
index faeb61c3b..6d994e4b8 100644
--- a/org.eclipse.tips.examples/src/org/eclipse/tips/examples/swttip/SwtTipImpl.java
+++ b/org.eclipse.tips.examples/src/org/eclipse/tips/examples/swttip/SwtTipImpl.java
@@ -44,7 +44,7 @@ public class SwtTipImpl extends Tip implements ISwtTip {
 
 	private String fSubject;
 
-	public SwtTipImpl(String providerId, int  number) {
+	public SwtTipImpl(String providerId, long number) {
 		super(providerId);
 		fSubject = "This is a tip " + number;
 	}
diff --git a/org.eclipse.tips.examples/src/org/eclipse/tips/examples/swttip/SwtTipsProvider.java b/org.eclipse.tips.examples/src/org/eclipse/tips/examples/swttip/SwtTipsProvider.java
index 5424fdd68..3c7280dd8 100644
--- a/org.eclipse.tips.examples/src/org/eclipse/tips/examples/swttip/SwtTipsProvider.java
+++ b/org.eclipse.tips.examples/src/org/eclipse/tips/examples/swttip/SwtTipsProvider.java
@@ -26,9 +26,10 @@ import org.eclipse.tips.core.internal.LogUtil;
 import org.osgi.framework.Bundle;
 import org.osgi.framework.FrameworkUtil;
 
+@SuppressWarnings("restriction")
 public class SwtTipsProvider extends TipProvider {
 
-	private TipImage fImage64, fImage48;
+	private TipImage fImage;
 	private int fCounter;
 	private boolean fFetching;
 
@@ -43,8 +44,8 @@ public class SwtTipsProvider extends TipProvider {
 	}
 
 	@Override
-	public synchronized List<Tip> getTips(boolean pFilter) {
-		List<Tip> tips = super.getTips(pFilter);
+	public synchronized List<Tip> getTips() {
+		List<Tip> tips = super.getTips();
 		if (tips.size() <= 1) {
 			Job job = new Job(getDescription() + " is getting more tips.") {
 
@@ -61,18 +62,17 @@ public class SwtTipsProvider extends TipProvider {
 
 	@Override
 	public TipImage getImage() {
-		if (fImage48 == null) {
+		if (fImage == null) {
 			Bundle bundle = FrameworkUtil.getBundle(getClass());
 			try {
-				fImage48 = new TipImage(bundle.getEntry("icons/48/swt.png")).setAspectRatio(1);
+				fImage = new TipImage(bundle.getEntry("icons/48/swt.png")).setAspectRatio(1);
 			} catch (IOException e) {
 				getManager().log(LogUtil.error(getClass(), e));
 			}
 		}
-		return fImage48;
+		return fImage;
 	}
 
-
 	@Override
 	public synchronized IStatus loadNewTips(IProgressMonitor pMonitor) {
 		SubMonitor subMonitor = SubMonitor.convert(pMonitor);
@@ -82,11 +82,11 @@ public class SwtTipsProvider extends TipProvider {
 		try {
 			subMonitor.beginTask("Loading Tips for " + getDescription(), -1);
 			List<Tip> tips = new ArrayList<>();
-			tips.add(new SwtTipImpl(getID(),1));
-			tips.add(new SwtTipImpl(getID(),2));
-			tips.add(new SwtTipImpl(getID(),3));
-			tips.add(new SwtTipImpl(getID(),4));
-			tips.add(new SwtTipImpl(getID(),5));
+			tips.add(new SwtTipImpl(getID(), System.currentTimeMillis() + 100));
+			tips.add(new SwtTipImpl(getID(), System.currentTimeMillis() + 200));
+			tips.add(new SwtTipImpl(getID(), System.currentTimeMillis() + 300));
+			tips.add(new SwtTipImpl(getID(), System.currentTimeMillis() + 400));
+			tips.add(new SwtTipImpl(getID(), System.currentTimeMillis() + 500));
 			addTips(tips);
 			return Status.OK_STATUS;
 		} finally {
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 a8ee1e26e..2df333370 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
@@ -12,6 +12,7 @@ package org.eclipse.tips.ide.internal;
 
 import java.io.ByteArrayInputStream;
 import java.util.ArrayList;
+import java.util.Collections;
 import java.util.List;
 
 import javax.xml.parsers.DocumentBuilderFactory;
@@ -24,17 +25,21 @@ import org.eclipse.core.runtime.IProgressMonitor;
 import org.eclipse.core.runtime.IStatus;
 import org.eclipse.core.runtime.Platform;
 import org.eclipse.core.runtime.Status;
+import org.eclipse.core.runtime.SubMonitor;
 import org.eclipse.core.runtime.jobs.Job;
+import org.eclipse.core.runtime.preferences.ConfigurationScope;
+import org.eclipse.core.runtime.preferences.IEclipsePreferences;
 import org.eclipse.tips.core.ITipManager;
 import org.eclipse.tips.core.Tip;
-import org.eclipse.tips.core.TipManager;
 import org.eclipse.tips.core.TipProvider;
 import org.eclipse.tips.core.internal.LogUtil;
-import org.eclipse.tips.ui.DefaultTipManager;
+import org.eclipse.tips.core.internal.TipManager;
+import org.eclipse.tips.ui.internal.DefaultTipManager;
 import org.eclipse.ui.PlatformUI;
 import org.eclipse.ui.services.IEvaluationService;
 import org.osgi.framework.Bundle;
 import org.osgi.framework.FrameworkUtil;
+import org.osgi.service.prefs.BackingStoreException;
 import org.w3c.dom.Document;
 import org.w3c.dom.Element;
 
@@ -57,7 +62,7 @@ public class IDETipManager extends DefaultTipManager {
 	/**
 	 * @return the tip manager instance.
 	 */
-	public static synchronized IDETipManager getInstance() {
+	public static synchronized ITipManager getInstance() {
 		if (instance.isDisposed()) {
 			instance = new IDETipManager();
 		}
@@ -86,26 +91,48 @@ public class IDETipManager extends DefaultTipManager {
 	}
 
 	@Override
-	public TipManager open(boolean startUp) {
-		 if (isOpen()) {
-		 return this;
-		 }
+	public ITipManager open(boolean startUp) {
+		if (isOpen()) {
+			return this;
+		}
 		if (!fSourceProviderAdded) {
 			IEvaluationService evaluationService = PlatformUI.getWorkbench().getService(IEvaluationService.class);
 			evaluationService.addSourceProvider(fSourceProvider);
 			fSourceProviderAdded = true;
 		}
+		fReadTips.addAll(Preferences.loadReadState());
 		return super.open(startUp);
 	}
 
 	/**
+	 * Saves the tip read status to disk.
+	 * 
+	 * @param pReadTips the tips to save
+	 *
+	 */
+	private void saveReadState(List<Integer> pReadTips) {
+		Job job = new Job("Tips save read state..") {
+			@Override
+			protected IStatus run(IProgressMonitor monitor) {
+				SubMonitor subMonitor = SubMonitor.convert(monitor, IProgressMonitor.UNKNOWN);
+				subMonitor.setTaskName("Saving read tips..");
+				IStatus status = Preferences.saveReadState(pReadTips);
+				subMonitor.done();
+				return status;
+			}
+		};
+		job.schedule();
+	}
+
+	/**
 	 * Calculates the new tip count to find if we need to expose the status trim
 	 * tool item.
 	 *
 	 * @param newTips
 	 */
-	private void refreshUI(boolean newTips) {
-		Job job = new Job("Tip of the Day..") {
+	private void refreshUI() {
+		boolean newTips = getProviders().stream().filter(p -> !p.getTips().isEmpty()).count() > 0;
+		Job job = new Job("Tips status bar refresh..") {
 			@Override
 			protected IStatus run(IProgressMonitor monitor) {
 				setNewTips(newTips);
@@ -151,7 +178,7 @@ public class IDETipManager extends DefaultTipManager {
 
 	@Override
 	public boolean isRead(Tip tip) {
-		if (fReadTips.contains(new Integer(tip.hashCode()))) {
+		if (fReadTips.contains(Integer.valueOf(tip.hashCode()))) {
 			return true;
 		}
 		return false;
@@ -160,7 +187,7 @@ public class IDETipManager extends DefaultTipManager {
 	@Override
 	public TipManager setAsRead(Tip tip) {
 		if (!isRead(tip)) {
-			fReadTips.add(new Integer(tip.hashCode()));
+			fReadTips.add(Integer.valueOf(tip.hashCode()));
 		}
 		return this;
 	}
@@ -176,8 +203,8 @@ public class IDETipManager extends DefaultTipManager {
 	@Override
 	public void dispose() {
 		try {
-			boolean newTips = getProviders().stream().filter(p -> !p.getTips(true).isEmpty()).count() > 0;
-			refreshUI(newTips);
+			refreshUI();
+			saveReadState(Collections.unmodifiableList(fReadTips));
 		} finally {
 			super.dispose();
 		}
@@ -191,8 +218,7 @@ public class IDETipManager extends DefaultTipManager {
 	 * enablement then the weight is 30. If there is a matching enablement then the
 	 * weight is 10.
 	 *
-	 * @param provider
-	 *            the provider
+	 * @param provider the provider
 	 *
 	 * @return the weight
 	 */
diff --git a/org.eclipse.tips.ide/src/org/eclipse/tips/ide/internal/Preferences.java b/org.eclipse.tips.ide/src/org/eclipse/tips/ide/internal/Preferences.java
index d03451bd0..5a08ae702 100644
--- a/org.eclipse.tips.ide/src/org/eclipse/tips/ide/internal/Preferences.java
+++ b/org.eclipse.tips.ide/src/org/eclipse/tips/ide/internal/Preferences.java
@@ -10,9 +10,17 @@
  *******************************************************************************/
 package org.eclipse.tips.ide.internal;
 
+import java.util.ArrayList;
+import java.util.List;
+
+import org.eclipse.core.runtime.IStatus;
+import org.eclipse.core.runtime.Platform;
+import org.eclipse.core.runtime.Status;
 import org.eclipse.core.runtime.preferences.AbstractPreferenceInitializer;
 import org.eclipse.core.runtime.preferences.ConfigurationScope;
 import org.eclipse.core.runtime.preferences.IEclipsePreferences;
+import org.osgi.framework.Bundle;
+import org.osgi.framework.FrameworkUtil;
 import org.osgi.service.prefs.BackingStoreException;
 
 /**
@@ -46,11 +54,59 @@ public class Preferences extends AbstractPreferenceInitializer {
 		}
 	}
 
+	/**
+	 * Loads the read tips from disk.
+	 * 
+	 * @return the integer array of read tips.
+	 */
+	public static List<Integer> loadReadState() {
+		ArrayList<Integer> result = new ArrayList<>();
+		IEclipsePreferences node = ConfigurationScope.INSTANCE.getNode("org.eclipse.tips.ide.read");
+		try {
+			for (String key : node.keys()) {
+				result.add(Integer.valueOf(node.getInt(key, 0)));
+			}
+		} catch (BackingStoreException e) {
+			Status status = new Status(IStatus.ERROR, "org.eclipse.tips.ide", e.getMessage(), e);
+			log(status);
+		}
+		return result;
+	}
+
+	/**
+	 * Saves the list with read tips to disk.
+	 * 
+	 * @param pReadTips the list with read tips
+	 * @return the status of the call
+	 */
+	public static IStatus saveReadState(List<Integer> pReadTips) {
+		try {
+			IEclipsePreferences node = ConfigurationScope.INSTANCE.getNode("org.eclipse.tips.ide.read");
+			node.clear();
+			pReadTips.forEach(value -> node.putInt(value.toString(), value.intValue()));
+			node.flush();
+			return Status.OK_STATUS;
+		} catch (BackingStoreException e) {
+			e.printStackTrace();
+			return new Status(IStatus.ERROR, "org.eclipse.tips.ide", e.getMessage(), e);
+		}
+	}
+
 	public static IEclipsePreferences getPreferences() {
 		IEclipsePreferences node = ConfigurationScope.INSTANCE.getNode(Constants.BUNDLE_ID);
 		return node;
 	}
 
+	private static void log(IStatus status) {
+		if (status.matches(IStatus.ERROR | IStatus.WARNING)) {
+			Bundle bundle = FrameworkUtil.getBundle(Preferences.class);
+			Platform.getLog(bundle).log(status);
+		}
+		if (System.getProperty("org.eclipse.tips.consolelog") != null) {
+			System.out.println(status.toString());
+		}
+	}
+
 	public static boolean isRunAtStartup() {
 		return getPreferences().getBoolean(PREF_RUN_AT_STARTUP, true);
 	}
diff --git a/org.eclipse.tips.ide/src/org/eclipse/tips/ide/internal/ProviderLoadJobChangeListener.java b/org.eclipse.tips.ide/src/org/eclipse/tips/ide/internal/ProviderLoadJobChangeListener.java
index 36af35e29..52cd60362 100644
--- a/org.eclipse.tips.ide/src/org/eclipse/tips/ide/internal/ProviderLoadJobChangeListener.java
+++ b/org.eclipse.tips.ide/src/org/eclipse/tips/ide/internal/ProviderLoadJobChangeListener.java
@@ -40,7 +40,7 @@ public class ProviderLoadJobChangeListener extends JobChangeAdapter {
 	 */
 	@Override
 	public void done(IJobChangeEvent event) {
-		for (Tip tip : fProvider.getTips(false)) {
+		for (Tip tip : fProvider.getTips(null)) {
 			if (!fManager.isRead(tip)) {
 				fManager.setNewTips(true);
 				return;
diff --git a/org.eclipse.tips.ide/src/org/eclipse/tips/ide/internal/Startup.java b/org.eclipse.tips.ide/src/org/eclipse/tips/ide/internal/Startup.java
index 8c3b67b9b..dc560e0ed 100644
--- a/org.eclipse.tips.ide/src/org/eclipse/tips/ide/internal/Startup.java
+++ b/org.eclipse.tips.ide/src/org/eclipse/tips/ide/internal/Startup.java
@@ -31,8 +31,8 @@ public class Startup implements IStartup {
 
 	@Override
 	public void earlyStartup() {
-		loadProviders();
 		openManager();
+		loadProviders();
 	}
 
 	/**
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 68bd6c94c..f198068c3 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
@@ -23,9 +23,9 @@ public class TipsHandler extends AbstractHandler {
 	@Override
 	public Object execute(ExecutionEvent event) throws ExecutionException {
 		if (!IDETipManager.getInstance().isOpen()) {
+			IDETipManager.getInstance().open(false);
 			Startup.loadProviders();
 		}
-		IDETipManager.getInstance().open(false);
 		return null;
 	}
 }
\ No newline at end of file
diff --git a/org.eclipse.tips.tests/src/org/eclipse/tips/core/TestTipManager.java b/org.eclipse.tips.tests/src/org/eclipse/tips/core/TestTipManager.java
index 83e17480b..85c016b27 100644
--- a/org.eclipse.tips.tests/src/org/eclipse/tips/core/TestTipManager.java
+++ b/org.eclipse.tips.tests/src/org/eclipse/tips/core/TestTipManager.java
@@ -15,7 +15,9 @@ import java.util.List;
 
 import org.eclipse.core.runtime.IStatus;
 import org.eclipse.core.runtime.NullProgressMonitor;
+import org.eclipse.tips.core.internal.TipManager;
 
+@SuppressWarnings("restriction")
 public class TestTipManager extends TipManager {
 
 	private boolean fShouldRun = true;
@@ -63,6 +65,7 @@ public class TestTipManager extends TipManager {
 
 	@Override
 	public TipManager open(boolean startUp) {
+		setOpen(true);
 		return this;
 	}
 
diff --git a/org.eclipse.tips.tests/src/org/eclipse/tips/core/TipManagerTest.java b/org.eclipse.tips.tests/src/org/eclipse/tips/core/TipManagerTest.java
index a7363f29d..2ef74005e 100644
--- a/org.eclipse.tips.tests/src/org/eclipse/tips/core/TipManagerTest.java
+++ b/org.eclipse.tips.tests/src/org/eclipse/tips/core/TipManagerTest.java
@@ -19,6 +19,7 @@ import org.eclipse.core.runtime.IProgressMonitor;
 import org.eclipse.core.runtime.IStatus;
 import org.eclipse.core.runtime.Status;
 import org.eclipse.tips.core.internal.LogUtil;
+import org.eclipse.tips.core.internal.TipManager;
 import org.junit.Before;
 import org.junit.Test;
 
@@ -32,6 +33,7 @@ public class TipManagerTest {
 	@Before
 	public void testTipManager() {
 		fManager = new TestTipManager();
+		fManager.open(false);
 		fProvider1 = new TestTipProvider();
 		fProvider2 = new TestTipProvider();
 	}
@@ -166,9 +168,9 @@ public class TipManagerTest {
 		fProvider1.setTips(Arrays.asList(new TestTip(fProvider1.getID(),"<b>bold</b>", "Tip 1"),
 				new TestTip(fProvider1.getID(),"<b>bold2</b>", "Tip 2")));
 		fManager.setAsRead(fProvider1.getCurrentTip());
-		assertTrue(fProvider1.getTips(true).size() + "", fProvider1.getTips(true).size() == 1);
+		assertTrue(fProvider1.getTips().size() + "", fProvider1.getTips().size() == 1);
 		fManager.setServeReadTips(true);
-		assertTrue(fProvider1.getTips(true).size() == 2);
+		assertTrue(fProvider1.getTips().size() == 2);
 	}
 
 	@Test
diff --git a/org.eclipse.tips.tests/src/org/eclipse/tips/core/TipProviderTest.java b/org.eclipse.tips.tests/src/org/eclipse/tips/core/TipProviderTest.java
index 26eba8ee8..983227280 100644
--- a/org.eclipse.tips.tests/src/org/eclipse/tips/core/TipProviderTest.java
+++ b/org.eclipse.tips.tests/src/org/eclipse/tips/core/TipProviderTest.java
@@ -20,9 +20,11 @@ import org.eclipse.core.runtime.IProgressMonitor;
 import org.eclipse.core.runtime.IStatus;
 import org.eclipse.core.runtime.NullProgressMonitor;
 import org.eclipse.core.runtime.Status;
+import org.eclipse.tips.core.internal.TipManager;
 import org.junit.Before;
 import org.junit.Test;
 
+@SuppressWarnings("restriction")
 public class TipProviderTest {
 
 	private TestTipManager fManager;
@@ -31,6 +33,7 @@ public class TipProviderTest {
 	@Before
 	public void testTipProvider() {
 		fManager = new TestTipManager();
+		fManager.open(false);
 		fProvider = (TestTipProvider) new TestTipProvider().setManager(fManager);
 	}
 
@@ -50,7 +53,6 @@ public class TipProviderTest {
 		assertTrue(fProvider.getID().equals(fProvider.getClass().getName()));
 	}
 
-
 	@Test
 	public void testGetImage() {
 		assertTrue(fProvider.getImage() != null);
@@ -58,19 +60,19 @@ public class TipProviderTest {
 
 	@Test
 	public void testGetTips() {
-		assertTrue(fProvider.getTips(false).size() == 0);
-		createTestDate();
+		assertTrue(fProvider.getTips(null).size() == 0);
+		createTestData();
 		fManager.setAsRead(fProvider.getNextTip());
-		assertTrue(fProvider.getTips(false).size() == 2);
-		assertTrue(fProvider.getTips(false).size() == 2);
-		assertTrue(fProvider.getTips(true).size() == 1);
+		assertTrue(fProvider.getTips(null).size() == 2);
+		assertTrue(fProvider.getTips(null).size() == 2);
+		assertTrue(fProvider.getTips().size() == 1);
 		((TipManager) fProvider.getManager()).setServeReadTips(true);
-		assertTrue(fProvider.getTips(false).size() == 2);
+		assertTrue(fProvider.getTips(null).size() == 2);
 	}
 
-	private void createTestDate() {
-		fProvider.setTips(Arrays.asList(new TestTip(fProvider.getID(),"<b>bold</b>", "Tip 1"),
-				new TestTip(fProvider.getID(),"<b>bold2</b>", "Tip 2")));
+	private void createTestData() {
+		fProvider.setTips(Arrays.asList(new TestTip(fProvider.getID(), "<b>bold</b>", "Tip 1"),
+				new TestTip(fProvider.getID(), "<b>bold2</b>", "Tip 2")));
 	}
 
 	@Test
@@ -85,7 +87,7 @@ public class TipProviderTest {
 
 	@Test
 	public void testGetNextTip() {
-		createTestDate();
+		createTestData();
 		fManager.setAsRead(fProvider.getNextTip());
 		assertTrue(fProvider.getNextTip().equals(fProvider.getCurrentTip()));
 		Tip nextTip = fProvider.getNextTip();
@@ -119,7 +121,7 @@ public class TipProviderTest {
 
 	@Test
 	public void testGetPreviousTip4() {
-		createTestDate();
+		createTestData();
 		assertTrue(fProvider.getPreviousTip() != null);
 		assertTrue(fProvider.getPreviousTip() != null);
 		assertTrue(fProvider.getPreviousTip() != null);
@@ -159,15 +161,15 @@ public class TipProviderTest {
 		TestTipProvider p = new TestTipProvider() {
 			@Override
 			public IStatus loadNewTips(IProgressMonitor pMonitor) {
-				assertTrue(getTips(false).size() == 0);
-				assertTrue(setTips(Arrays.asList(new Tip[] { new TestTip(getID(),"DDD", "XXX") })).getTips(false)
+				assertTrue(getTips(null).size() == 0);
+				assertTrue(setTips(Arrays.asList(new Tip[] { new TestTip(getID(), "DDD", "XXX") })).getTips(null)
 						.size() == 1);
 				return Status.OK_STATUS;
 			}
 		};
-		assertTrue(p.getTips(false).size() == 0);
+		assertTrue(p.getTips(null).size() == 0);
 		fManager.register(p);
-		assertTrue(p.getTips(false).size() == 1);
+		assertTrue(p.getTips(null).size() == 1);
 	}
 
 	@Test
@@ -175,10 +177,10 @@ public class TipProviderTest {
 		TestTipProvider p = new TestTipProvider() {
 			@Override
 			public IStatus loadNewTips(IProgressMonitor pMonitor) {
-				assertTrue(getTips(false).size() == 0);
-				assertTrue(setTips(Arrays.asList(new Tip[] { new TestTip(getID(),"DDD", "XXX") })).getTips(false)
+				assertTrue(getTips(null).size() == 0);
+				assertTrue(setTips(Arrays.asList(new Tip[] { new TestTip(getID(), "DDD", "XXX") })).getTips(null)
 						.size() == 1);
-				assertTrue(addTips(Arrays.asList(new Tip[] { new TestTip(getID(),"DDD", "XXX") })).getTips(false)
+				assertTrue(addTips(Arrays.asList(new Tip[] { new TestTip(getID(), "DDD", "XXX") })).getTips(null)
 						.size() == 2);
 				return Status.OK_STATUS;
 			}
diff --git a/org.eclipse.tips.tests/src/org/eclipse/tips/manual/tests/SleakTipManager.java b/org.eclipse.tips.tests/src/org/eclipse/tips/manual/tests/SleakTipManager.java
index b3734a962..20fa208c0 100644
--- a/org.eclipse.tips.tests/src/org/eclipse/tips/manual/tests/SleakTipManager.java
+++ b/org.eclipse.tips.tests/src/org/eclipse/tips/manual/tests/SleakTipManager.java
@@ -21,14 +21,15 @@ import org.eclipse.swt.widgets.Shell;
 import org.eclipse.tips.core.ITipManager;
 import org.eclipse.tips.core.JsonTestProvider;
 import org.eclipse.tips.core.Tip;
-import org.eclipse.tips.core.TipManager;
 import org.eclipse.tips.core.TipProvider;
+import org.eclipse.tips.core.internal.TipManager;
 import org.eclipse.tips.ui.internal.TipDialog;
 import org.eclipse.tips.ui.internal.util.ResourceManager;
 
 /**
  * Class to manage the tip providers and start the tip of the day UI.
  */
+@SuppressWarnings("restriction")
 public class SleakTipManager extends TipManager {
 
 	private static SleakTipManager instance = new SleakTipManager();
diff --git a/org.eclipse.tips.ui/META-INF/MANIFEST.MF b/org.eclipse.tips.ui/META-INF/MANIFEST.MF
index 72e9b2379..9cbcb0b51 100644
--- a/org.eclipse.tips.ui/META-INF/MANIFEST.MF
+++ b/org.eclipse.tips.ui/META-INF/MANIFEST.MF
@@ -9,5 +9,5 @@ Require-Bundle: org.eclipse.core.runtime;bundle-version="3.13.0",
  org.eclipse.tips.core;bundle-version="0.1.0"
 Export-Package: org.eclipse.tips.ui,
  org.eclipse.tips.ui.internal,
- org.eclipse.tips.ui.internal.util
+ org.eclipse.tips.ui.internal.util;x-friends:="org.eclipse.tips.tests"
 Automatic-Module-Name: org.eclipse.tips.ui
diff --git a/org.eclipse.tips.ui/src/org/eclipse/tips/ui/DefaultTipManager.java b/org.eclipse.tips.ui/src/org/eclipse/tips/ui/internal/DefaultTipManager.java
index 48b6159dd..cbb5ff483 100644
--- a/org.eclipse.tips.ui/src/org/eclipse/tips/ui/DefaultTipManager.java
+++ b/org.eclipse.tips.ui/src/org/eclipse/tips/ui/internal/DefaultTipManager.java
@@ -8,14 +8,14 @@
  * Contributors:
  *     wim.jongman@remainsoftware.com - initial API and implementation
  *******************************************************************************/
-package org.eclipse.tips.ui;
+package org.eclipse.tips.ui.internal;
 
 import org.eclipse.core.runtime.Assert;
 import org.eclipse.swt.widgets.Display;
-import org.eclipse.tips.core.TipManager;
+import org.eclipse.tips.core.ITipManager;
 import org.eclipse.tips.core.TipProvider;
 import org.eclipse.tips.core.internal.LogUtil;
-import org.eclipse.tips.ui.internal.TipDialog;
+import org.eclipse.tips.core.internal.TipManager;
 
 /**
  * Class to manage the tip providers and start the tip of the day UI.
@@ -30,40 +30,32 @@ public abstract class DefaultTipManager extends TipManager {
 	 * Opens the Tip of the Day dialog. Subclasses may override if they want to
 	 * present the Tips in a different way, e.g. in a view.
 	 *
-	 * @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).
 	 *
 	 * @see #isOpen()
 	 */
 	@Override
-	public TipManager open(boolean startUp) {
-		if (fOpen && (fTipDialog == null || fTipDialog.getShell() == null || fTipDialog.getShell().isDisposed())) {
-			fOpen = false;
-		}
+	public ITipManager open(boolean startUp) {
 		try {
-			Assert.isTrue(!fOpen, "Tip of the Day already open.");
+			Assert.isTrue(!isOpen(), "Tip of the Day already open.");
 		} catch (Exception e) {
 			log(LogUtil.error(getClass(), e));
 			throw e;
 		}
-
 		if (!mustOpen(startUp)) {
 			return this;
 		}
 
+		setOpen(true);
+
 		fTipDialog = new TipDialog(Display.getCurrent().getActiveShell(), this, TipDialog.DEFAULT_STYLE);
 		fTipDialog.open();
 		fTipDialog.getShell().addDisposeListener(pE -> {
-			try {
-				dispose();
-			} finally {
-				fOpen = false;
-			}
+			dispose();
 		});
-		fOpen = true;
 		return this;
 	}
 
@@ -74,7 +66,7 @@ public abstract class DefaultTipManager extends TipManager {
 		}
 		if (startUp && isRunAtStartup()) {
 			for (TipProvider provider : getProviders()) {
-				if (provider.isReady() && !provider.getTips(true).isEmpty()) {
+				if (provider.isReady() && !provider.getTips().isEmpty()) {
 					return true;
 				}
 			}
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 c5c7d0aa5..bf94357fe 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
@@ -29,9 +29,9 @@ import org.eclipse.swt.widgets.Button;
 import org.eclipse.swt.widgets.Composite;
 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.internal.LogUtil;
+import org.eclipse.tips.core.internal.TipManager;
 import org.eclipse.tips.ui.internal.util.ImageUtil;
 import org.eclipse.tips.ui.internal.util.ResourceManager;
 import org.eclipse.tips.ui.internal.util.SWTResourceManager;
@@ -159,7 +159,7 @@ public class Slider extends Composite {
 		int newSpacing = fSpacing + (emptyPixelsLeft / (spaceCount + 1));
 		for (int i = 0; i < Math.min(providerCount - fSliderIndex, spaceCount); i++) {
 			TipProvider provider = providers.get(i + fSliderIndex);
-			if (fSelectedProvider == null && !provider.getTips(true).isEmpty()) {
+			if (fSelectedProvider == null && !provider.getTips().isEmpty()) {
 				fSelectedProvider = provider;
 				notifyListeners(fSelectedProvider);
 			}
@@ -314,7 +314,7 @@ public class Slider extends Composite {
 
 	private void paintButton(GC gc, Composite providerButton, TipProvider provider) {
 		gc.setAdvanced(true);
-		if (fSelectedProvider.equals(provider)) {
+		if (provider.equals(fSelectedProvider)) {
 			gc.setForeground(getDisplay().getSystemColor(SWT.COLOR_LIST_SELECTION_TEXT));
 			gc.setBackground(getDisplay().getSystemColor(SWT.COLOR_LIST_SELECTION));
 			gc.drawRectangle(0, 0, fIconSize + 3, fIconSize + 3);
@@ -354,13 +354,13 @@ public class Slider extends Composite {
 	}
 
 	private Image getUnreadOverlay(Composite providerButton, TipProvider provider) {
-		if (provider.getTips(true).isEmpty()) {
+		if (provider.getTips().isEmpty()) {
 			return getProviderImage(provider, selectProviderImage(provider));
 		}
 		GC gc2 = new GC(providerButton);
 		gc2.setAdvanced(true);
 		gc2.setFont(SWTResourceManager.getBoldFont(gc2.getFont()));
-		int tipCount = provider.getTips(true).size();
+		int tipCount = provider.getTips().size();
 		Point textExtent = gc2.textExtent(tipCount + "");
 		gc2.dispose();
 
diff --git a/org.eclipse.tips.ui/src/org/eclipse/tips/ui/internal/TipComposite.java b/org.eclipse.tips.ui/src/org/eclipse/tips/ui/internal/TipComposite.java
index 57f81b032..f1a792c5f 100644
--- a/org.eclipse.tips.ui/src/org/eclipse/tips/ui/internal/TipComposite.java
+++ b/org.eclipse.tips.ui/src/org/eclipse/tips/ui/internal/TipComposite.java
@@ -48,9 +48,9 @@ import org.eclipse.tips.core.IUrlTip;
 import org.eclipse.tips.core.Tip;
 import org.eclipse.tips.core.TipAction;
 import org.eclipse.tips.core.TipImage;
-import org.eclipse.tips.core.TipManager;
 import org.eclipse.tips.core.TipProvider;
 import org.eclipse.tips.core.internal.LogUtil;
+import org.eclipse.tips.core.internal.TipManager;
 import org.eclipse.tips.ui.ISwtTip;
 import org.eclipse.tips.ui.internal.util.ImageUtil;
 import org.eclipse.tips.ui.internal.util.ResourceManager;
@@ -313,10 +313,10 @@ public class TipComposite extends Composite implements ProviderSelectionListener
 	}
 
 	private void getNextTip() {
-		if (fProvider.getTips(true).isEmpty() && !fTipManager.getProviders().isEmpty()) {
+		if (fProvider.getTips().isEmpty() && !fTipManager.getProviders().isEmpty()) {
 			fProvider.getNextTip(); // advance current tip
 			for (TipProvider provider : fTipManager.getProviders()) {
-				if (!provider.getTips(true).isEmpty()) {
+				if (!provider.getTips().isEmpty()) {
 					setProvider(provider);
 					break;
 				}
@@ -387,7 +387,7 @@ public class TipComposite extends Composite implements ProviderSelectionListener
 	 */
 	private void loadTimeOutScript() {
 		fBrowser.setText(getScaling() + getLoadingScript(500));
-		while (!getShell().isDisposed()) {
+		while (!isDisposed()) {
 			if (!getDisplay().readAndDispatch()) {
 				break;
 			}
diff --git a/org.eclipse.tips.ui/src/org/eclipse/tips/ui/internal/TipDialog.java b/org.eclipse.tips.ui/src/org/eclipse/tips/ui/internal/TipDialog.java
index fe76b7886..e1dbe7912 100644
--- a/org.eclipse.tips.ui/src/org/eclipse/tips/ui/internal/TipDialog.java
+++ b/org.eclipse.tips.ui/src/org/eclipse/tips/ui/internal/TipDialog.java
@@ -18,12 +18,13 @@ import org.eclipse.swt.layout.GridLayout;
 import org.eclipse.swt.widgets.Composite;
 import org.eclipse.swt.widgets.Control;
 import org.eclipse.swt.widgets.Shell;
-import org.eclipse.tips.core.TipManager;
+import org.eclipse.tips.core.internal.TipManager;
 
 /**
  * The dialog containing the tips.
  *
  */
+@SuppressWarnings("restriction")
 public class TipDialog extends Dialog {
 
 	/**