QFile::moveToTrash: fix error reporting on Windows

If a move-to-trash operation failed, e.g. because the file was opened by
another process (or QFile), then the moveToTrash function would still
return true.

MSDN documents the IFileOperation::PerformOperations to return whether
the operation succeeded, but evidently this is only a statement about
the execution of queued up operations, not a statement about any of the
operations' success.

If the operation succeeded is reported by an HRESULT parameter
of the IFileOperationProgressSink::PostDeleteItem implementation,
and we ignored that parameter so far.

Check it via the SUCCEEDED macro, and set a boolean sink variable based
on that, which we can inspect to return the correct value.

Augment the test case by opening those files we create ourselves, and
if that fails (which it will on Windows, but not necessarily on other
platforms), then try again after closing the file. If the first attempt
succeeded, then the source file must also be gone.

Pick-to: 6.6 6.5 6.2 5.15
Fixes: QTBUG-117383
Done-With: Thiago Macieira <thiago.macieira@intel.com>
Change-Id: Icb82a0c9d3b337585dded622d6656e07dee33d84
Reviewed-by: Thiago Macieira <thiago.macieira@intel.com>
Reviewed-by: Volker Hilsheimer <volker.hilsheimer@qt.io>
Reviewed-by: Qt CI Bot <qt_ci_bot@qt-project.org>

1 files changed, 82 insertions, 0 deletions

diff --git a/tests/auto/corelib/io/qfile/tst_qfile.cpp b/tests/auto/corelib/io/qfile/tst_qfile.cpp
index a668a6345b..5fda0c04e3 100644
--- a/tests/auto/corelib/io/qfile/tst_qfile.cpp
+++ b/tests/auto/corelib/io/qfile/tst_qfile.cpp
@@ -288,6 +288,8 @@ private slots:
 
     void moveToTrash_data();
     void moveToTrash();
+    void moveToTrashOpenFile_data();
+    void moveToTrashOpenFile();
 
     void stdfilesystem();
 
@@ -4057,6 +4059,86 @@ void tst_QFile::moveToTrash()
     }
 }
 
+void tst_QFile::moveToTrashOpenFile_data()
+{
+    QTest::addColumn<bool>("useStatic");
+    QTest::addColumn<bool>("success");
+
+    // QFile::moveToTrash() non-static member closes the file before trashing,
+    // so this must always succeed.
+    QTest::newRow("member") << false << true;
+
+    // QFile::moveToTrash() static member cannot close the file because it
+    // operates on another QFile, so this operation will fail on OSes that do
+    // not permit deleting open files.
+    QTest::newRow("static") << true
+#ifdef Q_OS_WIN
+                            << false;
+#else
+                            << true;
+#endif
+}
+
+void tst_QFile::moveToTrashOpenFile()
+{
+#if defined(Q_OS_ANDROID) || defined(Q_OS_WEBOS)
+    QSKIP("This platform doesn't implement a trash bin");
+#endif
+    QFETCH(bool, useStatic);
+    QFETCH(bool, success);
+    const QByteArrayView contents = "Hello, World\n";
+
+    QString newFileName, origFileName;
+    auto cleanup = qScopeGuard([&] {
+        if (!origFileName.isEmpty())
+            QFile::remove(origFileName);
+        if (!newFileName.isEmpty() && newFileName != origFileName)
+            QFile::remove(newFileName);
+    });
+
+    origFileName = []() {
+        QTemporaryFile temp(QDir::homePath() + "/tst_qfile.moveToTrashOpenFile.XXXXXX");
+        temp.setAutoRemove(false);
+        if (!temp.open())
+            qWarning("Failed to create temporary file: %ls", qUtf16Printable(temp.errorString()));
+        return temp.fileName();
+    }();
+
+    QFile f;
+    f.setFileName(origFileName);
+    QVERIFY2(f.open(QIODevice::ReadWrite | QIODevice::Unbuffered), qPrintable(f.errorString()));
+    f.write(contents.data(), contents.size());
+
+    QString errorString;
+    auto doMoveToTrash = [&](QFile *f) {
+        if (!f->moveToTrash())
+            errorString = f->errorString();
+        newFileName = f->fileName();
+    };
+    if (useStatic) {
+        // it's the same as the static QFile::moveToTrash(), but gives us
+        // the error string
+        QFile other(origFileName);
+        doMoveToTrash(&other);
+    } else {
+        doMoveToTrash(&f);
+    }
+    QCOMPARE_NE(f.fileName(), QString());
+
+    if (success) {
+        QCOMPARE(errorString, QString());
+        QCOMPARE_NE(newFileName, origFileName);         // must have changed!
+        QVERIFY(!QFile::exists(origFileName));
+        QVERIFY(QFile::exists(newFileName));
+        QCOMPARE(QFileInfo(newFileName).size(), contents.size());
+    } else {
+        QCOMPARE_NE(errorString, QString());
+        QCOMPARE(newFileName, origFileName);            // mustn't have changed!
+        QVERIFY(QFile::exists(origFileName));
+        QCOMPARE(QFileInfo(origFileName).size(), contents.size());
+    }
+}
+
 void tst_QFile::stdfilesystem()
 {
 #if QT_CONFIG(cxx17_filesystem)