[495707] 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: 495707
Change-Id: Ideb7bddffa044adae5782d585dbe20c620c634da
Signed-off-by: Laurent Redor <laurent.redor@obeo.fr>

6 files changed, 139 insertions, 73 deletions

diff --git a/plugins/org.eclipse.sirius.diagram.ui/src-diag/org/eclipse/sirius/diagram/ui/business/api/query/ViewQuery.java b/plugins/org.eclipse.sirius.diagram.ui/src-diag/org/eclipse/sirius/diagram/ui/business/api/query/ViewQuery.java
index 2e2118bdcc..f94778a3aa 100644
--- a/plugins/org.eclipse.sirius.diagram.ui/src-diag/org/eclipse/sirius/diagram/ui/business/api/query/ViewQuery.java
+++ b/plugins/org.eclipse.sirius.diagram.ui/src-diag/org/eclipse/sirius/diagram/ui/business/api/query/ViewQuery.java
@@ -1,5 +1,5 @@
 /*******************************************************************************
- * Copyright (c) 2012, 2014 THALES GLOBAL SERVICES and others.
+ * Copyright (c) 2012, 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
@@ -38,6 +38,8 @@ import org.eclipse.sirius.diagram.ui.internal.edit.parts.DNodeListNameEditPart;
 import org.eclipse.sirius.diagram.ui.internal.edit.parts.NotationViewIDs;
 import org.eclipse.sirius.diagram.ui.part.SiriusVisualIDRegistry;
 import org.eclipse.sirius.diagram.ui.provider.DiagramUIPlugin;
+import org.eclipse.sirius.ext.base.Option;
+import org.eclipse.sirius.ext.base.Options;
 import org.eclipse.sirius.viewpoint.DStylizable;
 import org.eclipse.swt.SWT;
 import org.eclipse.swt.graphics.FontData;
@@ -263,4 +265,28 @@ public class ViewQuery {
         }
         return result;
     }
+
+    /**
+     * Get the first ancestor, or itself, that has at least one of the
+     * <code>visualID</code>.
+     * 
+     * @param visualID
+     *            List of visual ID that the ancestor must be.
+     * 
+     * @return An optional {@link View}
+     */
+    public Option<View> getAncestor(int... visualID) {
+        Option<View> result = Options.newNone();
+        int type = SiriusVisualIDRegistry.getVisualID(view.getType());
+        for (int i = 0; i < visualID.length; i++) {
+            if (type == visualID[i]) {
+                result = Options.newSome(view);
+                break;
+            }
+        }
+        if (!result.some() && view.eContainer() instanceof View) {
+            result = new ViewQuery((View) view.eContainer()).getAncestor(visualID);
+        }
+        return result;
+    }
 }
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 1df4a13d28..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,29 +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 && new NodeQuery((Node) input).isDescendantOf(regionContainer);
-            }
-        });
-    }
-
     /*
      * @param vertical : vertical if true, horizontal if false.
      */
@@ -199,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) {
@@ -226,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
      */
@@ -238,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..a18b72924b 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
@@ -72,6 +72,7 @@ import org.eclipse.sirius.ext.base.Option;
 
 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 +95,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 +151,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 +173,59 @@ 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 = new ViewQuery(gmfView).getAncestor(DNodeContainerEditPart.VISUAL_ID, DNodeContainer2EditPart.VISUAL_ID);
+            if (realRegionsContainer.some()) {
+                regionsContainersToLayoutWithImpactStatus.put(realRegionsContainer.get(), Boolean.TRUE);
+            }
+        }
+    }
+
     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) {
diff --git a/plugins/org.eclipse.sirius.doc/doc/Release_Notes.html b/plugins/org.eclipse.sirius.doc/doc/Release_Notes.html
index 1b99ee685c..1305ca1aa4 100644
--- a/plugins/org.eclipse.sirius.doc/doc/Release_Notes.html
+++ b/plugins/org.eclipse.sirius.doc/doc/Release_Notes.html
@@ -99,6 +99,9 @@
 			<li><span class="label label-success">Added</span> 
 				<code>org.eclipse.sirius.diagram.ui.business.api.query.NodeQuery.isDescendantOf(View)</code> has been added to know if a view is a descendant of another view.
 			</li>
+			<li><span class="label label-success">Added</span> 
+				<code>org.eclipse.sirius.diagram.ui.business.api.query.ViewQuery.getAncestor(int...)</code> has been added to retrieve the first ancestor of the view, or itself, that has at least one of the visualID passed as parameter.
+			</li>
 		</ul>
 		<h2 id="sirius4.0.0">Changes in Sirius 4.0.0</h2>
 		<h3 id="UserVisibleChanges2">User-Visible Changes</h3>
diff --git a/plugins/org.eclipse.sirius.doc/doc/Release_Notes.textile b/plugins/org.eclipse.sirius.doc/doc/Release_Notes.textile
index c3b55f023f..9b71ea861c 100644
--- a/plugins/org.eclipse.sirius.doc/doc/Release_Notes.textile
+++ b/plugins/org.eclipse.sirius.doc/doc/Release_Notes.textile
@@ -15,6 +15,7 @@ h3. Developer-Visible Changes
 h4. Changes in @org.eclipse.sirius.diagram.ui@
 
 * <span class="label label-success">Added</span> @org.eclipse.sirius.diagram.ui.business.api.query.NodeQuery.isDescendantOf(View)@ has been added to know if a view is a descendant of another view.
+* <span class="label label-success">Added</span> @org.eclipse.sirius.diagram.ui.business.api.query.ViewQuery.getAncestor(int...)@ has been added to retrieve the first ancestor of the view, or itself, that has at least one of the visualID passed as parameter.
 
 h2(#sirius4.0.0). Changes in Sirius 4.0.0