Bug 399628 - IllegalArgumentException in InternalTreeModelViewer#insertv20130206-213315I20130212-0800

2 files changed, 57 insertions, 12 deletions

diff --git a/org.eclipse.debug.tests/src/org/eclipe/debug/tests/viewer/model/UpdateTests.java b/org.eclipse.debug.tests/src/org/eclipe/debug/tests/viewer/model/UpdateTests.java
index 1b23bdb4e..bd2deeaaf 100644
--- a/org.eclipse.debug.tests/src/org/eclipe/debug/tests/viewer/model/UpdateTests.java
+++ b/org.eclipse.debug.tests/src/org/eclipe/debug/tests/viewer/model/UpdateTests.java
@@ -22,6 +22,7 @@ import org.eclipse.core.commands.ExecutionException;
 import org.eclipse.debug.internal.ui.viewers.model.provisional.IChildrenCountUpdate;
 import org.eclipse.debug.internal.ui.viewers.model.provisional.IHasChildrenUpdate;
 import org.eclipse.debug.internal.ui.viewers.model.provisional.IModelDelta;
+import org.eclipse.debug.internal.ui.viewers.model.provisional.IModelDeltaVisitor;
 import org.eclipse.debug.internal.ui.viewers.model.provisional.ITreeModelViewer;
 import org.eclipse.debug.internal.ui.viewers.model.provisional.IViewerUpdate;
 import org.eclipse.debug.internal.ui.viewers.model.provisional.ModelDelta;
@@ -327,6 +328,49 @@ abstract public class UpdateTests extends TestCase implements ITestModelUpdatesL
         childrenCountUpdateListener.dispose();
     }
 
+
+    /**
+     * This test case attempts to create a race condition between processing 
+     * of the content updates and processing of add/remove model deltas. 
+     * <br>
+     * See <a href="https://bugs.eclipse.org/bugs/show_bug.cgi?id=304066">bug 304066</a> 
+     */
+    public void testInsertAtInvalidIndex() throws InterruptedException {
+        TestModel model = TestModel.simpleSingleLevel();
+        fViewer.setAutoExpandLevel(-1);
+
+        // Create the listener
+        fListener.reset(TreePath.EMPTY, model.getRootElement(), -1, false, false); 
+
+        // Set the input into the view and update the view.
+        fViewer.setInput(model.getRootElement());
+        while (!fListener.isFinished()) if (!fDisplay.readAndDispatch ()) Thread.sleep(0);
+        model.validateData(fViewer, TreePath.EMPTY);
+
+        // Insert element at the end of the list.
+        final int insertIndex = model.getRootElement().getChildren().length;
+        ModelDelta delta = model.insertElementChild(TreePath.EMPTY, insertIndex, new TestElement(model, "last - invalid index", new TestElement[0]));
+        // Change insert index to out of range
+        delta.accept(new IModelDeltaVisitor() {
+			
+			public boolean visit(IModelDelta visitorDelta, int depth) {
+				if ((visitorDelta.getFlags() & IModelDelta.INSERTED) != 0) {
+					((ModelDelta)visitorDelta).setIndex(insertIndex + 1);
+					return false;
+				}
+				return true;
+			}
+		});
+        
+        // Remove delta should generate no new updates, but we still need to wait for the event to
+        // be processed.
+        fListener.reset();
+        model.postDelta(delta);
+        
+        while (!fListener.isFinished(MODEL_CHANGED_COMPLETE | CONTENT_SEQUENCE_COMPLETE | LABEL_SEQUENCE_COMPLETE)) 
+            if (!fDisplay.readAndDispatch ()) Thread.sleep(0);
+        model.validateData(fViewer, TreePath.EMPTY);                
+    }
     
     /**
      * This test forces the viewer to reschedule pending content updates
diff --git a/org.eclipse.debug.ui/ui/org/eclipse/debug/internal/ui/viewers/model/TreeModelContentProvider.java b/org.eclipse.debug.ui/ui/org/eclipse/debug/internal/ui/viewers/model/TreeModelContentProvider.java
index 4f6bd9d9f..5a8f563fc 100644
--- a/org.eclipse.debug.ui/ui/org/eclipse/debug/internal/ui/viewers/model/TreeModelContentProvider.java
+++ b/org.eclipse.debug.ui/ui/org/eclipse/debug/internal/ui/viewers/model/TreeModelContentProvider.java
@@ -1365,20 +1365,21 @@ public class TreeModelContentProvider implements ITreeModelContentProvider, ICon
 		TreePath parentPath = getViewerTreePath(delta.getParentDelta());
 		Object element = delta.getElement();
 		int modelIndex = delta.getIndex();
-		if (modelIndex >= 0) {
-			int viewIndex = modelToViewIndex(parentPath, modelIndex);
-			if (viewIndex >= 0) {
-				if (DebugUIPlugin.DEBUG_CONTENT_PROVIDER && DebugUIPlugin.DEBUG_TEST_PRESENTATION_ID(getPresentationContext())) {
-					DebugUIPlugin.trace("handleInsert(" + parentPath.getLastSegment() + ", (model) " + modelIndex + " (view) " + viewIndex + ", " + element); //$NON-NLS-1$ //$NON-NLS-2$ //$NON-NLS-3$ //$NON-NLS-4$
-				}
-				getViewer().insert(parentPath, element, viewIndex);
-			} else {
-				if (DebugUIPlugin.DEBUG_CONTENT_PROVIDER && DebugUIPlugin.DEBUG_TEST_PRESENTATION_ID(getPresentationContext())) {
-					DebugUIPlugin.trace("[filtered] handleInsert(" + delta.getElement() + ") > modelIndex: " + modelIndex); //$NON-NLS-1$ //$NON-NLS-2$
-				}
-				// Element is filtered - do not insert
+		int viewIndex = modelIndex >= 0 ? modelToViewIndex(parentPath, modelIndex) : -1; 
+		int viewCount = getViewer().getChildCount(parentPath);
+		if (viewIndex >= 0 && viewIndex <= viewCount) {
+			// Index in range, insert.
+			if (DebugUIPlugin.DEBUG_CONTENT_PROVIDER && DebugUIPlugin.DEBUG_TEST_PRESENTATION_ID(getPresentationContext())) {
+				DebugUIPlugin.trace("handleInsert(" + parentPath.getLastSegment() + ", (model) " + modelIndex + " (view) " + viewIndex + ", " + element); //$NON-NLS-1$ //$NON-NLS-2$ //$NON-NLS-3$ //$NON-NLS-4$
+			}
+			getViewer().insert(parentPath, element, viewIndex);
+		} else if (modelIndex >= 0 && viewIndex < 0) {
+			// Element is filtered - do not insert
+			if (DebugUIPlugin.DEBUG_CONTENT_PROVIDER && DebugUIPlugin.DEBUG_TEST_PRESENTATION_ID(getPresentationContext())) {
+				DebugUIPlugin.trace("[filtered] handleInsert(" + delta.getElement() + ") > modelIndex: " + modelIndex); //$NON-NLS-1$ //$NON-NLS-2$
 			}
 		} else {
+			// Invalid index given, do a refresh instead.
 			if (DebugUIPlugin.DEBUG_CONTENT_PROVIDER && DebugUIPlugin.DEBUG_TEST_PRESENTATION_ID(getPresentationContext())) {
 				DebugUIPlugin.trace("handleInsert(" + delta.getElement() + ")"); //$NON-NLS-1$ //$NON-NLS-2$
 			}