Bug 508710: Fix asyncExec handling and SWTBotTreeItem.select(...)

Rework select and unselect methods to ensure that they block on syncExec
before returning.

Apply the same improvements that were made in SWTBotTree.select(...) to
SWTBotTreeItem.select(...) methods.

Change-Id: Ie7ad2e973dc330e0b3d998219dd9cc7e10fc2e6e
Signed-off-by: Patrick Tasse <patrick.tasse@gmail.com>
diff --git a/org.eclipse.swtbot.swt.finder.test/src/org/eclipse/swtbot/swt/finder/widgets/SWTBotTreeItemTest.java b/org.eclipse.swtbot.swt.finder.test/src/org/eclipse/swtbot/swt/finder/widgets/SWTBotTreeItemTest.java
index 11ddb85..d4cc3b0 100644
--- a/org.eclipse.swtbot.swt.finder.test/src/org/eclipse/swtbot/swt/finder/widgets/SWTBotTreeItemTest.java
+++ b/org.eclipse.swtbot.swt.finder.test/src/org/eclipse/swtbot/swt/finder/widgets/SWTBotTreeItemTest.java
@@ -21,6 +21,7 @@
 import static org.junit.Assert.assertTrue;
 import static org.junit.Assert.fail;
 
+import org.eclipse.swt.SWT;
 import org.eclipse.swt.graphics.Point;
 import org.eclipse.swt.graphics.Rectangle;
 import org.eclipse.swtbot.swt.finder.exceptions.AssertionFailedException;
@@ -51,8 +52,8 @@
 		SWTBotTreeItem node = tree.expandNode("Node 2").expandNode("Node 2.2").expandNode("Node 2.2.1");
 		bot.button("Clear").click();
 		node.contextMenu("getItem(Point) on mouse coordinates").click();
-		assertEventMatches(listeners, "MenuDetect [35]: Event {type=35 Tree {} time=175982645 data=null x=148 y=195 width=0 height=0 detail=0}");
-		assertEventMatches(listeners, "Selection [13]: SelectionEvent{Tree {} time=175985221 data=null item=TreeItem {Node 2.2.1} detail=0 x=0 y=0 width=0 height=0 stateMask=" + toStateMask(0, tree.widget) + " text=null doit=true}");
+		assertEventMatches(listeners, "MenuDetect [35]: Event {type=35 Tree {} time=0 data=null x=0 y=0 width=0 height=0 detail=0}");
+		assertEventMatches(listeners, "Selection [13]: SelectionEvent{Tree {} time=0 data=null item=TreeItem {Node 2.2.1} detail=0 x=0 y=0 width=0 height=0 stateMask=" + toStateMask(SWT.BUTTON1, tree.widget) + " text=null doit=true}");
 		assertEventMatches(listeners, "getItem(Point(Point {");
 	}
 
@@ -141,7 +142,7 @@
 			tree.getTreeItem("Node 2").expand().select("NonExisting");
 			fail("Was expecting an exception");
 		} catch (Exception e) {
-			assertEquals("assertion failed: ", e.getMessage());
+			assertEquals("Timed out waiting for tree item NonExisting", e.getMessage());
 		}
 	}
 
diff --git a/org.eclipse.swtbot.swt.finder/src/org/eclipse/swtbot/swt/finder/widgets/SWTBotTable.java b/org.eclipse.swtbot.swt.finder/src/org/eclipse/swtbot/swt/finder/widgets/SWTBotTable.java
index 51092ef..6263683 100644
--- a/org.eclipse.swtbot.swt.finder/src/org/eclipse/swtbot/swt/finder/widgets/SWTBotTable.java
+++ b/org.eclipse.swtbot.swt.finder/src/org/eclipse/swtbot/swt/finder/widgets/SWTBotTable.java
@@ -17,6 +17,7 @@
 package org.eclipse.swtbot.swt.finder.widgets;
 
 import java.util.ArrayList;
+import java.util.Arrays;
 import java.util.List;
 
 import org.eclipse.swt.SWT;
@@ -29,6 +30,7 @@
 import org.eclipse.swtbot.swt.finder.SWTBot;
 import org.eclipse.swtbot.swt.finder.SWTBotWidget;
 import org.eclipse.swtbot.swt.finder.exceptions.WidgetNotFoundException;
+import org.eclipse.swtbot.swt.finder.results.ArrayResult;
 import org.eclipse.swtbot.swt.finder.results.IntResult;
 import org.eclipse.swtbot.swt.finder.results.ListResult;
 import org.eclipse.swtbot.swt.finder.results.Result;
@@ -36,7 +38,6 @@
 import org.eclipse.swtbot.swt.finder.results.VoidResult;
 import org.eclipse.swtbot.swt.finder.results.WidgetResult;
 import org.eclipse.swtbot.swt.finder.utils.MessageFormat;
-import org.eclipse.swtbot.swt.finder.utils.StringUtils;
 import org.eclipse.swtbot.swt.finder.utils.TableCollection;
 import org.eclipse.swtbot.swt.finder.utils.TableRow;
 import org.eclipse.swtbot.swt.finder.utils.internal.Assert;
@@ -313,34 +314,29 @@
 	public void unselect() {
 		waitForEnabled();
 		setFocus();
-		asyncExec(new VoidResult() {
-			public void run() {
-				log.debug(MessageFormat.format("Unselecting all in {0}", widget)); //$NON-NLS-1$
-				int[] selectedIndices = widget.getSelectionIndices();
-				for (int i = 0; i < selectedIndices.length; i++) {
-					unselect(selectedIndices[i]);
-				}
+		log.debug(MessageFormat.format("Unselecting all in {0}", this)); //$NON-NLS-1$
+		TableItem[] selection = syncExec(new ArrayResult<TableItem>() {
+			public TableItem[] run() {
+				return widget.getSelection();
 			}
 		});
+		for (TableItem item : selection) {
+			unselect(item);
+			notifySelect(true);
+		}
 	}
 
 	/**
-	 * Unselects the given row.
-	 * @param row index of the row to unselect
+	 * Unselects the given table item.
+	 *
+	 * @param item
+	 *            table item to unselect
 	 */
-	private void unselect(final int row) {
-		waitForEnabled();
-		setFocus();
+	private void unselect(final TableItem item) {
 		asyncExec(new VoidResult() {
 			public void run() {
-				if (widget.isSelected(row)) {
-					log.debug(MessageFormat.format("Unselecting row {0} in {1}", row, widget)); //$NON-NLS-1$
-					widget.deselect(row);
-					lastSelectionItem = widget.getItem(row);
-					notifySelect(true);
-				} else {
-					log.debug(MessageFormat.format("Nothing to unselect in {0}", widget)); //$NON-NLS-1$
-				}
+				widget.deselect(widget.indexOf(item));
+				lastSelectionItem = item;
 			}
 		});
 	}
@@ -361,25 +357,19 @@
 			return;
 		}
 		setFocus();
-		log.debug(MessageFormat.format("Selecting rows {0} in table {1}", StringUtils.join(indices, ", "), this)); //$NON-NLS-1$ //$NON-NLS-2$
+		log.debug(MessageFormat.format("Selecting rows {0} in {1}", Arrays.toString(indices), this)); //$NON-NLS-1$ //$NON-NLS-2$
 		for (int i = 0; i < indices.length; i++) {
 			assertIsLegalRowIndex(indices[i]);
 		}
-		asyncExec(new VoidResult() {
-			public void run() {
-				for (int i = 0; i < indices.length; i++) {
-					lastSelectionItem = widget.getItem(indices[i]);
-					if (i == 0) {
-						// removes earlier selection
-						widget.setSelection(indices[0]);
-						notifySelect();
-					} else {
-						widget.select(indices[i]);
-						notifySelect(true);
-					}
-				}
-			}
-		});
+		final List<TableItem> selection = new ArrayList<TableItem>();
+		for (int index : indices) {
+			selection.add(getItem(index));
+		}
+		for (int i = 0; i < selection.size(); i++) {
+			boolean add = (i != 0);
+			processSelection(selection.get(i), add);
+			notifySelect(add);
+		}
 	}
 
 	private void assertMultiSelect() {
@@ -387,6 +377,28 @@
 	}
 
 	/**
+	 * Selects a table item
+	 *
+	 * @param item
+	 *            the table item to select
+	 * @param add
+	 *            true to add to current selection
+	 */
+	private void processSelection(final TableItem item, final boolean add) {
+		syncExec(new VoidResult() {
+			public void run() {
+				if (add) {
+					widget.select(widget.indexOf(item));
+				} else {
+					// removes earlier selection
+					widget.setSelection(item);
+				}
+				lastSelectionItem = item;
+			}
+		});
+	}
+
+	/**
 	 * Notifies the selection.
 	 */
 	protected void notifySelect() {
diff --git a/org.eclipse.swtbot.swt.finder/src/org/eclipse/swtbot/swt/finder/widgets/SWTBotTree.java b/org.eclipse.swtbot.swt.finder/src/org/eclipse/swtbot/swt/finder/widgets/SWTBotTree.java
index 547d04a..c01ad18 100644
--- a/org.eclipse.swtbot.swt.finder/src/org/eclipse/swtbot/swt/finder/widgets/SWTBotTree.java
+++ b/org.eclipse.swtbot.swt.finder/src/org/eclipse/swtbot/swt/finder/widgets/SWTBotTree.java
@@ -242,14 +242,11 @@
 			SWTBotTreeItem si = getTreeItem(item);
 			selection.add(si.widget);
 		}
-		asyncExec(new VoidResult() {
-			public void run() {
-				for (int i = 0; i < items.length; i++) {
-					lastSelectionItem = selection.get(i);
-					processSelection(i);
-				}
-			}
-		});
+		for (int i = 0; i < selection.size(); i++) {
+			boolean add = (i != 0);
+			processSelection(selection.get(i), add);
+			notifySelect(add);
+		}
 		return this;
 	}
 
@@ -271,14 +268,11 @@
 		} else if (items.length == 0) {
 			return unselect();
 		}
-		asyncExec(new VoidResult() {
-			public void run() {
-				for (int i = 0; i < items.length; i++) {
-					lastSelectionItem = items[i].widget;
-					processSelection(i);
-				}
-			}
-		});
+		for (int i = 0; i < items.length; i++) {
+			boolean add = (i != 0);
+			processSelection(items[i].widget, add);
+			notifySelect(add);
+		}
 		return this;
 	}
 
@@ -289,21 +283,35 @@
 	 */
 	public SWTBotTree unselect() {
 		waitForEnabled();
-		asyncExec(new VoidResult() {
-			public void run() {
-				log.debug(MessageFormat.format("Unselecting all in {0}", widget)); //$NON-NLS-1$
-				TreeItem[] items = widget.getSelection();
-				for (TreeItem item : items) {
-					widget.deselect(item);
-					lastSelectionItem = item;
-					notifySelect(true);
-				}
+		log.debug(MessageFormat.format("Unselecting all in {0}", this)); //$NON-NLS-1$
+		TreeItem[] selection = syncExec(new ArrayResult<TreeItem>() {
+			public TreeItem[] run() {
+				return widget.getSelection();
 			}
 		});
+		for (TreeItem item : selection) {
+			unselect(item);
+			notifySelect(true);
+		}
 		return this;
 	}
 
 	/**
+	 * Unselects the given tree item.
+	 *
+	 * @param item
+	 *            tree item to unselect
+	 */
+	private void unselect(final TreeItem item) {
+		asyncExec(new VoidResult() {
+			public void run() {
+				widget.deselect(item);
+				lastSelectionItem = item;
+			}
+		});
+	}
+
+	/**
 	 * Selects the indices provided. Replaces the current selection. If there is
 	 * more than one item to select, the tree must have the SWT.MULTI style.
 	 *
@@ -319,34 +327,39 @@
 		} else if (indices.length == 0) {
 			return unselect();
 		}
-		asyncExec(new VoidResult() {
-			public void run() {
-				log.debug(MessageFormat.format("Selecting rows [{0}] in tree{1}", StringUtils.join(indices, ", "), this)); //$NON-NLS-1$ //$NON-NLS-2$
-				for (int i = 0; i < indices.length; i++) {
-					lastSelectionItem = widget.getItem(indices[i]);
-					processSelection(i);
-				}
-			}
-		});
+		final List<TreeItem> selection = new ArrayList<TreeItem>();
+		for (int index : indices) {
+			selection.add(getItem(index));
+		}
+		log.debug(MessageFormat.format("Selecting rows {0} in {1}", Arrays.toString(indices), this)); //$NON-NLS-1$ //$NON-NLS-2$
+		for (int i = 0; i < selection.size(); i++) {
+			boolean add = (i != 0);
+			processSelection(selection.get(i), add);
+			notifySelect(add);
+		}
 		return this;
 	}
 
 	/**
-	 * Selects widget and notifies selection
+	 * Selects a tree item
 	 *
-	 * @param i
-	 *            index of item getting selected
+	 * @param item
+	 *            the tree item to select
+	 * @param add
+	 *            true to add to current selection
 	 */
-	private void processSelection(int i)
-	{
-		if (i == 0) {
-			// removes earlier selection
-			widget.setSelection(lastSelectionItem);
-			notifySelect();
-		} else {
-			widget.select(lastSelectionItem);
-			notifySelect(true);
-		}
+	private void processSelection(final TreeItem item, final boolean add) {
+		syncExec(new VoidResult() {
+			public void run() {
+				if (add) {
+					widget.select(item);
+				} else {
+					// removes earlier selection
+					widget.setSelection(item);
+				}
+				lastSelectionItem = item;
+			}
+		});
 	}
 
 	/**
@@ -516,6 +529,20 @@
 	}
 
 	/**
+	 * Gets the item at the given index.
+	 *
+	 * @param index the index of the item to return
+	 * @return the item at the given index.
+	 */
+	private TreeItem getItem(final int index) {
+		return syncExec(new WidgetResult<TreeItem>() {
+			public TreeItem run() {
+				return widget.getItem(index);
+			}
+		});
+	}
+
+	/**
 	 * Gets the item matching the given name.
 	 *
 	 * @param nodeText the text on the node.
diff --git a/org.eclipse.swtbot.swt.finder/src/org/eclipse/swtbot/swt/finder/widgets/SWTBotTreeItem.java b/org.eclipse.swtbot.swt.finder/src/org/eclipse/swtbot/swt/finder/widgets/SWTBotTreeItem.java
index 7794eaa..9ea6e23 100644
--- a/org.eclipse.swtbot.swt.finder/src/org/eclipse/swtbot/swt/finder/widgets/SWTBotTreeItem.java
+++ b/org.eclipse.swtbot.swt.finder/src/org/eclipse/swtbot/swt/finder/widgets/SWTBotTreeItem.java
@@ -41,7 +41,6 @@
 import org.eclipse.swtbot.swt.finder.results.WidgetResult;
 import org.eclipse.swtbot.swt.finder.utils.MessageFormat;
 import org.eclipse.swtbot.swt.finder.utils.SWTUtils;
-import org.eclipse.swtbot.swt.finder.utils.StringUtils;
 import org.eclipse.swtbot.swt.finder.utils.TableRow;
 import org.eclipse.swtbot.swt.finder.utils.TextDescription;
 import org.eclipse.swtbot.swt.finder.utils.internal.Assert;
@@ -55,10 +54,13 @@
  * @version $Id$
  */
 public class SWTBotTreeItem extends AbstractSWTBot<TreeItem> {
-	// private static final int expandKey = SWT.getPlatform().equals("gtk") ? SWT.KEYPAD_ADD : SWT.ARROW_RIGHT;
-	// private static final int collapseKey = SWT.getPlatform().equals("gtk") ? SWT.KEYPAD_SUBTRACT : SWT.ARROW_LEFT;
+
+	/** The parent tree */
 	private Tree	tree;
 
+	/** The last selected item */
+	private TreeItem	lastSelectionItem;
+
 	/**
 	 * @param treeItem the widget.
 	 * @throws WidgetNotFoundException if the widget is <code>null</code> or widget has been disposed.
@@ -248,6 +250,7 @@
 		notify(eventType, event, tree);
 	}
 
+	@Override
 	protected Event createEvent() {
 		Event event = super.createEvent();
 		event.widget = tree;
@@ -364,6 +367,7 @@
 			public void run() {
 				tree.setFocus();
 				tree.setSelection(widget);
+				lastSelectionItem = widget;
 			}
 		});
 		notifySelect();
@@ -408,6 +412,7 @@
 	 * @return the current node.
 	 * @since 1.2
 	 */
+	@Override
 	public SWTBotTreeItem click() {
 		waitForEnabled();
 		Point center = getCenter(getCellBounds());
@@ -516,26 +521,20 @@
 	 */
 	public SWTBotTreeItem select(final String... items) {
 		waitForEnabled();
+		setFocus();
 		if (items.length > 1) {
 			assertMultiSelect();
 		}
-		final List<String> nodes = Arrays.asList(items);
-		Assert.isTrue(getNodes().containsAll(nodes));
-
-		syncExec(new VoidResult() {
-			public void run() {
-				ArrayList<TreeItem> selection = new ArrayList<TreeItem>();
-
-				for (String item : items) {
-					SWTBotTreeItem si = getTreeItem(item);
-					selection.add(si.widget);
-				}
-				tree.setFocus();
-				tree.setSelection(selection.toArray(new TreeItem[selection.size()]));
-			}
-		});
-
-		notifySelect();
+		final List<TreeItem> selection = new ArrayList<TreeItem>();
+		for (String item : items) {
+			SWTBotTreeItem si = getTreeItem(item);
+			selection.add(si.widget);
+		}
+		for (int i = 0; i < selection.size(); i++) {
+			boolean add = (i != 0);
+			processSelection(selection.get(i), add);
+			notifySelect(add);
+		}
 		return this;
 	}
 
@@ -549,24 +548,23 @@
 	 */
 	public SWTBotTreeItem select(final int... indices) {
 		waitForEnabled();
+		setFocus();
 		if (indices.length > 1) {
 			assertMultiSelect();
 		}
 		for (int i = 0; i < indices.length; i++) {
 			assertIsLegalRowIndex(indices[i]);
 		}
-
-		asyncExec(new VoidResult() {
-			public void run() {
-				log.debug(MessageFormat.format("Selecting rows [{0}] in {1}", StringUtils.join(indices, ", "), this)); //$NON-NLS-1$ //$NON-NLS-2$
-				TreeItem items[] = new TreeItem[indices.length];
-				for (int i = 0; i < indices.length; i++)
-					items[i] = widget.getItem(indices[i]);
-				tree.setFocus();
-				tree.setSelection(items);
-			}
-		});
-		notifySelect();
+		final List<TreeItem> selection = new ArrayList<TreeItem>();
+		for (int index : indices) {
+			selection.add(getItem(index));
+		}
+		log.debug(MessageFormat.format("Selecting rows {0} in {1}", Arrays.toString(indices), this)); //$NON-NLS-1$ //$NON-NLS-2$
+		for (int i = 0; i < selection.size(); i++) {
+			boolean add = (i != 0);
+			processSelection(selection.get(i), add);
+			notifySelect(add);
+		}
 		return this;
 	}
 
@@ -583,22 +581,63 @@
 	}
 
 	/**
-	 * notifies the tree widget about selection changes.
+	 * Selects a tree item
 	 *
-	 * @since 1.0
+	 * @param item
+	 *            the tree item to select
+	 * @param add
+	 *            true to add to current selection
+	 */
+	private void processSelection(final TreeItem item, final boolean add) {
+		syncExec(new VoidResult() {
+			public void run() {
+				if (add) {
+					tree.select(item);
+				} else {
+					// removes earlier selection
+					tree.setSelection(item);
+				}
+				lastSelectionItem = item;
+			}
+		});
+	}
+
+	/**
+	 * Notifies the tree widget about selection changes
 	 */
 	private void notifySelect() {
+		notifySelect(false);
+	}
+
+	/**
+	 * Notifies the selection.
+	 *
+	 * @param ctrl
+	 *            true if CTRL key should be pressed while sending the event,
+	 *            false otherwise.
+	 */
+	private void notifySelect(boolean ctrl) {
+		int stateMask1 = (ctrl) ?  (SWT.NONE | SWT.CTRL) : SWT.NONE;
+		int stateMask2 = (ctrl) ?  (SWT.BUTTON1 | SWT.CTRL) : SWT.BUTTON1;
 		notifyTree(SWT.MouseEnter);
 		notifyTree(SWT.MouseMove);
 		notifyTree(SWT.Activate);
 		notifyTree(SWT.FocusIn);
-		notifyTree(SWT.MouseDown);
-		notifyTree(SWT.Selection);
-		notifyTree(SWT.MouseUp);
+		notifyTree(SWT.MouseDown, createMouseEvent(0, 0, 1, stateMask1, 1));
+		notifyTree(SWT.Selection, selectionEvent(stateMask2));
+		notifyTree(SWT.MouseUp, createMouseEvent(0, 0, 1, stateMask2, 1));
 		notifyTree(SWT.MouseHover);
 		notifyTree(SWT.MouseMove);
 		notifyTree(SWT.MouseExit);
 		notifyTree(SWT.Deactivate);
+		notifyTree(SWT.FocusOut);
+	}
+
+	private Event selectionEvent(int stateMask) {
+		Event createEvent = createEvent();
+		createEvent.item = lastSelectionItem;
+		createEvent.stateMask = stateMask;
+		return createEvent;
 	}
 
 	@Override
@@ -801,6 +840,20 @@
 	}
 
 	/**
+	 * Gets the item at the given index.
+	 *
+	 * @param index the index of the item to return
+	 * @return the item at the given index.
+	 */
+	private TreeItem getItem(final int index) {
+		return syncExec(new WidgetResult<TreeItem>() {
+			public TreeItem run() {
+				return widget.getItem(index);
+			}
+		});
+	}
+
+	/**
 	 * Gets the item matching the given name.
 	 *
 	 * @param nodeText the text on the node.
@@ -819,6 +872,7 @@
 		});
 	}
 
+	@Override
 	public boolean isEnabled() {
 		return syncExec(new BoolResult() {
 			public Boolean run() {