Notification API cleanup: direction type, parameter and method names

Change-Id: I0349f3eea0028a4df917af8599c073227e0d0ec1
Reviewed-by: Allan Sandfeld Jensen <allan.jensen@qt.io>

7 files changed, 33 insertions, 29 deletions

diff --git a/src/core/api/qwebenginenotification.cpp b/src/core/api/qwebenginenotification.cpp
index edbb8934f..89fd1eff9 100644
--- a/src/core/api/qwebenginenotification.cpp
+++ b/src/core/api/qwebenginenotification.cpp
@@ -217,18 +217,18 @@ QString QWebEngineNotification::language() const
     \brief The text direction for the notification's title and body.
     \sa title(), message()
 */
-QWebEngineNotification::Direction QWebEngineNotification::direction() const
+Qt::LayoutDirection QWebEngineNotification::direction() const
 {
     Q_D(const QWebEngineNotification);
-    return d ? static_cast<Direction>(d->controller->direction()) : DirectionAuto;
+    return d ? d->controller->direction() : Qt::LayoutDirectionAuto;
 }
 
 /*!
-    Returns \c true if the notification is a default constructed null notification.
+    Returns \c true if the notification is not a default constructed null notification.
 */
-bool QWebEngineNotification::isNull() const
+bool QWebEngineNotification::isValid() const
 {
-    return d_ptr.isNull();
+    return !d_ptr.isNull();
 }
 
 /*!
diff --git a/src/core/api/qwebenginenotification.h b/src/core/api/qwebenginenotification.h
index b6b7414f9..0012e5d78 100644
--- a/src/core/api/qwebenginenotification.h
+++ b/src/core/api/qwebenginenotification.h
@@ -64,22 +64,15 @@ class QWEBENGINECORE_EXPORT QWebEngineNotification : public QObject {
     Q_PROPERTY(QString message READ message CONSTANT FINAL)
     Q_PROPERTY(QString tag READ tag CONSTANT FINAL)
     Q_PROPERTY(QString language READ language CONSTANT FINAL)
-    Q_PROPERTY(Direction direction READ direction CONSTANT FINAL)
+    Q_PROPERTY(Qt::LayoutDirection direction READ direction CONSTANT FINAL)
 
 public:
     QWebEngineNotification();
-    QWebEngineNotification(const QWebEngineNotification &);
+    QWebEngineNotification(const QWebEngineNotification &other);
     virtual ~QWebEngineNotification();
-    const QWebEngineNotification &operator=(const QWebEngineNotification &);
+    const QWebEngineNotification &operator=(const QWebEngineNotification &other);
 
-    enum Direction {
-        LeftToRight = Qt::LeftToRight,
-        RightToLeft = Qt::RightToLeft,
-        DirectionAuto = Qt::LayoutDirectionAuto
-    };
-    Q_ENUM(Direction)
-
-    bool matches(const QWebEngineNotification &) const;
+    bool matches(const QWebEngineNotification &other) const;
 
     QUrl origin() const;
     QIcon icon() const;
@@ -87,9 +80,9 @@ public:
     QString message() const;
     QString tag() const;
     QString language() const;
-    Direction direction() const;
+    Qt::LayoutDirection direction() const;
 
-    bool isNull() const;
+    bool isValid() const;
 
 public Q_SLOTS:
     void show() const;
@@ -100,7 +93,7 @@ Q_SIGNALS:
     void closed();
 
 private:
-    QWebEngineNotification(const QSharedPointer<QtWebEngineCore::UserNotificationController> &);
+    QWebEngineNotification(const QSharedPointer<QtWebEngineCore::UserNotificationController> &controller);
     Q_DECLARE_PRIVATE(QWebEngineNotification)
     QScopedPointer<QWebEngineNotificationPrivate> d_ptr;
     friend class QQuickWebEngineProfilePrivate;
diff --git a/src/webenginewidgets/api/qwebenginenotificationpresenter.cpp b/src/webenginewidgets/api/qwebenginenotificationpresenter.cpp
index c15a80373..da97c4662 100644
--- a/src/webenginewidgets/api/qwebenginenotificationpresenter.cpp
+++ b/src/webenginewidgets/api/qwebenginenotificationpresenter.cpp
@@ -57,11 +57,15 @@ DefaultNotificationPresenter::~DefaultNotificationPresenter()
 
 void DefaultNotificationPresenter::show(const QWebEngineNotification &notification)
 {
-    if (!m_activeNotification.isNull())
+    if (m_activeNotification.isValid()) {
         m_activeNotification.close();
+        m_activeNotification.disconnect(this);
+    }
+
     m_activeNotification = notification;
+
 #ifndef QT_NO_SYSTEMTRAYICON
-    if (m_systemTrayIcon) {
+    if (m_activeNotification.isValid() && m_systemTrayIcon) {
         m_systemTrayIcon->show();
         QIcon icon = notification.icon();
         if (!icon.isNull())
@@ -76,7 +80,7 @@ void DefaultNotificationPresenter::show(const QWebEngineNotification &notificati
 
 void DefaultNotificationPresenter::messageClicked()
 {
-    if (!m_activeNotification.isNull())
+    if (m_activeNotification.isValid())
         m_activeNotification.click();
 }
 
diff --git a/tests/auto/quick/publicapi/tst_publicapi.cpp b/tests/auto/quick/publicapi/tst_publicapi.cpp
index b89652625..0012249c8 100644
--- a/tests/auto/quick/publicapi/tst_publicapi.cpp
+++ b/tests/auto/quick/publicapi/tst_publicapi.cpp
@@ -99,6 +99,7 @@ static const QStringList hardcodedTypes = QStringList()
     << "QQuickWebEngineErrorPage*"
     << "const QQuickWebEngineContextMenuData*"
     << "QWebEngineCookieStore*"
+    << "Qt::LayoutDirection"
     ;
 
 static const QStringList expectedAPI = QStringList()
@@ -760,10 +761,7 @@ static const QStringList expectedAPI = QStringList()
     << "QWebEngineNotification.message --> QString"
     << "QWebEngineNotification.tag --> QString"
     << "QWebEngineNotification.language --> QString"
-    << "QWebEngineNotification.direction --> Direction"
-    << "QWebEngineNotification.LeftToRight --> Direction"
-    << "QWebEngineNotification.RightToLeft --> Direction"
-    << "QWebEngineNotification.DirectionAuto --> Direction"
+    << "QWebEngineNotification.direction --> Qt::LayoutDirection"
     << "QWebEngineNotification.show() --> void"
     << "QWebEngineNotification.click() --> void"
     << "QWebEngineNotification.close() --> void"
diff --git a/tests/auto/quick/qmltests/data/tst_notification.qml b/tests/auto/quick/qmltests/data/tst_notification.qml
index 609a04f61..af4aebafc 100644
--- a/tests/auto/quick/qmltests/data/tst_notification.qml
+++ b/tests/auto/quick/qmltests/data/tst_notification.qml
@@ -37,6 +37,7 @@ TestWebEngineView {
 
     property bool permissionRequested: false
     property bool grantPermission: false
+    property url securityOrigin: ''
 
     signal consoleMessage(string message)
 
@@ -48,7 +49,8 @@ TestWebEngineView {
 
     onFeaturePermissionRequested: {
         if (feature === WebEngineView.Notifications) {
-            permissionRequested = true
+            view.permissionRequested = true
+            view.securityOrigin = securityOrigin
             view.grantFeaturePermission(securityOrigin, feature, grantPermission)
         }
     }
@@ -111,6 +113,10 @@ TestWebEngineView {
             tryVerify(function () { return notification !== null })
             compare(notification.title, title)
             compare(notification.message, message)
+            compare(notification.direction, Qt.RightToLeft)
+            compare(notification.origin, securityOrigin)
+            compare(notification.tag, 'tst')
+            compare(notification.language, 'de')
         }
     }
 }
diff --git a/tests/auto/shared/data/notification.html b/tests/auto/shared/data/notification.html
index cadcbd942..1d1e9c411 100644
--- a/tests/auto/shared/data/notification.html
+++ b/tests/auto/shared/data/notification.html
@@ -8,7 +8,7 @@
     function getPermission() { return document.Notification }
 
     function sendNotification(title, body) {
-        let notification = new Notification(title, { body: body })
+        let notification = new Notification(title, { body: body, dir: 'rtl', lang: 'de', tag: 'tst' })
         notification.onclick = function() { console.info('onclick') }
         notification.onclose = function() { console.info('onclose') }
         notification.onerror = function(error) { console.info('onerror: ' + error) }
diff --git a/tests/auto/widgets/qwebenginepage/tst_qwebenginepage.cpp b/tests/auto/widgets/qwebenginepage/tst_qwebenginepage.cpp
index 2cdefe27d..916db3d1e 100644
--- a/tests/auto/widgets/qwebenginepage/tst_qwebenginepage.cpp
+++ b/tests/auto/widgets/qwebenginepage/tst_qwebenginepage.cpp
@@ -3329,10 +3329,13 @@ void tst_QWebEnginePage::sendNotification()
 
     auto notification = presenter.waitForResult();
     QVERIFY(presenter.wasCalled());
-    QVERIFY(!notification.isNull());
+    QVERIFY(notification.isValid());
     QCOMPARE(notification.title(), title);
     QCOMPARE(notification.message(), message);
     QCOMPARE(notification.origin(), origin);
+    QCOMPARE(notification.direction(), Qt::RightToLeft);
+    QCOMPARE(notification.language(), "de");
+    QCOMPARE(notification.tag(), "tst");
 
     notification.show();
     QTRY_VERIFY2(page.messages.contains("onshow"), page.messages.join("\n").toLatin1().constData());