OSDN Git Service

Handle node identity changes
authorTor Norbye <tnorbye@google.com>
Wed, 19 Oct 2011 23:39:08 +0000 (16:39 -0700)
committerTor Norbye <tnorbye@google.com>
Thu, 20 Oct 2011 16:53:34 +0000 (09:53 -0700)
This changeset fixes a bug where sometimes dropping a new widget into
a layout would cause lots of other widget to be selected as well.

The root cause is that Eclipse's DOM *sometimes* decides to throw away
and replace all the children of a node, even when most of the children
just had attribute changes. In particular, this happens in some
GridLayout scenarios where a drop causes many attribute changes on the
children (to update row and column indices etc).

The workaround has two parts:

(1) The old implementation would find out which nodes were added by a
    drop handler by storing a list of all the children before the
    drop, then getting the list after the drop and removing the ones
    that were present before.

    This doesn't work when node ids change since *all* the children
    would get marked as "new".

    To address this, I made tracking created nodes more explicit:
    rather than "diffing" the lists, there's a NodeCreationListener
    interface you can hook into which (while you're listening) will
    tell you about *all* nodes created by any parent. In the drop
    handler we add a listener and make a list of all newly added nodes
    that have the drop target node as a parent.

(2) Even with an explicit list of which nodes were added, there's the
    problem that the nodes that were added have changed their
    identities, so we cannot look up a "view rectangle" for them and
    select them in the layout editor. To do this we need ot map from
    the previous children of a parent node to its new children.  We
    cannot make the comparison based on equality (since the reason
    we're getting into this trouble in the first place is that the
    various children are touched by the drop handler when setting
    attributes on them) so instead we use the sibling index. Since we
    track all the additions and removals of nodes within our drop
    target node, we can know exactly that the 4th node before Eclipse
    reparses will correspond to the 4th node after, and so on.

    Therefore, the "NodeCreationListener" interface passes not just
    the nodes that were added and removed, but also the exact *index*
    at which the node was added or removed. We then use this in the
    code which looks up CanvasViewInfos from nodes to look up nodes
    that can't be identified using the normal path.

Change-Id: Ibbf651b27ede7edfa40452de07bf1dbce02cd21e

eclipse/plugins/com.android.ide.eclipse.adt/src/com/android/ide/eclipse/adt/internal/editors/layout/gle2/MoveGesture.java
eclipse/plugins/com.android.ide.eclipse.adt/src/com/android/ide/eclipse/adt/internal/editors/layout/gle2/SelectionManager.java
eclipse/plugins/com.android.ide.eclipse.adt/src/com/android/ide/eclipse/adt/internal/editors/uimodel/UiElementNode.java

index 805e6f6..59486de 100644 (file)
@@ -21,8 +21,12 @@ import com.android.ide.common.api.InsertType;
 import com.android.ide.common.api.Point;
 import com.android.ide.common.api.Rect;
 import com.android.ide.eclipse.adt.AdtPlugin;
+import com.android.ide.eclipse.adt.internal.editors.layout.gre.NodeFactory;
 import com.android.ide.eclipse.adt.internal.editors.layout.gre.NodeProxy;
 import com.android.ide.eclipse.adt.internal.editors.layout.gre.RulesEngine;
+import com.android.ide.eclipse.adt.internal.editors.layout.uimodel.UiViewElementNode;
+import com.android.ide.eclipse.adt.internal.editors.uimodel.UiElementNode;
+import com.android.ide.eclipse.adt.internal.editors.uimodel.UiElementNode.NodeCreationListener;
 
 import org.eclipse.jface.viewers.ISelection;
 import org.eclipse.jface.viewers.TreePath;
@@ -36,9 +40,7 @@ import org.eclipse.swt.widgets.Display;
 import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.Collections;
-import java.util.HashSet;
 import java.util.List;
-import java.util.Set;
 
 /**
  * The Move gesture provides the operation for moving widgets around in the canvas.
@@ -340,46 +342,90 @@ public class MoveGesture extends DropGesture {
             return;
         }
 
-        final LayoutPoint canvasPoint = getDropLocation(event).toLayout();
-
-        // Record children of the target right before the drop (such that we can
-        // find out after the drop which exact children were inserted)
-        Set<INode> children = new HashSet<INode>();
-        for (INode node : mTargetNode.getChildren()) {
-            children.add(node);
-        }
-
         updateDropFeedback(mFeedback, event);
 
         final SimpleElement[] elementsFinal = elements;
+        final LayoutPoint canvasPoint = getDropLocation(event).toLayout();
         String label = computeUndoLabel(mTargetNode, elements, event.detail);
-        mCanvas.getLayoutEditor().wrapUndoEditXmlModel(label, new Runnable() {
-            public void run() {
-                InsertType insertType = getInsertType(event, mTargetNode);
-                mCanvas.getRulesEngine().callOnDropped(mTargetNode,
-                        elementsFinal,
-                        mFeedback,
-                        new Point(canvasPoint.x, canvasPoint.y),
-                        insertType);
-                mTargetNode.applyPendingChanges();
-                // Clean up drag if applicable
-                if (event.detail == DND.DROP_MOVE) {
-                    GlobalCanvasDragInfo.getInstance().removeSource();
+
+        // Create node listener which (during the drop) listens for node additions
+        // and stores the list of added node such that they can be selected afterwards.
+        final List<UiElementNode> added = new ArrayList<UiElementNode>();
+        // List of "index within parent" for each node
+        final List<Integer> indices = new ArrayList<Integer>();
+        NodeCreationListener listener = new NodeCreationListener() {
+            public void nodeCreated(UiElementNode parent, UiElementNode child, int index) {
+                if (parent == mTargetNode.getNode()) {
+                    added.add(child);
+
+                    // Adjust existing indices
+                    for (int i = 0, n = indices.size(); i < n; i++) {
+                        int idx = indices.get(i);
+                        if (idx >= index) {
+                            indices.set(i, idx + 1);
+                        }
+                    }
+
+                    indices.add(index);
+                }
+            }
+
+            public void nodeDeleted(UiElementNode parent, UiElementNode child, int previousIndex) {
+                if (parent == mTargetNode.getNode()) {
+                    // Adjust existing indices
+                    for (int i = 0, n = indices.size(); i < n; i++) {
+                        int idx = indices.get(i);
+                        if (idx >= previousIndex) {
+                            indices.set(i, idx - 1);
+                        }
+                    }
+
+                    // Make sure we aren't removing the same nodes that are being added
+                    assert !added.contains(child);
                 }
             }
-        });
-
-        // Now find out which nodes were added, and look up their corresponding
-        // CanvasViewInfos
-        final List<INode> added = new ArrayList<INode>();
-        for (INode node : mTargetNode.getChildren()) {
-            if (!children.contains(node)) {
-                added.add(node);
+        };
+
+        try {
+            UiElementNode.addNodeCreationListener(listener);
+            mCanvas.getLayoutEditor().wrapUndoEditXmlModel(label, new Runnable() {
+                public void run() {
+                    InsertType insertType = getInsertType(event, mTargetNode);
+                    mCanvas.getRulesEngine().callOnDropped(mTargetNode,
+                            elementsFinal,
+                            mFeedback,
+                            new Point(canvasPoint.x, canvasPoint.y),
+                            insertType);
+                    mTargetNode.applyPendingChanges();
+                    // Clean up drag if applicable
+                    if (event.detail == DND.DROP_MOVE) {
+                        GlobalCanvasDragInfo.getInstance().removeSource();
+                    }
+                }
+            });
+        } finally {
+            UiElementNode.removeNodeCreationListener(listener);
+        }
+
+        final List<INode> nodes = new ArrayList<INode>();
+        NodeFactory nodeFactory = mCanvas.getNodeFactory();
+        for (UiElementNode uiNode : added) {
+            if (uiNode instanceof UiViewElementNode) {
+                NodeProxy node = nodeFactory.create((UiViewElementNode) uiNode);
+                if (node != null) {
+                    nodes.add(node);
+                }
             }
         }
-        // Select the newly dropped nodes
+
+        // Select the newly dropped nodes:
+        // Find out which nodes were added, and look up their corresponding
+        // CanvasViewInfos.
         final SelectionManager selectionManager = mCanvas.getSelectionManager();
-        if (!selectionManager.selectDropped(added)) {
+        // Don't use the indices to search for corresponding nodes yet, since a
+        // render may not have happened yet and we'd rather use an up to date
+        // view hierarchy than indices to look up the right view infos.
+        if (!selectionManager.selectDropped(nodes, null /* indices */)) {
             // In some scenarios we can't find the actual view infos yet; this
             // seems to happen when you drag from one canvas to another (see the
             // related comment next to the setFocus() call below). In that case
@@ -387,7 +433,7 @@ public class MoveGesture extends DropGesture {
             // date.
             Display.getDefault().asyncExec(new Runnable() {
                 public void run() {
-                    selectionManager.selectDropped(added);
+                    selectionManager.selectDropped(nodes, indices);
                 }
             });
         }
index e045f49..872b635 100644 (file)
@@ -23,7 +23,9 @@ import com.android.ide.common.api.INode;
 import com.android.ide.common.layout.GridLayoutRule;
 import com.android.ide.eclipse.adt.internal.editors.descriptors.ElementDescriptor;
 import com.android.ide.eclipse.adt.internal.editors.layout.LayoutEditor;
+import com.android.ide.eclipse.adt.internal.editors.layout.gre.NodeProxy;
 import com.android.ide.eclipse.adt.internal.editors.layout.uimodel.UiViewElementNode;
+import com.android.ide.eclipse.adt.internal.editors.uimodel.UiElementNode;
 import com.android.sdklib.SdkConstants;
 import com.android.util.Pair;
 
@@ -582,6 +584,7 @@ public class SelectionManager implements ISelectionProvider {
         redraw();
     }
 
+    /** Clears the selection */
     public void selectNone() {
         mSelections.clear();
         mAltSelection = null;
@@ -813,12 +816,54 @@ public class SelectionManager implements ISelectionProvider {
      * attempt to select was successful.
      *
      * @param nodes The collection of nodes to be selected
+     * @param indices A list of indices within the parent for each node, or null
      * @return True if and only if all nodes were successfully selected
      */
-    public boolean selectDropped(Collection<INode> nodes) {
+    public boolean selectDropped(List<INode> nodes, List<Integer> indices) {
+        assert indices == null || nodes.size() == indices.size();
+
+        ViewHierarchy viewHierarchy = mCanvas.getViewHierarchy();
+
+        // Look up a list of view infos which correspond to the nodes.
         final Collection<CanvasViewInfo> newChildren = new ArrayList<CanvasViewInfo>();
-        for (INode node : nodes) {
-            CanvasViewInfo viewInfo = mCanvas.getViewHierarchy().findViewInfoFor(node);
+        for (int i = 0, n = nodes.size(); i < n; i++) {
+            INode node = nodes.get(i);
+
+            CanvasViewInfo viewInfo = viewHierarchy.findViewInfoFor(node);
+
+            // There are two scenarios where looking up a view info fails.
+            // The first one is that the node was just added and the render has not yet
+            // happened, so the ViewHierarchy has no record of the node. In this case
+            // there is nothing we can do, and the method will return false (which the
+            // caller will use to schedule a second attempt later).
+            // The second scenario is where the nodes *change identity*. This isn't
+            // common, but when a drop handler makes a lot of changes to its children,
+            // for example when dropping into a GridLayout where attributes are adjusted
+            // on nearly all the other children to update row or column attributes
+            // etc, then in some cases Eclipse's DOM model changes the identities of
+            // the nodes when applying all the edits, so the new Node we created (as
+            // well as possibly other nodes) are no longer the children we observe
+            // after the edit, and there are new copies there instead. In this case
+            // the UiViewModel also fails to map the nodes. To work around this,
+            // we track the *indices* (within the parent) during a drop, such that we
+            // know which children (according to their positions) the given nodes
+            // are supposed to map to, and then we use these view infos instead.
+            if (viewInfo == null && node instanceof NodeProxy && indices != null) {
+                INode parent = node.getParent();
+                CanvasViewInfo parentViewInfo = viewHierarchy.findViewInfoFor(parent);
+                if (parentViewInfo != null) {
+                    UiViewElementNode parentUiNode = parentViewInfo.getUiViewNode();
+                    if (parentUiNode != null) {
+                        List<UiElementNode> children = parentUiNode.getUiChildren();
+                        int index = indices.get(i);
+                        if (index >= 0 && index < children.size()) {
+                            UiElementNode replacedNode = children.get(index);
+                            viewInfo = viewHierarchy.findViewInfoFor(replacedNode);
+                        }
+                    }
+                }
+            }
+
             if (viewInfo != null) {
                 if (nodes.size() > 1 && viewInfo.isHidden()) {
                     // Skip spacers - unless you're dropping just one
@@ -844,7 +889,7 @@ public class SelectionManager implements ISelectionProvider {
     public void setOutlineSelection(final List<INode> nodes) {
         Display.getDefault().asyncExec(new Runnable() {
             public void run() {
-                selectDropped(nodes);
+                selectDropped(nodes, null /* indices */);
                 syncOutlineSelection();
             }
         });
index 41fbec5..8a15893 100644 (file)
@@ -1084,7 +1084,11 @@ public class UiElementNode implements IPropertySource {
 
         getEditor().scheduleNodeReformat(this, false);
 
+        // Notify per-node listeners
         invokeUiUpdateListeners(UiUpdateState.CREATED);
+        // Notify global listeners
+        fireNodeCreated(this, getUiSiblingIndex());
+
         return mXmlNode;
     }
 
@@ -1099,6 +1103,8 @@ public class UiElementNode implements IPropertySource {
             return null;
         }
 
+        int previousIndex = getUiSiblingIndex();
+
         // First clear the internals of the node and *then* actually deletes the XML
         // node (because doing so will generate an update even and this node may be
         // revisited via loadFromXmlNode).
@@ -1120,6 +1126,8 @@ public class UiElementNode implements IPropertySource {
         }
 
         invokeUiUpdateListeners(UiUpdateState.DELETED);
+        fireNodeDeleted(this, previousIndex);
+
         return oldXmlNode;
     }
 
@@ -2103,4 +2111,95 @@ public class UiElementNode implements IPropertySource {
 
         return null;
     }
+
+    // ---- Global node create/delete Listeners ----
+
+    /** List of listeners to be notified of newly created nodes, or null */
+    private static List<NodeCreationListener> sListeners;
+
+    /** Notify listeners that a new node has been created */
+    private void fireNodeCreated(UiElementNode newChild, int index) {
+        // Nothing to do if there aren't any listeners. We don't need to worry about
+        // the case where one thread is firing node changes while another is adding a listener
+        // (in that case it's still okay for this node firing not to be heard) so perform
+        // the check outside of synchronization.
+        if (sListeners == null) {
+            return;
+        }
+        synchronized (UiElementNode.class) {
+            if (sListeners != null) {
+                UiElementNode parent = newChild.getUiParent();
+                for (NodeCreationListener listener : sListeners) {
+                    listener.nodeCreated(parent, newChild, index);
+                }
+            }
+        }
+    }
+
+    /** Notify listeners that a new node has been deleted */
+    private void fireNodeDeleted(UiElementNode oldChild, int index) {
+        if (sListeners == null) {
+            return;
+        }
+        synchronized (UiElementNode.class) {
+            if (sListeners != null) {
+                UiElementNode parent = oldChild.getUiParent();
+                for (NodeCreationListener listener : sListeners) {
+                    listener.nodeDeleted(parent, oldChild, index);
+                }
+            }
+        }
+    }
+
+    /**
+     * Adds a {@link NodeCreationListener} to be notified when new nodes are created
+     *
+     * @param listener the listener to be notified
+     */
+    public static void addNodeCreationListener(NodeCreationListener listener) {
+        synchronized (UiElementNode.class) {
+            if (sListeners == null) {
+                sListeners = new ArrayList<NodeCreationListener>(1);
+            }
+            sListeners.add(listener);
+        }
+    }
+
+    /**
+     * Removes a {@link NodeCreationListener} from the set of listeners such that it is
+     * no longer notified when nodes are created.
+     *
+     * @param listener the listener to be removed from the notification list
+     */
+    public static void removeNodeCreationListener(NodeCreationListener listener) {
+        synchronized (UiElementNode.class) {
+            sListeners.remove(listener);
+            if (sListeners.size() == 0) {
+                sListeners = null;
+            }
+        }
+    }
+
+    /** Interface implemented by listeners to be notified of newly created nodes */
+    public interface NodeCreationListener {
+        /**
+         * Called when a new child node is created and added to the given parent
+         *
+         * @param parent the parent of the created node
+         * @param child the newly node
+         * @param index the index among the siblings of the child <b>after</b>
+         *            insertion
+         */
+        void nodeCreated(UiElementNode parent, UiElementNode child, int index);
+
+        /**
+         * Called when a child node is removed from the given parent
+         *
+         * @param parent the parent of the removed node
+         * @param child the removed node
+         * @param previousIndex the index among the siblings of the child
+         *            <b>before</b> removal
+         */
+        void nodeDeleted(UiElementNode parent, UiElementNode child, int previousIndex);
+    }
 }