[499831] Expand the scope of the layout of regions container

This commit fixes the scenario of [1].
The previous commit 4c22f9 [2] reduces the number of regions container
layout. But a case was no longer handled: the deletion of a semantic
element represented by a region. In this case, the layout was not called
and the remaining regions are incorrectly layouted.

[1] https://bugs.eclipse.org/bugs/show_bug.cgi?id=495707#c16
[2]
http://git.eclipse.org/c/sirius/org.eclipse.sirius.git/commit/?id=4c22f9b8234cafc16a93dea18dfe491194571342

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

3 files changed, 136 insertions, 95 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 6897384a15..cfcb9b8384 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
@@ -13,7 +13,6 @@ 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;
@@ -24,7 +23,6 @@ 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;
@@ -36,52 +34,55 @@ 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
- * Regions.
+ * Update and keep consistent the GMF Bounds of a regions container and its
+ * regions.
  * 
  * @author mporhel
  */
 public class RegionContainerUpdateLayoutOperation extends AbstractModelChangeOperation<Void> {
 
-    private final Node regionContainer;
+    private final Node regionsContainer;
 
-    private Set<View> createdNodeViews = Sets.newHashSet();
+    /**
+     * True if this layout operation is caused for a regions container that has
+     * at least one new region or one region less, false otherwise.
+     */
+    private final boolean containsCreatedOrDeletedRegions;
 
     /**
      * Constructor.
      * 
-     * @param regionContainer
-     *            The Region Container view to layout.
+     * @param regionsContainer
+     *            The regions container view to layout.
      */
-    public RegionContainerUpdateLayoutOperation(Node regionContainer) {
+    public RegionContainerUpdateLayoutOperation(Node regionsContainer) {
         super(Messages.RegionContainerUpdateLayoutOperation_name);
-        this.regionContainer = extractRealRegionContainer(regionContainer);
+        this.regionsContainer = extractRealRegionsContainer(regionsContainer);
+        this.containsCreatedOrDeletedRegions = false;
     }
 
     /**
      * 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).
+     * @param regionsContainer
+     *            The regions container view to layout.
+     * @param containsCreatedOrDeletedRegions
+     *            true if this layout operation is caused for a regions
+     *            container that has at least one new region or one region less.
      */
-    public RegionContainerUpdateLayoutOperation(Node regionContainer, Set<View> createdNodeViews) {
-        this(regionContainer);
-        this.createdNodeViews = createdNodeViews;
+    public RegionContainerUpdateLayoutOperation(Node regionsContainer, boolean containsCreatedOrDeletedRegions) {
+        super(Messages.RegionContainerUpdateLayoutOperation_name);
+        this.regionsContainer = extractRealRegionsContainer(regionsContainer);
+        this.containsCreatedOrDeletedRegions = true;
     }
 
-    private Node extractRealRegionContainer(Node node) {
+    private Node extractRealRegionsContainer(Node node) {
         if (node != null && node.eContainer() instanceof Node) {
             int visualID = SiriusVisualIDRegistry.getVisualID(node);
             if (DNodeContainerViewNodeContainerCompartmentEditPart.VISUAL_ID == visualID || DNodeContainerViewNodeContainerCompartment2EditPart.VISUAL_ID == visualID) {
@@ -93,9 +94,9 @@ public class RegionContainerUpdateLayoutOperation extends AbstractModelChangeOpe
 
     private List<Node> getRegionsToLayout() {
         List<Node> regionsToLayout = Lists.newArrayList();
-        if (regionContainer != null) {
-            Node labelNode = SiriusGMFHelper.getLabelNode(regionContainer);
-            List<Node> nodes = Lists.newArrayList(Iterables.filter(regionContainer.getChildren(), Node.class));
+        if (regionsContainer != null) {
+            Node labelNode = SiriusGMFHelper.getLabelNode(regionsContainer);
+            List<Node> nodes = Lists.newArrayList(Iterables.filter(regionsContainer.getChildren(), Node.class));
             if (labelNode != null && nodes.size() == 2) {
                 nodes.remove(labelNode);
                 Node compartment = nodes.iterator().next();
@@ -112,14 +113,17 @@ public class RegionContainerUpdateLayoutOperation extends AbstractModelChangeOpe
     public Void execute() {
         List<Node> regionsToLayout = getRegionsToLayout();
         if (!regionsToLayout.isEmpty()) {
-            EObject element = regionContainer.getElement();
+            EObject element = regionsContainer.getElement();
             if (element instanceof DNodeContainer) {
                 DNodeContainer dnc = (DNodeContainer) element;
                 List<Node> regionsToLayoutSortedByLocation = Lists.newArrayList(regionsToLayout);
                 sortRegionsAccordingToLocation(dnc, regionsToLayoutSortedByLocation);
                 sortRegions(dnc, regionsToLayout);
-                boolean isOrderChanged = !regionsToLayout.equals(regionsToLayoutSortedByLocation);
-                if (isOrderChanged || isImpactedByNewViews()) {
+                // Check if the order from location (x axis for horizontal stack
+                // and y for vertical) is the same as the stored order. If not a
+                // layout must be launched.
+                boolean isSameOrder = regionsToLayout.equals(regionsToLayoutSortedByLocation);
+                if (!isSameOrder || containsCreatedOrDeletedRegions) {
                     DNodeContainerExperimentalQuery query = new DNodeContainerExperimentalQuery(dnc);
                     if (query.isVerticalStackContainer()) {
                         updateRegionsLayoutContraints(regionsToLayout, true);
@@ -128,52 +132,10 @@ 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);
-            }
-
-            /**
-             * Tests whether the queried Node is a descendant of
-             * <code>container</code>.
-             * 
-             * @param container
-             *            The container
-             * @return true if the queried Node is a descendant of
-             *         <code>container</code>, false otherwise.
-             */
-            private 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;
-            }
-        });
-    }
-
     /*
      * @param vertical : vertical if true, horizontal if false.
      */
@@ -222,7 +184,7 @@ public class RegionContainerUpdateLayoutOperation extends AbstractModelChangeOpe
             }
         }
 
-        LayoutConstraint layoutConstraint = regionContainer.getLayoutConstraint();
+        LayoutConstraint layoutConstraint = regionsContainer.getLayoutConstraint();
         if (layoutConstraint instanceof Size) {
             Size size = (Size) layoutConstraint;
             if (vertical) {
@@ -249,7 +211,7 @@ public class RegionContainerUpdateLayoutOperation extends AbstractModelChangeOpe
      * a mapping comparator.
      * 
      * @param dNodeContainer
-     *            the {@link DNodeContainer} region container
+     *            the {@link DNodeContainer} regions container
      * @param modelChildren
      *            the regions
      */
@@ -261,7 +223,7 @@ public class RegionContainerUpdateLayoutOperation extends AbstractModelChangeOpe
      * Sort the regions with the current x or y location.
      * 
      * @param dNodeContainer
-     *            the {@link DNodeContainer} region container
+     *            the {@link DNodeContainer} regions container
      * @param modelChildren
      *            the regions
      */
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 1791716f66..0a302248c4 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
@@ -69,9 +69,11 @@ import org.eclipse.sirius.diagram.ui.part.SiriusVisualIDRegistry;
 import org.eclipse.sirius.diagram.ui.provider.DiagramUIPlugin;
 import org.eclipse.sirius.diagram.ui.tools.api.graphical.edit.styles.IBorderItemOffsets;
 import org.eclipse.sirius.ext.base.Option;
+import org.eclipse.sirius.ext.base.Options;
 
 import com.google.common.collect.Iterables;
 import com.google.common.collect.Lists;
+import com.google.common.collect.Maps;
 import com.google.common.collect.Sets;
 
 /**
@@ -94,9 +96,15 @@ public abstract class AbstractCanonicalSynchronizer implements CanonicalSynchron
     protected IViewProvider viewpointViewProvider = new SiriusViewProvider();
 
     /**
-     * Store region containers to layout.
+     * Store regions containers and its impacted status:
+     * <UL>
+     * <LI>A regions container is considered as impacted if it contains at least
+     * one new view or if at least one of its views has been deleted.</LI>
+     * <LI>if impacted, the regions container must be layouted</LI>
+     * </UL>
+     * .
      */
-    protected Collection<View> regionContainersToLayout = Sets.newLinkedHashSet();
+    protected Map<View, Boolean> regionsContainersToLayoutWithImpactStatus = Maps.newHashMap();
 
     /**
      * Default constructor.
@@ -144,6 +152,21 @@ public abstract class AbstractCanonicalSynchronizer implements CanonicalSynchron
             return Collections.emptySet();
         }
 
+        // isPartOfRegionsContainer is true for regions container and all its
+        // sub part as CompartmentView.
+        boolean isPartOfRegionsContainer = semanticView instanceof DNodeContainer && new DNodeContainerExperimentalQuery((DNodeContainer) semanticView).isRegionContainer();
+        // isRegionsContainer is true only for the real regions container view
+        boolean isRegionsContainer = false;
+        if (isPartOfRegionsContainer) {
+            // Only consider DNodeContainerEditPart and DNodeContainer2EditPart
+            // as real regionContainer potentially to layout.
+            int type = SiriusVisualIDRegistry.getVisualID(gmfView.getType());
+            if (type == DNodeContainerEditPart.VISUAL_ID || type == DNodeContainer2EditPart.VISUAL_ID) {
+                isRegionsContainer = true;
+                regionsContainersToLayoutWithImpactStatus.put(gmfView, Boolean.FALSE);
+            }
+        }
+
         //
         // current views
         final List<View> viewChildren = getViewChildren(gmfView);
@@ -151,30 +174,86 @@ public abstract class AbstractCanonicalSynchronizer implements CanonicalSynchron
 
         List<View> orphaned = cleanCanonicalSemanticChildren(gmfView, viewChildren, semanticChildren);
 
+        if (!orphaned.isEmpty()) {
+            // Some children views have been deleted, this container must be
+            // layouted if it is a regions container or a sub part of a regions
+            // container
+            setRegionsContainerAsImpacted(gmfView, isRegionsContainer, isPartOfRegionsContainer);
+        }
         deleteViews(orphaned);
 
         // create a view for each remaining semantic element.
         Set<View> createdViews = createViews(semanticChildren, gmfView.getType(), gmfView);
 
-        boolean regionContainer = semanticView instanceof DNodeContainer && new DNodeContainerExperimentalQuery((DNodeContainer) semanticView).isRegionContainer();
-        if (regionContainer) {
-            // 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);
-            }
+        if (!createdViews.isEmpty()) {
+            // There is at least one new child, this container must be
+            // layouted if it is a regions container or a sub part of a regions
+            // container.
+            setRegionsContainerAsImpacted(gmfView, isRegionsContainer, isPartOfRegionsContainer);
         }
-
         // Manage Nodes ordering in Compartment according to DNodeListElement
         // ordering
-        if (semanticView instanceof DNodeList || regionContainer) {
+        if (semanticView instanceof DNodeList || isPartOfRegionsContainer) {
             refreshSemanticChildrenOrdering(gmfView);
         }
 
         return createdViews;
     }
 
+    /**
+     * Set the gmfView, or its ancestor representation the real regions
+     * container, as impacted by the changes (new views, removed views or order
+     * change).
+     * 
+     * @param gmfView
+     *            The concerned view (corresponding to the regions container
+     *            itself or one of its sub-part)
+     * @param isRegionsContainer
+     *            true is the current view represents the regions container,
+     *            false otherwise
+     * @param isPartOfRegionsContainer
+     *            true is the current view represents the regions container or a
+     *            sub-part of the regions container (as CompartmentView for
+     *            example), false otherwise
+     */
+    private void setRegionsContainerAsImpacted(final View gmfView, boolean isRegionsContainer, boolean isPartOfRegionsContainer) {
+        if (isRegionsContainer) {
+            regionsContainersToLayoutWithImpactStatus.put(gmfView, Boolean.TRUE);
+        } else if (isPartOfRegionsContainer) {
+            Option<View> realRegionsContainer = getAncestor(gmfView, DNodeContainerEditPart.VISUAL_ID, DNodeContainer2EditPart.VISUAL_ID);
+            if (realRegionsContainer.some()) {
+                regionsContainersToLayoutWithImpactStatus.put(realRegionsContainer.get(), Boolean.TRUE);
+            }
+        }
+    }
+
+    /**
+     * Get the first ancestor, or itself, that has at least one of the
+     * <code>visualID</code>.
+     * 
+     * @param gmfView
+     *            The concerned view (corresponding to the regions container
+     *            itself or one of its sub-part)
+     * @param visualID
+     *            List of visual ID that the ancestor must be.
+     * 
+     * @return An optional {@link View}
+     */
+    public Option<View> getAncestor(View gmfView, int... visualID) {
+        Option<View> result = Options.newNone();
+        int type = SiriusVisualIDRegistry.getVisualID(gmfView.getType());
+        for (int i = 0; i < visualID.length; i++) {
+            if (type == visualID[i]) {
+                result = Options.newSome(gmfView);
+                break;
+            }
+        }
+        if (!result.some() && gmfView.eContainer() instanceof View) {
+            result = getAncestor((View) gmfView.eContainer(), visualID);
+        }
+        return result;
+    }
+
     private void markCreatedViewsAsToLayout(Collection<View> createdViews) {
         for (View createdView : createdViews) {
             createdView.eAdapters().add(SiriusLayoutDataManager.INSTANCE.getAdapterMarker());
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 f939796eae..f3cacd44de 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
@@ -121,25 +121,25 @@ public class DDiagramCanonicalSynchronizer extends AbstractCanonicalSynchronizer
 
             manageCollapse(createdNodeViews);
 
-            manageRegions(createdNodeViews);
+            manageRegions();
         }
     }
 
-    private void manageRegions(Set<View> createdNodeViews) {
-        if (regionContainersToLayout.isEmpty()) {
+    private void manageRegions() {
+        if (regionsContainersToLayoutWithImpactStatus.isEmpty()) {
             return;
         }
 
         // Step 1: update regions layout from the deepest ones.
-        List<View> newArrayList = Lists.newArrayList(regionContainersToLayout);
-        ListIterator<View> regionToLayoutListIterator = newArrayList.listIterator(newArrayList.size());
-        while (regionToLayoutListIterator.hasPrevious()) {
-            View regionContainer = regionToLayoutListIterator.previous();
-            if (regionContainer instanceof Node) {
-                new RegionContainerUpdateLayoutOperation((Node) regionContainer, createdNodeViews).execute();
+        List<View> newArrayList = Lists.newArrayList(regionsContainersToLayoutWithImpactStatus.keySet());
+        ListIterator<View> regionsContainersToLayoutListIterator = newArrayList.listIterator(newArrayList.size());
+        while (regionsContainersToLayoutListIterator.hasPrevious()) {
+            View regionsContainer = regionsContainersToLayoutListIterator.previous();
+            if (regionsContainer instanceof Node) {
+                new RegionContainerUpdateLayoutOperation((Node) regionsContainer, regionsContainersToLayoutWithImpactStatus.get(regionsContainer)).execute();
             }
         }
-        regionContainersToLayout.clear();
+        regionsContainersToLayoutWithImpactStatus.clear();
     }
 
     private void manageCreatedViewsLayout(Set<View> createdViews) {