Get rid of QQuickTextDocumentWithImageResources

Users want to be able to provide their own QTextDocument instances
to TextEdit; but we had been creating a subclass called
QQuickTextDocumentWithImageResources, which was an obstacle for that.

QTextDocumentPrivate has two QMaps to hold resources, but
QQuickTextDocumentWithImageResources existed for the purpose of caching
remote resources, which only QQuickPixmap knows how to fetch. (This
design was apparently invented as a workaround to the lack of
virtual-filesystem functionality in Qt Core. If QtCore already knew how
to fetch web resources from URLs in the background, QTextDocument could
use it to do its own fetching of remote resources.) What we want instead
of subclassing QTextDocument is to keep doing the fetching in Qt Quick
(because there's no other choice for now), but get the images into the
same QTextDocumentPrivate::cachedResources map where local-file
resources are cached.

As it turns out, since qtbase ac300a166f801a6f6c0b15278e6893720a5726f8
QTextDocument::loadResource() can use QMetaMethod::invoke() to call a
method with the signature QVariant loadResource(int,QUrl) if such a
method is found in QTD's parent object; so since QQuickTextEdit creates
its own document by default, and is the document's parent, we can keep
remote resource fetching functionality working by 1) providing the
QQuickTextEdit::loadResource() method to be invoked, and 2) moving the
document to the QML thread so that it can be invoked directly.
(QMetaMethod::invoke() doesn't work across a queued connection, because
we need to return a value: the QVariant.) QTD will already cache the
images as soon as the call to loadResource() returns a valid QVariant.
We ask for the QQuickPixmap not to be cached by passing an empty
QQuickPixmap::Option enum to its ctor, which gets passed through
to the load() function.

When we consider fetching resources from a web server, it's unfortunate
that the signature of QTextDocument::resource() sets the expectation
that resources can be loaded immediately. But as long as
QQuickTextEdit::loadResource() is waiting for fetching to be done, it
keeps returning a default-constructed QVariant, which won't be cached;
and it will be called again later, repeatedly, until it eventually
succeeds. To ensure that it is called again when fetching is done, we
call QTextDocument::resource() again, to provoke it to call our
QQuickTextEdit::loadResource() one last time.  If the returned image
wasn't cached before, it will be after that. Then it's ok to delete the
QQuickPixmap, and invalidate the TextEdit so that layout will be updated
to include the image, now that we have it.

But most of the time it's relatively boring: QTextDocument knows
how to load local files on its own, and caches them.

So we no longer need QQuickTextDocumentWithImageResources. But we
still needed to replace QTextImageHandler with a custom implementation,
as explained in bab2eaf3da299c471dd898c89cf356984b077412: we need
QQuickPixmap for its QML-specific URL resolution relative to the Text
item's context. And it turns out that in case the document contains
only a missing image, the minimum size is 16x16 pixels, to reserve
space to display a "broken image" icon as browsers sometimes do...
we have never actually done that in Qt Quick AFAICT, but autotests have
been enforcing the 16x16 minimum size all along. This is done in
QQuickTextImageHandler::intrinsicSize() now.

The same approach is taken with Text (when textFormat is RichText or
MarkdownText. In case of StyledText, there is no QTextDocument instance,
and resource loading is taken care of entirely within QQuickText.)

For the autotests, friendly use of QQuickPixmapCache::m_cache requires
QT_BEGIN_NAMESPACE.

Task-number: QTBUG-35688
Change-Id: I8ad8142b3b3790254dd56d6cfe5209d641465f08
Reviewed-by: Oliver Eftevaag <[email protected]>
Reviewed-by: Axel Spoerl <[email protected]>
Reviewed-by: Fabian Kosmale <[email protected]>

4 files changed, 50 insertions, 58 deletions

diff --git a/tests/auto/quick/qquicktext/tst_qquicktext.cpp b/tests/auto/quick/qquicktext/tst_qquicktext.cpp
index c31b2c3861..218d1d47ac 100644
--- a/tests/auto/quick/qquicktext/tst_qquicktext.cpp
+++ b/tests/auto/quick/qquicktext/tst_qquicktext.cpp
@@ -9,10 +9,10 @@
 #include <QtQuick/private/qquicktext_p.h>
 #include <QtQuick/private/qquickflickable_p.h>
 #include <QtQuick/private/qquickmousearea_p.h>
+#include <QtQuick/private/qquickpixmapcache_p.h>
 #include <QtQuickTest/QtQuickTest>
 #include <private/qquicktext_p_p.h>
 #include <private/qsginternaltextnode_p.h>
-#include <private/qquicktextdocument_p.h>
 #include <private/qquickvaluetypes_p.h>
 #include <QFontMetrics>
 #include <qmath.h>
@@ -29,12 +29,17 @@ DEFINE_BOOL_CONFIG_OPTION(qmlDisableDistanceField, QML_DISABLE_DISTANCEFIELD)
 
 Q_DECLARE_METATYPE(QQuickText::TextFormat)
 
-QT_BEGIN_NAMESPACE
-extern void qt_setQtEnableTestFont(bool value);
-QT_END_NAMESPACE
+typedef QVector<QPointF> PointVector;
+Q_DECLARE_METATYPE(PointVector);
+
+typedef qreal (*ExpectedBaseline)(QQuickText *item);
+Q_DECLARE_METATYPE(ExpectedBaseline)
 
 Q_LOGGING_CATEGORY(lcTests, "qt.quick.tests")
 
+QT_BEGIN_NAMESPACE
+extern void qt_setQtEnableTestFont(bool value);
+
 class tst_qquicktext : public QQmlDataTest
 {
     Q_OBJECT
@@ -1803,10 +1808,6 @@ public:
     QTextLayout layout;
 };
 
-
-typedef QVector<QPointF> PointVector;
-Q_DECLARE_METATYPE(PointVector);
-
 void tst_qquicktext::linkInteraction_data()
 {
     QTest::addColumn<QString>("text");
@@ -2181,13 +2182,10 @@ void tst_qquicktext::embeddedImages()
     if (!error.isEmpty())
         QTest::ignoreMessage(QtWarningMsg, error.toLatin1());
 
-    QScopedPointer<QQuickView> view(new QQuickView);
-    view->rootContext()->setContextProperty(QStringLiteral("serverBaseUrl"), server.baseUrl());
-    view->setSource(qmlfile);
-    view->show();
-    view->requestActivate();
-    QVERIFY(QTest::qWaitForWindowActive(view.get()));
-    QQuickText *textObject = qobject_cast<QQuickText*>(view->rootObject());
+    QQuickView view;
+    view.rootContext()->setContextProperty(QStringLiteral("serverBaseUrl"), server.baseUrl());
+    QVERIFY(QQuickTest::showView(view, qmlfile));
+    QQuickText *textObject = qobject_cast<QQuickText*>(view.rootObject());
 
     QVERIFY(textObject != nullptr);
     QTRY_COMPARE(textObject->resourcesLoading(), 0);
@@ -2201,6 +2199,10 @@ void tst_qquicktext::embeddedImages()
         QCOMPARE(textObject->width(), 16.0); // default size of QTextDocument broken image icon
         QCOMPARE(textObject->height(), 16.0);
     }
+
+    // QTextDocument images are cached in QTextDocumentPrivate::cachedResources,
+    // so verify that we don't redundantly cache them in QQuickPixmapCache
+    QCOMPARE(QQuickPixmapCache::instance()->m_cache.size(), 0);
 }
 
 void tst_qquicktext::lineCount()
@@ -4048,9 +4050,6 @@ void tst_qquicktext::fontFormatSizes()
     }
 }
 
-typedef qreal (*ExpectedBaseline)(QQuickText *item);
-Q_DECLARE_METATYPE(ExpectedBaseline)
-
 static qreal expectedBaselineTop(QQuickText *item)
 {
     QFontMetricsF fm(item->font());
@@ -4823,6 +4822,8 @@ void tst_qquicktext::displaySuperscriptedTag()
     QCOMPARE(color.green(), 255);
 }
 
+QT_END_NAMESPACE
+
 QTEST_MAIN(tst_qquicktext)
 
 #include "tst_qquicktext.moc"
diff --git a/tests/auto/quick/qquicktextdocument/tst_qquicktextdocument.cpp b/tests/auto/quick/qquicktextdocument/tst_qquicktextdocument.cpp
index 63d4f24a54..e6fb32acf6 100644
--- a/tests/auto/quick/qquicktextdocument/tst_qquicktextdocument.cpp
+++ b/tests/auto/quick/qquicktextdocument/tst_qquicktextdocument.cpp
@@ -6,7 +6,6 @@
 #include <QtQuick/QQuickTextDocument>
 #include <QtQuick/QQuickItem>
 #include <QtQuick/private/qquicktextedit_p.h>
-#include <QtQuick/private/qquicktextdocument_p.h>
 #include <QtGui/QTextDocument>
 #include <QtGui/QTextDocumentWriter>
 #include <QtQml/QQmlEngine>
@@ -21,7 +20,6 @@ public:
 
 private slots:
     void textDocumentWriter();
-    void textDocumentWithImage();
 };
 
 QString text = QStringLiteral("foo bar");
@@ -54,20 +52,6 @@ void tst_qquicktextdocument::textDocumentWriter()
     delete o;
 }
 
-void tst_qquicktextdocument::textDocumentWithImage()
-{
-    QQuickTextDocumentWithImageResources document(nullptr);
-    QImage image(1, 1, QImage::Format_Mono);
-    image.fill(1);
-
-    QString name = "image";
-    document.addResource(QTextDocument::ImageResource, name, image);
-    QTextImageFormat format;
-    format.setName(name);
-    QCOMPARE(image, document.image(format));
-    QCOMPARE(image, document.resource(QTextDocument::ImageResource, name).value<QImage>());
-}
-
 QTEST_MAIN(tst_qquicktextdocument)
 
 #include "tst_qquicktextdocument.moc"
diff --git a/tests/auto/quick/qquicktextedit/data/embeddedImagesRemote.qml b/tests/auto/quick/qquicktextedit/data/embeddedImagesRemote.qml
index 6fc12edf35..a80e669140 100644
--- a/tests/auto/quick/qquicktextedit/data/embeddedImagesRemote.qml
+++ b/tests/auto/quick/qquicktextedit/data/embeddedImagesRemote.qml
@@ -1,4 +1,4 @@
-import QtQuick 2.0
+import QtQuick
 
 TextEdit {
     property string serverBaseUrl;
diff --git a/tests/auto/quick/qquicktextedit/tst_qquicktextedit.cpp b/tests/auto/quick/qquicktextedit/tst_qquicktextedit.cpp
index bbb17e23d5..5ad49ffd1b 100644
--- a/tests/auto/quick/qquicktextedit/tst_qquicktextedit.cpp
+++ b/tests/auto/quick/qquicktextedit/tst_qquicktextedit.cpp
@@ -18,7 +18,7 @@
 #include <private/qquicktextedit_p.h>
 #include <private/qquicktextedit_p_p.h>
 #include <private/qquicktext_p.h>
-#include <private/qquicktextdocument_p.h>
+#include <QtQuick/private/qquickpixmapcache_p.h>
 #include <QFontMetrics>
 #include <QtQuick/QQuickView>
 #include <QDir>
@@ -40,6 +40,21 @@ Q_DECLARE_METATYPE(QQuickTextEdit::SelectionMode)
 Q_DECLARE_METATYPE(Qt::Key)
 DEFINE_BOOL_CONFIG_OPTION(qmlDisableDistanceField, QML_DISABLE_DISTANCEFIELD)
 
+typedef QList<int> IntList;
+Q_DECLARE_METATYPE(IntList)
+
+typedef QPair<int, QChar> Key;
+typedef QList<Key> KeyList;
+Q_DECLARE_METATYPE(KeyList)
+
+Q_DECLARE_METATYPE(QQuickTextEdit::HAlignment)
+Q_DECLARE_METATYPE(QQuickTextEdit::VAlignment)
+Q_DECLARE_METATYPE(QQuickTextEdit::TextFormat)
+
+#if QT_CONFIG(shortcut)
+Q_DECLARE_METATYPE(QKeySequence::StandardKey)
+#endif
+
 Q_LOGGING_CATEGORY(lcTests, "qt.quick.tests")
 
 static bool isPlatformWayland()
@@ -47,7 +62,7 @@ static bool isPlatformWayland()
     return !QGuiApplication::platformName().compare(QLatin1String("wayland"), Qt::CaseInsensitive);
 }
 
-typedef QPair<int, QChar> Key;
+QT_BEGIN_NAMESPACE
 
 class tst_qquicktextedit : public QQmlDataTest
 
@@ -237,16 +252,6 @@ private:
     QPointingDevice *touchDevice = QTest::createTouchDevice();
 };
 
-typedef QList<int> IntList;
-Q_DECLARE_METATYPE(IntList)
-
-typedef QList<Key> KeyList;
-Q_DECLARE_METATYPE(KeyList)
-
-Q_DECLARE_METATYPE(QQuickTextEdit::HAlignment)
-Q_DECLARE_METATYPE(QQuickTextEdit::VAlignment)
-Q_DECLARE_METATYPE(QQuickTextEdit::TextFormat)
-
 void tst_qquicktextedit::simulateKeys(QWindow *window, const QList<Key> &keys)
 {
     for (int i = 0; i < keys.size(); ++i) {
@@ -2375,8 +2380,6 @@ void tst_qquicktextedit::selectByKeyboard()
 
 #if QT_CONFIG(shortcut)
 
-Q_DECLARE_METATYPE(QKeySequence::StandardKey)
-
 void tst_qquicktextedit::keyboardSelection_data()
 {
     QTest::addColumn<QString>("text");
@@ -6053,30 +6056,32 @@ void tst_qquicktextedit::embeddedImages()
         QTest::ignoreMessage(QtWarningMsg, error.toLatin1());
 
     QQmlComponent textComponent(&engine, qmlfile);
-    QQuickTextEdit *textObject = qobject_cast<QQuickTextEdit*>(textComponent.beginCreate(engine.rootContext()));
-    QVERIFY(textObject != nullptr);
+    QScopedPointer<QQuickTextEdit> textObject(qobject_cast<QQuickTextEdit*>(textComponent.beginCreate(engine.rootContext())));
+    QVERIFY(!textObject.isNull());
 
     const int baseUrlPropertyIndex = textObject->metaObject()->indexOfProperty("serverBaseUrl");
     if (baseUrlPropertyIndex != -1) {
         QMetaProperty prop = textObject->metaObject()->property(baseUrlPropertyIndex);
-        QVERIFY(prop.write(textObject, server.baseUrl().toString()));
+        QVERIFY(prop.write(textObject.get(), server.baseUrl().toString()));
     }
 
     textComponent.completeCreate();
 
-    QTRY_COMPARE(QQuickTextEditPrivate::get(textObject)->document->resourcesLoading(), 0);
+    QTRY_COMPARE(textObject->resourcesLoading(), 0);
 
     QPixmap pm(testFile("http/exists.png"));
     if (error.isEmpty()) {
-        QCOMPARE(textObject->width(), double(pm.width()));
-        QCOMPARE(textObject->height(), double(pm.height()));
+        QCOMPARE(textObject->width(), pm.width());
+        QCOMPARE(textObject->height(), pm.height());
     } else {
         QVERIFY(16 != pm.width()); // check test is effective
-        QCOMPARE(textObject->width(), 16.0); // default size of QTextDocument broken image icon
-        QCOMPARE(textObject->height(), 16.0);
+        QCOMPARE(textObject->width(), 16); // default size of QTextDocument broken image icon
+        QCOMPARE(textObject->height(), 16);
     }
 
-    delete textObject;
+    // QTextDocument images are cached in QTextDocumentPrivate::cachedResources,
+    // so verify that we don't redundantly cache them in QQuickPixmapCache
+    QCOMPARE(QQuickPixmapCache::instance()->m_cache.size(), 0);
 }
 
 void tst_qquicktextedit::emptytags_QTBUG_22058()
@@ -6591,6 +6596,8 @@ void tst_qquicktextedit::rtlAlignmentInColumnLayout_QTBUG_112858()
     }
 }
 
+QT_END_NAMESPACE
+
 QTEST_MAIN(tst_qquicktextedit)
 
 #include "tst_qquicktextedit.moc"