QPixmap::load: ensure QBitmap stays a QBitmap even on failure

... and avoid detach()ing potentially large data for just
preserving the QPlatformPixmap::pixelType().

A QBitmap differs from a QPixmap (its base class, urgh)
by always having a data != nullptr and a Bitmap pixel
type, yet load() was unconditionally setting 'data' to
nullptr on failure, turning a QBitmap into a non-QBitmap.

Fix by move-assigning a null QBitmap instead of resetting
'data'.

Add some tests.

Change-Id: Ida58b3b24d96472a5f9d0f18f81cc763edcf3c16
Reviewed-by: Anton Kudryavtsev <a.kudryavtsev@netris.ru>
Reviewed-by: Edward Welbourne <edward.welbourne@qt.io>
Reviewed-by: Ulf Hermann <ulf.hermann@qt.io>

2 files changed, 51 insertions, 26 deletions

diff --git a/src/gui/image/qpixmap.cpp b/src/gui/image/qpixmap.cpp
index 78031bca97..3726b2124c 100644
--- a/src/gui/image/qpixmap.cpp
+++ b/src/gui/image/qpixmap.cpp
@@ -762,39 +762,37 @@ QBitmap QPixmap::createMaskFromColor(const QColor &maskColor, Qt::MaskMode mode)
 
 bool QPixmap::load(const QString &fileName, const char *format, Qt::ImageConversionFlags flags)
 {
-    if (fileName.isEmpty()) {
-        data.reset();
-        return false;
-    }
-
-    detach();
+    if (!fileName.isEmpty()) {
 
-    QFileInfo info(fileName);
-    QString key = QLatin1String("qt_pixmap")
-                  % info.absoluteFilePath()
-                  % HexString<uint>(info.lastModified().toTime_t())
-                  % HexString<quint64>(info.size())
-                  % HexString<uint>(data ? data->pixelType() : QPlatformPixmap::PixmapType);
+        QFileInfo info(fileName);
+        // Note: If no extension is provided, we try to match the
+        // file against known plugin extensions
+        if (info.completeSuffix().isEmpty() || info.exists()) {
 
-    // Note: If no extension is provided, we try to match the
-    // file against known plugin extensions
-    if (!info.completeSuffix().isEmpty() && !info.exists()) {
-        data.reset();
-        return false;
-    }
+            QString key = QLatin1String("qt_pixmap")
+                    % info.absoluteFilePath()
+                    % HexString<uint>(info.lastModified().toTime_t())
+                    % HexString<quint64>(info.size())
+                    % HexString<uint>(data ? data->pixelType() : QPlatformPixmap::PixmapType);
 
-    if (QPixmapCache::find(key, this))
-        return true;
+            if (QPixmapCache::find(key, this))
+                return true;
 
-    if (!data)
-        data = QPlatformPixmap::create(0, 0, QPlatformPixmap::PixmapType);
+            data = QPlatformPixmap::create(0, 0, data ? data->pixelType() : QPlatformPixmap::PixmapType);
 
-    if (data->fromFile(fileName, format, flags)) {
-        QPixmapCache::insert(key, *this);
-        return true;
+            if (data->fromFile(fileName, format, flags)) {
+                QPixmapCache::insert(key, *this);
+                return true;
+            }
+        }
     }
 
-    data.reset();
+    if (!isNull()) {
+        if (isQBitmap())
+            *this = QBitmap();
+        else
+            data.reset();
+    }
     return false;
 }
 
diff --git a/tests/auto/gui/image/qpixmap/tst_qpixmap.cpp b/tests/auto/gui/image/qpixmap/tst_qpixmap.cpp
index 4ffe357d09..286f00c111 100644
--- a/tests/auto/gui/image/qpixmap/tst_qpixmap.cpp
+++ b/tests/auto/gui/image/qpixmap/tst_qpixmap.cpp
@@ -1508,6 +1508,33 @@ void tst_QPixmap::loadAsBitmapOrPixmap()
     QVERIFY(!bitmap.isNull());
     QCOMPARE(bitmap.depth(), 1);
     QVERIFY(bitmap.isQBitmap());
+
+    // check that a QBitmap stays a QBitmap even when loading fails:
+    ok = bitmap.load(QString());
+    QVERIFY(!ok);
+    QVERIFY(bitmap.isNull());
+    QVERIFY(bitmap.isQBitmap());
+
+    ok = bitmap.load("does not exist");
+    QVERIFY(!ok);
+    QVERIFY(bitmap.isNull());
+    QVERIFY(bitmap.isQBitmap());
+
+    ok = bitmap.load("does not exist.png");
+    QVERIFY(!ok);
+    QVERIFY(bitmap.isNull());
+    QVERIFY(bitmap.isQBitmap());
+
+    QTemporaryFile garbage;
+    QVERIFY(garbage.open());
+    const QString garbagePath = garbage.fileName();
+    garbage.write(reinterpret_cast<const char *>(&garbage), sizeof garbage);
+    garbage.close();
+
+    ok = bitmap.load(garbagePath);
+    QVERIFY(!ok);
+    QVERIFY(bitmap.isNull());
+    QVERIFY(bitmap.isQBitmap());
 }
 
 void tst_QPixmap::toImageDeepCopy()