someone got confused as to the difference between "the layout that manages my children" and "the layout that manages me". this fixes that and stops crashes in applets that use vbox/hbox

* don't crash when parent=0 is passed in
* don't divid by 0 when we have no children
* have the (fugly) setManagingLayout and unsetManagingLayout and managingLayout methods
* do some memory management so layouts that are thrown around don't get lost in the heap

some unit tests would be nice. hell, a design document on the layout stuff would be nice. i know we're only keeping this around for 4.0, but  ... yeah. ugh.
CCMAIL:panel-devel@kde.org

svn path=/trunk/KDE/kdebase/workspace/libs/plasma/; revision=693361

widgets/boxlayout.cpp

@@ -37,15 +37,16 @@ BoxLayout::BoxLayout(LayoutItem *parent)
     : Layout(parent),
       d(new Private)
 {
-    parent->setLayout(this);
+    if (parent) {
+        parent->setLayout(this);
+    }
 }
 
 BoxLayout::~BoxLayout()
 {
-    foreach (LayoutItem *l, d->childList) {
+    foreach (LayoutItem* item, children()) {
-        l->resetLayout();
+        item->unsetManagingLayout(this);
     }
-
     delete d;
 }
 
@@ -90,7 +91,7 @@ void BoxLayout::insertItem(int index, LayoutItem *l)
         return;
     }
 
-    l->setLayout(this);
+    //l->setLayout(this);
     d->childList.insert(index, l);
     setGeometry(geometry());
 }
@@ -101,14 +102,18 @@ void BoxLayout::addItem(LayoutItem *l)
         return;
     }
 
-    l->setLayout(this);
+    l->setManagingLayout(this);
     d->childList.append(l);
-    qDebug("Added Child LayoutItem : %p", l);
     setGeometry(geometry());
 }
 
 void BoxLayout::removeItem(LayoutItem *l)
 {
+    if (!l) {
+        return;
+    }
+
+    l->unsetManagingLayout(this);
     d->childList.removeAll(l);
     setGeometry(geometry());
 }

widgets/hboxlayout.cpp

@@ -66,12 +66,10 @@ void HBoxLayout::setGeometry(const QRectF& geometry)
     QSizeF available = geometry.size() - QSizeF(2 * margin(), 2 * margin());
 
     foreach (LayoutItem *l, children()) {
-        kDebug() << "testing layout item " << l << endl;
         if (l->expandingDirections() & Qt::Horizontal) {
-            expandingChildren += l;
+            expandingChildren.append(l);
         } else {
-
+            fixedChildren.append(l);
-            fixedChildren += l;
         }
     }
 
@@ -81,10 +79,12 @@ void HBoxLayout::setGeometry(const QRectF& geometry)
         available -= QSizeF(hint.width() + spacing(), 0.0f);
     }
 
-    qreal expandWidth = (available.width() - ((expandingChildren.count() - 1) * spacing())) / expandingChildren.count();
+    qreal expandWidth = 0;
+    if (expandingChildren.count() > 0) {
+        expandWidth = (available.height() - ((expandingChildren.count() - 1) * spacing())) / expandingChildren.count();
+    }
 
     foreach (LayoutItem *l, expandingChildren) {
-
         sizes.insert(indexOf(l), QSizeF(expandWidth, available.height()));
     }
 

widgets/layout.cpp

@@ -25,27 +25,35 @@ namespace Plasma
 
 class Layout::Private
 {
-	public:
+    public:
-		Private() : margin(10.0), spacing(10.0), parent(0) {}
+        Private(LayoutItem* parent)
-		~Private() {}
+            : margin(12.0),
+              spacing(6.0),
+              parent(parent)
+        {
+        }
 
-		qreal margin;
+        ~Private() {}
-		qreal spacing;
 
-		LayoutItem *parent;
+        qreal margin;
+        qreal spacing;
+
+        LayoutItem *parent;
 };
 
 
 Layout::Layout(LayoutItem *parent)
-	: LayoutItem(),
+    : LayoutItem(),
-	  d(new Private)
+      d(new Private(parent))
 {
-	d->parent = parent;
 }
 
 Layout::~Layout()
 {
-        delete d;
+    if (parent()) {
+        parent()->setLayout(0);
+    }
+    delete d;
 }
 
 qreal Layout::margin() const

widgets/layoutitem.cpp

@@ -17,6 +17,9 @@
  */
 
 #include "layoutitem.h"
+
+#include <KDebug>
+
 #include "layout.h"
 
 namespace Plasma
@@ -26,27 +29,30 @@ class LayoutItem::Private
 {
     public:
         Private()
-            : layout(0)
+            : layout(0),
+              managingLayout(0)
         {
         }
 
         ~Private() {}
 
         Layout* layout;
+        Layout* managingLayout;
 };
 
 
 LayoutItem::LayoutItem()
-    : d(new Private)
+    : d(new Private())
 {
 }
 
 LayoutItem::~LayoutItem()
 {
-    if (d->layout) {
+    if (d->managingLayout) {
-        d->layout->removeItem(this);
+        d->managingLayout->removeItem(this);
     }
 
+    delete d->layout;
     delete d;
 }
 
@@ -70,15 +76,11 @@ qreal LayoutItem::widthForHeight(qreal h) const
 	return 0.0;
 }
 
-void LayoutItem::resetLayout()
-{
-    d->layout = 0;
-}
-
 void LayoutItem::setLayout(Layout* layout)
 {
-    if (d->layout) {
+    if (d->layout && layout) {
-        d->layout->removeItem(this);
+        kDebug() << k_funcinfo << " already have a layout." << endl;
+        return;
     }
 
     d->layout = layout;
@@ -89,4 +91,25 @@ Layout* LayoutItem::layout()
     return d->layout;
 }
 
+void LayoutItem::setManagingLayout(Layout* layout)
+{
+    if (d->managingLayout) {
+        d->managingLayout->removeItem(this);
+    }
+
+    d->managingLayout = layout;
+}
+
+void LayoutItem::unsetManagingLayout(Layout* layout)
+{
+    if (d->managingLayout == layout) {
+        d->managingLayout = 0;
+    }
+}
+
+Layout* LayoutItem::managingLayout()
+{
+    return d->managingLayout;
+}
+
 }

widgets/layoutitem.h

@@ -44,7 +44,7 @@ class PLASMA_EXPORT LayoutItem
         /**
          * Constructor.
          */
-        LayoutItem();
+        explicit LayoutItem();
 
         /**
          * Virtual Destructor.
@@ -106,28 +106,37 @@ class PLASMA_EXPORT LayoutItem
         virtual QSizeF sizeHint() const = 0;
 
         /**
-         * Resets the layout to 0 and doesn't notify the previous layout.
+         * Sets the layout that will manage children items
-         * Should only be used by the current layout when relinquishing the item,
-         * e.g. during layout destruction.
-         */
-        void resetLayout();
-
-        /**
-         * Sets the layout so that the LayoutItem may inform the layout of its
-         * deletion. Should only be used by the layout it is added to.
-         *
-         * If the layout item is currently associated with another layout, it will
-         * first remove itself from that layout.
          *
          * @param layout The Layout that this LayoutItem will be managed by.
          */
         void setLayout(Layout* layout);
 
         /**
-         * Returns the layout this item is currently associated with.
+         * @return the layout this item is currently associated with.
          */
         Layout* layout();
 
+        /**
+         * Sets the layout that manages this item's geometry
+         *
+         * @param layout the layout that manage this item's geometry
+         **/
+        void setManagingLayout(Layout* layout);
+
+        /**
+         * Resets the layout that manges this item's geometry if it is the
+         * currently associated layout
+         *
+         * @param layout to unset
+         **/
+        void unsetManagingLayout(Layout* layout);
+
+        /**
+         * @return the layout that manages this item's geometry, or 0 if none
+         **/
+        Layout* managingLayout();
+
     private:
         class Private;
         Private *const d;

widgets/vboxlayout.cpp

@@ -55,10 +55,11 @@ void VBoxLayout::setGeometry(const QRectF& geometry)
 {
     if (!geometry.isValid() || geometry.isEmpty()) {
         kDebug() << "Invalid Geometry " << geometry << endl;
+        BoxLayout::setGeometry(geometry);
         return;
     }
 
-    kDebug() << this << " Geometry process " << geometry << " for " << children().count() << " childrens"<< endl;
+    kDebug() << this << " Geometry process " << geometry << " for " << children().count() << " children"<< endl;
 
     QList<LayoutItem *> fixedChildren;
     QList<LayoutItem *> expandingChildren;
@@ -68,9 +69,9 @@ void VBoxLayout::setGeometry(const QRectF& geometry)
     foreach (LayoutItem *l, children()) {
         kDebug() << "testing layout item " << l << endl;
         if (l->expandingDirections() & Qt::Vertical) {
-            expandingChildren += l;
+            expandingChildren.append(l);
         } else {
-            fixedChildren += l;
+            fixedChildren.append(l);
         }
     }
 
@@ -80,7 +81,11 @@ void VBoxLayout::setGeometry(const QRectF& geometry)
         available -= QSizeF(0.0, hint.height() + spacing());
     }
 
-    qreal expandHeight = (available.height() - ((expandingChildren.count() - 1) * spacing())) / expandingChildren.count();
+    qreal expandHeight = 0;
+
+    if (expandingChildren.count() > 0) {
+        expandHeight = (available.height() - ((expandingChildren.count() - 1) * spacing())) / expandingChildren.count();
+    }
 
     foreach (LayoutItem *l, expandingChildren) {
         sizes.insert(indexOf(l),QSizeF(available.width(), expandHeight));

widgets/widget.cpp

@@ -33,8 +33,7 @@ class Widget::Private
 {
     public:
         Private()
-            : parent(0),
+            : parent(0)
-              layout(0)
         { }
         ~Private() { }
 
@@ -43,7 +42,6 @@ class Widget::Private
         QSizeF maximumSize;
 
         Widget *parent;
-        Layout *layout;
         QList<Widget *> childList;
 };
 
@@ -139,9 +137,7 @@ void Widget::setGeometry(const QRectF& geometry)
 void Widget::updateGeometry()
 {
     if (layout()) {
-
         kDebug() << (void *) this << " updating geometry to " << size() << endl;
-
         layout()->setGeometry(geometry());
     }
 }
@@ -180,20 +176,6 @@ void Widget::resize(qreal w, qreal h)
     resize(QSizeF(w, h));
 }
 
-void Widget::setLayout(Layout *l)
-{
-    if(!d->layout) {
-        d->layout = l;
-    } else {
-        kDebug() << "Widget " << this << "already has a layout!" << endl;
-    }
-}
-
-Layout *Widget::layout() const
-{
-    return d->layout;
-}
-
 Widget *Widget::parent() const
 {
     return d->parent;

widgets/widget.h

@@ -161,16 +161,6 @@ class PLASMA_EXPORT Widget : public QGraphicsItem,
          */
         void resize(qreal w, qreal h);
 
-        /**
-         * Sets the Layout that will manage this Widget's childrens.
-         */
-        void setLayout(Layout *l);
-
-        /**
-         * Returns the Layout associated with this Widget.
-         */
-        Layout *layout() const;
-
         /**
          * Returns the parent of this Widget.
          */