QQmlImport: Clean up & rename getTypeForUrl to fetchOrCreateTypeForUrl

The new name better matches what the method does. At the same time, clean it up
a little: make the flow a little clearer, extrapolate on some comments, remove
some old debug. Also add a small method doc explaining this method does.

Change-Id: I494efc2dfe8760734f228a9af440abf7067354b7
Reviewed-by: Simon Hausmann <simon.hausmann@qt.io>

1 files changed, 66 insertions, 34 deletions

diff --git a/src/qml/qml/qqmlimport.cpp b/src/qml/qml/qqmlimport.cpp
index 8712b638c5..659c7f1b9f 100644
--- a/src/qml/qml/qqmlimport.cpp
+++ b/src/qml/qml/qqmlimport.cpp
@@ -130,42 +130,74 @@ bool isPathAbsolute(const QString &path)
 #endif
 }
 
-// If the type does not already exist as a file import, add the type and return the new type
-QQmlType *getTypeForUrl(const QString &urlString, const QHashedStringRef& typeName,
+/*
+    \internal
+
+    Fetches the QQmlType instance registered for \a urlString, creating a
+    registration for it if it is not already registered, using the associated
+    \a typeName, \a isCompositeSingleton, \a majorVersion and \a minorVersion
+    details.
+
+    Errors (if there are any) are placed into \a errors, if it is nonzero. Note
+    that errors are treated as fatal if \a errors is not set.
+*/
+QQmlType *fetchOrCreateTypeForUrl(const QString &urlString, const QHashedStringRef& typeName,
                         bool isCompositeSingleton, QList<QQmlError> *errors,
                         int majorVersion=-1, int minorVersion=-1)
 {
-    QUrl url(urlString);
+    QUrl url(urlString); // ### unfortunate (costly) conversion
     QQmlType *ret = QQmlMetaType::qmlType(url);
-    if (!ret) { //QQmlType not yet existing for composite or composite singleton type
-        int dot = typeName.indexOf(QLatin1Char('.'));
-        QHashedStringRef unqualifiedtype = dot < 0 ? typeName : QHashedStringRef(typeName.constData() + dot + 1, typeName.length() - dot - 1);
-
-        //XXX: The constData of the string ref is pointing somewhere unsafe in qmlregister, so we need to create a temporary copy
-        QByteArray buf(unqualifiedtype.toString().toUtf8());
-
-        if (isCompositeSingleton) {
-            QQmlPrivate::RegisterCompositeSingletonType reg = {
-                url,
-                "", //Empty URI indicates loaded via file imports
-                majorVersion,
-                minorVersion,
-                buf.constData()
-            };
-            ret = QQmlMetaType::qmlTypeFromIndex(QQmlPrivate::qmlregister(QQmlPrivate::CompositeSingletonRegistration, &reg));
-        } else {
-            QQmlPrivate::RegisterCompositeType reg = {
-                url,
-                "", //Empty URI indicates loaded via file imports
-                majorVersion,
-                minorVersion,
-                buf.constData()
-            };
-            ret = QQmlMetaType::qmlTypeFromIndex(QQmlPrivate::qmlregister(QQmlPrivate::CompositeRegistration, &reg));
-        }
+    if (ret)
+        return ret;
+
+    int dot = typeName.indexOf(QLatin1Char('.'));
+    QHashedStringRef unqualifiedtype = dot < 0 ? typeName : QHashedStringRef(typeName.constData() + dot + 1, typeName.length() - dot - 1);
+
+    // We need a pointer, but we were passed a string. Take a copy so we
+    // can guarentee it will live long enough to reach qmlregister.
+    QByteArray buf(unqualifiedtype.toString().toUtf8());
+
+    // Register the type. Note that the URI parameters here are empty; for
+    // file type imports, we do not place them in a URI as we don't
+    // necessarily have a good and unique one (picture a library import,
+    // which may be found in multiple plugin locations on disk), but there
+    // are other reasons for this too.
+    //
+    // By not putting them in a URI, we prevent the types from being
+    // registered on a QQmlTypeModule; this is important, as once types are
+    // placed on there, they cannot be easily removed, meaning if the
+    // developer subsequently loads a different import (meaning different
+    // types) with the same URI (using, say, a different plugin path), it is
+    // very undesirable that we continue to associate the types from the
+    // "old" URI with that new module.
+    //
+    // Not having URIs also means that the types cannot be found by name
+    // etc, the only way to look them up is through QQmlImports -- for
+    // better or worse.
+    if (isCompositeSingleton) {
+        QQmlPrivate::RegisterCompositeSingletonType reg = {
+            url,
+            "", // uri
+            majorVersion,
+            minorVersion,
+            buf.constData()
+        };
+        ret = QQmlMetaType::qmlTypeFromIndex(QQmlPrivate::qmlregister(QQmlPrivate::CompositeSingletonRegistration, &reg));
+    } else {
+        QQmlPrivate::RegisterCompositeType reg = {
+            url,
+            "", // uri
+            majorVersion,
+            minorVersion,
+            buf.constData()
+        };
+        ret = QQmlMetaType::qmlTypeFromIndex(QQmlPrivate::qmlregister(QQmlPrivate::CompositeRegistration, &reg));
     }
-    if (!ret) {//Usually when a type name is "found" but invalid
-        //qDebug() << ret << urlString << QQmlMetaType::qmlType(url);
+
+    // This means that the type couldn't be found by URL, but could not be
+    // registered either, meaning we most likely were passed some kind of bad
+    // data.
+    if (!ret) {
         if (!errors) // Cannot list errors properly, just quit
             qFatal("%s", QQmlMetaType::typeRegistrationFailures().join('\n').toLatin1().constData());
         QQmlError error;
@@ -704,7 +736,7 @@ bool QQmlImportNamespace::Import::resolveType(QQmlTypeLoader *typeLoader,
         if (candidate != end) {
             int major = vmajor ? *vmajor : -1;
             int minor = vminor ? *vminor : -1;
-            QQmlType *returnType = getTypeForUrl(componentUrl, type, isCompositeSingleton, 0,
+            QQmlType *returnType = fetchOrCreateTypeForUrl(componentUrl, type, isCompositeSingleton, 0,
                                                  major, minor);
             if (type_return)
                 *type_return = returnType;
@@ -732,7 +764,7 @@ bool QQmlImportNamespace::Import::resolveType(QQmlTypeLoader *typeLoader,
                 if (typeRecursionDetected)
                     *typeRecursionDetected = true;
             } else {
-                QQmlType *returnType = getTypeForUrl(qmlUrl, type, false, 0);
+                QQmlType *returnType = fetchOrCreateTypeForUrl(qmlUrl, type, false, 0);
                 if (type_return)
                     *type_return = returnType;
                 return returnType != 0;
@@ -777,7 +809,7 @@ bool QQmlImportsPrivate::resolveType(const QHashedStringRef& type, int *vmajor,
             return true;
         if (s->imports.count() == 1 && !s->imports.at(0)->isLibrary && type_return && s != &unqualifiedset) {
             // qualified, and only 1 url
-            *type_return = getTypeForUrl(resolveLocalUrl(s->imports.at(0)->url, unqualifiedtype.toString() + QLatin1String(".qml")), type, false, errors);
+            *type_return = fetchOrCreateTypeForUrl(resolveLocalUrl(s->imports.at(0)->url, unqualifiedtype.toString() + QLatin1String(".qml")), type, false, errors);
             return (*type_return != 0);
         }
     }