QSGLoader shouldn't load component when active is false

Previously, QSGLoader still loaded the component specified, but didn't
instantiate the item. This commit ensures that no component is loaded
from the source, and that the onLoaded signal is emitted only when
loading occurs (when the loader is active).

Task-number: QTBUG-21710
Change-Id: I2d83603ef84d6942fb84141e9e146d2cf9654fc4
Reviewed-on: http://codereview.qt-project.org/5915
Reviewed-by: Qt Sanity Bot <qt_sanity_bot@ovi.com>
Reviewed-by: Michael Brasser <michael.brasser@nokia.com>

diff --git a/src/declarative/items/qsgloader.cpp b/src/declarative/items/qsgloader.cpp
index 0a0a3b9..91dce3b 100644 (file)
--- a/src/declarative/items/qsgloader.cpp
+++ b/src/declarative/items/qsgloader.cpp
@@ -53,8 +53,9 @@
 QT_BEGIN_NAMESPACE
 
 QSGLoaderPrivate::QSGLoaderPrivate()
-    : item(0), component(0), ownComponent(false), updatingSize(false),
-      itemWidthValid(false), itemHeightValid(false), active(true)
+    : item(0), component(0), updatingSize(false),
+      itemWidthValid(false), itemHeightValid(false),
+      active(true), loadingFromSource(false)
 {
 }
 
@@ -79,10 +80,9 @@ void QSGLoaderPrivate::clear()
 {
     disposeInitialPropertyValues();
 
-    if (ownComponent) {
+    if (loadingFromSource && component) {
         component->deleteLater();
         component = 0;
-        ownComponent = false;
     }
     source = QUrl();
 
@@ -340,11 +340,10 @@ void QSGLoader::loadFromSource()
         return;
     }
 
-    d->component = new QDeclarativeComponent(qmlEngine(this), d->source, this);
-    d->ownComponent = true;
-
-    if (isComponentComplete())
+    if (isComponentComplete()) {
+        d->component = new QDeclarativeComponent(qmlEngine(this), d->source, this);
         d->load();
+    }
 }
 
 /*!
@@ -384,7 +383,6 @@ void QSGLoader::setSourceComponent(QDeclarativeComponent *comp)
     d->clear();
 
     d->component = comp;
-    d->ownComponent = false;
     d->loadingFromSource = false;
 
     if (d->active)
@@ -520,7 +518,7 @@ void QSGLoaderPrivate::load()
                 q, SIGNAL(progressChanged()));
         emit q->statusChanged();
         emit q->progressChanged();
-        if (ownComponent)
+        if (loadingFromSource)
             emit q->sourceChanged();
         else
             emit q->sourceComponentChanged();
@@ -535,7 +533,7 @@ void QSGLoaderPrivate::_q_sourceLoaded()
     if (component) {
         if (!component->errors().isEmpty()) {
             QDeclarativeEnginePrivate::warning(qmlEngine(q), component->errors());
-            if (ownComponent)
+            if (loadingFromSource)
                 emit q->sourceChanged();
             else
                 emit q->sourceComponentChanged();
@@ -582,7 +580,7 @@ void QSGLoaderPrivate::_q_sourceLoaded()
             source = QUrl();
         }
         completeCreateWithInitialPropertyValues(component, obj, initialPropertyValues, qmlGlobalForIpv);
-        if (ownComponent)
+        if (loadingFromSource)
             emit q->sourceChanged();
         else
             emit q->sourceComponentChanged();
@@ -653,7 +651,12 @@ void QSGLoader::componentComplete()
 {
     Q_D(QSGLoader);
     QSGItem::componentComplete();
-    d->load();
+    if (active()) {
+        if (d->loadingFromSource) {
+            d->component = new QDeclarativeComponent(qmlEngine(this), d->source, this);
+        }
+        d->load();
+    }
 }
 
 /*!

diff --git a/src/declarative/items/qsgloader_p_p.h b/src/declarative/items/qsgloader_p_p.h
index 732ec86..13292ff 100644 (file)
--- a/src/declarative/items/qsgloader_p_p.h
+++ b/src/declarative/items/qsgloader_p_p.h
@@ -86,7 +86,6 @@ public:
     QDeclarativeComponent *component;
     v8::Persistent<v8::Object> initialPropertyValues;
     v8::Persistent<v8::Object> qmlGlobalForIpv;
-    bool ownComponent : 1;
     bool updatingSize: 1;
     bool itemWidthValid : 1;
     bool itemHeightValid : 1;

diff --git a/tests/auto/declarative/qsgloader/data/active.7.qml b/tests/auto/declarative/qsgloader/data/active.7.qml
new file mode 100644 (file)
index 0000000..a29e932
--- /dev/null
+++ b/tests/auto/declarative/qsgloader/data/active.7.qml
@@ -0,0 +1,14 @@
+import QtQuick 2.0
+Rectangle {
+    id: root
+    color: "blue"
+    width: 100; height: 100
+    property bool success: true
+
+    Loader {
+        active: false
+        anchors.fill: parent
+        sourceComponent: Rectangle { color: "red" }
+        onLoaded: { root.success = false; } // shouldn't be triggered if active is false
+    }
+}

diff --git a/tests/auto/declarative/qsgloader/data/active.8.qml b/tests/auto/declarative/qsgloader/data/active.8.qml
new file mode 100644 (file)
index 0000000..3a66d3e
--- /dev/null
+++ b/tests/auto/declarative/qsgloader/data/active.8.qml
@@ -0,0 +1,13 @@
+import QtQuick 2.0
+Rectangle {
+    id: root
+    color: "blue"
+    width: 100; height: 100
+    property bool success: false
+
+    Loader {
+        anchors.fill: parent
+        sourceComponent: Rectangle { color: "red" }
+        onLoaded: { root.success = true; } // should be triggered since active is true by default
+    }
+}

diff --git a/tests/auto/declarative/qsgloader/tst_qsgloader.cpp b/tests/auto/declarative/qsgloader/tst_qsgloader.cpp
index cd47925..ba8d5e0 100644 (file)
--- a/tests/auto/declarative/qsgloader/tst_qsgloader.cpp
+++ b/tests/auto/declarative/qsgloader/tst_qsgloader.cpp
@@ -141,9 +141,14 @@ void tst_QSGLoader::sourceOrComponent()
     QCOMPARE(static_cast<QSGItem*>(loader)->childItems().count(), error ? 0: 1);
 
     if (!error) {
-        QDeclarativeComponent *c = qobject_cast<QDeclarativeComponent*>(loader->children().at(0));
-        QVERIFY(c);
-        QCOMPARE(loader->sourceComponent(), c);
+        bool sourceComponentIsChildOfLoader = false;
+        for (int ii = 0; ii < loader->children().size(); ++ii) {
+            QDeclarativeComponent *c = qobject_cast<QDeclarativeComponent*>(loader->children().at(ii));
+            if (c && c == loader->sourceComponent()) {
+                sourceComponentIsChildOfLoader = true;
+            }
+        }
+        QVERIFY(sourceComponentIsChildOfLoader);
     }
 
     if (sourceOrComponent == "component") {
@@ -585,6 +590,24 @@ void tst_QSGLoader::active()
 
         delete object;
     }
+
+    // check that the component isn't loaded until active is set to true
+    {
+        QDeclarativeComponent component(&engine, TEST_FILE("active.7.qml"));
+        QObject *object = component.create();
+        QVERIFY(object != 0);
+        QCOMPARE(object->property("success").toBool(), true);
+        delete object;
+    }
+
+    // check that the component is loaded if active is not set (true by default)
+    {
+        QDeclarativeComponent component(&engine, TEST_FILE("active.8.qml"));
+        QObject *object = component.create();
+        QVERIFY(object != 0);
+        QCOMPARE(object->property("success").toBool(), true);
+        delete object;
+    }
 }
 
 void tst_QSGLoader::initialPropertyValues_data()