394020: [model] Persist Review models across workbench..

	•Adds Edit Factory Supporting Persistence
	•Enriches Consumer and Observers to support complex remote requests
	•Refactors Editors, Explorer and UI factories to implement above
	•Ensure that model changes don't happen off of model thread
	•Adds testing coverage

Change-Id: I3e4234f29ecc6d1a579300897b2db22c434c47df
Task-Url: https://bugs.eclipse.org/bugs/show_bug.cgi?id=394020

10 files changed, 378 insertions, 252 deletions

diff --git a/org.eclipse.mylyn.reviews.ui/src/org/eclipse/mylyn/internal/reviews/ui/ReviewsUiPlugin.java b/org.eclipse.mylyn.reviews.ui/src/org/eclipse/mylyn/internal/reviews/ui/ReviewsUiPlugin.java
index f8c5204ad..af6ba65a0 100644
--- a/org.eclipse.mylyn.reviews.ui/src/org/eclipse/mylyn/internal/reviews/ui/ReviewsUiPlugin.java
+++ b/org.eclipse.mylyn.reviews.ui/src/org/eclipse/mylyn/internal/reviews/ui/ReviewsUiPlugin.java
@@ -11,10 +11,13 @@
 package org.eclipse.mylyn.internal.reviews.ui;
 
 import org.eclipse.mylyn.commons.workbench.CommonImageManger;
-import org.eclipse.mylyn.reviews.core.spi.remote.AbstractRemoteFactoryProvider;
+import org.eclipse.mylyn.reviews.core.model.IRepository;
+import org.eclipse.mylyn.reviews.core.model.IReview;
+import org.eclipse.mylyn.reviews.core.spi.ReviewsConnector;
 import org.eclipse.mylyn.reviews.core.spi.remote.AbstractRemoteService;
+import org.eclipse.mylyn.reviews.core.spi.remote.emf.AbstractRemoteEmfFactoryProvider;
 import org.eclipse.mylyn.reviews.core.spi.remote.review.IReviewRemoteFactoryProvider;
-import org.eclipse.mylyn.reviews.internal.core.ReviewsConnector;
+import org.eclipse.mylyn.reviews.ui.spi.factories.UiDataLocator;
 import org.eclipse.mylyn.reviews.ui.spi.remote.RemoteUiService;
 import org.eclipse.mylyn.tasks.core.TaskRepository;
 import org.eclipse.mylyn.tasks.ui.TasksUi;
@@ -58,12 +61,16 @@ public class ReviewsUiPlugin extends AbstractUIPlugin {
 
 	public IReviewRemoteFactoryProvider getFactoryProvider(String connectorKind, TaskRepository repository) {
 		ReviewsConnector connector = (ReviewsConnector) TasksUi.getRepositoryConnector(connectorKind);
-		AbstractRemoteFactoryProvider factoryProvider = connector.getFactoryProvider(repository);
+		AbstractRemoteEmfFactoryProvider<IRepository, IReview> factoryProvider = connector.getFactoryProvider(repository);
+		if (service == null) {
+			service = new RemoteUiService();
+		}
+		factoryProvider.setService(service);
+		if (!(factoryProvider.getDataLocator() instanceof UiDataLocator)) {
+			factoryProvider.setDataLocator(new UiDataLocator());
+		}
+		factoryProvider.open();
 		if (factoryProvider instanceof IReviewRemoteFactoryProvider) {
-			if (service == null) {
-				service = new RemoteUiService();
-			}
-			factoryProvider.setService(service);
 			return (IReviewRemoteFactoryProvider) factoryProvider;
 		}
 		throw new RuntimeException("The connector factory propvider must implement IReviewRemoteFactoryProvider");
diff --git a/org.eclipse.mylyn.reviews.ui/src/org/eclipse/mylyn/internal/reviews/ui/dialogs/AddCommentDialog.java b/org.eclipse.mylyn.reviews.ui/src/org/eclipse/mylyn/internal/reviews/ui/dialogs/AddCommentDialog.java
index fd37f9a0a..a588af52c 100644
--- a/org.eclipse.mylyn.reviews.ui/src/org/eclipse/mylyn/internal/reviews/ui/dialogs/AddCommentDialog.java
+++ b/org.eclipse.mylyn.reviews.ui/src/org/eclipse/mylyn/internal/reviews/ui/dialogs/AddCommentDialog.java
@@ -12,7 +12,6 @@
 package org.eclipse.mylyn.internal.reviews.ui.dialogs;
 
 import java.lang.reflect.InvocationTargetException;
-import java.util.List;
 import java.util.concurrent.atomic.AtomicReference;
 
 import org.eclipse.core.runtime.IProgressMonitor;
@@ -25,13 +24,11 @@ import org.eclipse.mylyn.internal.reviews.ui.ReviewsUiPlugin;
 import org.eclipse.mylyn.internal.tasks.ui.TasksUiPlugin;
 import org.eclipse.mylyn.internal.tasks.ui.editors.RichTextEditor;
 import org.eclipse.mylyn.internal.tasks.ui.editors.TaskEditorExtensions;
+import org.eclipse.mylyn.reviews.core.model.IComment;
 import org.eclipse.mylyn.reviews.core.model.IFileItem;
 import org.eclipse.mylyn.reviews.core.model.IFileVersion;
 import org.eclipse.mylyn.reviews.core.model.ILocation;
 import org.eclipse.mylyn.reviews.core.model.IReviewItem;
-import org.eclipse.mylyn.reviews.core.model.IReviewItemSet;
-import org.eclipse.mylyn.reviews.core.model.IComment;
-import org.eclipse.mylyn.reviews.core.spi.remote.emf.RemoteEmfConsumer;
 import org.eclipse.mylyn.reviews.core.spi.remote.review.IReviewRemoteFactoryProvider;
 import org.eclipse.mylyn.reviews.ui.ProgressDialog;
 import org.eclipse.mylyn.reviews.ui.ReviewBehavior;
@@ -141,9 +138,9 @@ public class AddCommentDialog extends ProgressDialog {
 						reviewBehavior.getTask().getConnectorKind(), reviewBehavior.getTask().getRepositoryUrl());
 				IReviewRemoteFactoryProvider factoryProvider = ReviewsUiPlugin.getDefault().getFactoryProvider(
 						reviewBehavior.getTask().getConnectorKind(), taskRepository);
-				RemoteEmfConsumer<IReviewItemSet, List<IFileItem>, ?, String, String> consumer = factoryProvider.getReviewItemSetContentFactory()
-						.getConsumerForLocalKey(file.getSet(), file.getSet().getId());
-				consumer.updateObservers();
+				factoryProvider.getReviewItemSetContentFactory()
+						.getConsumerForLocalKey(file.getSet(), file.getSet().getId())
+						.updateObservers();
 			}
 			return true;
 		} else {
diff --git a/org.eclipse.mylyn.reviews.ui/src/org/eclipse/mylyn/internal/reviews/ui/providers/ReviewsLabelProvider.java b/org.eclipse.mylyn.reviews.ui/src/org/eclipse/mylyn/internal/reviews/ui/providers/ReviewsLabelProvider.java
index 7e3f9fe30..d45d2eb26 100644
--- a/org.eclipse.mylyn.reviews.ui/src/org/eclipse/mylyn/internal/reviews/ui/providers/ReviewsLabelProvider.java
+++ b/org.eclipse.mylyn.reviews.ui/src/org/eclipse/mylyn/internal/reviews/ui/providers/ReviewsLabelProvider.java
@@ -190,7 +190,11 @@ public abstract class ReviewsLabelProvider extends TableStyledLabelProvider {
 				return desc;
 			}
 			if (element instanceof IUser) {
-				return ((IUser) element).getDisplayName();
+				String displayName = ((IUser) element).getDisplayName();
+				if (!StringUtils.isEmpty(displayName)) {
+					return displayName;
+				}
+				return "Unknown";
 			}
 			if (element instanceof Date) {
 				return DateUtil.getRelativeDuration(System.currentTimeMillis() - ((Date) element).getTime()) + " ago";
@@ -469,11 +473,7 @@ public abstract class ReviewsLabelProvider extends TableStyledLabelProvider {
 				if (element instanceof GlobalCommentsNode) {
 					element = ((GlobalCommentsNode) element).getReview();
 				}
-				if (element instanceof ICommentContainer) {
-					ICommentContainer container = (ICommentContainer) element;
-					String statsText = getStatsText(container);
-					styledString.append(statsText, StyledString.DECORATIONS_STYLER);
-				}
+				addCommentContainerStatsStyle(element, styledString);
 				if (element instanceof IFileItem) {
 					IReviewItem item = (IReviewItem) element;
 					styledString.append("  " + item.getName(), StyledString.QUALIFIER_STYLER);
diff --git a/org.eclipse.mylyn.reviews.ui/src/org/eclipse/mylyn/internal/reviews/ui/views/ReviewExplorer.java b/org.eclipse.mylyn.reviews.ui/src/org/eclipse/mylyn/internal/reviews/ui/views/ReviewExplorer.java
index 407ff075d..5398aec67 100644
--- a/org.eclipse.mylyn.reviews.ui/src/org/eclipse/mylyn/internal/reviews/ui/views/ReviewExplorer.java
+++ b/org.eclipse.mylyn.reviews.ui/src/org/eclipse/mylyn/internal/reviews/ui/views/ReviewExplorer.java
@@ -14,6 +14,7 @@ package org.eclipse.mylyn.internal.reviews.ui.views;
 
 import java.util.Arrays;
 import java.util.Collection;
+import java.util.Date;
 import java.util.HashMap;
 import java.util.HashSet;
 import java.util.List;
@@ -48,6 +49,9 @@ import org.eclipse.mylyn.reviews.core.model.IReviewItemSet;
 import org.eclipse.mylyn.reviews.core.spi.remote.emf.RemoteEmfConsumer;
 import org.eclipse.mylyn.reviews.core.spi.remote.emf.RemoteEmfObserver;
 import org.eclipse.mylyn.reviews.core.spi.remote.review.IReviewRemoteFactoryProvider;
+import org.eclipse.mylyn.reviews.core.spi.remote.review.ReviewItemSetContentRemoteFactory;
+import org.eclipse.mylyn.reviews.core.spi.remote.review.ReviewItemSetContentRemoteFactory.Client;
+import org.eclipse.mylyn.reviews.core.spi.remote.review.ReviewRemoteFactory;
 import org.eclipse.mylyn.reviews.ui.spi.editor.AbstractReviewTaskEditorPage;
 import org.eclipse.mylyn.tasks.core.ITask;
 import org.eclipse.mylyn.tasks.ui.editors.AbstractTaskEditorPage;
@@ -103,29 +107,63 @@ public class ReviewExplorer extends CommonNavigator {
 
 	private TableStyledLabelProvider currentProvider;
 
-	private RemoteEmfConsumer<IRepository, IReview, ?, String, String> reviewConsumer;
+	private RemoteEmfConsumer<IRepository, IReview, String, ?, ?, Date> reviewConsumer;
 
 	private IReviewRemoteFactoryProvider factoryProvider;
 
 	private String taskId;
 
-	private final RemoteEmfObserver<IRepository, IReview> reviewObserver = new RemoteEmfObserver<IRepository, IReview>() {
+	private final ReviewRemoteFactory.Client reviewObserver = new ReviewRemoteFactory.Client() {
 		@Override
-		public void created(IRepository parentObject, IReview modelObject) {
-			if (modelObject.getId().equals(taskId) && modelObject != ReviewExplorer.this.review) {
-				setReview(modelObject);
-			}
+		public void create() {
+			setReview(getConsumer().getModelObject());
 		}
 
 		@Override
-		public void updated(IRepository parentObject, IReview modelObject, boolean modified) {
-			if (modified) {
-				updatePerservingSelection();
-			}
+		public void update() {
+			updatePatchSetObservers();
+			updatePerservingSelection();
+		}
+
+		@Override
+		protected boolean isClientReady() {
+			return getCommonViewer() != null && getCommonViewer().getControl() != null
+					&& !getCommonViewer().getControl().isDisposed();
 		}
 	};
 
-	private final Map<IReviewItemSet, RemoteEmfObserver<IReviewItemSet, List<IFileItem>>> patchSetObservers = new HashMap<IReviewItemSet, RemoteEmfObserver<IReviewItemSet, List<IFileItem>>>();
+	private final class PatchSetClient extends ReviewItemSetContentRemoteFactory.Client {
+		@Override
+		protected boolean isClientReady() {
+			return getCommonViewer() != null && getCommonViewer().getControl() != null
+					&& !getCommonViewer().getControl().isDisposed();
+		}
+
+		@Override
+		protected void update() {
+			if (showList) {
+				getCommonViewer().refresh(true);
+			} else {
+				//Tree, so we can just refresh the parent set, assuming it's currently displayed
+				TreeItem[] rootItems = getCommonViewer().getTree().getItems();
+				boolean parentDisplayed = false;
+				for (TreeItem treeItem : rootItems) {
+					Object data = treeItem.getData();
+					if (data == getConsumer().getParentObject()) {
+						parentDisplayed = true;
+						break;
+					}
+				}
+				if (parentDisplayed) {
+					getCommonViewer().refresh(getConsumer().getParentObject(), true);
+				} else { //treeitem is currently being filtered (as a non-commented set, for example)
+					getCommonViewer().refresh(true);
+				}
+			}
+		}
+	}
+
+	private final Map<IReviewItemSet, ReviewItemSetContentRemoteFactory.Client> patchSetObservers = new HashMap<IReviewItemSet, ReviewItemSetContentRemoteFactory.Client>();
 
 	private final IPartListener editorPartListener = new IPartListener() {
 		public void partOpened(IWorkbenchPart part) {
@@ -139,7 +177,8 @@ public class ReviewExplorer extends CommonNavigator {
 				currentPart = null;
 				review = null;
 				taskId = null;
-				disposeObservers();
+				reviewObserver.dispose();
+				disposePatchSetObservers();
 				update();
 			}
 		}
@@ -290,7 +329,7 @@ public class ReviewExplorer extends CommonNavigator {
 				if (event.getElement() instanceof IReviewItemSet) {
 					IReviewItemSet set = (IReviewItemSet) event.getElement();
 					if (set.getItems().size() == 0) {
-						RemoteEmfObserver<IReviewItemSet, List<IFileItem>> observer = patchSetObservers.get(set);
+						ReviewItemSetContentRemoteFactory.Client observer = getPatchSetObserver(set);
 						observer.getConsumer().retrieve(false);
 					}
 				}
@@ -467,7 +506,7 @@ public class ReviewExplorer extends CommonNavigator {
 	protected void refresh() {
 		if (reviewConsumer != null) {
 			reviewConsumer.retrieve(true);
-			for (RemoteEmfObserver<IReviewItemSet, List<IFileItem>> observer : patchSetObservers.values()) {
+			for (ReviewItemSetContentRemoteFactory.Client observer : patchSetObservers.values()) {
 				observer.getConsumer().retrieve(true);
 			}
 		}
@@ -498,40 +537,12 @@ public class ReviewExplorer extends CommonNavigator {
 		if (review != newReview) {
 			review = newReview;
 			update();
-			if (review != null) {
+			if (review != null
+					&& !factoryProvider.getReviewFactory().isCreateModelNeeded(factoryProvider.getRoot(), review)) {
 				reviewConsumer = factoryProvider.getReviewFactory().getConsumerForModel(factoryProvider.getRoot(),
 						newReview);
 				reviewConsumer.retrieve(false);
-				for (IReviewItemSet newSet : review.getSets()) {
-					RemoteEmfConsumer<IReviewItemSet, List<IFileItem>, ?, String, String> contentConsumer = factoryProvider.getReviewItemSetContentFactory()
-							.getConsumerForLocalKey(newSet, newSet.getId());
-					RemoteEmfObserver<IReviewItemSet, List<IFileItem>> patchSetObserver = new RemoteEmfObserver<IReviewItemSet, List<IFileItem>>(
-							contentConsumer) {
-						@Override
-						public void updated(IReviewItemSet parent, List<IFileItem> items, boolean modified) {
-							if (showList) {
-								getCommonViewer().refresh(true);
-							} else {
-								//Tree, so we can just refresh the parent set, assuming it's currently displayed
-								TreeItem[] rootItems = getCommonViewer().getTree().getItems();
-								boolean parentDisplayed = false;
-								for (TreeItem treeItem : rootItems) {
-									Object data = treeItem.getData();
-									if (data == parent) {
-										parentDisplayed = true;
-										break;
-									}
-								}
-								if (parentDisplayed) {
-									getCommonViewer().refresh(parent, true);
-								} else { //treeitem is currently being filtered (as a non-commented set, for example)
-									getCommonViewer().refresh(true);
-								}
-							}
-						}
-					};
-					patchSetObservers.put(newSet, patchSetObserver);
-				}
+				updatePatchSetObservers();
 			}
 		}
 	}
@@ -539,7 +550,8 @@ public class ReviewExplorer extends CommonNavigator {
 	protected void setReviewId(String newTaskId) {
 		if (!newTaskId.equals(taskId)) {
 			taskId = newTaskId;
-			disposeObservers();
+			reviewObserver.dispose();
+			disposePatchSetObservers();
 			reviewConsumer = factoryProvider.getReviewFactory().getConsumerForLocalKey(factoryProvider.getRoot(),
 					newTaskId);
 			reviewConsumer.addObserver(reviewObserver);
@@ -560,12 +572,25 @@ public class ReviewExplorer extends CommonNavigator {
 		}
 	}
 
-	private void disposeObservers() {
-		for (RemoteEmfObserver<IReviewItemSet, List<IFileItem>> observer : patchSetObservers.values()) {
+	private void disposePatchSetObservers() {
+		for (RemoteEmfObserver<IReviewItemSet, List<IFileItem>, String, Long> observer : patchSetObservers.values()) {
 			observer.dispose();
 		}
 		patchSetObservers.clear();
-		reviewObserver.dispose();
+	}
+
+	public void updatePatchSetObservers() {
+		for (IReviewItemSet set : review.getSets()) {
+			Client client = patchSetObservers.get(set);
+			if (client == null) {
+				ReviewItemSetContentRemoteFactory.Client patchSetObserver = new PatchSetClient();
+				RemoteEmfConsumer<IReviewItemSet, List<IFileItem>, String, ?, ?, Long> consumer = factoryProvider.getReviewItemSetContentFactory()
+						.getConsumerForLocalKey(set, set.getId());
+				patchSetObserver.setConsumer(consumer);
+				patchSetObservers.put(set, patchSetObserver);
+				patchSetObserver.checkUpdate(false);
+			}
+		}
 	}
 
 	/* (non-Javadoc)
@@ -578,7 +603,8 @@ public class ReviewExplorer extends CommonNavigator {
 		flatLabelProvider.doDispose();
 		treeLabelProvider.doDispose();
 		currentPart = null;
-		disposeObservers();
+		reviewObserver.dispose();
+		disposePatchSetObservers();
 		pageListener.pageActivated(null);
 	}
 
@@ -652,6 +678,10 @@ public class ReviewExplorer extends CommonNavigator {
 		getCommonViewer().refresh();
 	}
 
+	public Client getPatchSetObserver(IReviewItemSet set) {
+		return patchSetObservers.get(set);
+	}
+
 	public IWorkbenchPart getCurrentPart() {
 		return currentPart;
 	}
diff --git a/org.eclipse.mylyn.reviews.ui/src/org/eclipse/mylyn/reviews/ui/spi/editor/AbstractReviewSection.java b/org.eclipse.mylyn.reviews.ui/src/org/eclipse/mylyn/reviews/ui/spi/editor/AbstractReviewSection.java
index a3025bb6f..d97e138b6 100644
--- a/org.eclipse.mylyn.reviews.ui/src/org/eclipse/mylyn/reviews/ui/spi/editor/AbstractReviewSection.java
+++ b/org.eclipse.mylyn.reviews.ui/src/org/eclipse/mylyn/reviews/ui/spi/editor/AbstractReviewSection.java
@@ -11,15 +11,16 @@
 
 package org.eclipse.mylyn.reviews.ui.spi.editor;
 
-import org.eclipse.core.runtime.IStatus;
+import java.util.Date;
+
 import org.eclipse.jface.layout.GridLayoutFactory;
 import org.eclipse.mylyn.commons.ui.CommonUiUtil;
 import org.eclipse.mylyn.internal.tasks.ui.editors.AbstractTaskEditorSection;
 import org.eclipse.mylyn.reviews.core.model.IRepository;
 import org.eclipse.mylyn.reviews.core.model.IReview;
-import org.eclipse.mylyn.reviews.core.spi.remote.emf.IRemoteEmfObserver;
 import org.eclipse.mylyn.reviews.core.spi.remote.emf.RemoteEmfConsumer;
 import org.eclipse.mylyn.reviews.core.spi.remote.review.IReviewRemoteFactoryProvider;
+import org.eclipse.mylyn.reviews.core.spi.remote.review.ReviewRemoteFactory;
 import org.eclipse.mylyn.reviews.ui.spi.factories.IUiContext;
 import org.eclipse.mylyn.tasks.core.ITask;
 import org.eclipse.mylyn.tasks.core.TaskRepository;
@@ -40,8 +41,7 @@ import org.eclipse.ui.forms.widgets.Section;
  * @author Miles Parker
  * @author Steffen Pingel
  */
-public abstract class AbstractReviewSection extends AbstractTaskEditorSection implements IUiContext,
-		IRemoteEmfObserver<IRepository, IReview> {
+public abstract class AbstractReviewSection extends AbstractTaskEditorSection implements IUiContext {
 
 	protected Composite composite;
 
@@ -49,14 +49,33 @@ public abstract class AbstractReviewSection extends AbstractTaskEditorSection im
 
 	protected boolean modelContentsCurrent;
 
-	protected RemoteEmfConsumer<IRepository, IReview, ?, String, String> consumer;
+	protected RemoteEmfConsumer<IRepository, IReview, String, ?, ?, Date> reviewConsumer;
+
+	protected final ReviewRemoteFactory.Client reviewClient = new ReviewRemoteFactory.Client() {
+
+		@Override
+		protected void create() {
+			createModelContent();
+		}
+
+		@Override
+		protected boolean isClientReady() {
+			return composite != null;
+		}
+
+		@Override
+		protected void update() {
+			super.update();
+			updateModelContent();
+		}
+	};
 
 	@Override
 	public void initialize(AbstractTaskEditorPage taskEditorPage) {
 		super.initialize(taskEditorPage);
-		consumer = getFactoryProvider().getReviewFactory().getConsumerForLocalKey(getFactoryProvider().getRoot(),
+		reviewConsumer = getFactoryProvider().getReviewFactory().getConsumerForLocalKey(getFactoryProvider().getRoot(),
 				getTask().getTaskId());
-		consumer.addObserver(this);
+		reviewClient.setConsumer(reviewConsumer);
 	}
 
 	@Override
@@ -64,29 +83,15 @@ public abstract class AbstractReviewSection extends AbstractTaskEditorSection im
 		this.toolkit = toolkit;
 		composite = toolkit.createComposite(parent);
 		GridLayoutFactory.fillDefaults().extendedMargins(0, 0, 0, 5).applyTo(composite);
-		checkCreateModelContent();
+		reviewClient.checkUpdate(false);
+		reviewClient.requestUpdate(true);
 		return composite;
 	}
 
-	/**
-	 * We don't know whether the model or the controls will be available first, so we handle both cases here.
-	 */
-	private void checkCreateModelContent() {
-		if (composite != null) {
-			if (!composite.isDisposed()) {
-				if (!modelContentsCurrent && getReview() != null) {
-					modelContentsCurrent = true;
-					createModelContent();
-				}
-				updateMessage();
-			}
-		}
-	}
-
 	@SuppressWarnings("restriction")
 	private void updateMessage() {
 		if (composite != null) {
-			if (consumer != null && consumer.getModelObject() != null) {
+			if (reviewConsumer != null && reviewConsumer.getModelObject() != null) {
 				getSection().setText(CommonUiUtil.toLabel(getPartName()));
 			} else {
 				getSection().setText(
@@ -96,7 +101,10 @@ public abstract class AbstractReviewSection extends AbstractTaskEditorSection im
 		}
 	}
 
-	protected void createModelContent() {
+	protected abstract void createModelContent();
+
+	protected void updateModelContent() {
+		updateMessage();
 	}
 
 	public Label addTextClient(final FormToolkit toolkit, final Section section, String text) {
@@ -150,8 +158,8 @@ public abstract class AbstractReviewSection extends AbstractTaskEditorSection im
 	}
 
 	public IReview getReview() {
-		if (consumer != null) {
-			return consumer.getModelObject();
+		if (reviewConsumer != null) {
+			return reviewConsumer.getModelObject();
 		}
 		return null;
 	}
@@ -172,22 +180,13 @@ public abstract class AbstractReviewSection extends AbstractTaskEditorSection im
 		return (AbstractReviewTaskEditorPage) getTaskEditorPage();
 	}
 
-	public void created(IRepository parent, IReview object) {
-		//ignore
-	}
-
-	public void updating(IRepository parent, IReview object) {
-		//ignore
+	public ReviewRemoteFactory.Client getReviewClient() {
+		return reviewClient;
 	}
 
-	public void updated(IRepository parent, IReview object, boolean modified) {
-		checkCreateModelContent();
-		if (modified) {
-			refresh();
-		}
-	}
-
-	public void failed(IRepository parent, IReview object, IStatus status) {
-		// ignore
+	@Override
+	public void dispose() {
+		super.dispose();
+		reviewClient.dispose();
 	}
 }
diff --git a/org.eclipse.mylyn.reviews.ui/src/org/eclipse/mylyn/reviews/ui/spi/editor/AbstractReviewTaskEditorPage.java b/org.eclipse.mylyn.reviews.ui/src/org/eclipse/mylyn/reviews/ui/spi/editor/AbstractReviewTaskEditorPage.java
index 134ed987b..20fbd58c6 100644
--- a/org.eclipse.mylyn.reviews.ui/src/org/eclipse/mylyn/reviews/ui/spi/editor/AbstractReviewTaskEditorPage.java
+++ b/org.eclipse.mylyn.reviews.ui/src/org/eclipse/mylyn/reviews/ui/spi/editor/AbstractReviewTaskEditorPage.java
@@ -11,6 +11,9 @@
 
 package org.eclipse.mylyn.reviews.ui.spi.editor;
 
+import java.util.Date;
+import java.util.List;
+
 import org.eclipse.core.runtime.IStatus;
 import org.eclipse.core.runtime.Status;
 import org.eclipse.jface.dialogs.IMessageProvider;
@@ -18,11 +21,14 @@ import org.eclipse.mylyn.commons.core.StatusHandler;
 import org.eclipse.mylyn.internal.reviews.ui.ReviewsUiPlugin;
 import org.eclipse.mylyn.internal.tasks.ui.TasksUiPlugin;
 import org.eclipse.mylyn.internal.tasks.ui.util.TasksUiInternal;
+import org.eclipse.mylyn.reviews.core.model.IFileItem;
 import org.eclipse.mylyn.reviews.core.model.IRepository;
 import org.eclipse.mylyn.reviews.core.model.IReview;
-import org.eclipse.mylyn.reviews.core.spi.remote.emf.IRemoteEmfObserver;
+import org.eclipse.mylyn.reviews.core.model.IReviewItemSet;
 import org.eclipse.mylyn.reviews.core.spi.remote.emf.RemoteEmfConsumer;
 import org.eclipse.mylyn.reviews.core.spi.remote.review.IReviewRemoteFactoryProvider;
+import org.eclipse.mylyn.reviews.core.spi.remote.review.ReviewRemoteFactory;
+import org.eclipse.mylyn.reviews.spi.edit.remote.review.ReviewsRemoteEditFactoryProvider;
 import org.eclipse.mylyn.tasks.ui.editors.AbstractTaskEditorPage;
 import org.eclipse.mylyn.tasks.ui.editors.TaskEditor;
 import org.eclipse.swt.widgets.Display;
@@ -36,16 +42,44 @@ import org.eclipse.ui.forms.events.HyperlinkEvent;
  * 
  * @author Miles Parker
  */
-public abstract class AbstractReviewTaskEditorPage extends AbstractTaskEditorPage implements
-		IRemoteEmfObserver<IRepository, IReview> {
-
-	private RemoteEmfConsumer<IRepository, IReview, ?, String, String> consumer;
+public abstract class AbstractReviewTaskEditorPage extends AbstractTaskEditorPage {
 
 	private IReviewRemoteFactoryProvider factoryProvider;
 
-	private boolean intialRefreshRequested;
+	private final ReviewRemoteFactory.Client emfClient = new ReviewRemoteFactory.Client() {
+		@Override
+		protected boolean isClientReady() {
+			return getManagedForm() != null && !getManagedForm().getForm().isDisposed();
+		}
 
-	private boolean refreshRequested;
+		@Override
+		protected void rebuild() {
+			//Prevent CME from observer and allow other UI processes time to execute
+			Display.getCurrent().asyncExec(new Runnable() {
+				@Override
+				public void run() {
+					AbstractReviewTaskEditorPage.super.refresh();
+				}
+			});
+			updateSets();
+		}
+
+		@Override
+		public void failed(IRepository parent, IReview object, final IStatus status) {
+			StatusHandler.log(new Status(IStatus.ERROR, TasksUiPlugin.ID_PLUGIN,
+					"Error loading task", status.getException())); //$NON-NLS-1$
+			getTaskEditor().setMessage(
+					org.eclipse.mylyn.internal.tasks.ui.editors.Messages.AbstractTaskEditorPage_Error_opening_task,
+					IMessageProvider.ERROR, new HyperlinkAdapter() {
+						@Override
+						public void linkActivated(HyperlinkEvent event) {
+							TasksUiInternal.displayStatus(
+									org.eclipse.mylyn.internal.tasks.ui.editors.Messages.AbstractTaskEditorPage_Open_failed,
+									status);
+						}
+					});
+		}
+	};
 
 	public AbstractReviewTaskEditorPage(TaskEditor editor, String id, String label, String connectorKind) {
 		super(editor, id, label, connectorKind);
@@ -54,20 +88,16 @@ public abstract class AbstractReviewTaskEditorPage extends AbstractTaskEditorPag
 	@Override
 	public void init(final IEditorSite site, final IEditorInput input) {
 		AbstractReviewTaskEditorPage.super.init(site, input);
-		if (getFactoryProvider() != null) {
-			consumer = getFactoryProvider().getReviewFactory().getConsumerForRemoteKey(getFactoryProvider().getRoot(),
-					getTask().getTaskId());
-			consumer.addObserver(AbstractReviewTaskEditorPage.this);
-			intialRefreshRequested = true;
-			consumer.retrieve(false);
-		}
+		final RemoteEmfConsumer<IRepository, IReview, String, ?, ?, Date> reviewConsumer = getFactoryProvider().getReviewFactory()
+				.getConsumerForLocalKey(getFactoryProvider().getRoot(), getTask().getTaskId());
+		emfClient.setConsumer(reviewConsumer);
+		reviewConsumer.open();
 	}
 
 	@Override
 	public void refresh() {
-		refreshRequested = true;
 		//We defer the actual refresh until the model is also refreshed. See updated method.
-		consumer.retrieve(true);
+		emfClient.requestUpdate(true, true);
 	}
 
 	public IReviewRemoteFactoryProvider getFactoryProvider() {
@@ -77,40 +107,29 @@ public abstract class AbstractReviewTaskEditorPage extends AbstractTaskEditorPag
 		return factoryProvider;
 	}
 
-	public void created(IRepository parent, IReview object) {
-		//ignore
-	}
-
-	public void updating(IRepository parent, IReview object) {
-		//ignore
+	@Override
+	public void dispose() {
+		IReviewRemoteFactoryProvider provider = getFactoryProvider();
+		if (provider instanceof ReviewsRemoteEditFactoryProvider) {
+			ReviewsRemoteEditFactoryProvider reviewsProvider = (ReviewsRemoteEditFactoryProvider) provider;
+			reviewsProvider.close(emfClient.getConsumer().getModelObject());
+			reviewsProvider.save();
+		}
+		emfClient.dispose();
+		super.dispose();
 	}
 
-	public void updated(IRepository parent, IReview object, boolean modified) {
-		if (refreshRequested || (modified && !intialRefreshRequested)) {
-			//Prevent CME from observer and allow other UI processes time to execute
-			Display.getCurrent().asyncExec(new Runnable() {
-				@Override
-				public void run() {
-					AbstractReviewTaskEditorPage.super.refresh();
+	private void updateSets() {
+		for (IReviewItemSet set : emfClient.getConsumer().getModelObject().getSets()) {
+			if (set.getItems() != null) {
+				RemoteEmfConsumer<IReviewItemSet, List<IFileItem>, String, ?, ?, Long> contentConsumer = getFactoryProvider().getReviewItemSetContentFactory()
+						.getConsumerForModel(set, set.getItems());
+				//Don't retrieve items that have not already been explicitly requested by user..
+				if (contentConsumer != null && contentConsumer.getModelObject() != null
+						&& !contentConsumer.getModelObject().isEmpty()) {
+					contentConsumer.retrieve(true);
 				}
-			});
-			refreshRequested = false;
+			}
 		}
-		intialRefreshRequested = false;
-	}
-
-	public void failed(IRepository parent, IReview object, final IStatus status) {
-		StatusHandler.log(new Status(IStatus.ERROR, TasksUiPlugin.ID_PLUGIN,
-				"Error loading task", status.getException())); //$NON-NLS-1$
-		getTaskEditor().setMessage(
-				org.eclipse.mylyn.internal.tasks.ui.editors.Messages.AbstractTaskEditorPage_Error_opening_task,
-				IMessageProvider.ERROR, new HyperlinkAdapter() {
-					@Override
-					public void linkActivated(HyperlinkEvent event) {
-						TasksUiInternal.displayStatus(
-								org.eclipse.mylyn.internal.tasks.ui.editors.Messages.AbstractTaskEditorPage_Open_failed,
-								status);
-					}
-				});
 	}
 }
diff --git a/org.eclipse.mylyn.reviews.ui/src/org/eclipse/mylyn/reviews/ui/spi/editor/ReviewSetContentSection.java b/org.eclipse.mylyn.reviews.ui/src/org/eclipse/mylyn/reviews/ui/spi/editor/ReviewSetContentSection.java
index 17825a6b4..30c4dd4f8 100644
--- a/org.eclipse.mylyn.reviews.ui/src/org/eclipse/mylyn/reviews/ui/spi/editor/ReviewSetContentSection.java
+++ b/org.eclipse.mylyn.reviews.ui/src/org/eclipse/mylyn/reviews/ui/spi/editor/ReviewSetContentSection.java
@@ -14,6 +14,7 @@ package org.eclipse.mylyn.reviews.ui.spi.editor;
 
 import java.text.DateFormat;
 import java.util.Collections;
+import java.util.Date;
 import java.util.List;
 
 import org.eclipse.core.runtime.IStatus;
@@ -34,9 +35,14 @@ import org.eclipse.mylyn.internal.reviews.ui.providers.ReviewsLabelProvider;
 import org.eclipse.mylyn.internal.tasks.ui.TasksUiPlugin;
 import org.eclipse.mylyn.internal.tasks.ui.editors.EditorUtil;
 import org.eclipse.mylyn.reviews.core.model.IFileItem;
+import org.eclipse.mylyn.reviews.core.model.IRepository;
+import org.eclipse.mylyn.reviews.core.model.IReview;
 import org.eclipse.mylyn.reviews.core.model.IReviewItemSet;
 import org.eclipse.mylyn.reviews.core.spi.remote.emf.IRemoteEmfObserver;
 import org.eclipse.mylyn.reviews.core.spi.remote.emf.RemoteEmfConsumer;
+import org.eclipse.mylyn.reviews.core.spi.remote.emf.RemoteEmfObserver;
+import org.eclipse.mylyn.reviews.core.spi.remote.review.ReviewItemSetContentRemoteFactory;
+import org.eclipse.mylyn.reviews.core.spi.remote.review.ReviewItemSetRemoteFactory;
 import org.eclipse.osgi.util.NLS;
 import org.eclipse.swt.SWT;
 import org.eclipse.swt.widgets.Composite;
@@ -58,7 +64,7 @@ import org.eclipse.ui.forms.widgets.Section;
  * @author Miles Parker
  * @author Sam Davis
  */
-public class ReviewSetContentSection implements IRemoteEmfObserver<IReviewItemSet, List<IFileItem>> {
+public class ReviewSetContentSection {
 
 	private static final int MAXIMUM_ITEMS_SHOWN = 30;
 
@@ -70,50 +76,117 @@ public class ReviewSetContentSection implements IRemoteEmfObserver<IReviewItemSe
 
 	private TableViewer viewer;
 
-	private final RemoteEmfConsumer<IReviewItemSet, List<IFileItem>, ?, ?, String> consumer;
+	private boolean buttonsUpdated;
 
-	private boolean createdContentSection;
+	private final IRemoteEmfObserver<IRepository, IReview, String, Date> reviewObserver = new RemoteEmfObserver<IRepository, IReview, String, Date>() {
+		@Override
+		public void updated(IRepository parentObject, IReview modelObject, boolean modified) {
+			updateButtons();
+		}
+	};
+
+	private final ReviewItemSetRemoteFactory.Client setClient = new ReviewItemSetRemoteFactory.Client() {
+
+		@Override
+		protected boolean isClientReady() {
+			return section != null && !section.isDisposed();
+		}
+
+		@Override
+		protected void update() {
+			updateButtons();
+		}
+	};
+
+	private final ReviewItemSetContentRemoteFactory.Client itemsClient = new ReviewItemSetContentRemoteFactory.Client() {
 
-	private boolean requestedModelContents;
+		@Override
+		protected boolean isClientReady() {
+			return section != null && !section.isDisposed();
+		}
+
+		@Override
+		protected void create() {
+			updateMessage();
+			createItemSetTable(tableContainer);
+			viewer.setInput(set);
+			getParentSection().getTaskEditorPage().reflow();
+		}
 
-	private boolean modelContentsCurrent;
+		@Override
+		protected void update() {
+			super.update();
+			updateMessage();
+			viewer.setInput(set);
+			updateButtons();
+		}
+
+		@Override
+		protected void updating() {
+			updateMessage();
+		}
+
+		@Override
+		public void failed(IReviewItemSet parent, List<IFileItem> object, IStatus status) {
+			Status errorStatus = new Status(IStatus.ERROR, TasksUiPlugin.ID_PLUGIN, "Error loading patch set",
+					status.getException());
+			StatusHandler.log(errorStatus);
+			if (getParentSection().getSection().getTextClient() != null) {
+				AbstractReviewSection.appendMessage(getParentSection().getSection(), "Couldn't load patch set.");
+			}
+		}
+	};
 
 	private Composite tableContainer;
 
-	private int lastCommentCount;
+	private Composite actionContainer;
+
+	private Composite actionComposite;
+
+	private RemoteEmfConsumer<IRepository, IReview, String, ?, ?, Date> reviewConsumer;
 
 	public ReviewSetContentSection(ReviewSetSection parentSection, final IReviewItemSet set) {
 		this.parentSection = parentSection;
 		this.set = set;
+		//We assume that the last item is also the "current" item
+//		List<IReviewItemSet> items = set.getReview().getSets();
+//		if (items.get(items.size() - 1) == set) {
+//			style |= ExpandableComposite.EXPANDED;
+//		}
 		int style = ExpandableComposite.TWISTIE | ExpandableComposite.CLIENT_INDENT
 				| ExpandableComposite.LEFT_TEXT_CLIENT_ALIGNMENT;
-		//We assume that the last item is also the "current" item
-		List<IReviewItemSet> items = set.getReview().getSets();
-		if (items.get(items.size() - 1) == set) {
-			style |= ExpandableComposite.EXPANDED;
-		}
-		consumer = getParentSection().getReviewEditorPage()
-				.getFactoryProvider()
-				.getReviewItemSetContentFactory()
-				.getConsumerForRemoteKey(set, set.getId());
-		consumer.addObserver(this);
 		section = parentSection.getToolkit().createSection(parentSection.getComposite(), style);
 		GridDataFactory.fillDefaults().grab(true, false).applyTo(section);
 		section.setText(set.getName());
 		section.setTitleBarForeground(parentSection.getToolkit().getColors().getColor(IFormColors.TITLE));
 
 		parentSection.addTextClient(parentSection.getToolkit(), section, "", false); //$NON-NLS-1$
-		updateMessage();
 
-		if (section.isExpanded()) {
-			onExpanded();
-		}
 		section.addExpansionListener(new ExpansionAdapter() {
 			@Override
 			public void expansionStateChanged(ExpansionEvent e) {
-				onExpanded();
+				if (e.getState()) {
+					itemsClient.populate();
+					updateButtons();
+				}
 			}
 		});
+		createPatchSetControls();
+		updateMessage();
+	}
+
+	private void createPatchSetControls() {
+		RemoteEmfConsumer<IReview, IReviewItemSet, String, ?, ?, String> setConsumer = getParentSection().getReviewEditorPage()
+				.getFactoryProvider()
+				.getReviewItemSetFactory()
+				.getConsumerForModel(set.getParentReview(), set);
+		setClient.setConsumer(setConsumer);
+		RemoteEmfConsumer<IReviewItemSet, List<IFileItem>, String, ?, ?, Long> contentConsumer = getParentSection().getReviewEditorPage()
+				.getFactoryProvider()
+				.getReviewItemSetContentFactory()
+				.getConsumerForLocalKey(set, set.getId());
+		itemsClient.setConsumer(contentConsumer);
+		createMainSection();
 	}
 
 	public void updateMessage() {
@@ -127,23 +200,14 @@ public class ReviewSetContentSection implements IRemoteEmfObserver<IReviewItemSe
 			message = NLS.bind("{0}", time);
 		}
 
-		if (consumer.isRetrieving()) {
+		if (itemsClient.getConsumer().isRetrieving()) {
 			message += " " + org.eclipse.mylyn.internal.reviews.ui.Messages.Reviews_RetrievingContents;
 		}
 
 		AbstractReviewSection.appendMessage(getSection(), message);
 	}
 
-	protected void onExpanded() {
-		updateMessage();
-		if (!requestedModelContents) {
-			consumer.retrieve(false);
-			requestedModelContents = true;
-		}
-		checkCreateModelControls();
-	}
-
-	void createContents() {
+	void createMainSection() {
 		Composite composite = parentSection.getToolkit().createComposite(section);
 		GridLayoutFactory.fillDefaults().numColumns(2).applyTo(composite);
 		section.setClient(composite);
@@ -198,9 +262,9 @@ public class ReviewSetContentSection implements IRemoteEmfObserver<IReviewItemSe
 		GridDataFactory.fillDefaults().span(2, 1).grab(true, true).applyTo(tableContainer);
 		GridLayoutFactory.fillDefaults().numColumns(2).applyTo(tableContainer);
 
-		Composite actionComposite = getParentSection().getUiFactoryProvider().createButtons(getParentSection(),
-				composite, parentSection.getToolkit(), set);
-		GridDataFactory.fillDefaults().span(2, 1).applyTo(actionComposite);
+		actionContainer = new Composite(composite, SWT.NONE);
+		GridDataFactory.fillDefaults().span(2, 1).grab(true, true).applyTo(actionContainer);
+		GridLayoutFactory.fillDefaults().numColumns(2).applyTo(actionContainer);
 
 		parentSection.getTaskEditorPage().reflow();
 	}
@@ -261,25 +325,16 @@ public class ReviewSetContentSection implements IRemoteEmfObserver<IReviewItemSe
 		EditorUtil.addScrollListener(viewer.getTable());
 	}
 
-	/**
-	 * We don't know whether the model or the controls will be available first, so we handle both cases here.
-	 */
-	private void checkCreateModelControls() {
-		if (section.isExpanded()) {
-			if (!createdContentSection) {
-				createdContentSection = true;
-				createContents();
-			}
-			if (requestedModelContents && !set.getItems().isEmpty() && ((viewer == null || !modelContentsCurrent))) {
-				modelContentsCurrent = true;
-				if (viewer == null) {
-					createItemSetTable(tableContainer);
-				}
-				viewer.setInput(set);
-				getParentSection().getTaskEditorPage().reflow();
+	public void updateButtons() {
+		if (!buttonsUpdated && setClient.getConsumer().getRemoteObject() != null) {
+			if (actionComposite != null) {
+				actionComposite.dispose();
 			}
+			actionComposite = getParentSection().getUiFactoryProvider().createButtons(getParentSection(),
+					actionContainer, parentSection.getToolkit(), set);
+			getParentSection().getTaskEditorPage().reflow();
+			buttonsUpdated = true;
 		}
-		updateMessage();
 	}
 
 	public Section getSection() {
@@ -290,38 +345,12 @@ public class ReviewSetContentSection implements IRemoteEmfObserver<IReviewItemSe
 		return parentSection;
 	}
 
-	public void created(IReviewItemSet parent, List<IFileItem> object) {
-	}
-
-	public void updating(IReviewItemSet parent, List<IFileItem> object) {
-		updateMessage();
-	}
-
-	public void updated(IReviewItemSet parent, List<IFileItem> items, boolean modified) {
-		modelContentsCurrent &= !modified;
-		int currentCommentSize = 0;
-		for (IFileItem item : items) {
-			currentCommentSize += item.getAllComments().size();
-		}
-		modelContentsCurrent &= lastCommentCount == currentCommentSize;
-		requestedModelContents |= lastCommentCount != currentCommentSize;
-		lastCommentCount = currentCommentSize;
-		checkCreateModelControls();
-	}
-
-	public void failed(IReviewItemSet parent, List<IFileItem> object, IStatus status) {
-		Status errorStatus = new Status(IStatus.ERROR, TasksUiPlugin.ID_PLUGIN, "Error loading patch set",
-				status.getException());
-		StatusHandler.log(errorStatus);
-		if (getParentSection().getSection().getTextClient() != null) {
-			AbstractReviewSection.appendMessage(getParentSection().getSection(), "Couldn't load patch set.");
-		} else {
-			((AbstractReviewTaskEditorPage) getParentSection().getTaskEditorPage()).failed(null, null, errorStatus);
-		}
-	}
-
 	public void dispose() {
-		consumer.removeObserver(this);
+		itemsClient.dispose();
 		section.dispose();
 	}
+
+	public void updateReview() {
+		updateButtons();
+	}
 }
diff --git a/org.eclipse.mylyn.reviews.ui/src/org/eclipse/mylyn/reviews/ui/spi/editor/ReviewSetSection.java b/org.eclipse.mylyn.reviews.ui/src/org/eclipse/mylyn/reviews/ui/spi/editor/ReviewSetSection.java
index 1bcd15e6e..9e1a4112f 100644
--- a/org.eclipse.mylyn.reviews.ui/src/org/eclipse/mylyn/reviews/ui/spi/editor/ReviewSetSection.java
+++ b/org.eclipse.mylyn.reviews.ui/src/org/eclipse/mylyn/reviews/ui/spi/editor/ReviewSetSection.java
@@ -64,15 +64,19 @@ public abstract class ReviewSetSection extends AbstractReviewSection {
 		for (IReviewItem item : getReview().getSets()) {
 			if (item instanceof IReviewItemSet) {
 				IReviewItemSet set = (IReviewItemSet) item;
-				ReviewSetContentSection subSection = createContentSubSection(set);
+				ReviewSetContentSection subSection = new ReviewSetContentSection(this, set);
 				reviewSetSections.add(subSection);
 			}
 		}
+		revealPatchSet(reviewSetSections.size());
 		getTaskEditorPage().reflow();
 	}
 
-	protected ReviewSetContentSection createContentSubSection(IReviewItemSet set) {
-		return new ReviewSetContentSection(this, set);
+	@Override
+	protected void updateModelContent() {
+		for (ReviewSetContentSection section : reviewSetSections) {
+			section.updateReview();
+		}
 	}
 
 	@Override
diff --git a/org.eclipse.mylyn.reviews.ui/src/org/eclipse/mylyn/reviews/ui/spi/factories/UiDataLocator.java b/org.eclipse.mylyn.reviews.ui/src/org/eclipse/mylyn/reviews/ui/spi/factories/UiDataLocator.java
new file mode 100644
index 000000000..7e44d7a8a
--- /dev/null
+++ b/org.eclipse.mylyn.reviews.ui/src/org/eclipse/mylyn/reviews/ui/spi/factories/UiDataLocator.java
@@ -0,0 +1,33 @@
+/*******************************************************************************
+ * Copyright (c) 2013 Tasktop Technologies 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
+ * http://www.eclipse.org/legal/epl-v10.html
+ *
+ * Contributors:
+ *     Tasktop Technologies - initial API and implementation
+ *******************************************************************************/
+
+package org.eclipse.mylyn.reviews.ui.spi.factories;
+
+import java.io.File;
+
+import org.eclipse.core.runtime.IPath;
+import org.eclipse.core.runtime.Path;
+import org.eclipse.mylyn.internal.tasks.ui.TasksUiPlugin;
+import org.eclipse.mylyn.reviews.core.spi.remote.AbstractDataLocator;
+
+/**
+ * @author Miles Parker
+ */
+public class UiDataLocator extends AbstractDataLocator {
+
+	private static final String MODEL_DIR = "model"; //$NON-NLS-1$
+
+	@SuppressWarnings("restriction")
+	@Override
+	public IPath getSystemPath() {
+		return new Path(TasksUiPlugin.getTaskDataManager().getDataPath() + File.separator + MODEL_DIR);
+	}
+}
diff --git a/org.eclipse.mylyn.reviews.ui/src/org/eclipse/mylyn/reviews/ui/spi/remote/RemoteUiService.java b/org.eclipse.mylyn.reviews.ui/src/org/eclipse/mylyn/reviews/ui/spi/remote/RemoteUiService.java
index 6a4b9f00b..69770f5c7 100644
--- a/org.eclipse.mylyn.reviews.ui/src/org/eclipse/mylyn/reviews/ui/spi/remote/RemoteUiService.java
+++ b/org.eclipse.mylyn.reviews.ui/src/org/eclipse/mylyn/reviews/ui/spi/remote/RemoteUiService.java
@@ -23,18 +23,26 @@ public class RemoteUiService extends JobRemoteService {
 
 	@Override
 	public void modelExec(final Runnable runnable, boolean block) {
+		Display displayThread = Display.getCurrent();
+		if (displayThread == null) {
+			displayThread = Display.getDefault();
+		}
 		if (block) {
-			getDisplayThread().syncExec(runnable);
+			if (Display.getCurrent() != null) {
+				//Don't cause deadlock, just execute now!
+				runnable.run();
+			} else {
+				displayThread.syncExec(runnable);
+			}
 		} else {
-			getDisplayThread().asyncExec(runnable);
+			displayThread.asyncExec(runnable);
 		}
 	}
 
-	private Display getDisplayThread() {
-		Display displayThread = Display.getCurrent();
-		if (displayThread == null) {
-			displayThread = Display.getDefault();
+	@Override
+	public void ensureModelThread() {
+		if (Display.getCurrent() == null) {
+			throw new RuntimeException("Attempted to execute a model-related operation in a non-model thread.");
 		}
-		return displayThread;
 	}
 }