Merge remote-tracking branch 'origin/5.12' into dev

Change-Id: Ib374079ded76e43d4b72f725bbf16fe353ef827e

9 files changed, 83 insertions, 18 deletions

diff --git a/src/core/download_manager_delegate_qt.cpp b/src/core/download_manager_delegate_qt.cpp
index abf4a2a95..948a62047 100644
--- a/src/core/download_manager_delegate_qt.cpp
+++ b/src/core/download_manager_delegate_qt.cpp
@@ -82,35 +82,45 @@ void DownloadManagerDelegateQt::GetNextId(const content::DownloadIdCallback& cal
     callback.Run(++m_currentId);
 }
 
+download::DownloadItem *DownloadManagerDelegateQt::findDownloadById(quint32 downloadId)
+{
+    content::DownloadManager* dlm = content::BrowserContext::GetDownloadManager(m_profileAdapter->profile());
+    return dlm->GetDownload(downloadId);
+}
+
 void DownloadManagerDelegateQt::cancelDownload(const content::DownloadTargetCallback& callback)
 {
-    callback.Run(base::FilePath(), download::DownloadItem::TARGET_DISPOSITION_PROMPT, download::DownloadDangerType::DOWNLOAD_DANGER_TYPE_MAYBE_DANGEROUS_CONTENT, base::FilePath(), download::DownloadInterruptReason::DOWNLOAD_INTERRUPT_REASON_USER_CANCELED);
+    callback.Run(base::FilePath(),
+                 download::DownloadItem::TARGET_DISPOSITION_PROMPT,
+                 download::DownloadDangerType::DOWNLOAD_DANGER_TYPE_MAYBE_DANGEROUS_CONTENT,
+                 base::FilePath(),
+                 download::DownloadInterruptReason::DOWNLOAD_INTERRUPT_REASON_USER_CANCELED);
 }
 
 void DownloadManagerDelegateQt::cancelDownload(quint32 downloadId)
 {
-    content::DownloadManager* dlm = content::BrowserContext::GetDownloadManager(m_profileAdapter->profile());
-    download::DownloadItem *download = dlm->GetDownload(downloadId);
-    if (download)
+    if (download::DownloadItem *download = findDownloadById(downloadId))
         download->Cancel(/* user_cancel */ true);
 }
 
 void DownloadManagerDelegateQt::pauseDownload(quint32 downloadId)
 {
-    content::DownloadManager* dlm = content::BrowserContext::GetDownloadManager(m_profileAdapter->profile());
-    download::DownloadItem *download = dlm->GetDownload(downloadId);
-    if (download)
+    if (download::DownloadItem *download = findDownloadById(downloadId))
         download->Pause();
 }
 
 void DownloadManagerDelegateQt::resumeDownload(quint32 downloadId)
 {
-    content::DownloadManager* dlm = content::BrowserContext::GetDownloadManager(m_profileAdapter->profile());
-    download::DownloadItem *download = dlm->GetDownload(downloadId);
-    if (download)
+    if (download::DownloadItem *download = findDownloadById(downloadId))
         download->Resume();
 }
 
+void DownloadManagerDelegateQt::removeDownload(quint32 downloadId)
+{
+    if (download::DownloadItem *download = findDownloadById(downloadId))
+        download->Remove();
+}
+
 bool DownloadManagerDelegateQt::DetermineDownloadTarget(download::DownloadItem* item,
                                                         const content::DownloadTargetCallback& callback)
 {
diff --git a/src/core/download_manager_delegate_qt.h b/src/core/download_manager_delegate_qt.h
index fa5bd12ec..db965b12d 100644
--- a/src/core/download_manager_delegate_qt.h
+++ b/src/core/download_manager_delegate_qt.h
@@ -90,6 +90,7 @@ public:
     void cancelDownload(quint32 downloadId);
     void pauseDownload(quint32 downloadId);
     void resumeDownload(quint32 downloadId);
+    void removeDownload(quint32 downloadId);
 
     void markNextDownloadAsUserRequested() { m_nextDownloadIsUserRequested = true; }
 
@@ -99,6 +100,7 @@ public:
 
 private:
     void cancelDownload(const content::DownloadTargetCallback& callback);
+    download::DownloadItem *findDownloadById(quint32 downloadId);
     void savePackageDownloadCreated(download::DownloadItem *download);
     ProfileAdapter *m_profileAdapter;
 
diff --git a/src/core/profile_adapter.cpp b/src/core/profile_adapter.cpp
index 0375c8852..cee783118 100644
--- a/src/core/profile_adapter.cpp
+++ b/src/core/profile_adapter.cpp
@@ -209,6 +209,11 @@ void ProfileAdapter::resumeDownload(quint32 downloadId)
     downloadManagerDelegate()->resumeDownload(downloadId);
 }
 
+void ProfileAdapter::removeDownload(quint32 downloadId)
+{
+    downloadManagerDelegate()->removeDownload(downloadId);
+}
+
 ProfileAdapter *ProfileAdapter::createDefaultProfileAdapter()
 {
     return WebEngineContext::current()->createDefaultProfileAdapter();
diff --git a/src/core/profile_adapter.h b/src/core/profile_adapter.h
index 40633e802..9849d2788 100644
--- a/src/core/profile_adapter.h
+++ b/src/core/profile_adapter.h
@@ -100,6 +100,7 @@ public:
     void cancelDownload(quint32 downloadId);
     void pauseDownload(quint32 downloadId);
     void resumeDownload(quint32 downloadId);
+    void removeDownload(quint32 downloadId);
 
     ProfileQt *profile();
 
diff --git a/src/webengine/api/qquickwebenginedownloaditem.cpp b/src/webengine/api/qquickwebenginedownloaditem.cpp
index 4dce4ecd9..a80f163d5 100644
--- a/src/webengine/api/qquickwebenginedownloaditem.cpp
+++ b/src/webengine/api/qquickwebenginedownloaditem.cpp
@@ -615,6 +615,8 @@ QQuickWebEngineDownloadItem::QQuickWebEngineDownloadItem(QQuickWebEngineDownload
 
 QQuickWebEngineDownloadItem::~QQuickWebEngineDownloadItem()
 {
+    if (d_ptr->profile)
+        d_ptr->profile->d_ptr->profileAdapter()->removeDownload(d_ptr->downloadId);
 }
 
 QT_END_NAMESPACE
diff --git a/src/webenginewidgets/api/qwebenginedownloaditem.cpp b/src/webenginewidgets/api/qwebenginedownloaditem.cpp
index fc27e104d..55d4fcca8 100644
--- a/src/webenginewidgets/api/qwebenginedownloaditem.cpp
+++ b/src/webenginewidgets/api/qwebenginedownloaditem.cpp
@@ -141,8 +141,8 @@ static inline QWebEngineDownloadItem::DownloadInterruptReason toDownloadInterrup
     QWebEngineProfile being a long-lived object, it is in fact recommended that
     the application delete any items it is no longer interested in.
 
-    \note Deleting an item will not cancel a possible ongoing download. If that
-    is desirable, then cancel() must be called separately.
+    \note Deleting an item will also automatically cancel a download since 5.12.2,
+    but it is recommended to cancel manually before deleting for portability.
 
     \section2 Web Page Downloads
 
@@ -653,6 +653,8 @@ QWebEngineDownloadItem::QWebEngineDownloadItem(QWebEngineDownloadItemPrivate *p,
 */
 QWebEngineDownloadItem::~QWebEngineDownloadItem()
 {
+    if (auto profileAdapter = d_ptr->profile->profileAdapter())
+        profileAdapter->removeDownload(d_ptr->downloadId);
 }
 
 QT_END_NAMESPACE
diff --git a/tests/auto/shared/httpserver.cpp b/tests/auto/shared/httpserver.cpp
index 8d14c18ff..b85af9901 100644
--- a/tests/auto/shared/httpserver.cpp
+++ b/tests/auto/shared/httpserver.cpp
@@ -39,6 +39,7 @@ HttpServer::HttpServer(QObject *parent) : QObject(parent)
 bool HttpServer::start()
 {
     m_error = false;
+    m_expectingError = false;
 
     if (!m_tcpServer.listen()) {
         qCWarning(gHttpServerLog).noquote() << m_tcpServer.errorString();
@@ -55,7 +56,12 @@ bool HttpServer::start()
 bool HttpServer::stop()
 {
     m_tcpServer.close();
-    return !m_error;
+    return m_error == m_expectingError;
+}
+
+void HttpServer::setExpectError(bool b)
+{
+    m_expectingError = b;
 }
 
 QUrl HttpServer::url(const QString &path) const
diff --git a/tests/auto/shared/httpserver.h b/tests/auto/shared/httpserver.h
index ddbab433c..b4649244e 100644
--- a/tests/auto/shared/httpserver.h
+++ b/tests/auto/shared/httpserver.h
@@ -68,6 +68,8 @@ public:
     // Stops listening and performs final error checks.
     Q_REQUIRED_RESULT bool stop();
 
+    void setExpectError(bool b);
+
     // Full URL for given relative path
     QUrl url(const QString &path = QStringLiteral("/")) const;
 
@@ -82,6 +84,7 @@ private:
     QTcpServer m_tcpServer;
     QUrl m_url;
     bool m_error = false;
+    bool m_expectingError = false;
 };
 
 #endif // !HTTPSERVER_H
diff --git a/tests/auto/widgets/qwebenginedownloaditem/tst_qwebenginedownloaditem.cpp b/tests/auto/widgets/qwebenginedownloaditem/tst_qwebenginedownloaditem.cpp
index 68c549540..2af818928 100644
--- a/tests/auto/widgets/qwebenginedownloaditem/tst_qwebenginedownloaditem.cpp
+++ b/tests/auto/widgets/qwebenginedownloaditem/tst_qwebenginedownloaditem.cpp
@@ -34,6 +34,7 @@
 #include <QWebEngineDownloadItem>
 #include <QWebEnginePage>
 #include <QWebEngineProfile>
+#include <QWebEngineSettings>
 #include <QWebEngineView>
 #include <httpserver.h>
 
@@ -67,6 +68,7 @@ private Q_SLOTS:
     void downloadViaSetUrl();
     void downloadFileNot1();
     void downloadFileNot2();
+    void downloadDeleted();
 
 private:
     void saveLink(QPoint linkPos);
@@ -81,7 +83,8 @@ private:
     QWebEngineProfile *m_profile;
     QWebEnginePage *m_page;
     QWebEngineView *m_view;
-    QSet<QWebEngineDownloadItem *> m_downloads;
+    QSet<QWebEngineDownloadItem *> m_requestedDownloads;
+    QSet<QWebEngineDownloadItem *> m_finishedDownloads;
 };
 
 class ScopedConnection {
@@ -100,13 +103,15 @@ void tst_QWebEngineDownloadItem::initTestCase()
     m_server = new HttpServer();
     m_profile = new QWebEngineProfile;
     m_profile->setHttpCacheType(QWebEngineProfile::NoCache);
+    m_profile->settings()->setAttribute(QWebEngineSettings::AutoLoadIconsForPage, false);
     connect(m_profile, &QWebEngineProfile::downloadRequested, [this](QWebEngineDownloadItem *item) {
-        m_downloads.insert(item);
+        m_requestedDownloads.insert(item);
         connect(item, &QWebEngineDownloadItem::destroyed, [this, item](){
-            m_downloads.remove(item);
+            m_requestedDownloads.remove(item);
+            m_finishedDownloads.remove(item);
         });
         connect(item, &QWebEngineDownloadItem::finished, [this, item](){
-            m_downloads.remove(item);
+            m_finishedDownloads.insert(item);
         });
     });
     m_page = new QWebEnginePage(m_profile);
@@ -123,7 +128,11 @@ void tst_QWebEngineDownloadItem::init()
 
 void tst_QWebEngineDownloadItem::cleanup()
 {
-    QCOMPARE(m_downloads.count(), 0);
+    for (QWebEngineDownloadItem *item : m_finishedDownloads) {
+        item->deleteLater();
+    }
+    QTRY_COMPARE(m_requestedDownloads.count(), 0);
+    QCOMPARE(m_finishedDownloads.count(), 0);
     QVERIFY(m_server->stop());
 }
 
@@ -778,5 +787,30 @@ void tst_QWebEngineDownloadItem::downloadFileNot2()
     QCOMPARE(downloadItem->state(), QWebEngineDownloadItem::DownloadCancelled);
 }
 
+void tst_QWebEngineDownloadItem::downloadDeleted()
+{
+    QPointer<QWebEngineDownloadItem> downloadItem;
+    m_server->setExpectError(true);
+    int downloadCount = 0;
+    int finishedCount = 0;
+    ScopedConnection sc2 = connect(m_profile, &QWebEngineProfile::downloadRequested, [&](QWebEngineDownloadItem *item) {
+        QVERIFY(item);
+        QCOMPARE(item->state(), QWebEngineDownloadItem::DownloadRequested);
+        downloadItem = item;
+        connect(downloadItem, &QWebEngineDownloadItem::finished, [&]() {
+            finishedCount++;
+        });
+        item->accept();
+        downloadCount++;
+    });
+
+    m_page->download(m_server->url(QByteArrayLiteral("/file")));
+    QTRY_COMPARE(downloadCount, 1);
+    QVERIFY(downloadItem);
+    QCOMPARE(finishedCount, 0);
+    downloadItem->deleteLater();
+    QTRY_COMPARE(finishedCount, 1);
+}
+
 QTEST_MAIN(tst_QWebEngineDownloadItem)
 #include "tst_qwebenginedownloaditem.moc"