Bug 210448 [Viewers] [JFace] AbstractTreeViewer inputChanged setRedraw should be in try/finally
The inputChanged method on the AbstractTreeViewer makes use of setRedraw, which is good, but it doesn't use a try/finally block as per recommendations. If an exception is thrown from within one of the methods called, redraw will remain false. Additionally, the commented out portion is fine, but the fact that we have conditionals in the code that always evaluate to true is less fine. When we're ready to actually determine when to use redraw we could put them back in. Current code: protected void inputChanged(Object input, Object oldInput) { preservingSelection(new Runnable() { public void run() { Control tree = getControl(); boolean useRedraw = true; // (size > REDRAW_THRESHOLD) || (table.getItemCount() > // REDRAW_THRESHOLD); if (useRedraw) { tree.setRedraw(false); } removeAll(tree); tree.setData(getRoot()); internalInitializeTree(tree); if (useRedraw) { tree.setRedraw(true); } } }); } Suggestion: protected void inputChanged(Object input, Object oldInput) { preservingSelection(new Runnable() { public void run() { Control tree = getControl(); try { tree.setRedraw(false); removeAll(tree); tree.setData(getRoot()); internalInitializeTree(tree); } finally { tree.setRedraw(true); } } }); }
bundles/org.eclipse.jface/src/org/eclipse/jface/viewers/AbstractTreeViewer.java
* Removes a listener for expand and collapse events in this viewer. Has no
	 * affect if an identical listener is not registered.
	 *
	 * @param listener
	 *            a tree viewer listener
	 */
	public void removeTreeListener(ITreeViewerListener listener) {
		treeListeners.remove(listener);
	}
	/**
	 * This implementation of reveal() reveals the given element or tree path.
	 */
	public void reveal(Object elementOrTreePath) {
		Assert.isNotNull(elementOrTreePath);
		Widget w = internalExpand(elementOrTreePath, true);
		if (w instanceof Item) {
			showItem((Item) w);
		}
	}
	/**
	 * Returns the rightmost visible descendent of the given item. Returns the
	 * item itself if it has no children.
	 *
	 * @param item
	 *            the item to compute the descendent of
	 * @return the rightmost visible descendent or the item itself if it has no
	 *         children
	 */
	private Item rightMostVisibleDescendent(Item item) {
		Item[] children = getItems(item);