Bug 550220 Sometimes incomplete stack is shown for threads that were

expanded before resume

CheckState model delta visitor removes delta if parent flags are
NO_CHANGE and there are no pending updates reported for it from content
provider. In case of this bug, the updates for elements (threads in
view) are a bit late and CheckState visitor removes that delta. The
removal of delta causes improper view like expanded thread with only one
frame.

Change-Id: Ia1d33d6e59db285dbd6c1acf4503b7406809ecdb
Signed-off-by: Umair Sair <umair_sair@hotmail.com>

2 files changed, 189 insertions, 23 deletions

diff --git a/org.eclipse.debug.tests/src/org/eclipse/debug/tests/viewer/model/StateTests.java b/org.eclipse.debug.tests/src/org/eclipse/debug/tests/viewer/model/StateTests.java
index 312e928bc..1883dfcb9 100644
--- a/org.eclipse.debug.tests/src/org/eclipse/debug/tests/viewer/model/StateTests.java
+++ b/org.eclipse.debug.tests/src/org/eclipse/debug/tests/viewer/model/StateTests.java
@@ -17,6 +17,7 @@ package org.eclipse.debug.tests.viewer.model;
 import java.util.Arrays;
 import java.util.HashSet;
 import java.util.Set;
+import java.util.function.Supplier;
 
 import org.eclipse.debug.internal.ui.viewers.model.IInternalTreeModelViewer;
 import org.eclipse.debug.internal.ui.viewers.model.provisional.IModelDelta;
@@ -164,6 +165,68 @@ abstract public class StateTests extends AbstractViewerModelTest implements ITes
 		return model;
 	}
 
+	/**
+	 * Creates a model in the pattern of:
+	 *
+	 * <pre>
+	 * root
+	 *   1
+	 *     1.1
+	 *     1.2
+	 *     ...
+	 *     1.childrenCount
+	 *   2
+	 *     2.1
+	 *     2.2
+	 *     ...
+	 *     2.childrenCount
+	 *   3
+	 *     3.1
+	 *     3.2
+	 *     ...
+	 *     3.childrenCount
+	 *   ...
+	 *   (size)
+	 *     (size).1
+	 *     (size).2
+	 *     ...
+	 *     (size).childrenCount
+	 * </pre>
+	 *
+	 * @param size The number of elements in the tree
+	 * @param childrenCount Number of children of each element
+	 * @param shouldReturnChildren The supplier dictates whether children should
+	 *            be reported when fetched
+	 * @return The model
+	 */
+	static TestModel alternatingSubtreesModelWithChildren(int size, int childrenCount, Supplier<Boolean> shouldReturnChildren) {
+		TestModel model = new TestModel();
+		TestElement[] elements = new TestElement[size];
+		for (int i = 0; i < size; i++) {
+			String text = Integer.toString(i + 1);
+
+			TestElement[] children = new TestElement[childrenCount];
+			for (int x = 0; x < childrenCount; x++) {
+				children[x] = new TestElement(model, text + "." + (x + 1), new TestElement[0]);
+			}
+
+			elements[i] = new TestElement(model, text, children) {
+				@Override
+				public TestElement[] getChildren() {
+					if (shouldReturnChildren.get()) {
+						return super.getChildren();
+					}
+
+					return new TestElement[0];
+				}
+			};
+		}
+
+		model.setRoot(new TestElement(model, "root", elements));
+
+		return model;
+	}
+
 	static boolean areTreeSelectionsEqual(ITreeSelection sel1, ITreeSelection sel2) {
 		Set<TreePath> sel1Set = new HashSet<>();
 		sel1Set.addAll( Arrays.asList(sel1.getPaths()) );
@@ -347,6 +410,130 @@ new TreePath[] { model.findElement("5"), model.findElement("5.1"), model.findEle
 		assertTrue( fListener.checkCoalesced(TreePath.EMPTY, 0, 5) );
 	}
 
+	public void testKeepCollapsedAfterRemovingAndReaddingChildrenInExpandedTree() throws Exception {
+		boolean showChildren[] = new boolean[] { true };
+		int size = 3;
+		Supplier<Boolean> shouldShowChildren = () -> showChildren[0];
+
+		TestModel model = alternatingSubtreesModelWithChildren(size, 10, shouldShowChildren);
+
+		// Create the listener, only check the first level
+		fListener.reset(TreePath.EMPTY, model.getRootElement(), 1, true, false);
+
+		// Set the input into the view and update the view.
+		fViewer.setInput(model.getRootElement());
+		waitWhile(t -> !fListener.isFinished(), createListenerErrorMessage());
+		model.validateData(fViewer, TreePath.EMPTY, true);
+
+		/*
+		 * 1. Trigger model update with expansion of all elements
+		 */
+		{
+			fListener.reset();
+			fListener.setFailOnRedundantUpdates(false);
+
+			TestElement rootElement = model.getRootElement();
+			TestElement[] children = rootElement.getChildren();
+			ModelDelta rootDelta = new ModelDelta(rootElement, IModelDelta.NO_CHANGE);
+			ModelDelta expandDelta = model.getBaseDelta(rootDelta);
+			for (int i = 0; i < children.length; i++) {
+				TestElement element = children[i];
+				ModelDelta delta = expandDelta;
+				int index = i;
+				while (element.getChildren().length != 0) {
+					TreePath elementPath = model.findElement(element.getLabel());
+					fListener.addUpdates(elementPath, element, 1, CHILD_COUNT_UPDATES | CHILDREN_UPDATES);
+					delta = delta.addNode(element, index, IModelDelta.EXPAND | IModelDelta.CONTENT, element.getChildren().length);
+					element = element.getChildren()[0];
+					index = 0;
+				}
+			}
+
+			model.postDelta(rootDelta);
+
+			TestUtil.waitWhile(t -> !fListener.isFinished(ALL_UPDATES_COMPLETE | MODEL_CHANGED_COMPLETE), null, 300000, t -> "Listener not finished: " + fListener);
+		}
+
+		/*
+		 * 2. Trigger model change with no children
+		 */
+		{
+			fListener.reset();
+			fListener.setFailOnRedundantUpdates(false);
+
+			showChildren[0] = false;
+
+			TestElement rootElement = model.getRootElement();
+			ModelDelta rootDelta = new ModelDelta(rootElement, IModelDelta.CONTENT);
+			model.getBaseDelta(rootDelta);
+
+			TreePath elementPath = TreePath.EMPTY;
+			fListener.addUpdates(elementPath, rootElement, 2, CHILD_COUNT_UPDATES | CHILDREN_UPDATES);
+
+			model.postDelta(rootDelta);
+
+			TestUtil.waitWhile(t -> !fListener.isFinished(ALL_UPDATES_COMPLETE | MODEL_CHANGED_COMPLETE), null, 60000, t -> "Listener not finished: " + fListener);
+		}
+
+		/*
+		 * 3. Trigger model change with expansion for first element and its
+		 * first child selected
+		 */
+		{
+			fListener.reset();
+			fListener.setFailOnRedundantUpdates(false);
+
+			showChildren[0] = true;
+
+			TestElement rootElement = model.getRootElement();
+			TestElement[] children = rootElement.getChildren();
+			ModelDelta rootDelta = new ModelDelta(rootElement, IModelDelta.NO_CHANGE);
+			ModelDelta delta = model.getBaseDelta(rootDelta);
+
+			// Expand the element and select first child
+			TestElement element = children[0];
+			delta = delta.addNode(element, 0, IModelDelta.EXPAND, element.getChildren().length);
+
+			TreePath elementPath = model.findElement(element.getLabel());
+			fListener.addUpdates(elementPath, element, 2, CHILDREN_UPDATES);
+
+			element = element.getChildren()[0];
+			delta = delta.addNode(element, 0, IModelDelta.SELECT, -1);
+
+			model.postDelta(rootDelta);
+
+			TestUtil.waitWhile(t -> !fListener.isFinished(ALL_UPDATES_COMPLETE | MODEL_CHANGED_COMPLETE), null, 60000, t -> "Listener not finished: " + fListener);
+		}
+
+		/*
+		 * 4. Trigger model change to update all elements and their plus state
+		 */
+		{
+			fListener.reset();
+			fListener.setFailOnRedundantUpdates(false);
+
+			showChildren[0] = true;
+
+			TestElement rootElement = model.getRootElement();
+			ModelDelta rootDelta = new ModelDelta(rootElement, IModelDelta.CONTENT);
+			model.getBaseDelta(rootDelta);
+
+			TestElement element = rootElement.getChildren()[0];
+			TreePath elementPath = model.findElement(element.getLabel());
+			fListener.addUpdates(elementPath, element, 1, CHILDREN_UPDATES);
+
+			model.postDelta(rootDelta);
+
+			TestUtil.waitWhile(t -> !fListener.isFinished(ALL_UPDATES_COMPLETE | MODEL_CHANGED_COMPLETE), null, 6000000, t -> "Listener not finished: " + fListener);
+		}
+
+		/*
+		 * Only first element should be expanded, all other should be collapsed
+		 */
+		for (int i = 1; i <= size; i++) {
+			assertTrue(getInternalViewer().getExpandedState(model.findElement(Integer.toString(i))) == (i == 1));
+		}
+	}
 
 	public void testPreserveExpandedOnSubTreeContent() throws Exception {
 		//TreeModelViewerAutopopulateAgent autopopulateAgent = new TreeModelViewerAutopopulateAgent(fViewer);
diff --git a/org.eclipse.debug.ui/ui/org/eclipse/debug/internal/ui/viewers/model/ViewerStateTracker.java b/org.eclipse.debug.ui/ui/org/eclipse/debug/internal/ui/viewers/model/ViewerStateTracker.java
index 168ac1b6a..e750002ea 100644
--- a/org.eclipse.debug.ui/ui/org/eclipse/debug/internal/ui/viewers/model/ViewerStateTracker.java
+++ b/org.eclipse.debug.ui/ui/org/eclipse/debug/internal/ui/viewers/model/ViewerStateTracker.java
@@ -11,6 +11,7 @@
  * Contributors:
  *     Wind River Systems - initial API and implementation
  *     IBM Corporation - bug fixing
+ *     Umair Sair (Mentor Graphics) - Bug 550220
  *******************************************************************************/
 package org.eclipse.debug.internal.ui.viewers.model;
 
@@ -84,13 +85,6 @@ class ViewerStateTracker {
 	static final int STATE_RESTORE_SEQUENCE_COMPLETE = 7;
 
 	/**
-	 * Dummy marker element used in the state delta. The marker indicates that a
-	 * given element in the pending state delta has been removed. It replaces
-	 * the original element so that it may optionally be garbage collected.
-	 */
-	private final static String ELEMENT_REMOVED = "ELEMENT_REMOVED"; //$NON-NLS-1$
-
-	/**
 	 * Collector of memento encoding requests.
 	 */
 	interface IElementMementoCollector {
@@ -1035,7 +1029,7 @@ class ViewerStateTracker {
 
 				if (flags != IModelDelta.NO_CHANGE) {
 					IModelDelta parentDelta = delta.getParentDelta();
-					// Remove the delta if :
+					// Do not visit children if :
 					// - The parent delta has no more flags on it (the content flag is removed as well),
 					// which means that parent element's children have been completely exposed.
 					// - There are no more pending updates for the element.
@@ -1045,7 +1039,6 @@ class ViewerStateTracker {
 						if ( !fContentProvider.areElementUpdatesPending(deltaPath) &&
 							 (!(delta.getElement() instanceof IMemento) || !areMementoUpdatesPending(delta)) )
 						{
-							removeDelta(delta);
 							return false;
 						}
 					}
@@ -1070,20 +1063,6 @@ class ViewerStateTracker {
 				}
 				return false;
 			}
-
-			private void removeDelta(IModelDelta delta) {
-				if (DebugUIPlugin.DEBUG_STATE_SAVE_RESTORE && DebugUIPlugin.DEBUG_TEST_PRESENTATION_ID(fContentProvider.getPresentationContext())) {
-					DebugUIPlugin.trace("\tRESTORE REMOVED: " + delta.getElement()); //$NON-NLS-1$
-				}
-
-				delta.accept((_visitorDelta, depth) -> {
-					ModelDelta visitorDelta = (ModelDelta) _visitorDelta;
-					visitorDelta.setElement(ELEMENT_REMOVED);
-					visitorDelta.setFlags(IModelDelta.NO_CHANGE);
-					return true;
-				});
-
-			}
 		}
 
 		CheckState state = new CheckState();