Bug 291267 - Additional fixes for intermittent failures in restoring viewer state.
diff --git a/org.eclipse.debug.tests/src/org/eclipe/debug/tests/viewer/model/StateTests.java b/org.eclipse.debug.tests/src/org/eclipe/debug/tests/viewer/model/StateTests.java index 70058c5..04571b6 100644 --- a/org.eclipse.debug.tests/src/org/eclipe/debug/tests/viewer/model/StateTests.java +++ b/org.eclipse.debug.tests/src/org/eclipe/debug/tests/viewer/model/StateTests.java
@@ -19,6 +19,7 @@ import org.eclipse.debug.internal.ui.viewers.model.provisional.IModelDelta; import org.eclipse.debug.internal.ui.viewers.model.provisional.ModelDelta; import org.eclipse.jface.viewers.TreePath; +import org.eclipse.jface.viewers.TreeSelection; import org.eclipse.swt.layout.FillLayout; import org.eclipse.swt.widgets.Display; import org.eclipse.swt.widgets.Shell; @@ -224,7 +225,6 @@ viewer.setExpandedState(path, true); while (!fListener.isFinished()) if (!fDisplay.readAndDispatch ()) fDisplay.sleep (); model.validateData(fViewer, TreePath.EMPTY, true); - } public void testPreserveExpandedOnRemove() { @@ -243,6 +243,10 @@ expandAlternateElements(model); + // Set a selection in view + TreeSelection originalSelection = new TreeSelection(model.findElement("5.1.1")); + fViewer.setSelection(originalSelection); + // Update the model ModelDelta delta = model.removeElementChild(TreePath.EMPTY, 0); @@ -251,15 +255,17 @@ model.postDelta(delta); while (!fListener.isFinished(MODEL_CHANGED_COMPLETE)) if (!fDisplay.readAndDispatch ()) fDisplay.sleep (); + + // Validate data model.validateData(fViewer, TreePath.EMPTY, true); - ITreeModelContentProviderTarget viewer = (ITreeModelContentProviderTarget)fViewer; - Assert.assertTrue(viewer.getExpandedState(model.findElement("2")) == false); - Assert.assertTrue(viewer.getExpandedState(model.findElement("3")) == true); - Assert.assertTrue(viewer.getExpandedState(model.findElement("3.1")) == true); - Assert.assertTrue(viewer.getExpandedState(model.findElement("4")) == false); - Assert.assertTrue(viewer.getExpandedState(model.findElement("5")) == true); - Assert.assertTrue(viewer.getExpandedState(model.findElement("5.1")) == true); - Assert.assertTrue(viewer.getExpandedState(model.findElement("6")) == false); + Assert.assertTrue(getCTargetViewer().getExpandedState(model.findElement("2")) == false); + Assert.assertTrue(getCTargetViewer().getExpandedState(model.findElement("3")) == true); + Assert.assertTrue(getCTargetViewer().getExpandedState(model.findElement("3.1")) == true); + Assert.assertTrue(getCTargetViewer().getExpandedState(model.findElement("4")) == false); + Assert.assertTrue(getCTargetViewer().getExpandedState(model.findElement("5")) == true); + Assert.assertTrue(getCTargetViewer().getExpandedState(model.findElement("5.1")) == true); + Assert.assertTrue(getCTargetViewer().getExpandedState(model.findElement("6")) == false); + Assert.assertEquals(originalSelection, fViewer.getSelection()); } public void testPreserveExpandedOnInsert() { @@ -278,6 +284,10 @@ expandAlternateElements(model); + // Set a selection in view + TreeSelection originalSelection = new TreeSelection(model.findElement("5.1.1")); + fViewer.setSelection(originalSelection); + // Update the model ModelDelta delta = model.insertElementChild(TreePath.EMPTY, 0, new TestElement(model, "0 - new", new TestElement[0])); @@ -289,21 +299,22 @@ model.postDelta(delta); while (!fListener.isFinished(MODEL_CHANGED_COMPLETE | ALL_UPDATES_COMPLETE)) if (!fDisplay.readAndDispatch ()) fDisplay.sleep (); + + // Validate data model.validateData(fViewer, TreePath.EMPTY, true); - ITreeModelContentProviderTarget viewer = (ITreeModelContentProviderTarget)fViewer; - Assert.assertTrue(viewer.getExpandedState(model.findElement("1")) == true); - Assert.assertTrue(viewer.getExpandedState(model.findElement("1.1")) == true); - Assert.assertTrue(viewer.getExpandedState(model.findElement("2")) == false); - Assert.assertTrue(viewer.getExpandedState(model.findElement("3")) == true); - Assert.assertTrue(viewer.getExpandedState(model.findElement("3.1")) == true); - Assert.assertTrue(viewer.getExpandedState(model.findElement("4")) == false); - Assert.assertTrue(viewer.getExpandedState(model.findElement("5")) == true); - Assert.assertTrue(viewer.getExpandedState(model.findElement("5.1")) == true); - Assert.assertTrue(viewer.getExpandedState(model.findElement("6")) == false); + Assert.assertTrue(getCTargetViewer().getExpandedState(model.findElement("1")) == true); + Assert.assertTrue(getCTargetViewer().getExpandedState(model.findElement("1.1")) == true); + Assert.assertTrue(getCTargetViewer().getExpandedState(model.findElement("2")) == false); + Assert.assertTrue(getCTargetViewer().getExpandedState(model.findElement("3")) == true); + Assert.assertTrue(getCTargetViewer().getExpandedState(model.findElement("3.1")) == true); + Assert.assertTrue(getCTargetViewer().getExpandedState(model.findElement("4")) == false); + Assert.assertTrue(getCTargetViewer().getExpandedState(model.findElement("5")) == true); + Assert.assertTrue(getCTargetViewer().getExpandedState(model.findElement("5.1")) == true); + Assert.assertTrue(getCTargetViewer().getExpandedState(model.findElement("6")) == false); + Assert.assertEquals(originalSelection, fViewer.getSelection()); } - - public void testPreserveExpandedOnContent() { + public void testPreserveExpandedOnContentStress() { //TreeModelViewerAutopopulateAgent autopopulateAgent = new TreeModelViewerAutopopulateAgent(fViewer); TestModel model = alternatingSubsreesModel(); @@ -319,28 +330,59 @@ expandAlternateElements(model); - // Update the model - model.removeElementChild(TreePath.EMPTY, 0); - - // Content delta should generate updates for the elements being re-expanded - ITreeModelContentProviderTarget viewer = (ITreeModelContentProviderTarget)fViewer; - // Note: Re-expanding nodes causes redundant updates. - fListener.reset(false, false); - fListener.addUpdates(viewer, TreePath.EMPTY, model.getRootElement(), -1, ALL_UPDATES_COMPLETE); - model.postDelta(new ModelDelta(model.getRootElement(), IModelDelta.CONTENT)); - while (!fListener.isFinished(ALL_UPDATES_COMPLETE)) - if (!fDisplay.readAndDispatch ()) fDisplay.sleep (); - model.validateData(fViewer, TreePath.EMPTY, true); - Assert.assertTrue(viewer.getExpandedState(model.findElement("2")) == false); - Assert.assertTrue(viewer.getExpandedState(model.findElement("3")) == true); - Assert.assertTrue(viewer.getExpandedState(model.findElement("3.1")) == true); - Assert.assertTrue(viewer.getExpandedState(model.findElement("4")) == false); - Assert.assertTrue(viewer.getExpandedState(model.findElement("5")) == true); - Assert.assertTrue(viewer.getExpandedState(model.findElement("5.1")) == true); - Assert.assertTrue(viewer.getExpandedState(model.findElement("6")) == false); + // Set a selection in view + TreeSelection originalSelection = new TreeSelection(model.findElement("5.1.1")); + fViewer.setSelection(originalSelection); + Assert.assertEquals(originalSelection, fViewer.getSelection()); + + // Run this test ten times as we've seen intermittent failures related + // to timing in it. + for (int i = 0; i < 10; i++) { + // Update the model + model.removeElementChild(TreePath.EMPTY, 0); + + // Note: Re-expanding nodes causes redundant updates. + fListener.reset(false, false); + fListener.addUpdates(getCTargetViewer(), TreePath.EMPTY, model.getRootElement(), -1, ALL_UPDATES_COMPLETE); + model.postDelta(new ModelDelta(model.getRootElement(), IModelDelta.CONTENT)); + while (!fListener.isFinished(ALL_UPDATES_COMPLETE | STATE_RESTORE_COMPLETE)) + if (!fDisplay.readAndDispatch ()) fDisplay.sleep (); + + // Validate data + model.validateData(fViewer, TreePath.EMPTY, true); + Assert.assertTrue(getCTargetViewer().getExpandedState(model.findElement("2")) == false); + Assert.assertTrue(getCTargetViewer().getExpandedState(model.findElement("3")) == true); + Assert.assertTrue(getCTargetViewer().getExpandedState(model.findElement("3.1")) == true); + Assert.assertTrue(getCTargetViewer().getExpandedState(model.findElement("4")) == false); + Assert.assertTrue(getCTargetViewer().getExpandedState(model.findElement("5")) == true); + Assert.assertTrue(getCTargetViewer().getExpandedState(model.findElement("5.1")) == true); + Assert.assertTrue(getCTargetViewer().getExpandedState(model.findElement("6")) == false); + Assert.assertEquals(originalSelection, fViewer.getSelection()); + + // Update the model again + model.addElementChild(TreePath.EMPTY, 0, new TestElement(model, "1", new TestElement[0])); + + // Note: Re-expanding nodes causes redundant updates. + fListener.reset(false, false); + fListener.addUpdates(getCTargetViewer(), TreePath.EMPTY, model.getRootElement(), -1, ALL_UPDATES_COMPLETE); + model.postDelta(new ModelDelta(model.getRootElement(), IModelDelta.CONTENT)); + while (!fListener.isFinished(ALL_UPDATES_COMPLETE | STATE_RESTORE_COMPLETE)) + if (!fDisplay.readAndDispatch ()) fDisplay.sleep (); + + // Validate data + model.validateData(fViewer, TreePath.EMPTY, true); + Assert.assertTrue(getCTargetViewer().getExpandedState(model.findElement("2")) == false); + Assert.assertTrue(getCTargetViewer().getExpandedState(model.findElement("3")) == true); + Assert.assertTrue(getCTargetViewer().getExpandedState(model.findElement("3.1")) == true); + Assert.assertTrue(getCTargetViewer().getExpandedState(model.findElement("4")) == false); + Assert.assertTrue(getCTargetViewer().getExpandedState(model.findElement("5")) == true); + Assert.assertTrue(getCTargetViewer().getExpandedState(model.findElement("5.1")) == true); + Assert.assertTrue(getCTargetViewer().getExpandedState(model.findElement("6")) == false); + Assert.assertEquals(originalSelection, fViewer.getSelection()); + } } - public void testSaveAndRstoreOnInputChange1() { + public void testSaveAndRstore1() { //TreeModelViewerAutopopulateAgent autopopulateAgent = new TreeModelViewerAutopopulateAgent(fViewer); TestModel model = alternatingSubsreesModel(); @@ -357,6 +399,9 @@ // Expand some, but not all elements expandAlternateElements(model); + // Set a selection in view + fViewer.setSelection(new TreeSelection(model.findElement("5.1.1"))); + // Extract the original state from viewer ModelDelta originalState = new ModelDelta(model.getRootElement(), IModelDelta.NO_CHANGE); fViewer.saveElementState(TreePath.EMPTY, originalState, IModelDelta.EXPAND | IModelDelta.SELECT); @@ -382,4 +427,51 @@ Assert.assertTrue( deltaMatches(originalState, restoredState) ); } + public void testSaveAndRstore2() { + //TreeModelViewerAutopopulateAgent autopopulateAgent = new TreeModelViewerAutopopulateAgent(fViewer); + TestModel model = TestModel.simpleMultiLevel(); + + // expand all elements + fViewer.setAutoExpandLevel(-1); + + // 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()); + while (!fListener.isFinished()) if (!fDisplay.readAndDispatch ()) fDisplay.sleep (); + model.validateData(fViewer, TreePath.EMPTY); + + // Set a selection in view + fViewer.setSelection(new TreeSelection(model.findElement("3.2.3"))); + + // Turn off the auto-expand now since we want to text the auto-expand logic + fViewer.setAutoExpandLevel(-1); + + // Extract the original state from viewer + ModelDelta originalState = new ModelDelta(model.getRootElement(), IModelDelta.NO_CHANGE); + fViewer.saveElementState(TreePath.EMPTY, originalState, IModelDelta.EXPAND | IModelDelta.SELECT); + + // Set the viewer input to null. This will trigger the view to save the viewer state. + fListener.reset(true, false); + fListener.addStateUpdates(getCTargetViewer(), TreePath.EMPTY, model.getRootElement()); + + fViewer.setInput(null); + while (!fListener.isFinished(STATE_SAVE_COMPLETE)) if (!fDisplay.readAndDispatch ()) fDisplay.sleep (); + + // Set the viewer input back to the model. When view updates are complete + // the viewer + // Note: disable redundant updates because the reveal delta triggers one. + fListener.reset(TreePath.EMPTY, model.getRootElement(), 1, false, false); + // TODO: add state updates somehow? + fViewer.setInput(model.getRootElement()); + while (!fListener.isFinished()) if (!fDisplay.readAndDispatch ()) fDisplay.sleep (); + + // Extract the restored state from viewer + ModelDelta restoredState = new ModelDelta(model.getRootElement(), IModelDelta.NO_CHANGE); + fViewer.saveElementState(TreePath.EMPTY, restoredState, IModelDelta.EXPAND | IModelDelta.SELECT); + + Assert.assertTrue( deltaMatches(originalState, restoredState) ); + } + }
diff --git a/org.eclipse.debug.tests/src/org/eclipe/debug/tests/viewer/model/TestModelUpdatesListener.java b/org.eclipse.debug.tests/src/org/eclipe/debug/tests/viewer/model/TestModelUpdatesListener.java index ea866c7..ea8e094 100644 --- a/org.eclipse.debug.tests/src/org/eclipe/debug/tests/viewer/model/TestModelUpdatesListener.java +++ b/org.eclipse.debug.tests/src/org/eclipe/debug/tests/viewer/model/TestModelUpdatesListener.java
@@ -53,7 +53,10 @@ private boolean fStateRestoreComplete; private int fViewerUpdatesRunning; private int fLabelUpdatesRunning; - + private int fTimeoutInterval = 60000; + private long fTimeoutTime; + + public TestModelUpdatesListener(boolean failOnRedundantUpdates, boolean failOnMultipleUpdateSequences) { setFailOnRedundantUpdates(failOnRedundantUpdates); setFailOnMultipleUpdateSequences(failOnMultipleUpdateSequences); @@ -66,8 +69,15 @@ public void setFailOnMultipleUpdateSequences(boolean failOnMultipleUpdateSequences) { fFailOnMultipleUpdateSequences = failOnMultipleUpdateSequences; } - - + + /** + * Sets the the maximum amount of time (in milliseconds) that the update listener + * is going to wait. If set to -1, the listener will wait indefinitely. + */ + public void setTimeoutInterval(int milis) { + fTimeoutInterval = milis; + } + public void reset(TreePath path, TestElement element, int levels, boolean failOnRedundantUpdates, boolean failOnMultipleUpdateSequences) { reset(); addUpdates(path, element, levels); @@ -93,6 +103,7 @@ fModelChangedComplete = false; fStateSaveComplete = false; fStateRestoreComplete = false; + fTimeoutTime = System.currentTimeMillis() + fTimeoutInterval; } public void addHasChildrenUpdate(TreePath path) { @@ -202,6 +213,10 @@ } public boolean isFinished(int flags) { + if (fTimeoutInterval > 0 && fTimeoutTime < System.currentTimeMillis()) { + throw new RuntimeException("Test timed out"); + } + if ( (flags & LABEL_UPDATES_COMPLETE) != 0) { if (!fLabelUpdatesComplete) return false; }
diff --git a/org.eclipse.debug.ui/ui/org/eclipse/debug/internal/ui/viewers/model/ChildrenCountUpdate.java b/org.eclipse.debug.ui/ui/org/eclipse/debug/internal/ui/viewers/model/ChildrenCountUpdate.java index 19b0be0..073ff0b 100644 --- a/org.eclipse.debug.ui/ui/org/eclipse/debug/internal/ui/viewers/model/ChildrenCountUpdate.java +++ b/org.eclipse.debug.ui/ui/org/eclipse/debug/internal/ui/viewers/model/ChildrenCountUpdate.java
@@ -76,7 +76,7 @@ return true; } else if (getElementContentProvider().equals(request.getElementContentProvider())) { if (fBatchedRequests == null) { - fBatchedRequests = new ArrayList(); + fBatchedRequests = new ArrayList(4); fBatchedRequests.add(this); } fBatchedRequests.add(request); @@ -103,6 +103,19 @@ getElementContentProvider().update(updates); } } + + boolean containsUpdate(TreePath path) { + if (getElementPath().equals(path)) { + return true; + } else if (fBatchedRequests != null) { + for (int i = 0; i < fBatchedRequests.size(); i++) { + if (((ViewerUpdateMonitor)fBatchedRequests.get(i)).getElementPath().equals(path)) { + return true; + } + } + } + return false; + } /* (non-Javadoc) * @see org.eclipse.debug.internal.ui.viewers.model.ViewerUpdateMonitor#getPriority()
diff --git a/org.eclipse.debug.ui/ui/org/eclipse/debug/internal/ui/viewers/model/ChildrenUpdate.java b/org.eclipse.debug.ui/ui/org/eclipse/debug/internal/ui/viewers/model/ChildrenUpdate.java index 21db89d..387403e 100644 --- a/org.eclipse.debug.ui/ui/org/eclipse/debug/internal/ui/viewers/model/ChildrenUpdate.java +++ b/org.eclipse.debug.ui/ui/org/eclipse/debug/internal/ui/viewers/model/ChildrenUpdate.java
@@ -126,7 +126,13 @@ } return false; } + + boolean containsUpdate(TreePath path) { + return getElementPath().equals(path); + } + + /* (non-Javadoc) * @see org.eclipse.debug.internal.ui.viewers.model.provisional.IChildrenUpdate#getLength() */
diff --git a/org.eclipse.debug.ui/ui/org/eclipse/debug/internal/ui/viewers/model/HasChildrenUpdate.java b/org.eclipse.debug.ui/ui/org/eclipse/debug/internal/ui/viewers/model/HasChildrenUpdate.java index 61bb05d..9572326 100644 --- a/org.eclipse.debug.ui/ui/org/eclipse/debug/internal/ui/viewers/model/HasChildrenUpdate.java +++ b/org.eclipse.debug.ui/ui/org/eclipse/debug/internal/ui/viewers/model/HasChildrenUpdate.java
@@ -106,6 +106,20 @@ } } + boolean containsUpdate(TreePath path) { + if (getElementPath().equals(path)) { + return true; + } else if (fBatchedRequests != null) { + for (int i = 0; i < fBatchedRequests.size(); i++) { + if (((ViewerUpdateMonitor)fBatchedRequests.get(i)).getElementPath().equals(path)) { + return true; + } + } + } + return false; + } + + /* (non-Javadoc) * @see org.eclipse.debug.internal.ui.viewers.model.ViewerUpdateMonitor#getPriority() */
diff --git a/org.eclipse.debug.ui/ui/org/eclipse/debug/internal/ui/viewers/model/ModelContentProvider.java b/org.eclipse.debug.ui/ui/org/eclipse/debug/internal/ui/viewers/model/ModelContentProvider.java index 1b793f5..133df6b 100644 --- a/org.eclipse.debug.ui/ui/org/eclipse/debug/internal/ui/viewers/model/ModelContentProvider.java +++ b/org.eclipse.debug.ui/ui/org/eclipse/debug/internal/ui/viewers/model/ModelContentProvider.java
@@ -191,10 +191,16 @@ * int) */ public boolean visit(IModelDelta delta, int depth) { - if (delta.getFlags() != IModelDelta.NO_CHANGE) { + // Filster out the CONTENT flags from the delta flags, the content + // flag is only used as a marker indicating that all the sub-elements + // of a given delta have been retrieved. + int flags = (delta.getFlags() & ~IModelDelta.CONTENT); + + if (flags != IModelDelta.NO_CHANGE) { IModelDelta parentDelta = delta.getParentDelta(); // Remove the delta if : - // - The parent delta has no more flags on it. + // - 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. // - If element is a memento, there are no state requests pending. if (parentDelta != null && parentDelta.getFlags() == IModelDelta.NO_CHANGE) { @@ -207,7 +213,7 @@ } } - if (delta.getFlags() == IModelDelta.REVEAL && !(delta.getElement() instanceof IMemento)) { + if (flags == IModelDelta.REVEAL && !(delta.getElement() instanceof IMemento)) { topDelta = delta; } else { complete = false; @@ -241,7 +247,7 @@ if (requests != null) { for (int i = 0; i < requests.size(); i++) { ViewerUpdateMonitor update = (ViewerUpdateMonitor) requests.get(i); - if (update.getElement().equals(path.getLastSegment())) { + if (update.containsUpdate(path)) { return true; } }
diff --git a/org.eclipse.debug.ui/ui/org/eclipse/debug/internal/ui/viewers/model/ViewerUpdateMonitor.java b/org.eclipse.debug.ui/ui/org/eclipse/debug/internal/ui/viewers/model/ViewerUpdateMonitor.java index 9a137d2..9ba15c5 100644 --- a/org.eclipse.debug.ui/ui/org/eclipse/debug/internal/ui/viewers/model/ViewerUpdateMonitor.java +++ b/org.eclipse.debug.ui/ui/org/eclipse/debug/internal/ui/viewers/model/ViewerUpdateMonitor.java
@@ -201,6 +201,13 @@ * @return whether it worked */ abstract boolean coalesce(ViewerUpdateMonitor update); + + /** + * Returns whether this update or any coalesced updates is for an + * element at the given path. + * @since 3.6 + */ + abstract boolean containsUpdate(TreePath path); /** * Starts this request. Subclasses must override startRequest().