Encapsulate QQmlContextData

This class is not a private detail of QQmlContext. And it is incredibly
hard to see who owns what in there. Let's add some civilization ...

We enforce refcounting for QQmlContextData across the code base, with
two exceptions:

1. QQmlContextPrivate may or may not own its QQmlContextData.
2. We may request a QQmlContextData owned by its parent QQmlContextData.

For these two cases we keep flags in QQmlContextData and when the
respective field (m_parent or m_publicContext) is reset, we release()
once.

Furthermore, QQmlContextData and QQmlGuardedContextData are moved to
their own files, in order to de-spaghettify qqmlcontext_p.h and
qqmlcontext.cpp.

When the QQmlEngine is deleted, any QQmlComponents drop their object
creators now, in order to release any context data held by those.
Before, the context data would be deleted, but the object creators would
retain the dangling pointer.

[ChangeLog][QML][Important Behavior Changes] QQmlContext::baseUrl() does
what the documentation says now: It prefers explicitly set baseUrls over
compilation unit URLs. Only if no baseUrl is set, the CU's URL is
returned. It used to prefer the CU's URL.

Change-Id: Ieeb5dcb07b45d891526191321386d5443b8f5738
Reviewed-by: Fabian Kosmale <fabian.kosmale@qt.io>

7 files changed, 64 insertions, 59 deletions

diff --git a/tests/auto/qml/debugger/qqmlenginedebugservice/tst_qqmlenginedebugservice.cpp b/tests/auto/qml/debugger/qqmlenginedebugservice/tst_qqmlenginedebugservice.cpp
index 12befeb1ec..3ae40bf687 100644
--- a/tests/auto/qml/debugger/qqmlenginedebugservice/tst_qqmlenginedebugservice.cpp
+++ b/tests/auto/qml/debugger/qqmlenginedebugservice/tst_qqmlenginedebugservice.cpp
@@ -1395,17 +1395,14 @@ void tst_QQmlEngineDebugService::invalidContexts()
     QQmlContext context(m_engine);
     getContexts();
     QCOMPARE(m_dbg->rootContext().contexts.count(), base + 1);
-    QQmlContextData *contextData = QQmlContextData::get(&context);
+    QQmlRefPointer<QQmlContextData> contextData = QQmlContextData::get(&context);
     contextData->invalidate();
     getContexts();
     QCOMPARE(m_dbg->rootContext().contexts.count(), base);
-    QQmlContextData *rootData = QQmlContextData::get(m_engine->rootContext());
+    QQmlRefPointer<QQmlContextData> rootData = QQmlContextData::get(m_engine->rootContext());
     rootData->invalidate();
     getContexts();
     QCOMPARE(m_dbg->rootContext().contexts.count(), 0);
-    contextData->setParent(rootData); // makes context valid again, but not root.
-    getContexts();
-    QCOMPARE(m_dbg->rootContext().contexts.count(), 0);
 }
 
 void tst_QQmlEngineDebugService::createObjectOnDestruction()
diff --git a/tests/auto/qml/qqmlcomponent/tst_qqmlcomponent.cpp b/tests/auto/qml/qqmlcomponent/tst_qqmlcomponent.cpp
index 43cbd93396..79014a4450 100644
--- a/tests/auto/qml/qqmlcomponent/tst_qqmlcomponent.cpp
+++ b/tests/auto/qml/qqmlcomponent/tst_qqmlcomponent.cpp
@@ -35,7 +35,7 @@
 #include <QtQuick>
 #include <QtQuick/private/qquickrectangle_p.h>
 #include <QtQuick/private/qquickmousearea_p.h>
-#include <private/qqmlcontext_p.h>
+#include <private/qqmlguardedcontextdata_p.h>
 #include <private/qv4qmlcontext_p.h>
 #include <private/qv4scopedvalue_p.h>
 #include <private/qv4qmlcontext_p.h>
diff --git a/tests/auto/qml/qqmlcontext/tst_qqmlcontext.cpp b/tests/auto/qml/qqmlcontext/tst_qqmlcontext.cpp
index 6754f22049..7b62469003 100644
--- a/tests/auto/qml/qqmlcontext/tst_qqmlcontext.cpp
+++ b/tests/auto/qml/qqmlcontext/tst_qqmlcontext.cpp
@@ -34,6 +34,7 @@
 #include <QQmlComponent>
 #include <QQmlExpression>
 #include <private/qqmlcontext_p.h>
+#include <private/qqmlguardedcontextdata_p.h>
 #include <private/qv4qmlcontext_p.h>
 #include <private/qv4object_p.h>
 #include "../../shared/util.h"
@@ -737,7 +738,7 @@ void tst_qqmlcontext::qobjectDerived()
     // the engine
     QQmlContext context(engine.rootContext());
 
-    QObject *o1 = component.create(&context);
+    QScopedPointer<QObject> o1(component.create(&context));
     Q_UNUSED(o1);
 
     QCOMPARE(command.count, 2);
@@ -800,32 +801,33 @@ void tst_qqmlcontext::contextLeak()
 
         QQmlData *ddata = QQmlData::get(obj.data());
         QVERIFY(ddata);
-        QQmlContextData *context = ddata->context;
+        QQmlRefPointer<QQmlContextData> context = ddata->context;
         QVERIFY(context);
-        QVERIFY(!context->importedScripts.isNullOrUndefined());
-        QCOMPARE(int(context->importedScripts.valueRef()->as<QV4::Object>()->getLength()), 1);
-
-        {
-            QV4::Scope scope(ddata->jsWrapper.engine());
-            QV4::ScopedValue scriptContextWrapper(scope);
-            scriptContextWrapper = context->importedScripts.valueRef()->as<QV4::Object>()->get(uint(0));
-            scriptContext = scriptContextWrapper->as<QV4::QQmlContextWrapper>()->getContext();
-        }
+        QVERIFY(!context->importedScripts().isNullOrUndefined());
+        QCOMPARE(int(context->importedScripts().valueRef()->as<QV4::Object>()->getLength()), 1);
+
+        QV4::Scope scope(ddata->jsWrapper.engine());
+        QV4::ScopedValue scriptContextWrapper(scope);
+        scriptContextWrapper = context->importedScripts().valueRef()
+                ->as<QV4::Object>()->get(uint(0));
+        scriptContext = scriptContextWrapper->as<QV4::QQmlContextWrapper>()->getContext();
     }
 
     engine.collectGarbage();
 
     // Each time a JS file (non-pragma-shared) is imported, we create a QQmlContext(Data) for it.
     // Make sure that context does not leak.
-    QVERIFY(scriptContext.isNull());
+    // The QQmlGuardedContextData also holds a reference. Therefore, the refCount is still 1.
+    // All other references should be gone by now.
+    QCOMPARE(scriptContext->refCount(), 1);
 }
 
 
 static bool buildObjectList(QQmlContext *ctxt)
 {
     static QHash<QObject *, QString> deletedObjects;
-    QQmlContextData *p = QQmlContextData::get(ctxt);
-    QObject *object = p->contextObject;
+    QQmlRefPointer<QQmlContextData> p = QQmlContextData::get(ctxt);
+    QObject *object = p->contextObject();
     if (object) {
         // If the object was actually deleted this is likely to crash in one way or another.
         // Either the memory is still intact, then we will probably find the objectName, or it is
@@ -838,11 +840,11 @@ static bool buildObjectList(QQmlContext *ctxt)
         });
     }
 
-    QQmlContextData *child = p->childContexts;
+    QQmlRefPointer<QQmlContextData> child = p->childContexts();
     while (child) {
         if (!buildObjectList(child->asQQmlContext()))
             return false;
-        child = child->nextChild;
+        child = child->nextChild();
     }
 
     return true;
@@ -878,10 +880,14 @@ void tst_qqmlcontext::outerContextObject()
 void tst_qqmlcontext::contextObjectHierarchy()
 {
     QQmlEngine engine;
-    QQmlComponent component(&engine);
-    component.loadUrl(testFileUrl("contextObjectHierarchy.qml"));
-    QVERIFY(component.isReady());
-    QScopedPointer<QObject> root(component.create());
+    QScopedPointer<QObject> root;
+    {
+        // Drop the component after create(), to release the root context.
+        QQmlComponent component(&engine);
+        component.loadUrl(testFileUrl("contextObjectHierarchy.qml"));
+        QVERIFY(component.isReady());
+        root.reset(component.create());
+    }
     QVERIFY(!root.isNull());
 
     for (const QObject *child : root->children())
diff --git a/tests/auto/qml/qqmlecmascript/tst_qqmlecmascript.cpp b/tests/auto/qml/qqmlecmascript/tst_qqmlecmascript.cpp
index cfbc329198..b3e4296a13 100644
--- a/tests/auto/qml/qqmlecmascript/tst_qqmlecmascript.cpp
+++ b/tests/auto/qml/qqmlecmascript/tst_qqmlecmascript.cpp
@@ -386,7 +386,7 @@ private slots:
 
 private:
 //    static void propertyVarWeakRefCallback(v8::Persistent<v8::Value> object, void* parameter);
-    static void verifyContextLifetime(QQmlContextData *ctxt);
+    static void verifyContextLifetime(const QQmlRefPointer<QQmlContextData> &ctxt);
 
     // When calling into JavaScript, the specific type of the return value can differ if that return
     // value is a number. This is not only the case for non-integral numbers, or numbers that do not
@@ -4289,16 +4289,16 @@ void tst_qqmlecmascript::singletonTypeResolution()
     delete object;
 }
 
-void tst_qqmlecmascript::verifyContextLifetime(QQmlContextData *ctxt) {
-    QQmlContextData *childCtxt = ctxt->childContexts;
+void tst_qqmlecmascript::verifyContextLifetime(const QQmlRefPointer<QQmlContextData> &ctxt) {
+    QQmlRefPointer<QQmlContextData> childCtxt = ctxt->childContexts();
 
-    if (!ctxt->importedScripts.isNullOrUndefined()) {
-        QV4::ExecutionEngine *v4 = ctxt->engine->handle();
+    if (!ctxt->importedScripts().isNullOrUndefined()) {
+        QV4::ExecutionEngine *v4 = ctxt->engine()->handle();
         QV4::Scope scope(v4);
-        QV4::ScopedArrayObject scripts(scope, ctxt->importedScripts.value());
+        QV4::ScopedArrayObject scripts(scope, ctxt->importedScripts().value());
         QV4::Scoped<QV4::QQmlContextWrapper> qml(scope);
         for (quint32 i = 0; i < scripts->getLength(); ++i) {
-            QQmlContextData *scriptContext, *newContext;
+            QQmlRefPointer<QQmlContextData> scriptContext, newContext;
             qml = scripts->get(i);
 
             scriptContext = qml ? qml->getContext() : nullptr;
@@ -4310,17 +4310,16 @@ void tst_qqmlecmascript::verifyContextLifetime(QQmlContextData *ctxt) {
                 Q_UNUSED(temporaryScope)
             }
 
-            ctxt->engine->collectGarbage();
+            ctxt->engine()->collectGarbage();
             qml = scripts->get(i);
             newContext = qml ? qml->getContext() : nullptr;
-            QCOMPARE(scriptContext, newContext);
+            QCOMPARE(scriptContext.data(), newContext.data());
         }
     }
 
     while (childCtxt) {
         verifyContextLifetime(childCtxt);
-
-        childCtxt = childCtxt->nextChild;
+        childCtxt = childCtxt->nextChild();
     }
 }
 
@@ -4592,9 +4591,7 @@ void tst_qqmlecmascript::importScripts()
         QVERIFY(!object);
     } else {
         QVERIFY(object != nullptr);
-
-        QQmlContextData *ctxt = QQmlContextData::get(engine.rootContext());
-        tst_qqmlecmascript::verifyContextLifetime(ctxt);
+        tst_qqmlecmascript::verifyContextLifetime(QQmlContextData::get(engine.rootContext()));
 
         for (int i = 0; i < propertyNames.size(); ++i)
             QCOMPARE(object->property(propertyNames.at(i).toLatin1().constData()), propertyValues.at(i));
@@ -6138,24 +6135,22 @@ void tst_qqmlecmascript::nullObjectInitializer()
 // Test that bindings don't evaluate once the engine has been destroyed
 void tst_qqmlecmascript::deletedEngine()
 {
-    QQmlEngine *engine = new QQmlEngine;
-    QQmlComponent component(engine, testFileUrl("deletedEngine.qml"));
+    QScopedPointer<QQmlEngine> engine(new QQmlEngine);
+    QQmlComponent component(engine.data(), testFileUrl("deletedEngine.qml"));
+    QScopedPointer<QObject> object(component.create());
 
-    QObject *object = component.create();
     QVERIFY(object != nullptr);
 
     QCOMPARE(object->property("a").toInt(), 39);
     object->setProperty("b", QVariant(9));
     QCOMPARE(object->property("a").toInt(), 117);
 
-    delete engine;
+    engine.reset(); // This drops the engine's context hierarchy and the object gets notified.
 
     QCOMPARE(object->property("a").toInt(), 0);
     object->setProperty("b", QVariant(10));
     object->setProperty("b", QVariant());
     QCOMPARE(object->property("a").toInt(), 0);
-
-    delete object;
 }
 
 // Test the crashing part of QTBUG-9705
@@ -6217,25 +6212,30 @@ void tst_qqmlecmascript::variants()
 void tst_qqmlecmascript::qtbug_9792()
 {
     QQmlEngine engine;
-    QQmlComponent component(&engine, testFileUrl("qtbug_9792.qml"));
 
-    QQmlContext *context = new QQmlContext(engine.rootContext());
+    QScopedPointer<QQmlContext> context(new QQmlContext(engine.rootContext()));
+    QScopedPointer<QObject> object;
 
-    MyQmlObject *object = qobject_cast<MyQmlObject*>(component.create(context));
-    QVERIFY(object != nullptr);
+    MyQmlObject *myQmlObject = nullptr;
+    {
+        // Drop the component after creation as that holds a strong reference
+        // to the root QQmlContextData. We want the context data to be destroyed.
+        QQmlComponent component(&engine, testFileUrl("qtbug_9792.qml"));
+        object.reset(component.create(context.data()));
+        myQmlObject = qobject_cast<MyQmlObject*>(object.data());
+    }
+    QVERIFY(myQmlObject != nullptr);
 
     QTest::ignoreMessage(QtDebugMsg, "Hello world!");
-    object->basicSignal();
+    myQmlObject->basicSignal();
 
-    delete context;
+    context.reset();
 
     QQmlTestMessageHandler messageHandler;
 
-    object->basicSignal();
+    myQmlObject->basicSignal();
 
     QVERIFY2(messageHandler.messages().isEmpty(), qPrintable(messageHandler.messageString()));
-
-    delete object;
 }
 
 // Verifies that QPointer<>s used in the vmemetaobject are cleaned correctly
diff --git a/tests/auto/qml/qqmllanguage/testtypes.cpp b/tests/auto/qml/qqmllanguage/testtypes.cpp
index 31a4135d89..2c0e5f1d8c 100644
--- a/tests/auto/qml/qqmllanguage/testtypes.cpp
+++ b/tests/auto/qml/qqmllanguage/testtypes.cpp
@@ -149,7 +149,7 @@ void CustomBinding::componentComplete()
 
         int bindingId = binding->value.compiledScriptIndex;
 
-        QQmlContextData *context = QQmlContextData::get(qmlContext(this));
+        QQmlRefPointer<QQmlContextData> context = QQmlContextData::get(qmlContext(this));
 
         QQmlProperty property(m_target, name, qmlContext(this));
         QV4::Scope scope(qmlEngine(this)->handle());
diff --git a/tests/auto/qml/qqmllanguage/tst_qqmllanguage.cpp b/tests/auto/qml/qqmllanguage/tst_qqmllanguage.cpp
index 6d932c2665..f2fa301565 100644
--- a/tests/auto/qml/qqmllanguage/tst_qqmllanguage.cpp
+++ b/tests/auto/qml/qqmllanguage/tst_qqmllanguage.cpp
@@ -4871,8 +4871,9 @@ static void beginDeferredOnce(QQmlEnginePrivate *enginePriv,
         QQmlComponentPrivate::ConstructionState *state = new QQmlComponentPrivate::ConstructionState;
         state->completePending = true;
 
-        QQmlContextData *creationContext = nullptr;
-        state->creator.reset(new QQmlObjectCreator(deferData->context->parent, deferData->compilationUnit, creationContext));
+        state->creator.reset(new QQmlObjectCreator(
+                                 deferData->context->parent(), deferData->compilationUnit,
+                                 QQmlRefPointer<QQmlContextData>()));
 
         enginePriv->inProgressCreations++;
 
@@ -4904,7 +4905,7 @@ static void testExecuteDeferredOnce(const QQmlProperty &property)
     QObject *object = property.object();
     QQmlData *data = QQmlData::get(object);
     if (data && !data->deferredData.isEmpty() && !data->wasDeleted(object)) {
-        QQmlEnginePrivate *ep = QQmlEnginePrivate::get(data->context->engine);
+        QQmlEnginePrivate *ep = QQmlEnginePrivate::get(data->context->engine());
 
         QQmlComponentPrivate::DeferredState state;
         beginDeferredOnce(ep, property, &state);
diff --git a/tests/auto/qml/qqmlpropertycache/tst_qqmlpropertycache.cpp b/tests/auto/qml/qqmlpropertycache/tst_qqmlpropertycache.cpp
index 2e040eec18..956759668e 100644
--- a/tests/auto/qml/qqmlpropertycache/tst_qqmlpropertycache.cpp
+++ b/tests/auto/qml/qqmlpropertycache/tst_qqmlpropertycache.cpp
@@ -32,6 +32,7 @@
 #include <QtQml/qqmlcontext.h>
 #include <QtQml/qqmlcomponent.h>
 #include <private/qmetaobjectbuilder_p.h>
+#include <private/qqmlcontextdata_p.h>
 #include <QCryptographicHash>
 #include "../../shared/util.h"