QGraphicsAnchorLayout: Update m_vertexList reference count system

The reference count system used to delete AnchorVertex objects
stored in m_vertexList had an issue that prevented the count values
from being stored properly.

This is a tentative fix for the memory leaks in the layout, changes
were made to the m_vertexList access methods as well as those that
use them.

1) addInternalVertex()

  After incrementing the ref count, store it in m_vertexList.

2) removeInternalVertex()

  To make it symmetrical to addInternalVertex() we delete the anchor
  vertex if its reach count reaches zero. From now on, the only
  methods that create or delete vertices are these two. All other
  methods should rely on add/removeInternalVertex.

  Also made the method 'void'. This method is called to release
  the associated object and may trigger a deletion of it, thus
  any use of the object reference after that is unsafe. Other
  methods can get a reference with internalVertex().

3) removeAnchor()

  Follow above instructions. No longer relies on the reference
  returned by removeInternalVertex(), instead, use the reference
  first then release.

  Note: In this particular case, we know that "removeEdge(v1, v2)"
        will only access the contents of the pointers, not the
        referenced object. Yet, the change probably pays itself
        for the sake of clarity and safety.

4) removeAnchors()

  Each tiime an anchor is created, the reference count of its
  both vertices is incremented, thus we should decrement that
  count every time we remove an anchor. (Instead of removing
  v1 only once).

  Also, leave the task of deleting v1 to removeInternalVertex()
  as explained above.

  Removing the constraints handling code. I understand that we
  cannot leave constraints with dangling pointers inside of them,
  however that raises an even worse problem.

  Every time an item is added, we create an item center constraint,
  thus we must be sure to delete those when the item is removed.
  Once we do that, we won't have bad constraints laying around.
  This issue remains open in this commit but will be solved soon.

Signed-off-by: Eduardo M. Fleury <eduardo.fleury@openbossa.org>

2 files changed, 30 insertions, 26 deletions

diff --git a/src/gui/graphicsview/qgraphicsanchorlayout_p.cpp b/src/gui/graphicsview/qgraphicsanchorlayout_p.cpp
index 1f6813f..b0ffea4 100644
--- a/src/gui/graphicsview/qgraphicsanchorlayout_p.cpp
+++ b/src/gui/graphicsview/qgraphicsanchorlayout_p.cpp
@@ -250,12 +250,13 @@ AnchorVertex *QGraphicsAnchorLayoutPrivate::addInternalVertex(QGraphicsLayoutIte
 {
     QPair<QGraphicsLayoutItem *, QGraphicsAnchorLayout::Edge> pair(item, edge);
     QPair<AnchorVertex *, int> v = m_vertexList.value(pair);
+
     if (!v.first) {
         Q_ASSERT(v.second == 0);
         v.first = new AnchorVertex(item, edge);
-        m_vertexList.insert(pair, v);
     }
     v.second++;
+    m_vertexList.insert(pair, v);
     return v.first;
 }
 
@@ -265,20 +266,26 @@ AnchorVertex *QGraphicsAnchorLayoutPrivate::addInternalVertex(QGraphicsLayoutIte
  * returns the AnchorVertex that was dereferenced, also when it was removed.
  * returns 0 if it did not exist.
  */
-AnchorVertex *QGraphicsAnchorLayoutPrivate::removeInternalVertex(QGraphicsLayoutItem *item,
-                                                                 QGraphicsAnchorLayout::Edge edge)
+void QGraphicsAnchorLayoutPrivate::removeInternalVertex(QGraphicsLayoutItem *item,
+                                                        QGraphicsAnchorLayout::Edge edge)
 {
     QPair<QGraphicsLayoutItem *, QGraphicsAnchorLayout::Edge> pair(item, edge);
     QPair<AnchorVertex *, int> v = m_vertexList.value(pair);
-    if (v.first) {
-        v.second--;
-        if (v.second == 0) {
-            m_vertexList.remove(pair);
-        }
-    } else {
+
+    if (!v.first) {
         qWarning("This item with this edge is not in the graph");
+        return;
+    }
+
+    v.second--;
+    if (v.second == 0) {
+        // Remove reference and delete vertex
+        m_vertexList.remove(pair);
+        delete v.first;
+    } else {
+        // Update reference count
+        m_vertexList.insert(pair, v);
     }
-    return v.first;
 }
 
 void QGraphicsAnchorLayoutPrivate::removeAnchor(QGraphicsLayoutItem *firstItem,
@@ -286,14 +293,18 @@ void QGraphicsAnchorLayoutPrivate::removeAnchor(QGraphicsLayoutItem *firstItem,
                                                 QGraphicsLayoutItem *secondItem,
                                                 QGraphicsAnchorLayout::Edge secondEdge)
 {
-    // Is there a representation for the Vertex (firstItem, firstEdge)
-    // in our internal structure?
-    AnchorVertex *v1 = removeInternalVertex(firstItem, firstEdge);
-    AnchorVertex *v2 = removeInternalVertex(secondItem, secondEdge);
+    // Look for both vertices
+    AnchorVertex *v1 = internalVertex(firstItem, firstEdge);
+    AnchorVertex *v2 = internalVertex(secondItem, secondEdge);
 
+    // Remove edge from graph
     if (v1 && v2) {
         graph[edgeOrientation(firstEdge)].removeEdge(v1, v2);
     }
+
+    // Decrease vertices reference count (may trigger a deletion)
+    removeInternalVertex(firstItem, firstEdge);
+    removeInternalVertex(secondItem, secondEdge);
 }
 
 void QGraphicsAnchorLayoutPrivate::removeAnchors(QGraphicsLayoutItem *item)
@@ -307,22 +318,15 @@ void QGraphicsAnchorLayoutPrivate::removeAnchors(QGraphicsLayoutItem *item)
         // Remove all vertex for all edges
         QGraphicsAnchorLayout::Edge e = static_cast<QGraphicsAnchorLayout::Edge>(edge);
 
-        if ((v1 = removeInternalVertex(item, e))) {
+        if ((v1 = internalVertex(item, e))) {
             // Remove all edges
             allVertex = graph[edgeOrientation(e)].adjacentVertices(v1);
 
-            QList<QSimplexConstraint *> constraints = itemCenterConstraints[edgeOrientation(e)];
             foreach (v2, allVertex) {
-                AnchorData *data = graph[edgeOrientation(e)].takeEdge(v1, v2);
-                Q_ASSERT(data);
-                for (int i = 0; i < constraints.count(); ++i) {
-                    QSimplexConstraint *c = constraints.at(i);
-                    c->variables.remove(data);
-                }
-                delete data;
+                graph[edgeOrientation(e)].removeEdge(v1, v2);
+                removeInternalVertex(item, e);
+                removeInternalVertex(v2->m_item, v2->m_edge);
             }
-            qDebug("removing anchor: %s", qPrintable(v1->toString()));
-            delete v1;
         }
     }
 }
diff --git a/src/gui/graphicsview/qgraphicsanchorlayout_p.h b/src/gui/graphicsview/qgraphicsanchorlayout_p.h
index ee1a181..27855e6 100644
--- a/src/gui/graphicsview/qgraphicsanchorlayout_p.h
+++ b/src/gui/graphicsview/qgraphicsanchorlayout_p.h
@@ -276,7 +276,7 @@ public:
     }
 
     AnchorVertex *addInternalVertex(QGraphicsLayoutItem *item, QGraphicsAnchorLayout::Edge edge);
-    AnchorVertex *removeInternalVertex(QGraphicsLayoutItem *item, QGraphicsAnchorLayout::Edge edge);
+    void removeInternalVertex(QGraphicsLayoutItem *item, QGraphicsAnchorLayout::Edge edge);
 
     // Geometry interpolation methods
     void setItemsGeometries();