Notification API cleanup

Task-number: QTBUG-74543
Change-Id: Ice5a0dbfc3485c8b7e6fa900ef427a9aed871d42
Reviewed-by: Allan Sandfeld Jensen <allan.jensen@qt.io>

12 files changed, 64 insertions, 92 deletions

diff --git a/src/core/api/qwebenginenotification.cpp b/src/core/api/qwebenginenotification.cpp
index ba9739b35..0b1d48ce9 100644
--- a/src/core/api/qwebenginenotification.cpp
+++ b/src/core/api/qwebenginenotification.cpp
@@ -58,8 +58,8 @@ using QtWebEngineCore::UserNotificationController;
 
     Web engine notifications are passed to the user in the
     \l QWebEngineProfile::setNotificationPresenter() and
-    \l QQuickWebEngineProfile::userNotification() calls and the
-    \l WebEngineProfile::userNotification() signal.
+    \l QQuickWebEngineProfile::presentNotification() calls and the
+    \l WebEngineProfile::presentNotification() signal.
 */
 
 class QWebEngineNotificationPrivate : public UserNotificationController::Client {
@@ -86,14 +86,6 @@ public:
     QWebEngineNotification *q;
 };
 
-
-/*!
-    Creates a null QWebEngineNotification.
-
-    \sa isNull()
-*/
-QWebEngineNotification::QWebEngineNotification() { }
-
 /*! \internal
 */
 QWebEngineNotification::QWebEngineNotification(const QSharedPointer<UserNotificationController> &controller)
@@ -102,25 +94,10 @@ QWebEngineNotification::QWebEngineNotification(const QSharedPointer<UserNotifica
 
 /*! \internal
 */
-QWebEngineNotification::QWebEngineNotification(const QWebEngineNotification &other)
-    : QObject()
-    , d_ptr(new QWebEngineNotificationPrivate(this, other.d_ptr->controller))
-{ }
-
-/*! \internal
-*/
 QWebEngineNotification::~QWebEngineNotification()
 {
 }
 
-/*! \internal
-*/
-const QWebEngineNotification &QWebEngineNotification::operator=(const QWebEngineNotification &other)
-{
-    d_ptr.reset(new QWebEngineNotificationPrivate(this, other.d_ptr->controller));
-    return *this;
-}
-
 /*!
     Returns \c true if the two notifications belong to the same message chain.
     That is, if their tag() and origin() are the same. This means one is
@@ -128,13 +105,15 @@ const QWebEngineNotification &QWebEngineNotification::operator=(const QWebEngine
 
     \sa tag(), origin()
 */
-bool QWebEngineNotification::matches(const QWebEngineNotification &other) const
+bool QWebEngineNotification::matches(const QWebEngineNotification *other) const
 {
+    if (!other)
+        return false;
     if (!d_ptr)
-        return !other.d_ptr;
-    if (!other.d_ptr)
+        return !other->d_ptr;
+    if (!other->d_ptr)
         return false;
-    return tag() == other.tag() && origin() == other.origin();
+    return tag() == other->tag() && origin() == other->origin();
 }
 
 /*!
@@ -187,15 +166,14 @@ QUrl QWebEngineNotification::origin() const
 }
 
 /*!
-    \property QWebEngineNotification::icon
-    \brief The icon to be shown with the notification.
+    Returns the icon to be shown with the notification.
 
-    If no icon is set by the sender, an null QIcon is returned.
+    If no icon is set by the sender, a null QImage is returned.
 */
-QIcon QWebEngineNotification::icon() const
+QImage QWebEngineNotification::icon() const
 {
     Q_D(const QWebEngineNotification);
-    return d ? d->controller->icon() : QIcon();
+    return d ? d->controller->icon() : QImage();
 }
 
 /*!
@@ -224,14 +202,6 @@ Qt::LayoutDirection QWebEngineNotification::direction() const
 }
 
 /*!
-    Returns \c true if the notification is not a default constructed null notification.
-*/
-bool QWebEngineNotification::isValid() const
-{
-    return !d_ptr.isNull();
-}
-
-/*!
     Creates and dispatches a JavaScript \e {show event} on notification.
 
     Should be called by the notification platform when the notification has been shown to user.
diff --git a/src/core/api/qwebenginenotification.h b/src/core/api/qwebenginenotification.h
index 6c9a8f891..08fd629be 100644
--- a/src/core/api/qwebenginenotification.h
+++ b/src/core/api/qwebenginenotification.h
@@ -46,7 +46,6 @@
 #include <QtCore/QScopedPointer>
 #include <QtCore/QSharedPointer>
 #include <QtCore/QUrl>
-#include <QtGui/QIcon>
 
 namespace QtWebEngineCore {
 class UserNotificationController;
@@ -59,7 +58,6 @@ class QWebEngineNotificationPrivate;
 class Q_WEBENGINECORE_EXPORT QWebEngineNotification : public QObject {
     Q_OBJECT
     Q_PROPERTY(QUrl origin READ origin CONSTANT FINAL)
-    Q_PROPERTY(QIcon icon READ icon CONSTANT FINAL)
     Q_PROPERTY(QString title READ title CONSTANT FINAL)
     Q_PROPERTY(QString message READ message CONSTANT FINAL)
     Q_PROPERTY(QString tag READ tag CONSTANT FINAL)
@@ -67,23 +65,18 @@ class Q_WEBENGINECORE_EXPORT QWebEngineNotification : public QObject {
     Q_PROPERTY(Qt::LayoutDirection direction READ direction CONSTANT FINAL)
 
 public:
-    QWebEngineNotification();
-    QWebEngineNotification(const QWebEngineNotification &other);
-    virtual ~QWebEngineNotification();
-    const QWebEngineNotification &operator=(const QWebEngineNotification &other);
+    virtual ~QWebEngineNotification() override;
 
-    bool matches(const QWebEngineNotification &other) const;
+    bool matches(const QWebEngineNotification *other) const;
 
     QUrl origin() const;
-    QIcon icon() const;
+    QImage icon() const;
     QString title() const;
     QString message() const;
     QString tag() const;
     QString language() const;
     Qt::LayoutDirection direction() const;
 
-    bool isValid() const;
-
 public Q_SLOTS:
     void show() const;
     void click() const;
@@ -93,8 +86,9 @@ Q_SIGNALS:
     void closed();
 
 private:
-    QWebEngineNotification(const QSharedPointer<QtWebEngineCore::UserNotificationController> &controller);
+    Q_DISABLE_COPY(QWebEngineNotification)
     Q_DECLARE_PRIVATE(QWebEngineNotification)
+    QWebEngineNotification(const QSharedPointer<QtWebEngineCore::UserNotificationController> &controller);
     QScopedPointer<QWebEngineNotificationPrivate> d_ptr;
     friend class QQuickWebEngineProfilePrivate;
     friend class QWebEngineProfilePrivate;
diff --git a/src/core/user_notification_controller.cpp b/src/core/user_notification_controller.cpp
index 82cb57e51..d169bf854 100644
--- a/src/core/user_notification_controller.cpp
+++ b/src/core/user_notification_controller.cpp
@@ -86,7 +86,7 @@ public:
     std::unique_ptr<UserNotificationController::Delegate> m_delegate;
     blink::NotificationResources m_resources;
     UserNotificationController::Client *m_client;
-    QIcon m_icon;
+    QImage m_icon;
     QImage m_image;
     QImage m_badge;
     bool m_iconGenerated;
@@ -155,15 +155,12 @@ QUrl UserNotificationController::origin() const
     return toQt(d->m_origin);
 }
 
-QIcon UserNotificationController::icon() const
+QImage UserNotificationController::icon() const
 {
     if (!d->m_iconGenerated) {
         d->m_iconGenerated = true;
-        if (!d->m_resources.notification_icon.isNull()) {
-            QImage image = toQImage(d->m_resources.notification_icon);
-            if (!image.isNull())
-                d->m_icon = QIcon(QPixmap::fromImage(std::move(image), Qt::NoFormatConversion));
-        }
+        if (!d->m_resources.notification_icon.isNull())
+            d->m_icon = toQImage(d->m_resources.notification_icon);
     }
     return d->m_icon;
 }
diff --git a/src/core/user_notification_controller.h b/src/core/user_notification_controller.h
index 213629334..bab85c7ec 100644
--- a/src/core/user_notification_controller.h
+++ b/src/core/user_notification_controller.h
@@ -86,7 +86,7 @@ public:
     void closeNotification();
 
     QUrl origin() const;
-    QIcon icon() const;
+    QImage icon() const;
     QImage image() const;
     QImage badge() const;
     QString title() const;
diff --git a/src/webengine/api/qquickwebengineprofile.cpp b/src/webengine/api/qquickwebengineprofile.cpp
index ac75b5356..4832ba303 100644
--- a/src/webengine/api/qquickwebengineprofile.cpp
+++ b/src/webengine/api/qquickwebengineprofile.cpp
@@ -152,7 +152,7 @@ ASSERT_ENUMS_MATCH(QQuickWebEngineDownloadItem::MimeHtmlSaveFormat, QtWebEngineC
 */
 
 /*!
-    \fn QQuickWebEngineProfile::userNotification(QWebEngineNotification *notification)
+    \fn QQuickWebEngineProfile::presentNotification(QWebEngineNotification *notification)
 
     This signal is emitted whenever there is a newly created user notification.
     The \a notification argument holds the notification instance to query data and interact with.
@@ -304,7 +304,7 @@ void QQuickWebEngineProfilePrivate::showNotification(QSharedPointer<QtWebEngineC
     Q_Q(QQuickWebEngineProfile);
     auto notification = new QWebEngineNotification(controller);
     QQmlEngine::setObjectOwnership(notification, QQmlEngine::JavaScriptOwnership);
-    Q_EMIT q->userNotification(notification);
+    Q_EMIT q->presentNotification(notification);
 }
 
 void QQuickWebEngineProfilePrivate::userScripts_append(QQmlListProperty<QQuickWebEngineScript> *p, QQuickWebEngineScript *script)
@@ -387,7 +387,7 @@ void QQuickWebEngineProfilePrivate::userScripts_clear(QQmlListProperty<QQuickWeb
 */
 
 /*!
-    \qmlsignal WebEngineProfile::userNotification(WebEngineNotification notification)
+    \qmlsignal WebEngineProfile::presentNotification(WebEngineNotification notification)
     \since QtWebEngine 1.9
 
     This signal is emitted whenever there is a newly created user notification.
diff --git a/src/webengine/api/qquickwebengineprofile.h b/src/webengine/api/qquickwebengineprofile.h
index e6f9fb73d..e5f7ff713 100644
--- a/src/webengine/api/qquickwebengineprofile.h
+++ b/src/webengine/api/qquickwebengineprofile.h
@@ -176,7 +176,7 @@ Q_SIGNALS:
     void downloadRequested(QQuickWebEngineDownloadItem *download);
     void downloadFinished(QQuickWebEngineDownloadItem *download);
 
-    Q_REVISION(5) void userNotification(QWebEngineNotification *notification);
+    Q_REVISION(5) void presentNotification(QWebEngineNotification *notification);
 
 private:
     Q_DECLARE_PRIVATE(QQuickWebEngineProfile)
diff --git a/src/webengine/plugin/plugins.qmltypes b/src/webengine/plugin/plugins.qmltypes
index cc2ed502d..0037861e5 100644
--- a/src/webengine/plugin/plugins.qmltypes
+++ b/src/webengine/plugin/plugins.qmltypes
@@ -531,7 +531,7 @@ Module {
             Parameter { name: "download"; type: "QQuickWebEngineDownloadItem"; isPointer: true }
         }
         Signal {
-            name: "userNotification"
+            name: "presentNotification"
             revision: 5
             Parameter { name: "notification"; type: "QWebEngineNotification"; isPointer: true }
         }
diff --git a/src/webenginewidgets/api/qwebenginenotificationpresenter.cpp b/src/webenginewidgets/api/qwebenginenotificationpresenter.cpp
index da97c4662..667605c37 100644
--- a/src/webenginewidgets/api/qwebenginenotificationpresenter.cpp
+++ b/src/webenginewidgets/api/qwebenginenotificationpresenter.cpp
@@ -39,6 +39,7 @@
 
 #include "qwebenginenotificationpresenter_p.h"
 
+#include <QApplication>
 #include <QSystemTrayIcon>
 
 QT_BEGIN_NAMESPACE
@@ -55,50 +56,53 @@ DefaultNotificationPresenter::~DefaultNotificationPresenter()
 {
 }
 
-void DefaultNotificationPresenter::show(const QWebEngineNotification &notification)
+void DefaultNotificationPresenter::show(std::unique_ptr<QWebEngineNotification> notification)
 {
-    if (m_activeNotification.isValid()) {
-        m_activeNotification.close();
-        m_activeNotification.disconnect(this);
+    Q_ASSERT(notification);
+    if (m_activeNotification) {
+        m_activeNotification->close();
+        m_activeNotification->disconnect(this);
     }
 
-    m_activeNotification = notification;
+    m_activeNotification = std::move(notification);
 
 #ifndef QT_NO_SYSTEMTRAYICON
-    if (m_activeNotification.isValid() && m_systemTrayIcon) {
+    if (m_activeNotification && m_systemTrayIcon) {
+        m_systemTrayIcon->setIcon(qApp->windowIcon());
         m_systemTrayIcon->show();
-        QIcon icon = notification.icon();
-        if (!icon.isNull())
-            m_systemTrayIcon->showMessage(notification.title(), notification.message(), icon);
+        QImage notificationIconImage = m_activeNotification->icon();
+        m_notificationIcon = QIcon(QPixmap::fromImage(std::move(notificationIconImage), Qt::NoFormatConversion));
+        if (!m_notificationIcon.isNull())
+            m_systemTrayIcon->showMessage(m_activeNotification->title(), m_activeNotification->message(), m_notificationIcon);
         else
-            m_systemTrayIcon->showMessage(notification.title(), notification.message());
-        notification.show();
-        connect(&m_activeNotification, &QWebEngineNotification::closed, this, &DefaultNotificationPresenter::closeNotification);
+            m_systemTrayIcon->showMessage(m_activeNotification->title(), m_activeNotification->message());
+        m_activeNotification->show();
+        connect(m_activeNotification.get(), &QWebEngineNotification::closed, this, &DefaultNotificationPresenter::closeNotification);
     }
 #endif
 }
 
 void DefaultNotificationPresenter::messageClicked()
 {
-    if (m_activeNotification.isValid())
-        m_activeNotification.click();
+    if (m_activeNotification)
+        m_activeNotification->click();
 }
 
 void DefaultNotificationPresenter::closeNotification()
 {
 #ifndef QT_NO_SYSTEMTRAYICON
     const QWebEngineNotification *canceled = static_cast<const QWebEngineNotification *>(QObject::sender());
-    if (m_systemTrayIcon && canceled->matches(m_activeNotification))
+    if (m_systemTrayIcon && canceled->matches(m_activeNotification.get()))
         m_systemTrayIcon->hide();
 #endif
 }
 
-void defaultNotificationPresenter(const QWebEngineNotification &notification)
+void defaultNotificationPresenter(std::unique_ptr<QWebEngineNotification> notification)
 {
     static DefaultNotificationPresenter *presenter = nullptr;
     if (!presenter)
         presenter = new DefaultNotificationPresenter();
-    presenter->show(notification);
+    presenter->show(std::move(notification));
 }
 
 
diff --git a/src/webenginewidgets/api/qwebenginenotificationpresenter_p.h b/src/webenginewidgets/api/qwebenginenotificationpresenter_p.h
index a66dbc1b2..49d774806 100644
--- a/src/webenginewidgets/api/qwebenginenotificationpresenter_p.h
+++ b/src/webenginewidgets/api/qwebenginenotificationpresenter_p.h
@@ -54,6 +54,9 @@
 #include <QtWebEngineCore/QWebEngineNotification>
 
 #include <QtCore/QObject>
+#include <QtGui/QIcon>
+
+#include <memory>
 
 QT_BEGIN_NAMESPACE
 
@@ -65,7 +68,7 @@ public:
     DefaultNotificationPresenter(QObject *parent = nullptr);
     virtual ~DefaultNotificationPresenter();
 
-    void show(const QWebEngineNotification &notification);
+    void show(std::unique_ptr<QWebEngineNotification> notification);
 
 private Q_SLOTS:
     void messageClicked();
@@ -73,10 +76,11 @@ private Q_SLOTS:
 
 private:
     QSystemTrayIcon *m_systemTrayIcon;
-    QWebEngineNotification m_activeNotification;
+    QIcon m_notificationIcon;
+    std::unique_ptr<QWebEngineNotification> m_activeNotification;
 };
 
-void defaultNotificationPresenter(const QWebEngineNotification &notification);
+void defaultNotificationPresenter(std::unique_ptr<QWebEngineNotification> notification);
 
 QT_END_NAMESPACE
 
diff --git a/src/webenginewidgets/api/qwebengineprofile.cpp b/src/webenginewidgets/api/qwebengineprofile.cpp
index d69ddb343..47b3f4363 100644
--- a/src/webenginewidgets/api/qwebengineprofile.cpp
+++ b/src/webenginewidgets/api/qwebengineprofile.cpp
@@ -141,10 +141,12 @@ using QtWebEngineCore::ProfileAdapter;
             Both session and persistent cookies are saved to and restored from disk.
 */
 
-void QWebEngineProfilePrivate::showNotification(QSharedPointer<QtWebEngineCore::UserNotificationController> &notification)
+void QWebEngineProfilePrivate::showNotification(QSharedPointer<QtWebEngineCore::UserNotificationController> &controller)
 {
-    if (m_notificationPresenter)
-        m_notificationPresenter(QWebEngineNotification(notification));
+    if (m_notificationPresenter) {
+        std::unique_ptr<QWebEngineNotification> notification(new QWebEngineNotification(controller));
+        m_notificationPresenter(std::move(notification));
+    }
 }
 
 /*!
@@ -668,7 +670,7 @@ QWebEngineScriptCollection *QWebEngineProfile::scripts() const
     \since 5.13
     \sa QWebEngineNotification
 */
-void QWebEngineProfile::setNotificationPresenter(std::function<void(const QWebEngineNotification &)> notificationPresenter)
+void QWebEngineProfile::setNotificationPresenter(std::function<void(std::unique_ptr<QWebEngineNotification>)> notificationPresenter)
 {
     Q_D(QWebEngineProfile);
     d->m_notificationPresenter = std::move(notificationPresenter);
diff --git a/src/webenginewidgets/api/qwebengineprofile.h b/src/webenginewidgets/api/qwebengineprofile.h
index e3ddb594a..794ba6279 100644
--- a/src/webenginewidgets/api/qwebengineprofile.h
+++ b/src/webenginewidgets/api/qwebengineprofile.h
@@ -47,6 +47,7 @@
 #include <QtCore/qstring.h>
 
 #include <functional>
+#include <memory>
 
 QT_BEGIN_NAMESPACE
 
@@ -141,7 +142,7 @@ public:
     QString downloadPath() const;
     void setDownloadPath(const QString &path);
 
-    void setNotificationPresenter(std::function<void(const QWebEngineNotification &)> notificationPresenter);
+    void setNotificationPresenter(std::function<void(std::unique_ptr<QWebEngineNotification>)> notificationPresenter);
 
     QWebEngineClientCertificateStore *clientCertificateStore();
 
diff --git a/src/webenginewidgets/api/qwebengineprofile_p.h b/src/webenginewidgets/api/qwebengineprofile_p.h
index 91c43cf0a..64e9500b0 100644
--- a/src/webenginewidgets/api/qwebengineprofile_p.h
+++ b/src/webenginewidgets/api/qwebengineprofile_p.h
@@ -100,7 +100,7 @@ private:
     QPointer<QtWebEngineCore::ProfileAdapter> m_profileAdapter;
     QScopedPointer<QWebEngineScriptCollection> m_scriptCollection;
     QMap<quint32, QPointer<QWebEngineDownloadItem> > m_ongoingDownloads;
-    std::function<void(const QWebEngineNotification &)> m_notificationPresenter;
+    std::function<void(std::unique_ptr<QWebEngineNotification>)> m_notificationPresenter;
 };
 
 QT_END_NAMESPACE