Reduce usage of SubProgressMonitor

Replaces some usage of SubProgressMonitor with SubMonitor

- SubMonitor does not need to call done()
- split will check for cancellation and throw OperationCanceledException
- newChild will NOT check for cancellation

CompositeChange#initializeValidationData also consumed more ticks than
allocated, this has been corrected.

Author: vogella

bundles/org.eclipse.ltk.core.refactoring/src/org/eclipse/ltk/core/refactoring/Change.java

@@ -21,39 +21,34 @@
 import org.eclipse.core.runtime.Platform;
 
 /**
- * An abstract base implementation for object representing a generic change
- * to the workbench. A <code>Change</code> object is typically created by
- * calling {@link Refactoring#createChange(IProgressMonitor)}. This class should be
- * subclassed by clients wishing to provide new changes.
+ * An abstract base implementation for object representing a generic change to the workbench. A
+ * <code>Change</code> object is typically created by calling
+ * {@link Refactoring#createChange(IProgressMonitor)}. This class should be subclassed by clients
+ * wishing to provide new changes.
  * <p>
- * Changes are best executed by using a {@link PerformChangeOperation}. If clients
- * execute a change directly then the following life cycle has to be honored:</p>
+ * Changes are best executed by using a {@link PerformChangeOperation}. If clients execute a change
+ * directly then the following life cycle has to be honored:
+ * </p>
  * <ul>
- *   <li>After a single change or a tree of changes has been created, the
- *       method <code>initializeValidationData</code> has to be called.</li>
- *   <li>The method <code>isValid</code> can be used to determine if a change
- *       can still be applied to the workspace. If the method returns a {@link
- *       RefactoringStatus} with a severity of FATAL then the change has to be
- *       treated as invalid. Performing an invalid change isn't allowed and
- *       results in an unspecified result. This method can be called multiple
- *       times.
- *   <li>Then the method <code>perform</code> can be called. A disabled change
- *       must not be executed. The <code>perform</code> method can only be called
- *       once. After a change has been executed, only the method <code>dispose</code>
- *       must be called.</li>
- *   <li>the method <code>dispose</code> has to be called either after the
- *       <code>perform</code> method
- *       has been called or if a change is no longer needed. The second case
- *       for example occurs when the undo stack gets flushed and all change
- *       objects managed by the undo stack are no longer needed. The method
- *       <code>dispose</code> is typically implemented to unregister listeners
- *       registered during the
- *       method <code>initializeValidationData</code>. There is no guarantee
- *       that <code>initializeValidationData</code>, <code>isValid</code>,
- *       or <code>perform</code> has been called before <code>dispose</code>
- *       is called.
+ * <li>After a single change or a tree of changes has been created, the method
+ * <code>initializeValidationData</code> has to be called.</li>
+ * <li>The method <code>isValid</code> can be used to determine if a change can still be applied to
+ * the workspace. If the method returns a {@link RefactoringStatus} with a severity of FATAL then
+ * the change has to be treated as invalid. Performing an invalid change isn't allowed and results
+ * in an unspecified result. This method can be called multiple times.
+ * <li>Then the method <code>perform</code> can be called. A disabled change must not be executed.
+ * The <code>perform</code> method can only be called once. After a change has been executed, only
+ * the method <code>dispose</code> must be called.</li>
+ * <li>the method <code>dispose</code> has to be called either after the <code>perform</code> method
+ * has been called or if a change is no longer needed. The second case for example occurs when the
+ * undo stack gets flushed and all change objects managed by the undo stack are no longer needed.
+ * The method <code>dispose</code> is typically implemented to unregister listeners registered
+ * during the method <code>initializeValidationData</code>. There is no guarantee that
+ * <code>initializeValidationData</code>, <code>isValid</code>, or <code>perform</code> has been
+ * called before <code>dispose</code> is called.
  * </ul>
  * Here is a code snippet that can be used to execute a change:
+ *
  * <pre>
  *   Change change= createChange();
  *   try {
@@ -63,26 +58,25 @@
  *
  *     if (!change.isEnabled())
  *         return;
- *     RefactoringStatus valid= change.isValid(new SubProgressMonitor(pm, 1));
+ *     RefactoringStatus valid= change.isValid(subMonitor.newChild(1));
  *     if (valid.hasFatalError())
  *         return;
- *     Change undo= change.perform(new SubProgressMonitor(pm, 1));
+ *     Change undo= change.perform(subMonitor.newChild(1));
  *     if (undo != null) {
- *        undo.initializeValidationData(new SubProgressMonitor(pm, 1));
+ *        undo.initializeValidationData(subMonitor.newChild(1));
  *        // do something with the undo object
  *     }
  *   } finally {
  *     change.dispose();
  *   }
  * </pre>
  * <p>
- * It is important that implementors of this abstract class provide an adequate
- * implementation of <code>isValid</code> and that they provide an undo change
- * via the return value of the method <code>perform</code>. If no undo can be
- * provided then the <code>perform</code> method is allowed to return <code>null</code>. But
- * implementors should be aware that not providing an undo object for a change
- * object that is part of a larger change tree will result in the fact that for
- * the whole change tree no undo object will be present.
+ * It is important that implementors of this abstract class provide an adequate implementation of
+ * <code>isValid</code> and that they provide an undo change via the return value of the method
+ * <code>perform</code>. If no undo can be provided then the <code>perform</code> method is allowed
+ * to return <code>null</code>. But implementors should be aware that not providing an undo object
+ * for a change object that is part of a larger change tree will result in the fact that for the
+ * whole change tree no undo object will be present.
  * </p>
  * <p>
  * Changes which are returned as top-level changes (e.g. by <code>Refactoring.createChange()</code>)

bundles/org.eclipse.ltk.core.refactoring/src/org/eclipse/ltk/core/refactoring/CompositeChange.java

@@ -26,7 +26,6 @@
 import org.eclipse.core.runtime.OperationCanceledException;
 import org.eclipse.core.runtime.SafeRunner;
 import org.eclipse.core.runtime.SubMonitor;
-import org.eclipse.core.runtime.SubProgressMonitor;
 
 import org.eclipse.ltk.internal.core.refactoring.RefactoringCoreMessages;
 import org.eclipse.ltk.internal.core.refactoring.RefactoringCorePlugin;
@@ -213,10 +212,9 @@ public void setEnabled(boolean enabled) {
 	 */
 	@Override
 	public void initializeValidationData(IProgressMonitor pm) {
-		pm.beginTask("", fChanges.size()); //$NON-NLS-1$
+		SubMonitor subMonitor= SubMonitor.convert(pm, fChanges.size());
 		for (Change change : fChanges) {
-			change.initializeValidationData(new SubProgressMonitor(pm, 1));
-			pm.worked(1);
+			change.initializeValidationData(subMonitor.newChild(1));
 		}
 	}
 
@@ -235,17 +233,16 @@ public void initializeValidationData(IProgressMonitor pm) {
 	@Override
 	public RefactoringStatus isValid(IProgressMonitor pm) throws CoreException {
 		RefactoringStatus result= new RefactoringStatus();
-		pm.beginTask("", fChanges.size()); //$NON-NLS-1$
+
+		SubMonitor subMonitor= SubMonitor.convert(pm, fChanges.size());
 		for (Iterator<Change> iter= fChanges.iterator(); iter.hasNext() && !result.hasFatalError();) {
 			Change change= iter.next();
-			if (change.isEnabled())
-				result.merge(change.isValid(new SubProgressMonitor(pm, 1)));
-			else
+			if (change.isEnabled()) {
+				result.merge(change.isValid(subMonitor.split(1)));
+			} else {
 				pm.worked(1);
-			if (pm.isCanceled())
-				throw new OperationCanceledException();
+			}
 		}
-		pm.done();
 		return result;
 	}
 

bundles/org.eclipse.ltk.core.refactoring/src/org/eclipse/ltk/core/refactoring/resource/DeleteResourceChange.java

@@ -164,16 +164,14 @@ public Change perform(IProgressMonitor pm) throws CoreException {
 
 	private static void saveFileIfNeeded(IFile file, IProgressMonitor pm) throws CoreException {
 		ITextFileBuffer buffer= FileBuffers.getTextFileBufferManager().getTextFileBuffer(file.getFullPath(), LocationKind.IFILE);
+		SubMonitor subMonitor= SubMonitor.convert(pm, 2);
 		if (buffer != null && buffer.isDirty() && buffer.isStateValidated() && buffer.isSynchronized()) {
-			SubMonitor subMonitor= SubMonitor.convert(pm, 2);
-			pm.beginTask("", 2); //$NON-NLS-1$
 			buffer.commit(subMonitor.newChild(1), false);
 			file.refreshLocal(IResource.DEPTH_ONE, subMonitor.newChild(1));
-			pm.done();
+			buffer.commit(subMonitor.newChild(1), false);
+			file.refreshLocal(IResource.DEPTH_ONE, subMonitor.newChild(1));
 		} else {
-			pm.beginTask("", 1); //$NON-NLS-1$
-			pm.worked(1);
-			pm.done();
+			subMonitor.worked(2);
 		}
 	}
 

bundles/org.eclipse.ltk.core.refactoring/src/org/eclipse/ltk/core/refactoring/resource/MoveRenameResourceChange.java

@@ -19,7 +19,6 @@
 import org.eclipse.core.runtime.IProgressMonitor;
 import org.eclipse.core.runtime.OperationCanceledException;
 import org.eclipse.core.runtime.SubMonitor;
-import org.eclipse.core.runtime.SubProgressMonitor;
 
 import org.eclipse.core.resources.IContainer;
 import org.eclipse.core.resources.IResource;
@@ -135,30 +134,23 @@ public final Change perform(IProgressMonitor monitor) throws CoreException, Oper
 	}
 
 	private Change performDestinationDelete(IResource newResource, IProgressMonitor monitor) throws CoreException {
-		monitor.beginTask(RefactoringCoreMessages.MoveResourceChange_progress_delete_destination, 3);
-		try {
-			DeleteResourceChange deleteChange= new DeleteResourceChange(newResource.getFullPath(), true);
-			deleteChange.initializeValidationData(new SubProgressMonitor(monitor, 1));
-			RefactoringStatus deleteStatus= deleteChange.isValid(new SubProgressMonitor(monitor, 1));
-			if (!deleteStatus.hasFatalError()) {
-				return deleteChange.perform(new SubProgressMonitor(monitor, 1));
-			}
-			return null;
-		} finally {
-			monitor.done();
+		SubMonitor subMonitor= SubMonitor.convert(monitor, RefactoringCoreMessages.MoveResourceChange_progress_delete_destination, 3);
+
+		DeleteResourceChange deleteChange= new DeleteResourceChange(newResource.getFullPath(), true);
+		deleteChange.initializeValidationData(subMonitor.newChild(1));
+		RefactoringStatus deleteStatus= deleteChange.isValid(subMonitor.newChild(1));
+		if (!deleteStatus.hasFatalError()) {
+			return deleteChange.perform(subMonitor.newChild(1));
 		}
+		return null;
 	}
 
 	private void performSourceRestore(IProgressMonitor monitor) throws CoreException {
-		monitor.beginTask(RefactoringCoreMessages.MoveResourceChange_progress_restore_source, 3);
-		try {
-			fRestoreSourceChange.initializeValidationData(new SubProgressMonitor(monitor, 1));
-			RefactoringStatus restoreStatus= fRestoreSourceChange.isValid(new SubProgressMonitor(monitor, 1));
-			if (!restoreStatus.hasFatalError()) {
-				fRestoreSourceChange.perform(new SubProgressMonitor(monitor, 1));
-			}
-		} finally {
-			monitor.done();
+		SubMonitor subMonitor= SubMonitor.convert(monitor, RefactoringCoreMessages.MoveResourceChange_progress_restore_source, 3);
+		fRestoreSourceChange.initializeValidationData(subMonitor.newChild(1));
+		RefactoringStatus restoreStatus= fRestoreSourceChange.isValid(subMonitor.newChild(1));
+		if (!restoreStatus.hasFatalError()) {
+			fRestoreSourceChange.perform(subMonitor.newChild(1));
 		}
 	}
 

bundles/org.eclipse.ltk.core.refactoring/src/org/eclipse/ltk/core/refactoring/resource/MoveResourceChange.java

@@ -16,9 +16,8 @@
 import org.eclipse.core.runtime.CoreException;
 import org.eclipse.core.runtime.IPath;
 import org.eclipse.core.runtime.IProgressMonitor;
-import org.eclipse.core.runtime.NullProgressMonitor;
 import org.eclipse.core.runtime.OperationCanceledException;
-import org.eclipse.core.runtime.SubProgressMonitor;
+import org.eclipse.core.runtime.SubMonitor;
 
 import org.eclipse.core.resources.IContainer;
 import org.eclipse.core.resources.IResource;
@@ -96,70 +95,56 @@ public void setDescriptor(ChangeDescriptor descriptor) {
 
 	@Override
 	public final Change perform(IProgressMonitor monitor) throws CoreException, OperationCanceledException {
-		try {
-			if (monitor == null)
-				monitor= new NullProgressMonitor();
-
-			monitor.beginTask(getName(), 4);
-
-			Change deleteUndo= null;
-
-			// delete destination if required
-			IResource resourceAtDestination= fTarget.findMember(fSource.getName());
-			if (resourceAtDestination != null && resourceAtDestination.exists()) {
-				deleteUndo= performDestinationDelete(resourceAtDestination, new SubProgressMonitor(monitor, 1));
-			} else {
-				monitor.worked(1);
-			}
-
-			// move resource
-			long currentStamp= fSource.getModificationStamp();
-			IPath destinationPath= fTarget.getFullPath().append(fSource.getName());
-			fSource.move(destinationPath, IResource.KEEP_HISTORY | IResource.SHALLOW, new SubProgressMonitor(monitor, 2));
-			resourceAtDestination= ResourcesPlugin.getWorkspace().getRoot().findMember(destinationPath);
-
-			// restore timestamp at destination
-			if (fStampToRestore != IResource.NULL_STAMP) {
-				resourceAtDestination.revertModificationStamp(fStampToRestore);
-			}
-
-			// restore file at source
-			if (fRestoreSourceChange != null) {
-				performSourceRestore(new SubProgressMonitor(monitor, 1));
-			} else {
-				monitor.worked(1);
-			}
-			return new MoveResourceChange(resourceAtDestination, fSource.getParent(), currentStamp, deleteUndo);
-		} finally {
-			monitor.done();
+		SubMonitor subMonitor= SubMonitor.convert(monitor, getName(), 4);
+
+		Change deleteUndo= null;
+
+		// delete destination if required
+		IResource resourceAtDestination= fTarget.findMember(fSource.getName());
+		if (resourceAtDestination != null && resourceAtDestination.exists()) {
+			deleteUndo= performDestinationDelete(resourceAtDestination, subMonitor.newChild(1));
+		} else {
+			subMonitor.worked(1);
+		}
+
+		// move resource
+		long currentStamp= fSource.getModificationStamp();
+		IPath destinationPath= fTarget.getFullPath().append(fSource.getName());
+		fSource.move(destinationPath, IResource.KEEP_HISTORY | IResource.SHALLOW, subMonitor.newChild(2));
+		resourceAtDestination= ResourcesPlugin.getWorkspace().getRoot().findMember(destinationPath);
+
+		// restore timestamp at destination
+		if (fStampToRestore != IResource.NULL_STAMP) {
+			resourceAtDestination.revertModificationStamp(fStampToRestore);
+		}
+
+		// restore file at source
+		if (fRestoreSourceChange != null) {
+			performSourceRestore(subMonitor.newChild(1));
+		} else {
+			subMonitor.worked(1);
 		}
+		return new MoveResourceChange(resourceAtDestination, fSource.getParent(), currentStamp, deleteUndo);
+
 	}
 
 	private Change performDestinationDelete(IResource newResource, IProgressMonitor monitor) throws CoreException {
-		monitor.beginTask(RefactoringCoreMessages.MoveResourceChange_progress_delete_destination, 3);
-		try {
-			DeleteResourceChange deleteChange= new DeleteResourceChange(newResource.getFullPath(), true);
-			deleteChange.initializeValidationData(new SubProgressMonitor(monitor, 1));
-			RefactoringStatus deleteStatus= deleteChange.isValid(new SubProgressMonitor(monitor, 1));
-			if (!deleteStatus.hasFatalError()) {
-				return deleteChange.perform(new SubProgressMonitor(monitor, 1));
-			}
-			return null;
-		} finally {
-			monitor.done();
+		SubMonitor subMonitor= SubMonitor.convert(monitor, RefactoringCoreMessages.MoveResourceChange_progress_delete_destination, 3);
+		DeleteResourceChange deleteChange= new DeleteResourceChange(newResource.getFullPath(), true);
+		deleteChange.initializeValidationData(subMonitor.newChild(1));
+		RefactoringStatus deleteStatus= deleteChange.isValid(subMonitor.newChild(1));
+		if (!deleteStatus.hasFatalError()) {
+			return deleteChange.perform(subMonitor.newChild(1));
 		}
+		return null;
 	}
 
 	private void performSourceRestore(IProgressMonitor monitor) throws CoreException {
-		monitor.beginTask(RefactoringCoreMessages.MoveResourceChange_progress_restore_source, 3);
-		try {
-			fRestoreSourceChange.initializeValidationData(new SubProgressMonitor(monitor, 1));
-			RefactoringStatus restoreStatus= fRestoreSourceChange.isValid(new SubProgressMonitor(monitor, 1));
-			if (!restoreStatus.hasFatalError()) {
-				fRestoreSourceChange.perform(new SubProgressMonitor(monitor, 1));
-			}
-		} finally {
-			monitor.done();
+		SubMonitor subMonitor= SubMonitor.convert(monitor, RefactoringCoreMessages.MoveResourceChange_progress_restore_source, 3);
+		fRestoreSourceChange.initializeValidationData(subMonitor.newChild(1));
+		RefactoringStatus restoreStatus= fRestoreSourceChange.isValid(subMonitor.newChild(1));
+		if (!restoreStatus.hasFatalError()) {
+			fRestoreSourceChange.perform(subMonitor.newChild(1));
 		}
 	}