Bug 576559 - fixed some non deferring calls insidePreservingSelection
insidePreservingSelection was introduced to avoid costly updates of
selection. But the implementation forgot to defer some calls to costly
getSelection().
Note that calls of preservingSelection() can be nested.
Avoiding getSelection() is mandatory to get away from O(n*n) behaviour
on batch updates.
Change-Id: I6ff0995bf51d11fabe98c2bca477ceb8e28ec2ec
Signed-off-by: Joerg Kubitz <jkubitz-eclipse@gmx.de>
Reviewed-on: https://git.eclipse.org/r/c/platform/eclipse.platform.ui/+/186357
Tested-by: Platform Bot <platform-bot@eclipse.org>
Reviewed-by: Mickael Istria <mistria@redhat.com>
diff --git a/bundles/org.eclipse.jface/src/org/eclipse/jface/viewers/TreeViewer.java b/bundles/org.eclipse.jface/src/org/eclipse/jface/viewers/TreeViewer.java
index 0da6da9..83934b9 100644
--- a/bundles/org.eclipse.jface/src/org/eclipse/jface/viewers/TreeViewer.java
+++ b/bundles/org.eclipse.jface/src/org/eclipse/jface/viewers/TreeViewer.java
@@ -358,6 +358,8 @@
updateCode.run();
return;
}
+ // avoid costly calls to setSelectionToWidget() and getSelection() during
+ // updateCode.run():
insidePreservingSelection = true;
try {
super.preservingSelection(updateCode, reveal);
@@ -422,8 +424,8 @@
final Object element) {
if (checkBusy())
return;
- Item[] selectedItems = getSelection(getControl());
- TreeSelection selection = (TreeSelection) getSelection();
+ Item[] selectedItems = insidePreservingSelection ? null : getSelection(getControl());
+ TreeSelection selection = insidePreservingSelection ? null : (TreeSelection) getSelection();
Widget[] itemsToDisassociate;
if (parentElementOrTreePath instanceof TreePath) {
TreePath elementPath = ((TreePath) parentElementOrTreePath)
@@ -435,7 +437,9 @@
if (internalIsInputOrEmptyPath(parentElementOrTreePath)) {
if (index < tree.getItemCount()) {
TreeItem item = tree.getItem(index);
- selection = adjustSelectionForReplace(selectedItems, selection, item, element, getRoot());
+ if (!insidePreservingSelection) {
+ selection = adjustSelectionForReplace(selectedItems, selection, item, element, getRoot());
+ }
// disassociate any different item that represents the
// same element under the same parent (the tree)
for (Widget widget : itemsToDisassociate) {
@@ -462,7 +466,10 @@
TreeItem parentItem = (TreeItem) widget;
if (index < parentItem.getItemCount()) {
TreeItem item = parentItem.getItem(index);
- selection = adjustSelectionForReplace(selectedItems, selection, item, element, parentItem.getData());
+ if (!insidePreservingSelection) {
+ selection = adjustSelectionForReplace(selectedItems, selection, item, element,
+ parentItem.getData());
+ }
// disassociate any different item that represents the
// same element under the same parent (the tree)
for (Widget widgetToDisassociate : itemsToDisassociate) {
@@ -805,7 +812,8 @@
public void remove(final Object parentOrTreePath, final int index) {
if (checkBusy())
return;
- final List<TreePath> oldSelection = new LinkedList<>(
+ // in case preservingSelection() is nested avoid getSelection():
+ final List<TreePath> oldSelection = insidePreservingSelection ? null : new LinkedList<>(
Arrays.asList(((TreeSelection) getSelection()).getPaths()));
preservingSelection(() -> {
TreePath removedPath = null;
@@ -845,7 +853,7 @@
}
}
}
- if (removedPath != null) {
+ if (removedPath != null && oldSelection != null) {
boolean removed = false;
for (Iterator<TreePath> it = oldSelection.iterator(); it.hasNext();) {
TreePath path = it.next();
