Unify logic of enum error+string holding and changing notification

We use the pair error + errorsString in several places,
let's handle the update in notification in one place and
have unit tests for the logic.

Thus, the change fixes errors notification for camera.

Pick-to: 6.5
Change-Id: Ie94ec728b051047bdd63ae3860e8942c3c245b1e
Reviewed-by: Artem Dyomin <artem.dyomin@qt.io>
(cherry picked from commit a69450f5e12d3cd0ff3367980c2fa9f4ab205457)
Reviewed-by: Qt Cherry-pick Bot <cherrypick_bot@qt-project.org>

14 files changed, 209 insertions, 60 deletions

diff --git a/src/multimedia/CMakeLists.txt b/src/multimedia/CMakeLists.txt
index 82ae52995..93fbdc4a4 100644
--- a/src/multimedia/CMakeLists.txt
+++ b/src/multimedia/CMakeLists.txt
@@ -63,6 +63,7 @@ qt_internal_add_module(Multimedia
         qmediatimerange.cpp qmediatimerange.h
         qmultimediautils.cpp qmultimediautils_p.h
         qtmultimediaglobal.h qtmultimediaglobal_p.h
+        qerrorinfo_p.h
         recording/qmediacapturesession.cpp recording/qmediacapturesession.h
         recording/qmediarecorder.cpp recording/qmediarecorder.h recording/qmediarecorder_p.h
         recording/qscreencapture.cpp recording/qscreencapture.h
diff --git a/src/multimedia/camera/qcamera.cpp b/src/multimedia/camera/qcamera.cpp
index ca9200b04..8f6b581c9 100644
--- a/src/multimedia/camera/qcamera.cpp
+++ b/src/multimedia/camera/qcamera.cpp
@@ -157,11 +157,7 @@ void QCameraPrivate::_q_error(int error, const QString &errorString)
 {
     Q_Q(QCamera);
 
-    this->error = QCamera::Error(error);
-    this->errorString = errorString;
-
-    emit q->errorChanged();
-    emit q->errorOccurred(this->error, errorString);
+    this->error.setAndNotify(QCamera::Error(error), errorString, *q);
 }
 
 void QCameraPrivate::init(const QCameraDevice &device)
@@ -171,8 +167,7 @@ void QCameraPrivate::init(const QCameraDevice &device)
     auto maybeControl = QPlatformMediaIntegration::instance()->createCamera(q);
     if (!maybeControl) {
         qWarning() << "Failed to initialize QCamera" << maybeControl.error();
-        error = QCamera::CameraError;
-        errorString =  maybeControl.error();
+        error = { QCamera::CameraError, maybeControl.error() };
         return;
     }
     control = maybeControl.value();
@@ -301,7 +296,7 @@ void QCamera::setActive(bool active)
 
 QCamera::Error QCamera::error() const
 {
-    return d_func()->error;
+    return d_func()->error.code();
 }
 
 /*!
@@ -317,7 +312,7 @@ QCamera::Error QCamera::error() const
 */
 QString QCamera::errorString() const
 {
-    return d_func()->errorString;
+    return d_func()->error.description();
 }
 
 /*! \enum QCamera::Feature
diff --git a/src/multimedia/camera/qcamera_p.h b/src/multimedia/camera/qcamera_p.h
index 3255b241c..c0477c242 100644
--- a/src/multimedia/camera/qcamera_p.h
+++ b/src/multimedia/camera/qcamera_p.h
@@ -16,6 +16,7 @@
 //
 
 #include "private/qobject_p.h"
+#include "private/qerrorinfo_p.h"
 #include "qcamera.h"
 #include "qcameradevice.h"
 
@@ -28,25 +29,18 @@ class QCameraPrivate : public QObjectPrivate
 {
     Q_DECLARE_PUBLIC(QCamera)
 public:
-    QCameraPrivate()
-        : QObjectPrivate(),
-          error(QCamera::NoError)
-    {
-    }
-
     void init(const QCameraDevice &device);
 
     QMediaCaptureSession *captureSession = nullptr;
     QPlatformCamera *control = nullptr;
 
-    QCamera::Error error;
-    QString errorString;
+    QErrorInfo<QCamera::Error> error;
 
     QCameraDevice cameraDevice;
     QCameraFormat cameraFormat;
 
     void _q_error(int error, const QString &errorString);
-    void unsetError() { error = QCamera::NoError; errorString.clear(); }
+    void unsetError() { error = {}; }
 };
 
 QT_END_NAMESPACE
diff --git a/src/multimedia/platform/qplatformmediarecorder.cpp b/src/multimedia/platform/qplatformmediarecorder.cpp
index c2e4df053..6aa76e066 100644
--- a/src/multimedia/platform/qplatformmediarecorder.cpp
+++ b/src/multimedia/platform/qplatformmediarecorder.cpp
@@ -47,13 +47,7 @@ void QPlatformMediaRecorder::actualLocationChanged(const QUrl &location)
 
 void QPlatformMediaRecorder::error(QMediaRecorder::Error error, const QString &errorString)
 {
-    if (error == m_error && errorString == m_errorString)
-        return;
-    m_error = error;
-    m_errorString = errorString;
-    if (error != QMediaRecorder::NoError)
-        emit q->errorOccurred(error, errorString);
-    emit q->errorChanged();
+    m_error.setAndNotify(error, errorString, *q);
 }
 
 void QPlatformMediaRecorder::metaDataChanged()
diff --git a/src/multimedia/platform/qplatformmediarecorder_p.h b/src/multimedia/platform/qplatformmediarecorder_p.h
index 1ff26f5bf..31df21710 100644
--- a/src/multimedia/platform/qplatformmediarecorder_p.h
+++ b/src/multimedia/platform/qplatformmediarecorder_p.h
@@ -22,6 +22,7 @@
 #include <QtMultimedia/qmediarecorder.h>
 #include <QtMultimedia/qmediametadata.h>
 #include <QtMultimedia/qmediaformat.h>
+#include <QtMultimedia/private/qerrorinfo_p.h>
 #include <QtCore/private/qglobal_p.h>
 
 QT_BEGIN_NAMESPACE
@@ -115,8 +116,8 @@ public:
     virtual void setMetaData(const QMediaMetaData &) {}
     virtual QMediaMetaData metaData() const { return {}; }
 
-    QMediaRecorder::Error error() const { return m_error;}
-    QString errorString() const { return m_errorString; }
+    QMediaRecorder::Error error() const { return m_error.code(); }
+    QString errorString() const { return m_error.description(); }
 
     QUrl outputLocation() const { return m_outputLocation; }
     virtual void setOutputLocation(const QUrl &location) { m_outputLocation = location; }
@@ -137,8 +138,7 @@ protected:
 
 private:
     QMediaRecorder *q = nullptr;
-    QMediaRecorder::Error m_error = QMediaRecorder::NoError;
-    QString m_errorString;
+    QErrorInfo<QMediaRecorder::Error> m_error;
     QUrl m_actualLocation;
     QUrl m_outputLocation;
     qint64 m_duration = 0;
diff --git a/src/multimedia/platform/qplatformsurfacecapture.cpp b/src/multimedia/platform/qplatformsurfacecapture.cpp
index 89adf3715..a56f48b62 100644
--- a/src/multimedia/platform/qplatformsurfacecapture.cpp
+++ b/src/multimedia/platform/qplatformsurfacecapture.cpp
@@ -53,27 +53,17 @@ void QPlatformSurfaceCapture::setSource(Source source)
 
 QPlatformSurfaceCapture::Error QPlatformSurfaceCapture::error() const
 {
-    return m_error;
+    return m_error.code();
 }
 
 QString QPlatformSurfaceCapture::errorString() const
 {
-    return m_errorString;
+    return m_error.description();
 }
 
 void QPlatformSurfaceCapture::updateError(Error error, const QString &errorString)
 {
-    bool changed = error != m_error || errorString != m_errorString;
-    m_error = error;
-    m_errorString = errorString;
-    if (changed) {
-        if (m_error != NoError) {
-            emit errorOccurred(error, errorString);
-            qWarning() << "Screen capture fail:" << error << "," << errorString;
-        }
-
-        emit errorChanged();
-    }
+    m_error.setAndNotify(error, errorString, *this);
 }
 
 bool QPlatformSurfaceCapture::checkScreenWithError(ScreenSource &screen)
diff --git a/src/multimedia/platform/qplatformsurfacecapture_p.h b/src/multimedia/platform/qplatformsurfacecapture_p.h
index c99708f9a..42fbda474 100644
--- a/src/multimedia/platform/qplatformsurfacecapture_p.h
+++ b/src/multimedia/platform/qplatformsurfacecapture_p.h
@@ -19,6 +19,7 @@
 #include "qscreen.h"
 #include "qcapturablewindow.h"
 #include "qpointer.h"
+#include "private/qerrorinfo_p.h"
 
 #include <optional>
 #include <variant>
@@ -77,8 +78,7 @@ Q_SIGNALS:
     void errorOccurred(Error error, QString errorString);
 
 private:
-    Error m_error = NoError;
-    QString m_errorString;
+    QErrorInfo<Error> m_error;
     Source m_source;
     bool m_active = false;
 };
diff --git a/src/multimedia/playback/qmediaplayer.cpp b/src/multimedia/playback/qmediaplayer.cpp
index 2ae206bbf..9c1cf6541 100644
--- a/src/multimedia/playback/qmediaplayer.cpp
+++ b/src/multimedia/playback/qmediaplayer.cpp
@@ -120,14 +120,7 @@ void QMediaPlayerPrivate::setError(QMediaPlayer::Error error, const QString &err
 {
     Q_Q(QMediaPlayer);
 
-    auto prevError = std::exchange(this->error, error);
-    auto prevErrorString = std::exchange(this->errorString, errorString);
-
-    if (prevError != error || prevErrorString != errorString)
-        emit q->errorChanged();
-
-    if (error != QMediaPlayer::NoError)
-        emit q->errorOccurred(error, errorString);
+    this->error.setAndNotify(error, errorString, *q);
 }
 
 void QMediaPlayerPrivate::setMedia(const QUrl &media, QIODevice *stream)
@@ -469,7 +462,7 @@ void QMediaPlayer::setLoops(int loops)
 */
 QMediaPlayer::Error QMediaPlayer::error() const
 {
-    return d_func()->error;
+    return d_func()->error.code();
 }
 
 /*!
@@ -486,7 +479,7 @@ QMediaPlayer::Error QMediaPlayer::error() const
 */
 QString QMediaPlayer::errorString() const
 {
-    return d_func()->errorString;
+    return d_func()->error.description();
 }
 
 /*!
diff --git a/src/multimedia/playback/qmediaplayer_p.h b/src/multimedia/playback/qmediaplayer_p.h
index 04f35e884..74d631839 100644
--- a/src/multimedia/playback/qmediaplayer_p.h
+++ b/src/multimedia/playback/qmediaplayer_p.h
@@ -20,6 +20,7 @@
 #include "qvideosink.h"
 #include "qaudiooutput.h"
 #include <private/qplatformmediaplayer_p.h>
+#include <private/qerrorinfo_p.h>
 
 #include "private/qobject_p.h"
 #include <QtCore/qobject.h>
@@ -40,8 +41,7 @@ class QMediaPlayerPrivate : public QObjectPrivate
 
 public:
     QMediaPlayerPrivate() = default;
-    QPlatformMediaPlayer* control = nullptr;
-    QString errorString;
+    QPlatformMediaPlayer *control = nullptr;
 
     QAudioOutput *audioOutput = nullptr;
     QVideoSink *videoSink = nullptr;
@@ -52,7 +52,7 @@ public:
     QIODevice *stream = nullptr;
 
     QMediaPlayer::PlaybackState state = QMediaPlayer::StoppedState;
-    QMediaPlayer::Error error = QMediaPlayer::NoError;
+    QErrorInfo<QMediaPlayer::Error> error;
 
     void setMedia(const QUrl &media, QIODevice *stream = nullptr);
 
diff --git a/src/multimedia/qerrorinfo_p.h b/src/multimedia/qerrorinfo_p.h
new file mode 100644
index 000000000..6428cb00f
--- /dev/null
+++ b/src/multimedia/qerrorinfo_p.h
@@ -0,0 +1,57 @@
+// Copyright (C) 2023 The Qt Company Ltd.
+// SPDX-License-Identifier: LicenseRef-Qt-Commercial OR LGPL-3.0-only OR GPL-2.0-only OR GPL-3.0-only
+
+#ifndef QERRORINFO_P_H
+#define QERRORINFO_P_H
+
+//
+//  W A R N I N G
+//  -------------
+//
+// This file is not part of the Qt API. It exists purely as an
+// implementation detail. This header file may change from version to
+// version without notice, or even be removed.
+//
+// We mean it.
+//
+
+#include <QtMultimedia/qtmultimediaglobal.h>
+#include <QString>
+
+QT_BEGIN_NAMESPACE
+
+template <typename ErrorCode, ErrorCode NoError = ErrorCode::NoError>
+class QErrorInfo
+{
+public:
+    QErrorInfo(ErrorCode error = NoError, QString description = {})
+        : m_code(error), m_description(std::move(description))
+    {
+    }
+
+    template <typename Notifier>
+    void setAndNotify(ErrorCode code, QString description, Notifier &notifier)
+    {
+        const bool changed = code != m_code || description != m_description;
+
+        m_code = code;
+        m_description = std::move(description);
+
+        if (code != NoError)
+            emit notifier.errorOccurred(m_code, m_description);
+
+        if (changed)
+            emit notifier.errorChanged();
+    }
+
+    ErrorCode code() const { return m_code; }
+    QString description() const { return m_description; };
+
+private:
+    ErrorCode m_code;
+    QString m_description;
+};
+
+QT_END_NAMESPACE
+
+#endif // QERRORINFO_P_H
diff --git a/tests/auto/integration/qwindowcapturebackend/tst_qwindowcapturebackend.cpp b/tests/auto/integration/qwindowcapturebackend/tst_qwindowcapturebackend.cpp
index e927da061..ad3ec88e4 100644
--- a/tests/auto/integration/qwindowcapturebackend/tst_qwindowcapturebackend.cpp
+++ b/tests/auto/integration/qwindowcapturebackend/tst_qwindowcapturebackend.cpp
@@ -53,7 +53,6 @@ private slots:
     {
         WindowCaptureFixture fixture;
 
-        QTest::ignoreMessage(QtWarningMsg, QRegularExpression{ ".*Screen capture fail.*" });
         fixture.m_capture.setActive(true);
 
         QVERIFY(!fixture.m_capture.isActive());
@@ -237,8 +236,6 @@ private slots:
         // Get capturing started
         fixture.m_grabber.waitAndTakeFrames(3);
 
-        QTest::ignoreMessage(QtWarningMsg, QRegularExpression{ ".*Screen capture fail.*" });
-
         // Closing the process waits for it to exit
         fixture.m_windowProcess.close();
 
diff --git a/tests/auto/unit/multimedia/CMakeLists.txt b/tests/auto/unit/multimedia/CMakeLists.txt
index 0aa1198a3..a690f5c6c 100644
--- a/tests/auto/unit/multimedia/CMakeLists.txt
+++ b/tests/auto/unit/multimedia/CMakeLists.txt
@@ -24,3 +24,4 @@ add_subdirectory(qaudiodecoder)
 add_subdirectory(qsamplecache)
 add_subdirectory(qscreencapture)
 add_subdirectory(qmediadevices)
+add_subdirectory(qerrorinfo)
diff --git a/tests/auto/unit/multimedia/qerrorinfo/CMakeLists.txt b/tests/auto/unit/multimedia/qerrorinfo/CMakeLists.txt
new file mode 100644
index 000000000..ea6ac5690
--- /dev/null
+++ b/tests/auto/unit/multimedia/qerrorinfo/CMakeLists.txt
@@ -0,0 +1,10 @@
+# Copyright (C) 2023 The Qt Company Ltd.
+# SPDX-License-Identifier: BSD-3-Clause
+
+qt_internal_add_test(tst_qerrorinfo
+    SOURCES
+        tst_qerrorinfo.cpp
+    LIBRARIES
+        Qt::Gui
+        Qt::MultimediaPrivate
+)
diff --git a/tests/auto/unit/multimedia/qerrorinfo/tst_qerrorinfo.cpp b/tests/auto/unit/multimedia/qerrorinfo/tst_qerrorinfo.cpp
new file mode 100644
index 000000000..4fce3c8ba
--- /dev/null
+++ b/tests/auto/unit/multimedia/qerrorinfo/tst_qerrorinfo.cpp
@@ -0,0 +1,117 @@
+// Copyright (C) 2023 The Qt Company Ltd.
+// SPDX-License-Identifier: LicenseRef-Qt-Commercial OR GPL-3.0-only WITH Qt-GPL-exception-1.0
+
+// TESTED_COMPONENT=src/multimedia
+
+#include <QtTest/QtTest>
+#include <QSignalSpy>
+#include <private/qerrorinfo_p.h>
+
+QT_USE_NAMESPACE
+
+enum class TestError { ErrorA, ErrorB, NoError };
+
+using TestErrorInfo = QErrorInfo<TestError>;
+
+class TestNotifier : public QObject
+{
+    Q_OBJECT
+public:
+signals:
+    void errorOccurred(TestError error, QString errorDescription);
+    void errorChanged();
+};
+
+class tst_QErrorInfo : public QObject
+{
+    Q_OBJECT
+
+private slots:
+    void defaultConstructor_setsNoError();
+    void constructor_setsPassedError();
+
+    void setAndNotify_setsErrorAndNotifes_data();
+    void setAndNotify_setsErrorAndNotifes();
+};
+
+void tst_QErrorInfo::defaultConstructor_setsNoError()
+{
+    TestErrorInfo errorInfo;
+    QCOMPARE(errorInfo.code(), TestError::NoError);
+    QCOMPARE(errorInfo.description(), QString());
+}
+
+void tst_QErrorInfo::constructor_setsPassedError()
+{
+    TestErrorInfo errorInfo(TestError::ErrorB, "test error");
+    QCOMPARE(errorInfo.code(), TestError::ErrorB);
+    QCOMPARE(errorInfo.description(), "test error");
+}
+
+void tst_QErrorInfo::setAndNotify_setsErrorAndNotifes_data()
+{
+    QTest::addColumn<TestError>("initialError");
+    QTest::addColumn<QString>("initialErrorDescription");
+    QTest::addColumn<TestError>("error");
+    QTest::addColumn<QString>("errorDescription");
+    QTest::addColumn<bool>("errorChangedEmitted");
+    QTest::addColumn<bool>("errorOccurredEmitted");
+
+    QTest::newRow("No error -> No error")
+            << TestError::NoError << QString() << TestError::NoError << QString() << false << false;
+    QTest::newRow("No error -> An error with empty string")
+            << TestError::NoError << QString() << TestError::ErrorA << QString() << true << true;
+    QTest::newRow("No error -> An error with non-empty string")
+            << TestError::NoError << QString() << TestError::ErrorB << QString("error") << true
+            << true;
+    QTest::newRow("An error with empty string -> No error")
+            << TestError::ErrorA << QString() << TestError::NoError << QString() << true << false;
+    QTest::newRow("An error with non-empty string -> No Error")
+            << TestError::ErrorA << QString("error") << TestError::NoError << QString() << true
+            << false;
+    QTest::newRow("An error -> Another error")
+            << TestError::ErrorA << QString("error A") << TestError::ErrorB << QString("error B")
+            << true << true;
+    QTest::newRow("An error -> Another error with empty string")
+            << TestError::ErrorA << QString("error A") << TestError::ErrorB << QString() << true
+            << true;
+    QTest::newRow("An error -> The same error with changed string")
+            << TestError::ErrorA << QString("error") << TestError::ErrorA
+            << QString("another error") << true << true;
+}
+
+void tst_QErrorInfo::setAndNotify_setsErrorAndNotifes()
+{
+    QFETCH(TestError, initialError);
+    QFETCH(QString, initialErrorDescription);
+    QFETCH(TestError, error);
+    QFETCH(QString, errorDescription);
+    QFETCH(bool, errorChangedEmitted);
+    QFETCH(bool, errorOccurredEmitted);
+
+    TestErrorInfo errorInfo(initialError, initialErrorDescription);
+
+    TestNotifier notifier;
+    QSignalSpy errorOccurredSpy(&notifier, &TestNotifier::errorOccurred);
+    QSignalSpy errorChangedSpy(&notifier, &TestNotifier::errorChanged);
+
+    errorInfo.setAndNotify(error, errorDescription, notifier);
+
+    QCOMPARE(errorInfo.code(), error);
+    QCOMPARE(errorInfo.description(), errorDescription);
+
+    QList<QList<QVariant>> expectedErrorChanged;
+    if (errorChangedEmitted)
+        expectedErrorChanged.push_back({});
+
+    QList<QList<QVariant>> expectedErrorOccured;
+    if (errorOccurredEmitted)
+        expectedErrorOccured.push_back({ QVariant::fromValue(error), errorDescription });
+
+    QCOMPARE(errorOccurredSpy, expectedErrorOccured);
+    QCOMPARE(errorChangedSpy, expectedErrorChanged);
+}
+
+QTEST_GUILESS_MAIN(tst_QErrorInfo)
+
+#include "tst_qerrorinfo.moc"