Merge "Fix bugs with the new lightweight decorator for RepositoryTreeNodes"
diff --git a/org.eclipse.egit.ui/src/org/eclipse/egit/ui/internal/repository/RepositoryTreeNodeDecorator.java b/org.eclipse.egit.ui/src/org/eclipse/egit/ui/internal/repository/RepositoryTreeNodeDecorator.java
index 206a2de..139f990 100644
--- a/org.eclipse.egit.ui/src/org/eclipse/egit/ui/internal/repository/RepositoryTreeNodeDecorator.java
+++ b/org.eclipse.egit.ui/src/org/eclipse/egit/ui/internal/repository/RepositoryTreeNodeDecorator.java
@@ -19,7 +19,6 @@
 import org.eclipse.egit.ui.Activator;
 import org.eclipse.egit.ui.internal.CommonUtils;
 import org.eclipse.egit.ui.internal.GitLabels;
-import org.eclipse.egit.ui.internal.UIIcons;
 import org.eclipse.egit.ui.internal.UIText;
 import org.eclipse.egit.ui.internal.decorators.GitDecorator;
 import org.eclipse.egit.ui.internal.repository.tree.AdditionalRefNode;
@@ -101,7 +100,6 @@
 		if (repository != null) {
 			try {
 				decorateText(node, repository, decoration);
-				decorateImage(node, repository, decoration);
 			} catch (IOException e) {
 				Activator.logError(MessageFormat.format(
 						UIText.GitLabelProvider_UnableToRetrieveLabel,
@@ -113,31 +111,38 @@
 	private void decorateText(RepositoryTreeNode<?> node,
 			@NonNull Repository repository, IDecoration decoration)
 			throws IOException {
+		boolean decorated = false;
 		switch (node.getType()) {
 		case REPO:
-			decorateRepository(node, repository, decoration);
+			decorated = decorateRepository(node, repository, decoration);
 			break;
 		case ADDITIONALREF:
-			decorateAdditionalRef((AdditionalRefNode) node, decoration);
+			decorated = decorateAdditionalRef((AdditionalRefNode) node,
+					decoration);
 			break;
 		case REF:
-			decorateRef((RefNode) node, decoration);
+			decorated = decorateRef((RefNode) node, decoration);
 			break;
 		case TAG:
-			decorateTag((TagNode) node, decoration);
+			decorated = decorateTag((TagNode) node, decoration);
 			break;
 		case STASHED_COMMIT:
-			decorateStash((StashedCommitNode) node, decoration);
+			decorated = decorateStash((StashedCommitNode) node, decoration);
 			break;
 		case SUBMODULES:
-			decorateSubmodules(repository, decoration);
+			decorated = decorateSubmodules(repository, decoration);
 			break;
 		default:
-			break;
+			return;
+		}
+		if (!decorated) {
+			// Ensure the caching of last labels in
+			// RepositoryTreeNodeLabelProvider works
+			decoration.addSuffix(" "); //$NON-NLS-1$
 		}
 	}
 
-	private void decorateAdditionalRef(AdditionalRefNode node,
+	private boolean decorateAdditionalRef(AdditionalRefNode node,
 			IDecoration decoration) {
 		Ref ref = node.getObject();
 		StringBuilder suffix = new StringBuilder();
@@ -157,19 +162,22 @@
 					UIText.RepositoriesViewLabelProvider_UnbornBranchText);
 		}
 		decoration.addSuffix(suffix.toString());
+		return true;
 	}
 
-	private void decorateRef(RefNode node, IDecoration decoration) {
+	private boolean decorateRef(RefNode node, IDecoration decoration) {
 		if (verboseBranchMode) {
 			RevCommit latest = getLatestCommit(node);
 			if (latest != null) {
 				decoration.addSuffix(" " + abbreviate(latest) + ' ' //$NON-NLS-1$
 						+ latest.getShortMessage());
+				return true;
 			}
 		}
+		return false;
 	}
 
-	private void decorateRepository(RepositoryTreeNode<?> node,
+	private boolean decorateRepository(RepositoryTreeNode<?> node,
 			@NonNull Repository repository, IDecoration decoration)
 			throws IOException {
 		boolean isSubModule = node.getParent() != null && node.getParent()
@@ -181,7 +189,7 @@
 		if (isSubModule) {
 			Ref head = repository.exactRef(Constants.HEAD);
 			if (head == null) {
-				return;
+				return false;
 			}
 			suffix.append(" ["); //$NON-NLS-1$
 			if (head.isSymbolic()) {
@@ -204,7 +212,7 @@
 			String branch = Activator.getDefault().getRepositoryUtil()
 					.getShortBranch(repository);
 			if (branch == null) {
-				return;
+				return false;
 			}
 			suffix.append(" ["); //$NON-NLS-1$
 			suffix.append(branch);
@@ -226,100 +234,34 @@
 			suffix.append(']');
 		}
 		decoration.addSuffix(suffix.toString());
+		return true;
 	}
 
-	private void decorateStash(StashedCommitNode node, IDecoration decoration) {
+	private boolean decorateStash(StashedCommitNode node,
+			IDecoration decoration) {
 		RevCommit commit = node.getObject();
 		decoration.addSuffix(
 				" [" + abbreviate(commit) + "] " + commit.getShortMessage()); //$NON-NLS-1$ //$NON-NLS-2$
+		return true;
 	}
 
-	private void decorateSubmodules(@NonNull Repository repository,
+	private boolean decorateSubmodules(@NonNull Repository repository,
 			IDecoration decoration) throws IOException {
 		if (haveSubmoduleChanges(repository)) {
 			decoration.addPrefix("> "); //$NON-NLS-1$
+			return true;
 		}
+		return false;
 	}
 
-	private void decorateTag(TagNode node, IDecoration decoration) {
+	private boolean decorateTag(TagNode node, IDecoration decoration) {
 		if (verboseBranchMode && node.getCommitId() != null
 				&& node.getCommitId().length() > 0) {
 			decoration.addSuffix(" " + node.getCommitId().substring(0, 7) + ' ' //$NON-NLS-1$
 					+ node.getCommitShortMessage());
+			return true;
 		}
-	}
-
-	private void decorateImage(RepositoryTreeNode<?> node,
-			@NonNull Repository repository, IDecoration decoration)
-			throws IOException {
-
-		switch (node.getType()) {
-		case TAG:
-		case ADDITIONALREF:
-		case REF:
-			// if the branch or tag is checked out,
-			// we want to decorate the corresponding
-			// node with a little check indicator
-			String refName = ((Ref) node.getObject()).getName();
-			Ref leaf = ((Ref) node.getObject()).getLeaf();
-
-			String compareString = null;
-
-			String branchName = repository.getFullBranch();
-			if (branchName == null) {
-				return;
-			}
-			if (refName.startsWith(Constants.R_HEADS)) {
-				// local branch: HEAD would be on the branch
-				compareString = refName;
-			} else if (refName.startsWith(Constants.R_TAGS)) {
-				// tag: HEAD would be on the commit id to which the tag is
-				// pointing
-				TagNode tagNode = (TagNode) node;
-				compareString = tagNode.getCommitId();
-			} else if (refName.startsWith(Constants.R_REMOTES)) {
-				// remote branch: HEAD would be on the commit id to which
-				// the branch is pointing
-				ObjectId id = repository.resolve(refName);
-				if (id == null) {
-					return;
-				}
-				try (RevWalk rw = new RevWalk(repository)) {
-					RevCommit commit = rw.parseCommit(id);
-					compareString = commit.getId().name();
-				}
-			} else if (refName.equals(Constants.HEAD)) {
-				decoration.addOverlay(UIIcons.OVR_CHECKEDOUT,
-						IDecoration.TOP_LEFT);
-				return;
-			} else {
-				String leafname = leaf.getName();
-				if (leafname.startsWith(Constants.R_REFS)
-						&& leafname.equals(branchName)) {
-					decoration.addOverlay(UIIcons.OVR_CHECKEDOUT,
-							IDecoration.TOP_LEFT);
-					return;
-				}
-				ObjectId objectId = leaf.getObjectId();
-				if (objectId != null && objectId
-						.equals(repository.resolve(Constants.HEAD))) {
-					decoration.addOverlay(UIIcons.OVR_CHECKEDOUT,
-							IDecoration.TOP_LEFT);
-					return;
-				}
-				// some other symbolic reference
-				return;
-			}
-
-			if (compareString != null && compareString.equals(branchName)) {
-				decoration.addOverlay(UIIcons.OVR_CHECKEDOUT,
-						IDecoration.TOP_LEFT);
-			}
-
-			break;
-		default:
-			break;
-		}
+		return false;
 	}
 
 	private RevCommit getLatestCommit(RepositoryTreeNode node) {
diff --git a/org.eclipse.egit.ui/src/org/eclipse/egit/ui/internal/repository/RepositoryTreeNodeLabelProvider.java b/org.eclipse.egit.ui/src/org/eclipse/egit/ui/internal/repository/RepositoryTreeNodeLabelProvider.java
index ebee104..18fb1d4 100644
--- a/org.eclipse.egit.ui/src/org/eclipse/egit/ui/internal/repository/RepositoryTreeNodeLabelProvider.java
+++ b/org.eclipse.egit.ui/src/org/eclipse/egit/ui/internal/repository/RepositoryTreeNodeLabelProvider.java
@@ -38,6 +38,19 @@
 
 	private final WorkbenchLabelProvider labelProvider;
 
+	/**
+	 * Keeps the last label. If the label we originally get is undecorated, we
+	 * return this last decorated label instead to prevent flickering. When the
+	 * asynchronous lightweight decorator then has computed the decoration, the
+	 * label will be updated. Note that this works only because our
+	 * RepositoryTreeNodeDecorator always decorates! (If there's no decoration,
+	 * it appends a single blank to ensure the decorated label is different from
+	 * the undecorated one.)
+	 * <p>
+	 * For images, there is no such work-around, and thus we need to do the
+	 * image decorations in the label provider (in the
+	 * RepositoryTreeNodeWorkbenchAdapter in our case) in the UI thread.
+	 */
 	private final WeakHashMap<Object, StyledString> previousDecoratedLabels = new WeakHashMap<>();
 
 	/**
@@ -63,7 +76,9 @@
 	@Override
 	public StyledString getStyledText(Object element) {
 		StyledString decoratedLabel = super.getStyledText(element);
-		if (decoratedLabel.getString().equals(labelProvider.getText(element))) {
+		String decoratedValue = decoratedLabel.getString();
+		String simpleValue = labelProvider.getText(element);
+		if (decoratedValue.equals(simpleValue)) {
 			// Decoration not available yet... but may be shortly. Try to
 			// prevent flickering by returning the previous decorated label, if
 			// any.
@@ -71,6 +86,9 @@
 			if (previousLabel != null) {
 				return previousLabel;
 			}
+		} else if (decoratedValue.trim().equals(simpleValue)) {
+			// No decoration...
+			decoratedLabel = labelProvider.getStyledText(element);
 		}
 		if (element instanceof RepositoryNode) {
 			Repository repository = ((RepositoryNode) element).getRepository();
diff --git a/org.eclipse.egit.ui/src/org/eclipse/egit/ui/internal/repository/RepositoryTreeNodeWorkbenchAdapter.java b/org.eclipse.egit.ui/src/org/eclipse/egit/ui/internal/repository/RepositoryTreeNodeWorkbenchAdapter.java
index 442ec59..479aaec 100644
--- a/org.eclipse.egit.ui/src/org/eclipse/egit/ui/internal/repository/RepositoryTreeNodeWorkbenchAdapter.java
+++ b/org.eclipse.egit.ui/src/org/eclipse/egit/ui/internal/repository/RepositoryTreeNodeWorkbenchAdapter.java
@@ -11,9 +11,11 @@
 package org.eclipse.egit.ui.internal.repository;
 
 import java.io.File;
+import java.io.IOException;
 import java.text.MessageFormat;
 
 import org.eclipse.core.runtime.IPath;
+import org.eclipse.egit.ui.internal.DecorationOverlayDescriptor;
 import org.eclipse.egit.ui.internal.GitLabels;
 import org.eclipse.egit.ui.internal.ResourcePropertyTester;
 import org.eclipse.egit.ui.internal.UIIcons;
@@ -23,9 +25,14 @@
 import org.eclipse.egit.ui.internal.repository.tree.StashedCommitNode;
 import org.eclipse.egit.ui.internal.repository.tree.TagNode;
 import org.eclipse.jface.resource.ImageDescriptor;
+import org.eclipse.jface.viewers.IDecoration;
+import org.eclipse.jgit.annotations.NonNull;
 import org.eclipse.jgit.lib.Constants;
+import org.eclipse.jgit.lib.ObjectId;
 import org.eclipse.jgit.lib.Ref;
 import org.eclipse.jgit.lib.Repository;
+import org.eclipse.jgit.revwalk.RevCommit;
+import org.eclipse.jgit.revwalk.RevWalk;
 import org.eclipse.ui.PlatformUI;
 import org.eclipse.ui.model.WorkbenchAdapter;
 
@@ -53,7 +60,26 @@
 
 	@Override
 	public ImageDescriptor getImageDescriptor(Object object) {
+		if (object == null) {
+			return null;
+		}
 		RepositoryTreeNode<?> node = (RepositoryTreeNode) object;
+		ImageDescriptor base = getBaseImageDescriptor(node);
+		if (base == null) {
+			return null;
+		}
+		// We have to decorate here: if we let an asynchronous lightweight
+		// decorator do it, image decorations may flicker in the repositories
+		// view and elsewhere where we'd refresh viewers.
+		try {
+			return decorateImageDescriptor(base, node);
+		} catch (IOException e) {
+			return base;
+		}
+	}
+
+	private ImageDescriptor getBaseImageDescriptor(
+			@NonNull RepositoryTreeNode<?> node) {
 		switch (node.getType()) {
 		case FILE: {
 			Object item = node.getObject();
@@ -82,6 +108,76 @@
 		return node.getType().getIcon();
 	}
 
+	private ImageDescriptor decorateImageDescriptor(
+			@NonNull ImageDescriptor base, @NonNull RepositoryTreeNode<?> node)
+			throws IOException {
+		switch (node.getType()) {
+		case TAG:
+		case ADDITIONALREF:
+		case REF:
+			// if the branch or tag is checked out,
+			// we want to decorate the corresponding
+			// node with a little check indicator
+			String refName = ((Ref) node.getObject()).getName();
+			Ref leaf = ((Ref) node.getObject()).getLeaf();
+
+			String compareString = null;
+			Repository repository = node.getRepository();
+			String branchName = repository.getFullBranch();
+			if (branchName == null) {
+				return base;
+			}
+			if (refName.startsWith(Constants.R_HEADS)) {
+				// local branch: HEAD would be on the branch
+				compareString = refName;
+			} else if (refName.startsWith(Constants.R_TAGS)) {
+				// tag: HEAD would be on the commit id to which the tag is
+				// pointing
+				TagNode tagNode = (TagNode) node;
+				compareString = tagNode.getCommitId();
+			} else if (refName.startsWith(Constants.R_REMOTES)) {
+				// remote branch: HEAD would be on the commit id to which
+				// the branch is pointing
+				ObjectId id = repository.resolve(refName);
+				if (id == null) {
+					return base;
+				}
+				try (RevWalk rw = new RevWalk(repository)) {
+					RevCommit commit = rw.parseCommit(id);
+					compareString = commit.getId().name();
+				}
+			} else if (refName.equals(Constants.HEAD)) {
+				return new DecorationOverlayDescriptor(base,
+						UIIcons.OVR_CHECKEDOUT, IDecoration.TOP_LEFT);
+			} else {
+				String leafname = leaf.getName();
+				if (leafname.startsWith(Constants.R_REFS)
+						&& leafname.equals(branchName)) {
+					return new DecorationOverlayDescriptor(base,
+							UIIcons.OVR_CHECKEDOUT, IDecoration.TOP_LEFT);
+				}
+				ObjectId objectId = leaf.getObjectId();
+				if (objectId != null && objectId
+						.equals(repository.resolve(Constants.HEAD))) {
+					return new DecorationOverlayDescriptor(base,
+							UIIcons.OVR_CHECKEDOUT, IDecoration.TOP_LEFT);
+				}
+				// some other symbolic reference
+				return base;
+			}
+
+			if (compareString != null && compareString.equals(branchName)) {
+				return new DecorationOverlayDescriptor(base,
+						UIIcons.OVR_CHECKEDOUT, IDecoration.TOP_LEFT);
+			}
+
+			break;
+		default:
+			break;
+		}
+		return base;
+	}
+
 	@Override
 	public String getLabel(Object object) {
 		RepositoryTreeNode<?> node = (RepositoryTreeNode) object;