Tracing: Simplify saving and loading

We don't need the success and canceled signals from TimelineTraceFile,
as the future produced by runAsync tells us when a job is finished or
canceled. Also, we can just create the QFile inside the runnable to save
some code.

Change-Id: I7d91c60c1f798077573712cf624243e9f5969fd6
Reviewed-by: Christian Kandeler <christian.kandeler@qt.io>

src/libs/tracing/timelinetracefile.cpp

@@ -61,6 +61,12 @@ void TimelineTraceFile::setDeviceProgress(QIODevice *device)
                               * (MaximumProgress - MinimumProgress) / device->size());
 }
 
+void TimelineTraceFile::fail(const QString &message)
+{
+    emit error(message);
+    m_future.cancel();
+}
+
 bool TimelineTraceFile::isCanceled() const
 {
     return m_future.isCanceled();

src/libs/tracing/timelinetracefile.h

@@ -64,6 +64,7 @@ public:
 
     virtual void save(QIODevice *device) = 0;
     virtual void load(QIODevice *device) = 0;
+    void fail(const QString &message);
 
     void setTraceStart(qint64 traceStart) { m_traceStart = traceStart; }
     qint64 traceStart() const { return m_traceStart; }
@@ -79,8 +80,6 @@ public:
 
 signals:
     void error(const QString &error);
-    void success();
-    void canceled();
 
 private:
     qint64 m_traceStart = -1;

src/libs/tracing/timelinetracemanager.cpp

@@ -230,100 +230,66 @@ void TimelineTraceManager::finalize()
 
 QFuture<void> TimelineTraceManager::save(const QString &filename)
 {
-    QFile *file = new QFile(filename);
-    if (!file->open(QIODevice::WriteOnly)) {
-        delete file;
-        return Utils::runAsync([this, filename](QFutureInterface<void> &future) {
-            future.setProgressRange(0, 1);
-            future.setProgressValue(1);
-            emit error(tr("Could not open %1 for writing.").arg(filename));
-            emit saveFinished();
-        });
-    }
-
     TimelineTraceFile *writer = createTraceFile();
     writer->setTraceTime(traceStart(), traceEnd(), traceDuration());
     writer->setTraceManager(this);
     writer->setNotes(d->notesModel);
 
-    connect(writer, &QObject::destroyed, this, &TimelineTraceManager::saveFinished,
+    connect(writer, &QObject::destroyed, this, &TimelineTraceManager::saveFinished);
-            Qt::QueuedConnection);
+    connect(writer, &TimelineTraceFile::error, this, &TimelineTraceManager::error);
 
-    connect(writer, &TimelineTraceFile::error, this, [this, file](const QString &message) {
+    return Utils::runAsync([filename, writer] (QFutureInterface<void> &future) {
-        file->close();
-        file->remove();
-        delete file;
-        if (!message.isEmpty())
-            emit error(message);
-    }, Qt::QueuedConnection);
-
-    connect(writer, &TimelineTraceFile::success, this, [file]() {
-        file->close();
-        delete file;
-    }, Qt::QueuedConnection);
-
-    connect(writer, &TimelineTraceFile::canceled, this, [file]() {
-        file->close();
-        file->remove();
-        delete file;
-    }, Qt::QueuedConnection);
-
-    return Utils::runAsync([file, writer] (QFutureInterface<void> &future) {
         writer->setFuture(future);
-        writer->save(file);
+        QFile file(filename);
+
+        if (file.open(QIODevice::WriteOnly))
+            writer->save(&file);
+        else
+            writer->fail(tr("Could not open %1 for writing.").arg(filename));
+
+        if (future.isCanceled())
+            file.remove();
         writer->deleteLater();
     });
 }
 
 QFuture<void> TimelineTraceManager::load(const QString &filename)
 {
-    QFile *file = new QFile(filename, this);
-    if (!file->open(QIODevice::ReadOnly)) {
-        delete file;
-        return Utils::runAsync([this, filename] (QFutureInterface<void> &future) {
-            future.setProgressRange(0, 1);
-            future.setProgressValue(1);
-            emit error(tr("Could not open %1 for reading.").arg(filename));
-            emit loadFinished();
-        });
-    }
-
     clearAll();
     initialize();
     TimelineTraceFile *reader = createTraceFile();
     reader->setTraceManager(this);
     reader->setNotes(d->notesModel);
 
-    connect(reader, &QObject::destroyed, this, &TimelineTraceManager::loadFinished,
+    connect(reader, &QObject::destroyed, this, &TimelineTraceManager::loadFinished);
-            Qt::QueuedConnection);
+    connect(reader, &TimelineTraceFile::error, this, &TimelineTraceManager::error);
 
-    connect(reader, &TimelineTraceFile::success, this, [this, reader]() {
+    QFuture<void> future = Utils::runAsync([filename, reader] (QFutureInterface<void> &future) {
-        if (reader->traceStart() >= 0)
-            decreaseTraceStart(reader->traceStart());
-        if (reader->traceEnd() >= 0)
-            increaseTraceEnd(reader->traceEnd());
-        finalize();
-        delete reader;
-    }, Qt::QueuedConnection);
-
-    connect(reader, &TimelineTraceFile::error, this, [this, reader](const QString &message) {
-        clearAll();
-        delete reader;
-        if (!message.isEmpty())
-            emit error(message);
-    }, Qt::QueuedConnection);
-
-    connect(reader, &TimelineTraceFile::canceled, this, [this, reader]() {
-        clearAll();
-        delete reader;
-    }, Qt::QueuedConnection);
-
-    return Utils::runAsync([file, reader] (QFutureInterface<void> &future) {
         reader->setFuture(future);
-        reader->load(file);
+        QFile file(filename);
-        file->close();
+
-        file->deleteLater();
+        if (file.open(QIODevice::ReadOnly))
+            reader->load(&file);
+        else
+            reader->fail(tr("Could not open %1 for reading.").arg(filename));
+
+        reader->deleteLater();
     });
+
+    QFutureWatcher<void> *watcher = new QFutureWatcher<void>(reader);
+    watcher->setFuture(future);
+    connect(watcher, &QFutureWatcherBase::canceled, this, &TimelineTraceManager::clearAll);
+    connect(watcher, &QFutureWatcherBase::finished, this, [this, reader]() {
+        if (!reader->isCanceled()) {
+            if (reader->traceStart() >= 0)
+                decreaseTraceStart(reader->traceStart());
+            if (reader->traceEnd() >= 0)
+                increaseTraceEnd(reader->traceEnd());
+            finalize();
+        }
+    });
+
+    return future;
 }
 
 qint64 TimelineTraceManager::traceStart() const

src/plugins/qmlprofiler/qmlprofilertracefile.cpp

@@ -189,12 +189,8 @@ void QmlProfilerTraceFile::loadQtd(QIODevice *device)
         }
     }
 
-    if (isCanceled())
+    if (stream.hasError())
-        emit canceled();
+        fail(tr("Error while parsing trace data file: %1").arg(stream.errorString()));
-    else if (stream.hasError())
-        emit error(tr("Error while parsing trace data file: %1").arg(stream.errorString()));
-    else
-        emit success();
 }
 
 void QmlProfilerTraceFile::loadQzt(QIODevice *device)
@@ -205,7 +201,7 @@ void QmlProfilerTraceFile::loadQzt(QIODevice *device)
     QByteArray magic;
     stream >> magic;
     if (magic != QByteArray("QMLPROFILER")) {
-        emit error(tr("Invalid magic: %1").arg(QLatin1String(magic)));
+        fail(tr("Invalid magic: %1").arg(QLatin1String(magic)));
         return;
     }
 
@@ -213,7 +209,7 @@ void QmlProfilerTraceFile::loadQzt(QIODevice *device)
     stream >> dataStreamVersion;
 
     if (dataStreamVersion > QDataStream::Qt_DefaultCompiledVersion) {
-        emit error(tr("Unknown data stream version: %1").arg(dataStreamVersion));
+        fail(tr("Unknown data stream version: %1").arg(dataStreamVersion));
         return;
     }
     stream.setVersion(dataStreamVersion);
@@ -237,7 +233,7 @@ void QmlProfilerTraceFile::loadQzt(QIODevice *device)
         quint32 numEventTypes;
         bufferStream >> numEventTypes;
         if (numEventTypes > quint32(std::numeric_limits<int>::max())) {
-            emit error(tr("Excessive number of event types: %1").arg(numEventTypes));
+            fail(tr("Excessive number of event types: %1").arg(numEventTypes));
             return;
         }
 
@@ -270,7 +266,7 @@ void QmlProfilerTraceFile::loadQzt(QIODevice *device)
             bufferStream >> event;
             if (bufferStream.status() == QDataStream::Ok) {
                 if (event.typeIndex() >= traceManager()->numEventTypes()) {
-                    emit error(tr("Invalid type index %1").arg(event.typeIndex()));
+                    fail(tr("Invalid type index %1").arg(event.typeIndex()));
                     return;
                 }
                 addFeature(manager->eventType(event.typeIndex()).feature());
@@ -279,7 +275,7 @@ void QmlProfilerTraceFile::loadQzt(QIODevice *device)
             } else if (bufferStream.status() == QDataStream::ReadPastEnd) {
                 break; // Apparently EOF is a character so we end up here after the last event.
             } else if (bufferStream.status() == QDataStream::ReadCorruptData) {
-                emit error(tr("Corrupt data before position %1.").arg(device->pos()));
+                fail(tr("Corrupt data before position %1.").arg(device->pos()));
                 return;
             } else {
                 Q_UNREACHABLE();
@@ -289,12 +285,6 @@ void QmlProfilerTraceFile::loadQzt(QIODevice *device)
         buffer.close();
         setDeviceProgress(device);
     }
-
-    if (isCanceled()) {
-        emit canceled();
-    } else {
-        emit success();
-    }
 }
 
 void QmlProfilerTraceFile::addEventsProgress(qint64 timestamp)
@@ -694,10 +684,8 @@ void QmlProfilerTraceFile::saveQtd(QIODevice *device)
     addStageProgress(ProgressTypes);
     stream.writeEndElement(); // eventData
 
-    if (isCanceled()) {
+    if (isCanceled())
-        emit canceled();
         return;
-    }
 
     QStack<QmlEvent> stack;
     qint64 lastProgressTimestamp = traceStart();
@@ -794,15 +782,11 @@ void QmlProfilerTraceFile::saveQtd(QIODevice *device)
         stream.writeEndElement(); // trace
         stream.writeEndDocument();
 
-        if (isCanceled())
+        if (stream.hasError())
-            emit canceled();
+            fail(tr("Error writing trace file."));
-        else if (stream.hasError())
-            emit error(tr("Error writing trace file."));
-        else
-            emit success();
     }, [this](const QString &message) {
-        emit error(tr("Could not re-read events from temporary trace file: %s\nSaving failed.")
+        fail(tr("Could not re-read events from temporary trace file: %s\nSaving failed.")
-                   .arg(message));
+             .arg(message));
     }, future());
 }
 
@@ -841,10 +825,8 @@ void QmlProfilerTraceFile::saveQzt(QIODevice *device)
         addStageProgress(ProgressNotes);
     }
 
-    if (isCanceled()) {
+    if (isCanceled())
-        emit canceled();
         return;
-    }
 
     qint64 lastProgressTimestamp = traceStart();
     modelManager()->replayQmlEvents([&](const QmlEvent &event, const QmlEventType &type) {
@@ -864,18 +846,15 @@ void QmlProfilerTraceFile::saveQzt(QIODevice *device)
     }, [&]() {
         buffer.open(QIODevice::WriteOnly);
     }, [&]() {
-        if (isCanceled()) {
+        if (!isCanceled()) {
-            emit canceled();
-        } else {
             stream << qCompress(buffer.data());
             buffer.close();
             buffer.buffer().clear();
             addEventsProgress(traceEnd() - lastProgressTimestamp);
-            emit success();
         }
     }, [this](const QString &message) {
-        emit error(tr("Could not re-read events from temporary trace file: %s\nSaving failed.")
+        fail(tr("Could not re-read events from temporary trace file: %s\nSaving failed.")
-                   .arg(message));
+             .arg(message));
     }, future());
 }