Improve QWebEngineDownloadRequest::isSavePageDownload()

WebContentsImpl::IsSavable() decides which file formats are saveable,
but it allows this only for text-based files. All other files will go
around on the code path of downloading. MHTML (multipart) and PDF files
are saved like this, which means they aren't marked as isSavePageDownload.

Just simply set this flag if the save was requested by the user even if
it was a download.

This fixes the flag for saving MHTML and PDF files from context menu,
and leaves the flag false when a PDF download was initiated by the
button of the web UI.

Pick-to: 6.6
Task-number: QTBUG-114859
Change-Id: Id65f26a96952c5a43876338ad37f40570e544f3a
Reviewed-by: Allan Sandfeld Jensen <allan.jensen@qt.io>

4 files changed, 48 insertions, 19 deletions

diff --git a/src/core/download_manager_delegate_qt.cpp b/src/core/download_manager_delegate_qt.cpp
index 2838b59ff..36551bc29 100644
--- a/src/core/download_manager_delegate_qt.cpp
+++ b/src/core/download_manager_delegate_qt.cpp
@@ -102,10 +102,22 @@ bool DownloadManagerDelegateQt::DetermineDownloadTarget(download::DownloadItem *
         return true;
     }
 
+    bool isSavePageDownload = false;
+    WebContentsAdapterClient *adapterClient = nullptr;
+    if (content::WebContents *webContents = content::DownloadItemUtils::GetWebContents(item)) {
+        WebContentsDelegateQt *contentsDelegate = static_cast<WebContentsDelegateQt *>(webContents->GetDelegate());
+        adapterClient = contentsDelegate->adapterClient();
+        if (contentsDelegate->savePageInfo()) {
+            // We end up here when saving non text-based files (MHTML, PDF or images)
+            isSavePageDownload = true;
+            // Clear the delegate's SavePageInfo. It's only valid for the page currently being saved.
+            contentsDelegate->setSavePageInfo(nullptr);
+        }
+    }
+
     QString suggestedFilename = toQt(item->GetSuggestedFilename());
     QString mimeTypeString = toQt(item->GetMimeType());
 
-
     if (suggestedFilename.isEmpty())
         suggestedFilename = toQt(net::HttpContentDisposition(item->GetContentDisposition(), net::kCharsetLatin1).filename());
 
@@ -132,11 +144,6 @@ bool DownloadManagerDelegateQt::DetermineDownloadTarget(download::DownloadItem *
     item->AddObserver(this);
     QList<ProfileAdapterClient*> clients = m_profileAdapter->clients();
     if (!clients.isEmpty()) {
-        content::WebContents *webContents = content::DownloadItemUtils::GetWebContents(item);
-        WebContentsAdapterClient *adapterClient = nullptr;
-        if (webContents)
-            adapterClient = static_cast<WebContentsDelegateQt *>(webContents->GetDelegate())->adapterClient();
-
         Q_ASSERT(m_currentId == item->GetId());
         ProfileAdapterClient::DownloadItemInfo info = {
             item->GetId(),
@@ -150,7 +157,7 @@ bool DownloadManagerDelegateQt::DetermineDownloadTarget(download::DownloadItem *
             false /* accepted */,
             false /* paused */,
             false /* done */,
-            false /* isSavePageDownload */,
+            isSavePageDownload,
             item->GetLastReason(),
             adapterClient,
             suggestedFilename,
@@ -220,12 +227,18 @@ void DownloadManagerDelegateQt::ChooseSavePath(content::WebContents *web_content
     if (clients.isEmpty())
         return;
 
+    bool acceptedByDefault = false;
+    QString suggestedFilePath;
+    ProfileAdapterClient::SavePageFormat suggestedSaveFormat = ProfileAdapterClient::UnknownSavePageFormat;
     WebContentsDelegateQt *contentsDelegate = static_cast<WebContentsDelegateQt *>(
             web_contents->GetDelegate());
-    const SavePageInfo &spi = contentsDelegate->savePageInfo();
+    if (SavePageInfo *spi = contentsDelegate->savePageInfo()) {
+        suggestedFilePath = spi->requestedFilePath;
+        suggestedSaveFormat = static_cast<ProfileAdapterClient::SavePageFormat>(spi->requestedFormat);
+        // Clear the delegate's SavePageInfo. It's only valid for the page currently being saved.
+        contentsDelegate->setSavePageInfo(nullptr);
+    }
 
-    bool acceptedByDefault = false;
-    QString suggestedFilePath = spi.requestedFilePath;
     if (suggestedFilePath.isEmpty()) {
         suggestedFilePath = QFileInfo(toQt(suggested_path.AsUTF8Unsafe())).completeBaseName()
                 + QStringLiteral(".mhtml");
@@ -237,14 +250,9 @@ void DownloadManagerDelegateQt::ChooseSavePath(content::WebContents *web_content
         suggestedFilePath = downloadDir.absoluteFilePath(suggestedFilePath);
     }
 
-    ProfileAdapterClient::SavePageFormat suggestedSaveFormat
-            = static_cast<ProfileAdapterClient::SavePageFormat>(spi.requestedFormat);
     if (suggestedSaveFormat == ProfileAdapterClient::UnknownSavePageFormat)
         suggestedSaveFormat = ProfileAdapterClient::MimeHtmlSaveFormat;
 
-    // Clear the delegate's SavePageInfo. It's only valid for the page currently being saved.
-    contentsDelegate->setSavePageInfo(SavePageInfo());
-
     WebContentsAdapterClient *adapterClient = nullptr;
     if (web_contents)
         adapterClient = static_cast<WebContentsDelegateQt *>(web_contents->GetDelegate())->adapterClient();
diff --git a/src/core/web_contents_adapter.cpp b/src/core/web_contents_adapter.cpp
index 7e13064b3..31a3ab93c 100644
--- a/src/core/web_contents_adapter.cpp
+++ b/src/core/web_contents_adapter.cpp
@@ -754,7 +754,7 @@ void WebContentsAdapter::save(const QString &filePath, int savePageFormat)
 {
     CHECK_INITIALIZED();
     base::RecordAction(base::UserMetricsAction("SavePage"));
-    m_webContentsDelegate->setSavePageInfo(SavePageInfo(filePath, savePageFormat));
+    m_webContentsDelegate->setSavePageInfo(new SavePageInfo(filePath, savePageFormat));
     m_webContents->OnSavePage();
 }
 
diff --git a/src/core/web_contents_delegate_qt.h b/src/core/web_contents_delegate_qt.h
index d49380471..23115d126 100644
--- a/src/core/web_contents_delegate_qt.h
+++ b/src/core/web_contents_delegate_qt.h
@@ -139,8 +139,8 @@ public:
     void launchExternalURL(const QUrl &url, ui::PageTransition page_transition, bool is_main_frame, bool has_user_gesture);
     FindTextHelper *findTextHelper();
 
-    void setSavePageInfo(const SavePageInfo &spi) { m_savePageInfo = spi; }
-    const SavePageInfo &savePageInfo() { return m_savePageInfo; }
+    void setSavePageInfo(SavePageInfo *spi) { m_savePageInfo.reset(spi); }
+    SavePageInfo *savePageInfo() { return m_savePageInfo.get(); }
 
     WebEngineSettings *webEngineSettings() const;
     WebContentsAdapter *webContentsAdapter() const;
@@ -180,7 +180,7 @@ private:
 
     WebContentsAdapterClient *m_viewClient;
     QScopedPointer<FindTextHelper> m_findTextHelper;
-    SavePageInfo m_savePageInfo;
+    std::unique_ptr<SavePageInfo> m_savePageInfo;
     QSharedPointer<FilePickerController> m_filePickerController;
     LoadingState m_loadingState;
     FrameFocusedObserver m_frameFocusedObserver;
diff --git a/tests/auto/quick/qmltests/data/tst_save.qml b/tests/auto/quick/qmltests/data/tst_save.qml
index 9757f8fac..f07a5f212 100644
--- a/tests/auto/quick/qmltests/data/tst_save.qml
+++ b/tests/auto/quick/qmltests/data/tst_save.qml
@@ -14,6 +14,7 @@ TestWebEngineView {
     property int receivedBytes: 0
     property string downloadDir: ""
     property string downloadFileName: ""
+    property bool isSavePageDownload: false
     property var downloadState: []
     property int savePageFormat: WebEngineDownloadRequest.MimeHtmlSaveFormat;
 
@@ -42,6 +43,7 @@ TestWebEngineView {
             savePageFormat = download.savePageFormat
             downloadDir = download.downloadDirectory;
             downloadFileName = download.downloadFileName
+            isSavePageDownload = download.isSavePageDownload
         }
         onDownloadFinished: function(download) {
             receivedBytes = download.receivedBytes
@@ -73,6 +75,7 @@ TestWebEngineView {
             receivedBytes = 0
             downloadDir = ""
             downloadFileName = ""
+            isSavePageDownload = false
             downloadState = []
             downloadUrl = ""
         }
@@ -104,6 +107,7 @@ TestWebEngineView {
             compare(savePageFormat, saveFormat)
             compare(downloadDir, fileDir)
             compare(downloadFileName, fileName)
+            compare(isSavePageDownload, true)
             compare(downloadState[0], WebEngineDownloadRequest.DownloadInProgress)
             downloadFinishedSpy.wait()
             compare(downloadFinishedSpy.count, 1)
@@ -119,5 +123,22 @@ TestWebEngineView {
             verify(webEngineView.waitForLoadSucceeded())
             verify(verifyData())
         }
+
+        function test_saveImage() {
+            var fileDir = tempDir.path()
+            var fileName = "favicon.png"
+            var filePath = fileDir + "/"+ fileName
+
+            // Load an image
+            webEngineView.url = Qt.resolvedUrl("icons/favicon.png")
+            verify(webEngineView.waitForLoadSucceeded())
+
+            webEngineView.save(filePath)
+            downLoadRequestedSpy.wait()
+            compare(downLoadRequestedSpy.count, 1)
+            compare(downloadUrl, webEngineView.url)
+            compare(isSavePageDownload, true)
+            compare(downloadState[0], WebEngineDownloadRequest.DownloadRequested)
+        }
     }
 }