[497398] Fix unexpected refresh of regions container

Some refresh of regions container are done but they should not. This
commit reduce them: The refresh is done only if new views are created in
an existing regions container or if the order of regions is changed.
This commit resolves no bug but clean the code.

Bug: 497398
Cherry-picked-from: 495707
Change-Id: I46e40cd51960e5dcac18ca0248c1d7141cc6169f
Signed-off-by: Laurent Redor <laurent.redor@obeo.fr>

4 files changed, 124 insertions, 17 deletions

diff --git a/plugins/org.eclipse.sirius.diagram.ui/src-diag/org/eclipse/sirius/diagram/ui/internal/operation/RegionContainerUpdateLayoutOperation.java b/plugins/org.eclipse.sirius.diagram.ui/src-diag/org/eclipse/sirius/diagram/ui/internal/operation/RegionContainerUpdateLayoutOperation.java
index 0bba021673..386981860a 100644
--- a/plugins/org.eclipse.sirius.diagram.ui/src-diag/org/eclipse/sirius/diagram/ui/internal/operation/RegionContainerUpdateLayoutOperation.java
+++ b/plugins/org.eclipse.sirius.diagram.ui/src-diag/org/eclipse/sirius/diagram/ui/internal/operation/RegionContainerUpdateLayoutOperation.java
@@ -10,8 +10,10 @@
  *******************************************************************************/
 package org.eclipse.sirius.diagram.ui.internal.operation;
 
+import java.util.Collections;
 import java.util.List;
 import java.util.Map;
+import java.util.Set;
 
 import org.eclipse.draw2d.geometry.Rectangle;
 import org.eclipse.emf.ecore.EObject;
@@ -22,6 +24,7 @@ import org.eclipse.gmf.runtime.notation.Size;
 import org.eclipse.gmf.runtime.notation.View;
 import org.eclipse.sirius.diagram.DNodeContainer;
 import org.eclipse.sirius.diagram.business.internal.query.DNodeContainerExperimentalQuery;
+import org.eclipse.sirius.diagram.ui.business.api.query.NodeQuery;
 import org.eclipse.sirius.diagram.ui.business.api.view.SiriusGMFHelper;
 import org.eclipse.sirius.diagram.ui.business.internal.operation.AbstractModelChangeOperation;
 import org.eclipse.sirius.diagram.ui.internal.edit.parts.DNodeContainerViewNodeContainerCompartment2EditPart;
@@ -32,9 +35,13 @@ import org.eclipse.sirius.diagram.ui.provider.Messages;
 import org.eclipse.sirius.viewpoint.DRepresentationElement;
 import org.eclipse.sirius.viewpoint.description.RepresentationElementMapping;
 
+import com.google.common.base.Function;
+import com.google.common.base.Predicate;
 import com.google.common.collect.Iterables;
 import com.google.common.collect.Lists;
 import com.google.common.collect.Maps;
+import com.google.common.collect.Ordering;
+import com.google.common.collect.Sets;
 
 /**
  * Update and keep consistent the GMF Bounds of a Region Container and its
@@ -46,6 +53,8 @@ public class RegionContainerUpdateLayoutOperation extends AbstractModelChangeOpe
 
     private final Node regionContainer;
 
+    private Set<View> createdNodeViews = Sets.newHashSet();
+
     /**
      * Constructor.
      * 
@@ -57,6 +66,21 @@ public class RegionContainerUpdateLayoutOperation extends AbstractModelChangeOpe
         this.regionContainer = extractRealRegionContainer(regionContainer);
     }
 
+    /**
+     * Constructor.
+     * 
+     * @param regionContainer
+     *            The Region Container view to layout.
+     * @param createdNodeViews
+     *            List of views created since the last call to the
+     *            RegionContainerUpdateLayoutOperation (and that can have effect
+     *            on the layout of the regions container).
+     */
+    public RegionContainerUpdateLayoutOperation(Node regionContainer, Set<View> createdNodeViews) {
+        this(regionContainer);
+        this.createdNodeViews = createdNodeViews;
+    }
+
     private Node extractRealRegionContainer(Node node) {
         if (node != null && node.eContainer() instanceof Node) {
             int visualID = SiriusVisualIDRegistry.getVisualID(node);
@@ -91,12 +115,17 @@ public class RegionContainerUpdateLayoutOperation extends AbstractModelChangeOpe
             EObject element = regionContainer.getElement();
             if (element instanceof DNodeContainer) {
                 DNodeContainer dnc = (DNodeContainer) element;
+                List<Node> regionsToLayoutSortedByLocation = Lists.newArrayList(regionsToLayout);
+                sortRegionsAccordingToLocation(dnc, regionsToLayoutSortedByLocation);
                 sortRegions(dnc, regionsToLayout);
-                DNodeContainerExperimentalQuery query = new DNodeContainerExperimentalQuery(dnc);
-                if (query.isVerticalStackContainer()) {
-                    updateRegionsLayoutContraints(regionsToLayout, true);
-                } else if (query.isHorizontaltackContainer()) {
-                    updateRegionsLayoutContraints(regionsToLayout, false);
+                boolean isOrderChanged = !regionsToLayout.equals(regionsToLayoutSortedByLocation);
+                if (isOrderChanged || isImpactedByNewViews()) {
+                    DNodeContainerExperimentalQuery query = new DNodeContainerExperimentalQuery(dnc);
+                    if (query.isVerticalStackContainer()) {
+                        updateRegionsLayoutContraints(regionsToLayout, true);
+                    } else if (query.isHorizontaltackContainer()) {
+                        updateRegionsLayoutContraints(regionsToLayout, false);
+                    }
                 }
             }
 
@@ -104,6 +133,24 @@ public class RegionContainerUpdateLayoutOperation extends AbstractModelChangeOpe
         return null;
     }
 
+    /**
+     * Check if at least one of the new views concerned the current region
+     * container.
+     * 
+     * @param dnc
+     *            The container to which we want to know if it is impacted
+     * @return true if at least one of the created views is contained by the
+     *         <code>regionContainer</code>, false otherwise.
+     */
+    private boolean isImpactedByNewViews() {
+        return Iterables.any(createdNodeViews, new Predicate<View>() {
+            @Override
+            public boolean apply(View input) {
+                return input instanceof Node && isDescendantOf((Node) input, regionContainer);
+            }
+        });
+    }
+
     /*
      * @param vertical : vertical if true, horizontal if false.
      */
@@ -187,6 +234,35 @@ public class RegionContainerUpdateLayoutOperation extends AbstractModelChangeOpe
         new RegionComparisonHelper(dNodeContainer).sort(modelChildren);
     }
 
+    /**
+     * Sort the regions with the current x or y location.
+     * 
+     * @param dNodeContainer
+     *            the {@link DNodeContainer} region container
+     * @param modelChildren
+     *            the regions
+     */
+    public static void sortRegionsAccordingToLocation(DNodeContainer dNodeContainer, List<? extends View> modelChildren) {
+        final DNodeContainerExperimentalQuery query = new DNodeContainerExperimentalQuery(dNodeContainer);
+        Function<View, Integer> locationIndex = new Function<View, Integer>() {
+            @Override
+            public Integer apply(View view) {
+                if (view instanceof Node) {
+                    LayoutConstraint constraint = ((Node) view).getLayoutConstraint();
+                    if (constraint instanceof Location) {
+                        if (query.isHorizontaltackContainer()) {
+                            return ((Location) constraint).getX();
+                        } else if (query.isVerticalStackContainer()) {
+                            return ((Location) constraint).getY();
+                        }
+                    }
+                }
+                return Integer.MAX_VALUE;
+            }
+        };
+        Collections.sort(modelChildren, Ordering.natural().onResultOf(locationIndex));
+    }
+
     private static class RegionComparisonHelper extends ComparisonHelper {
         private DNodeContainer self;
 
@@ -204,4 +280,29 @@ public class RegionContainerUpdateLayoutOperation extends AbstractModelChangeOpe
             return self.getActualMapping().getAllContainerMappings();
         }
     }
+
+    /**
+     * Tests whether the <code>node</code> is a descendant of
+     * <code>container</code>.
+     * 
+     * @param node
+     *            The node to query
+     * @param container
+     *            The container
+     * @return true if the queried Node is a descendant of
+     *         <code>container</code>, false otherwise.
+     */
+    public boolean isDescendantOf(Node node, View container) {
+        boolean result = false;
+        if (node.eContainer() instanceof Node) {
+            if (container instanceof Node && new NodeQuery((Node) container).isContainer()) {
+                if (container.equals(node.eContainer())) {
+                    result = true;
+                } else {
+                    result = isDescendantOf((Node) node.eContainer(), container);
+                }
+            }
+        }
+        return result;
+    }
 }
diff --git a/plugins/org.eclipse.sirius.diagram.ui/src-diag/org/eclipse/sirius/diagram/ui/internal/refresh/AbstractCanonicalSynchronizer.java b/plugins/org.eclipse.sirius.diagram.ui/src-diag/org/eclipse/sirius/diagram/ui/internal/refresh/AbstractCanonicalSynchronizer.java
index 83414073a2..1791716f66 100644
--- a/plugins/org.eclipse.sirius.diagram.ui/src-diag/org/eclipse/sirius/diagram/ui/internal/refresh/AbstractCanonicalSynchronizer.java
+++ b/plugins/org.eclipse.sirius.diagram.ui/src-diag/org/eclipse/sirius/diagram/ui/internal/refresh/AbstractCanonicalSynchronizer.java
@@ -1,5 +1,5 @@
 /*******************************************************************************
- * Copyright (c) 2011, 2015 THALES GLOBAL SERVICES and others.
+ * Copyright (c) 2011, 2016 THALES GLOBAL SERVICES and others.
  * All rights reserved. This program and the accompanying materials
  * are made available under the terms of the Eclipse Public License v1.0
  * which accompanies this distribution, and is available at
@@ -57,6 +57,8 @@ import org.eclipse.sirius.diagram.ui.business.api.query.ViewQuery;
 import org.eclipse.sirius.diagram.ui.business.api.view.SiriusLayoutDataManager;
 import org.eclipse.sirius.diagram.ui.business.internal.query.DNodeQuery;
 import org.eclipse.sirius.diagram.ui.business.internal.view.LayoutData;
+import org.eclipse.sirius.diagram.ui.internal.edit.parts.DNodeContainer2EditPart;
+import org.eclipse.sirius.diagram.ui.internal.edit.parts.DNodeContainerEditPart;
 import org.eclipse.sirius.diagram.ui.internal.providers.SiriusViewProvider;
 import org.eclipse.sirius.diagram.ui.internal.refresh.borderednode.CanonicalDBorderItemLocator;
 import org.eclipse.sirius.diagram.ui.internal.view.factories.AbstractContainerViewFactory;
@@ -156,7 +158,12 @@ public abstract class AbstractCanonicalSynchronizer implements CanonicalSynchron
 
         boolean regionContainer = semanticView instanceof DNodeContainer && new DNodeContainerExperimentalQuery((DNodeContainer) semanticView).isRegionContainer();
         if (regionContainer) {
-            regionContainersToLayout.add(gmfView);
+            // Only consider DNodeContainerEditPart and DNodeContainer2EditPart
+            // as regionContainer to layout.
+            int type = SiriusVisualIDRegistry.getVisualID(gmfView.getType());
+            if (type == DNodeContainerEditPart.VISUAL_ID || type == DNodeContainer2EditPart.VISUAL_ID) {
+                regionContainersToLayout.add(gmfView);
+            }
         }
 
         // Manage Nodes ordering in Compartment according to DNodeListElement
diff --git a/plugins/org.eclipse.sirius.diagram.ui/src-diag/org/eclipse/sirius/diagram/ui/internal/refresh/diagram/DDiagramCanonicalSynchronizer.java b/plugins/org.eclipse.sirius.diagram.ui/src-diag/org/eclipse/sirius/diagram/ui/internal/refresh/diagram/DDiagramCanonicalSynchronizer.java
index b090990bdf..7b2898f0f7 100644
--- a/plugins/org.eclipse.sirius.diagram.ui/src-diag/org/eclipse/sirius/diagram/ui/internal/refresh/diagram/DDiagramCanonicalSynchronizer.java
+++ b/plugins/org.eclipse.sirius.diagram.ui/src-diag/org/eclipse/sirius/diagram/ui/internal/refresh/diagram/DDiagramCanonicalSynchronizer.java
@@ -1,5 +1,5 @@
 /*******************************************************************************
- * Copyright (c) 2011, 2015 THALES GLOBAL SERVICES and others.
+ * Copyright (c) 2011, 2016 THALES GLOBAL SERVICES and others.
  * All rights reserved. This program and the accompanying materials
  * are made available under the terms of the Eclipse Public License v1.0
  * which accompanies this distribution, and is available at
@@ -46,8 +46,6 @@ import org.eclipse.sirius.diagram.ui.business.api.helper.graphicalfilters.Collap
 import org.eclipse.sirius.diagram.ui.business.api.view.SiriusLayoutDataManager;
 import org.eclipse.sirius.diagram.ui.business.internal.dialect.SetBestHeightHeaderCommand;
 import org.eclipse.sirius.diagram.ui.internal.edit.parts.DDiagramEditPart;
-import org.eclipse.sirius.diagram.ui.internal.edit.parts.DNodeContainer2EditPart;
-import org.eclipse.sirius.diagram.ui.internal.edit.parts.DNodeContainerEditPart;
 import org.eclipse.sirius.diagram.ui.internal.operation.RegionContainerUpdateLayoutOperation;
 import org.eclipse.sirius.diagram.ui.internal.refresh.AbstractCanonicalSynchronizer;
 import org.eclipse.sirius.diagram.ui.part.SiriusDiagramUpdater;
@@ -123,23 +121,22 @@ public class DDiagramCanonicalSynchronizer extends AbstractCanonicalSynchronizer
 
             manageCollapse(createdNodeViews);
 
-            manageRegions();
+            manageRegions(createdNodeViews);
         }
     }
 
-    private void manageRegions() {
+    private void manageRegions(Set<View> createdNodeViews) {
         if (regionContainersToLayout.isEmpty()) {
             return;
         }
 
         // Step 1: update regions layout from the deepest ones.
         List<View> newArrayList = Lists.newArrayList(regionContainersToLayout);
-        ListIterator<View> regionToLayoutListIterator = newArrayList.listIterator(newArrayList.size() - 1);
+        ListIterator<View> regionToLayoutListIterator = newArrayList.listIterator(newArrayList.size());
         while (regionToLayoutListIterator.hasPrevious()) {
             View regionContainer = regionToLayoutListIterator.previous();
-            int type = SiriusVisualIDRegistry.getVisualID(regionContainer.getType());
-            if (regionContainer instanceof Node && (type == DNodeContainerEditPart.VISUAL_ID || type == DNodeContainer2EditPart.VISUAL_ID)) {
-                new RegionContainerUpdateLayoutOperation((Node) regionContainer).execute();
+            if (regionContainer instanceof Node) {
+                new RegionContainerUpdateLayoutOperation((Node) regionContainer, createdNodeViews).execute();
             }
         }
         regionContainersToLayout.clear();
diff --git a/plugins/org.eclipse.sirius.diagram.ui/src-diag/org/eclipse/sirius/diagram/ui/tools/internal/layout/provider/ArrangeSelectionLayoutProvider.java b/plugins/org.eclipse.sirius.diagram.ui/src-diag/org/eclipse/sirius/diagram/ui/tools/internal/layout/provider/ArrangeSelectionLayoutProvider.java
index cbf7233098..b1e91c6ff3 100644
--- a/plugins/org.eclipse.sirius.diagram.ui/src-diag/org/eclipse/sirius/diagram/ui/tools/internal/layout/provider/ArrangeSelectionLayoutProvider.java
+++ b/plugins/org.eclipse.sirius.diagram.ui/src-diag/org/eclipse/sirius/diagram/ui/tools/internal/layout/provider/ArrangeSelectionLayoutProvider.java
@@ -139,7 +139,9 @@ public class ArrangeSelectionLayoutProvider extends AbstractLayoutProvider {
                         addEdgesToNotSelectedUnpinnedList((DiagramEditPart) igep.getRoot().getChildren().get(0));
                     }
 
-                    // Update Layout Hint to find later the list of unselected
+                    // Update Layout Hint to find later (in
+                    // org.eclipse.sirius.diagram.ui.tools.internal.layout.provider.AbstractCompositeLayoutProvider.layoutEditParts(List,
+                    // IAdaptable) for example) the list of unselected
                     // diagram element on diagram that are unpinned as elements
                     // to keep fixed in PinnedElementsHandler
                     final IAdaptable originalHint = layoutHint;