QmlDesigner: Remove generator from AsynchronousImageFactory

The generator was introduced to decouple the generation thread from the
fetch thread. But for the factory you only generate the image so you
can use the factory thread for the collector.

Change-Id: If569defba0c52dd85b19451a78f0daf5043b4d01
Reviewed-by: <github-actions-qt-creator@cristianadam.eu>
Reviewed-by: Qt CI Bot <qt_ci_bot@qt-project.org>
Reviewed-by: Thomas Hartmann <thomas.hartmann@qt.io>

4 files changed, 71 insertions, 81 deletions

diff --git a/src/plugins/qmldesigner/designercore/imagecache/asynchronousimagefactory.cpp b/src/plugins/qmldesigner/designercore/imagecache/asynchronousimagefactory.cpp
index 367f53afe5..81c7aa53ee 100644
--- a/src/plugins/qmldesigner/designercore/imagecache/asynchronousimagefactory.cpp
+++ b/src/plugins/qmldesigner/designercore/imagecache/asynchronousimagefactory.cpp
@@ -25,6 +25,7 @@
 
 #include "asynchronousimagefactory.h"
 
+#include "imagecachecollector.h"
 #include "imagecachegenerator.h"
 #include "imagecachestorage.h"
 #include "timestampprovider.h"
@@ -32,11 +33,11 @@
 namespace QmlDesigner {
 
 AsynchronousImageFactory::AsynchronousImageFactory(ImageCacheStorageInterface &storage,
-                                                   ImageCacheGeneratorInterface &generator,
-                                                   TimeStampProviderInterface &timeStampProvider)
+                                                   TimeStampProviderInterface &timeStampProvider,
+                                                   ImageCacheCollectorInterface &collector)
     : m_storage(storage)
-    , m_generator(generator)
     , m_timeStampProvider(timeStampProvider)
+    , m_collector(collector)
 {
     m_backgroundThread = std::thread{[this] {
         while (isRunning()) {
@@ -45,8 +46,8 @@ AsynchronousImageFactory::AsynchronousImageFactory(ImageCacheStorageInterface &s
                         entry->extraId,
                         std::move(entry->auxiliaryData),
                         m_storage,
-                        m_generator,
-                        m_timeStampProvider);
+                        m_timeStampProvider,
+                        m_collector);
             }
 
             waitForEntries();
@@ -107,8 +108,8 @@ void AsynchronousImageFactory::request(Utils::SmallStringView name,
                                        Utils::SmallStringView extraId,
                                        ImageCache::AuxiliaryData auxiliaryData,
                                        ImageCacheStorageInterface &storage,
-                                       ImageCacheGeneratorInterface &generator,
-                                       TimeStampProviderInterface &timeStampProvider)
+                                       TimeStampProviderInterface &timeStampProvider,
+                                       ImageCacheCollectorInterface &collector)
 {
     const auto id = extraId.empty() ? Utils::PathString{name}
                                     : Utils::PathString::join({name, "+", extraId});
@@ -118,19 +119,20 @@ void AsynchronousImageFactory::request(Utils::SmallStringView name,
 
     if (currentModifiedTime == storageModifiedTime)
         return;
-
-    generator.generateImage(name,
-                            extraId,
-                            currentModifiedTime,
-                            ImageCache::CaptureImageWithSmallImageCallback{},
-                            ImageCache::AbortCallback{},
-                            std::move(auxiliaryData));
+    auto capture = [=](const QImage &image, const QImage &smallImage) {
+        m_storage.storeImage(id, currentModifiedTime, image, smallImage);
+    };
+
+    collector.start(name,
+                    extraId,
+                    std::move(auxiliaryData),
+                    std::move(capture),
+                    ImageCache::AbortCallback{});
 }
 
 void AsynchronousImageFactory::clean()
 {
     clearEntries();
-    m_generator.clean();
 }
 
 void AsynchronousImageFactory::wait()
diff --git a/src/plugins/qmldesigner/designercore/imagecache/asynchronousimagefactory.h b/src/plugins/qmldesigner/designercore/imagecache/asynchronousimagefactory.h
index d9da32d3a5..1331e1af41 100644
--- a/src/plugins/qmldesigner/designercore/imagecache/asynchronousimagefactory.h
+++ b/src/plugins/qmldesigner/designercore/imagecache/asynchronousimagefactory.h
@@ -39,15 +39,14 @@ namespace QmlDesigner {
 
 class TimeStampProviderInterface;
 class ImageCacheStorageInterface;
-class ImageCacheGeneratorInterface;
 class ImageCacheCollectorInterface;
 
 class AsynchronousImageFactory
 {
 public:
     AsynchronousImageFactory(ImageCacheStorageInterface &storage,
-                             ImageCacheGeneratorInterface &generator,
-                             TimeStampProviderInterface &timeStampProvider);
+                             TimeStampProviderInterface &timeStampProvider,
+                             ImageCacheCollectorInterface &collector);
 
     ~AsynchronousImageFactory();
 
@@ -83,8 +82,8 @@ private:
                  Utils::SmallStringView extraId,
                  ImageCache::AuxiliaryData auxiliaryData,
                  ImageCacheStorageInterface &storage,
-                 ImageCacheGeneratorInterface &generator,
-                 TimeStampProviderInterface &timeStampProvider);
+                 TimeStampProviderInterface &timeStampProvider,
+                 ImageCacheCollectorInterface &collector);
     void wait();
     void clearEntries();
     void stopThread();
@@ -95,8 +94,8 @@ private:
     std::condition_variable m_condition;
     std::thread m_backgroundThread;
     ImageCacheStorageInterface &m_storage;
-    ImageCacheGeneratorInterface &m_generator;
     TimeStampProviderInterface &m_timeStampProvider;
+    ImageCacheCollectorInterface &m_collector;
     bool m_finishing{false};
 };
 
diff --git a/src/plugins/qmldesigner/qmldesignerprojectmanager.cpp b/src/plugins/qmldesigner/qmldesignerprojectmanager.cpp
index 3e5b4bf629..9ae83b5451 100644
--- a/src/plugins/qmldesigner/qmldesignerprojectmanager.cpp
+++ b/src/plugins/qmldesigner/qmldesignerprojectmanager.cpp
@@ -83,10 +83,9 @@ public:
     ImageCacheConnectionManager connectionManager;
     ImageCacheCollector collector{connectionManager,
                                   ImageCacheCollectorNullImageHandling::DontCaptureNullImage};
-    ImageCacheGenerator generator{collector, storage};
     TimeStampProvider timeStampProvider;
     AsynchronousExplicitImageCache cache{storage};
-    AsynchronousImageFactory factory{storage, generator, timeStampProvider};
+    AsynchronousImageFactory factory{storage, timeStampProvider, collector};
 };
 
 class QmlDesignerProjectManagerProjectData
diff --git a/tests/unit/unittest/asynchronousimagefactory-test.cpp b/tests/unit/unittest/asynchronousimagefactory-test.cpp
index ab968f220e..dfc40ceb5b 100644
--- a/tests/unit/unittest/asynchronousimagefactory-test.cpp
+++ b/tests/unit/unittest/asynchronousimagefactory-test.cpp
@@ -25,6 +25,7 @@
 
 #include "googletest.h"
 
+#include "imagecachecollectormock.h"
 #include "mockimagecachegenerator.h"
 #include "mockimagecachestorage.h"
 #include "mocktimestampprovider.h"
@@ -41,67 +42,64 @@ class AsynchronousImageFactory : public testing::Test
 protected:
     AsynchronousImageFactory()
     {
-        ON_CALL(mockTimeStampProvider, timeStamp(Eq("/path/to/Component.qml")))
+        ON_CALL(timeStampProviderMock, timeStamp(Eq("/path/to/Component.qml")))
             .WillByDefault(Return(Sqlite::TimeStamp{123}));
     }
 
 protected:
     Notification notification;
     Notification waitInThread;
-    NiceMock<MockImageCacheStorage> mockStorage;
-    NiceMock<MockImageCacheGenerator> mockGenerator;
-    NiceMock<MockTimeStampProvider> mockTimeStampProvider;
-    QmlDesigner::AsynchronousImageFactory factory{mockStorage, mockGenerator, mockTimeStampProvider};
+    NiceMock<MockImageCacheStorage> storageMock;
+    NiceMock<ImageCacheCollectorMock> collectorMock;
+    NiceMock<MockTimeStampProvider> timeStampProviderMock;
+    QmlDesigner::AsynchronousImageFactory factory{storageMock, timeStampProviderMock, collectorMock};
     QImage image1{10, 10, QImage::Format_ARGB32};
     QImage smallImage1{1, 1, QImage::Format_ARGB32};
 };
 
-TEST_F(AsynchronousImageFactory, RequestImageRequestImageFromGenerator)
+TEST_F(AsynchronousImageFactory, RequestImageRequestImageFromCollector)
 {
-    EXPECT_CALL(mockGenerator,
-                generateImage(Eq("/path/to/Component.qml"),
-                              IsEmpty(),
-                              Eq(Sqlite::TimeStamp{123}),
-                              _,
-                              _,
-                              VariantWith<Utils::monostate>(Utils::monostate{})))
-        .WillRepeatedly([&](auto, auto, auto, auto, auto, auto) { notification.notify(); });
+    EXPECT_CALL(collectorMock,
+                start(Eq("/path/to/Component.qml"),
+                      IsEmpty(),
+                      VariantWith<Utils::monostate>(Utils::monostate{}),
+                      _,
+                      _))
+        .WillRepeatedly([&](auto, auto, auto, auto, auto) { notification.notify(); });
 
     factory.generate("/path/to/Component.qml");
     notification.wait();
 }
 
-TEST_F(AsynchronousImageFactory, RequestImageWithExtraIdRequestImageFromGenerator)
+TEST_F(AsynchronousImageFactory, RequestImageWithExtraIdRequestImageFromCollector)
 {
-    EXPECT_CALL(mockGenerator,
-                generateImage(Eq("/path/to/Component.qml"),
-                              Eq("foo"),
-                              Eq(Sqlite::TimeStamp{123}),
-                              _,
-                              _,
-                              VariantWith<Utils::monostate>(Utils::monostate{})))
-        .WillRepeatedly([&](auto, auto, auto, auto, auto, auto) { notification.notify(); });
+    EXPECT_CALL(collectorMock,
+                start(Eq("/path/to/Component.qml"),
+                      Eq("foo"),
+                      VariantWith<Utils::monostate>(Utils::monostate{}),
+                      _,
+                      _))
+        .WillRepeatedly([&](auto, auto, auto, auto, auto) { notification.notify(); });
 
     factory.generate("/path/to/Component.qml", "foo");
     notification.wait();
 }
 
-TEST_F(AsynchronousImageFactory, RequestImageWithAuxiliaryDataRequestImageFromGenerator)
+TEST_F(AsynchronousImageFactory, RequestImageWithAuxiliaryDataRequestImageFromCollector)
 {
     std::vector<QSize> sizes{{20, 11}};
 
-    EXPECT_CALL(mockGenerator,
-                generateImage(Eq("/path/to/Component.qml"),
-                              Eq("foo"),
-                              Eq(Sqlite::TimeStamp{123}),
-                              _,
-                              _,
-                              VariantWith<FontCollectorSizesAuxiliaryData>(AllOf(
-                                  Field(&FontCollectorSizesAuxiliaryData::sizes,
-                                        ElementsAre(QSize{20, 11})),
-                                  Field(&FontCollectorSizesAuxiliaryData::colorName, Eq(u"color")),
-                                  Field(&FontCollectorSizesAuxiliaryData::text, Eq(u"some text"))))))
-        .WillRepeatedly([&](auto, auto, auto, auto, auto, auto) { notification.notify(); });
+    EXPECT_CALL(collectorMock,
+                start(Eq("/path/to/Component.qml"),
+                      Eq("foo"),
+                      VariantWith<FontCollectorSizesAuxiliaryData>(
+                          AllOf(Field(&FontCollectorSizesAuxiliaryData::sizes,
+                                      ElementsAre(QSize{20, 11})),
+                                Field(&FontCollectorSizesAuxiliaryData::colorName, Eq(u"color")),
+                                Field(&FontCollectorSizesAuxiliaryData::text, Eq(u"some text")))),
+                      _,
+                      _))
+        .WillRepeatedly([&](auto, auto, auto, auto, auto) { notification.notify(); });
 
     factory.generate("/path/to/Component.qml",
                      "foo",
@@ -111,16 +109,16 @@ TEST_F(AsynchronousImageFactory, RequestImageWithAuxiliaryDataRequestImageFromGe
     notification.wait();
 }
 
-TEST_F(AsynchronousImageFactory, DontRequestImageRequestImageFromGeneratorIfFileWasNotUpdated)
+TEST_F(AsynchronousImageFactory, DontRequestImageRequestImageFromCollectorIfFileWasNotUpdated)
 {
-    ON_CALL(mockStorage, fetchModifiedImageTime(Eq("/path/to/Component.qml"))).WillByDefault([&](auto) {
+    ON_CALL(storageMock, fetchModifiedImageTime(Eq("/path/to/Component.qml"))).WillByDefault([&](auto) {
         notification.notify();
         return Sqlite::TimeStamp{124};
     });
-    ON_CALL(mockTimeStampProvider, timeStamp(Eq("/path/to/Component.qml")))
+    ON_CALL(timeStampProviderMock, timeStamp(Eq("/path/to/Component.qml")))
         .WillByDefault(Return(Sqlite::TimeStamp{124}));
 
-    EXPECT_CALL(mockGenerator, generateImage(_, _, _, _, _, _)).Times(0);
+    EXPECT_CALL(collectorMock, start(_, _, _, _, _)).Times(0);
 
     factory.generate("/path/to/Component.qml");
     notification.wait();
@@ -128,36 +126,28 @@ TEST_F(AsynchronousImageFactory, DontRequestImageRequestImageFromGeneratorIfFile
 
 TEST_F(AsynchronousImageFactory, CleanRemovesEntries)
 {
-    EXPECT_CALL(mockGenerator, generateImage(Eq("/path/to/Component1.qml"), _, _, _, _, _))
-        .WillRepeatedly([&](auto, auto, auto, auto, auto, auto) { waitInThread.wait(); });
+    EXPECT_CALL(collectorMock, start(Eq("/path/to/Component1.qml"), _, _, _, _))
+        .WillRepeatedly([&](auto, auto, auto, auto, auto) { waitInThread.wait(); });
     factory.generate("/path/to/Component1.qml");
 
-    EXPECT_CALL(mockGenerator, generateImage(Eq("/path/to/Component3.qml"), _, _, _, _, _)).Times(0);
+    EXPECT_CALL(collectorMock, start(Eq("/path/to/Component3.qml"), _, _, _, _)).Times(0);
 
     factory.generate("/path/to/Component3.qml");
     factory.clean();
     waitInThread.notify();
 }
 
-TEST_F(AsynchronousImageFactory, CleanCallsGeneratorClean)
-{
-    EXPECT_CALL(mockGenerator, clean()).Times(AtLeast(1));
-
-    factory.clean();
-}
-
 TEST_F(AsynchronousImageFactory, AfterCleanNewJobsWorks)
 {
     factory.clean();
 
-    EXPECT_CALL(mockGenerator,
-                generateImage(Eq("/path/to/Component.qml"),
-                              IsEmpty(),
-                              Eq(Sqlite::TimeStamp{123}),
-                              _,
-                              _,
-                              VariantWith<Utils::monostate>(Utils::monostate{})))
-        .WillRepeatedly([&](auto, auto, auto, auto, auto, auto) { notification.notify(); });
+    EXPECT_CALL(collectorMock,
+                start(Eq("/path/to/Component.qml"),
+                      IsEmpty(),
+                      VariantWith<Utils::monostate>(Utils::monostate{}),
+                      _,
+                      _))
+        .WillRepeatedly([&](auto, auto, auto, auto, auto) { notification.notify(); });
 
     factory.generate("/path/to/Component.qml");
     notification.wait();