Fixed count(), itemAt() and removeAt() in QGraphicsLinearLayout.

Due to this, the behaviour of count(), itemAt() and removeAt() was
different between QGraphicsLinearLayout and QGraphicsGridLayout.
QGraphicsGridLayout does it right.

This adds some behaviour changes:
1. QGraphicsLinearLayout::count() is no longer affected by inserted
   stretches (through insertStretch)
  This means that code like this will break:

    QGraphicsLinearLayout *linearLayout = new QGraphicsLinearLayout;
    linearLayout->addItem(new QGraphicsWidget);
    linearLayout->addStretch();
    int count = linearLayout->count();
    linearLayout->removeAt(count - 1);
    // before this patch it would do nothing (and it wouldn't
    // actually remove the stretch), with the new patch it would
    // remove the QGraphicsWidget.

2. count(), itemAt() and removeAt() now prints a warning for an invalid
   index.
  The documentation actually says that "The reimplementation can assume
  that index is valid (i.e., it respects the value of count()", but I
  decided that its too risky to not assume that it is valid, since it
  would break the following common pattern:

    while(layout->itemAt(0)) {
      layout->removeAt(0);
    }

  Cleaned up autotests (and a small codeblock) that assumed
  itemAt/removeAt with an invalid index to work, since we should try to
  follow our own advice. :-)
  This change is also an alignment with what QGraphicsGridLayout does
  (it checks the index argument and prints a warning too.)

4 files changed, 45 insertions, 29 deletions

diff --git a/src/gui/graphicsview/qgraphicslayout_p.cpp b/src/gui/graphicsview/qgraphicslayout_p.cpp
index 6296207..83bf14b 100644
--- a/src/gui/graphicsview/qgraphicslayout_p.cpp
+++ b/src/gui/graphicsview/qgraphicslayout_p.cpp
@@ -120,8 +120,8 @@ static bool removeLayoutItemFromLayout(QGraphicsLayout *lay, QGraphicsLayoutItem
     if (!lay)
         return false;
 
-    QGraphicsLayoutItem *child;
-    for (int i = 0; (child = lay->itemAt(i)); ++i) {
+    for (int i = lay->count() - 1; i >= 0; --i) {
+        QGraphicsLayoutItem *child = lay->itemAt(i);
         if (child && child->isLayout()) {
             if (removeLayoutItemFromLayout(static_cast<QGraphicsLayout*>(child), layoutItem))
                 return true;
diff --git a/src/gui/graphicsview/qgraphicslinearlayout.cpp b/src/gui/graphicsview/qgraphicslinearlayout.cpp
index 2e78fc0..3b037cf 100644
--- a/src/gui/graphicsview/qgraphicslinearlayout.cpp
+++ b/src/gui/graphicsview/qgraphicslinearlayout.cpp
@@ -323,7 +323,11 @@ void QGraphicsLinearLayout::removeItem(QGraphicsLayoutItem *item)
 void QGraphicsLinearLayout::removeAt(int index)
 {
     Q_D(QGraphicsLinearLayout);
-    if (QGridLayoutItem *gridItem = d->engine.itemAt(d->gridRow(index), d->gridColumn(index))) {
+    if (index < 0 || index >= d->engine.itemCount()) {
+        qWarning("QGraphicsLinearLayout::removeAt: invalid index %d", index);
+        return;
+    }
+    if (QGridLayoutItem *gridItem = d->engine.itemAt(index)) {
         if (QGraphicsLayoutItem *layoutItem = gridItem->layoutItem())
             layoutItem->setParentLayoutItem(0);
         d->removeGridItem(gridItem);
@@ -463,7 +467,7 @@ QSizePolicy::ControlTypes QGraphicsLinearLayout::controlTypes(LayoutSide side) c
 int QGraphicsLinearLayout::count() const
 {
     Q_D(const QGraphicsLinearLayout);
-    return d->engine.rowCount(d->orientation);
+    return d->engine.itemCount();
 }
 
 /*!
@@ -472,8 +476,12 @@ int QGraphicsLinearLayout::count() const
 QGraphicsLayoutItem *QGraphicsLinearLayout::itemAt(int index) const
 {
     Q_D(const QGraphicsLinearLayout);
+    if (index < 0 || index >= d->engine.itemCount()) {
+        qWarning("QGraphicsLinearLayout::itemAt: invalid index %d", index);
+        return 0;
+    }
     QGraphicsLayoutItem *item = 0;
-    if (QGridLayoutItem *gridItem = d->engine.itemAt(d->gridRow(index), d->gridColumn(index)))
+    if (QGridLayoutItem *gridItem = d->engine.itemAt(index))
         item = gridItem->layoutItem();
     return item;
 }
diff --git a/tests/auto/qgraphicsgridlayout/tst_qgraphicsgridlayout.cpp b/tests/auto/qgraphicsgridlayout/tst_qgraphicsgridlayout.cpp
index ce4828a..2f70b62 100644
--- a/tests/auto/qgraphicsgridlayout/tst_qgraphicsgridlayout.cpp
+++ b/tests/auto/qgraphicsgridlayout/tst_qgraphicsgridlayout.cpp
@@ -153,8 +153,11 @@ void tst_QGraphicsGridLayout::qgraphicsgridlayout()
     layout.columnStretchFactor(0);
     layout.count();
     layout.horizontalSpacing();
+    QTest::ignoreMessage(QtWarningMsg, "QGraphicsGridLayout::itemAt: invalid row, column 0, 0");
     layout.itemAt(0, 0);
+    QTest::ignoreMessage(QtWarningMsg, "QGraphicsGridLayout::itemAt: invalid index 0");
     layout.itemAt(0);
+    QTest::ignoreMessage(QtWarningMsg, "QGraphicsGridLayout::removeAt: invalid index 0");
     layout.removeAt(0);
     layout.rowAlignment(0);
     layout.rowCount();
diff --git a/tests/auto/qgraphicslinearlayout/tst_qgraphicslinearlayout.cpp b/tests/auto/qgraphicslinearlayout/tst_qgraphicslinearlayout.cpp
index b61b2bb..c5ce1b9 100644
--- a/tests/auto/qgraphicslinearlayout/tst_qgraphicslinearlayout.cpp
+++ b/tests/auto/qgraphicslinearlayout/tst_qgraphicslinearlayout.cpp
@@ -210,17 +210,16 @@ void tst_QGraphicsLinearLayout::qgraphicslinearlayout()
     layout.setOrientation(Qt::Vertical);
     layout.orientation();
     QTest::ignoreMessage(QtWarningMsg, "QGraphicsLinearLayout::insertItem: cannot insert null item");
-    //QCOMPARE(layout.count(), 0);
+    QCOMPARE(layout.count(), 0);
     layout.addItem(0);
-    //QCOMPARE(layout.count(), 0);
+    QCOMPARE(layout.count(), 0);
     layout.addStretch(0);
-    //QCOMPARE(layout.count(), 1);
+    QCOMPARE(layout.count(), 0);
     QTest::ignoreMessage(QtWarningMsg, "QGraphicsLinearLayout::insertItem: cannot insert null item");
     layout.insertItem(0, 0);
     layout.insertStretch(0, 0);
     layout.removeItem(0);
-    layout.removeAt(0);
-    //QCOMPARE(layout.count(), 1);
+    QCOMPARE(layout.count(), 0);
     layout.setSpacing(0);
     layout.spacing();
     QTest::ignoreMessage(QtWarningMsg, "QGraphicsLinearLayout::setStretchFactor: cannot assign a stretch factor to a null item");
@@ -231,8 +230,7 @@ void tst_QGraphicsLinearLayout::qgraphicslinearlayout()
     QCOMPARE(layout.alignment(0), 0);
     layout.setGeometry(QRectF());
     layout.geometry();
-    //QCOMPARE(layout.count(), 1);
-    layout.itemAt(0);
+    QCOMPARE(layout.count(), 0);
     layout.invalidate();
     layout.sizeHint(Qt::MinimumSize, QSizeF());
 }
@@ -537,7 +535,10 @@ void tst_QGraphicsLinearLayout::insertItem()
     QSizeF oldSizeHint = layout.sizeHint(Qt::PreferredSize, QSizeF());
     layout.insertItem(insertItemAt, item);
     QCOMPARE(layout.count(), itemCount + layoutCount + 1);
-    QCOMPARE(layout.itemAt(insertItemAt), insertItemAt == -1 ? (QGraphicsLayoutItem*)0 : item);
+    
+    if (insertItemAt >= 0 && (itemCount + layoutCount >= 0)) {
+        QCOMPARE(layout.itemAt(itemCount + layoutCount), item);
+    }
 
     layout.activate();
     QSizeF newSizeHint = layout.sizeHint(Qt::PreferredSize, QSizeF());
@@ -599,8 +600,7 @@ void tst_QGraphicsLinearLayout::insertStretch()
     }
     widget->setLayout(layout);
     layout->insertStretch(insertItemAt, stretch);
-    QCOMPARE(layout->itemAt(insertItemAt), (QGraphicsLayoutItem*)0);
-    QCOMPARE(layout->count(), itemCount + layoutCount + 1);
+    QCOMPARE(layout->count(), itemCount + layoutCount);
 
     layout->activate();
     view.show();
@@ -669,7 +669,6 @@ void tst_QGraphicsLinearLayout::invalidate()
 void tst_QGraphicsLinearLayout::itemAt_data()
 {
     QTest::addColumn<int>("index");
-    QTest::newRow("-1") << -1;
     QTest::newRow("0") << 0;
     QTest::newRow("1") << 1;
     QTest::newRow("2") << 2;
@@ -681,7 +680,10 @@ void tst_QGraphicsLinearLayout::itemAt()
     // see also the insertItem() etc tests
     QFETCH(int, index);
     SubQGraphicsLinearLayout layout;
-    QCOMPARE(layout.itemAt(index), (QGraphicsLayoutItem*)0);
+    for (int i = 0; i < 3; ++i)
+        layout.addItem(new QGraphicsWidget);
+
+    QVERIFY(layout.itemAt(index) != 0);
 }
 
 void tst_QGraphicsLinearLayout::orientation_data()
@@ -779,19 +781,18 @@ void tst_QGraphicsLinearLayout::removeAt()
         layout.addItem(new SubQGraphicsLinearLayout);
     QSizeF oldSizeHint = layout.sizeHint(Qt::PreferredSize, QSizeF());
 
-    QGraphicsLayoutItem *w = layout.itemAt(removeItemAt);
-    QGraphicsLayoutItem *wParent = 0;
+    QGraphicsLayoutItem *w = 0;
+    if (removeItemAt >= 0 && removeItemAt < layout.count())
+        w = layout.itemAt(removeItemAt);
     if (w) {
-        wParent = w->parentLayoutItem();
+        QGraphicsLayoutItem *wParent = w->parentLayoutItem();
         QCOMPARE(wParent, static_cast<QGraphicsLayoutItem *>(&layout));
-    }
-    layout.removeAt(removeItemAt);
-    if (w) {
+        layout.removeAt(removeItemAt);
         wParent = w->parentLayoutItem();
         QCOMPARE(wParent, static_cast<QGraphicsLayoutItem *>(0));
+        delete w;
     }
-    delete w;
-    QCOMPARE(layout.count(), itemCount + layoutCount - ((removeItemAt == -1) ? 0 : 1));
+    QCOMPARE(layout.count(), itemCount + layoutCount - (w ? 1 : 0));
 
     layout.activate();
     QSizeF newSizeHint = layout.sizeHint(Qt::PreferredSize, QSizeF());
@@ -829,11 +830,15 @@ void tst_QGraphicsLinearLayout::removeItem()
     for (int i = 0; i < layoutCount; ++i)
         layout.addItem(new SubQGraphicsLinearLayout);
 
-    QGraphicsLayoutItem *w = layout.itemAt(removeItemAt);
+    QGraphicsLayoutItem *w = 0;
+    if (removeItemAt >= 0 && removeItemAt < layout.count())
+        w = layout.itemAt(removeItemAt);
     QSizeF oldSizeHint = layout.sizeHint(Qt::PreferredSize, QSizeF());
-    layout.removeItem(w);
-    delete w;
-    QCOMPARE(layout.count(), itemCount + layoutCount - ((removeItemAt == -1) ? 0 : 1));
+    if (w) {
+        layout.removeItem(w);
+        delete w;
+    }
+    QCOMPARE(layout.count(), itemCount + layoutCount - (w ? 1 : 0));
 
     layout.activate();
     QSizeF newSizeHint = layout.sizeHint(Qt::PreferredSize, QSizeF());