Bug 2066435 - delete downloaded support files when download is blocked r=mak

Pass in the path to the `filesFolder` to the `nsITransfer` so it can delete that folder if the download gets blocked or removed. Differential Revision: https://phabricator.services.mozilla.com/D321684
2026-09-10 02:19:43 +00:00
parent 0accc4119a
commit 0fd9b6ff4e
10 changed files with 220 additions and 32 deletions
@@ -150,3 +150,9 @@ support-files = [
  "browser_print_pdf_content_analysis_impl.js",
  "file_pdf.pdf",
 ]
+
+["browser_save_page_complete_content_analysis.js"]
+support-files = [
+  "save_page_complete.css",
+  "save_page_complete.html",
+]
@@ -0,0 +1,117 @@
+/* Any copyright is dedicated to the Public Domain.
+   http://creativecommons.org/publicdomain/zero/1.0/ */
+
+"use strict";
+
+let MockFilePicker = SpecialPowers.MockFilePicker;
+let mockCA;
+
+const TEST_URL =
+  "https://example.com/browser/toolkit/components/contentanalysis/tests/browser/save_page_complete.html";
+
+let tempDir;
+
+add_setup(async function test_setup() {
+  mockCA = await mockContentAnalysisService(makeMockContentAnalysis());
+
+  tempDir = Services.dirsvc.get("TmpD", Ci.nsIFile);
+  tempDir.append("test-save-page-complete-dir");
+  tempDir.createUnique(Ci.nsIFile.DIRECTORY_TYPE, 0o755);
+
+  MockFilePicker.init();
+  MockFilePicker.returnValue = MockFilePicker.returnOK;
+  MockFilePicker.displayDirectory = tempDir;
+
+  registerCleanupFunction(async function () {
+    MockFilePicker.cleanup();
+    let list = await Downloads.getList(Downloads.ALL);
+    await list.removeFinished();
+    tempDir.remove(true);
+  });
+});
+
+function createPromiseForFilePicker() {
+  return new Promise(resolve => {
+    MockFilePicker.showCallback = fp => {
+      let destFile = tempDir.clone();
+      destFile.append(fp.defaultString);
+      let filesFolder = tempDir.clone();
+      filesFolder.append(fp.defaultString.replace(/\.[^.]*$/, "") + "_files");
+      MockFilePicker.setFiles([destFile]);
+      MockFilePicker.filterIndex = 0; // kSaveAsType_Complete
+      resolve({ destFile, filesFolder });
+    };
+  });
+}
+
+function promiseDownloadFinished(list) {
+  return new Promise(resolve => {
+    list.addView({
+      onDownloadChanged(download) {
+        download.launchWhenSucceeded = false;
+        if (download.succeeded || download.error) {
+          list.removeView(this);
+          resolve(download);
+        }
+      },
+    });
+  });
+}
+
+async function testSavePageComplete(allow) {
+  mockCA.setupForTest(allow);
+  await SpecialPowers.pushPrefEnv({
+    set: [
+      ["browser.contentanalysis.interception_point.download.enabled", true],
+    ],
+  });
+
+  await BrowserTestUtils.withNewTab({ gBrowser, url: TEST_URL }, async () => {
+    let downloadList = await Downloads.getList(Downloads.ALL);
+    let filePickerShown = createPromiseForFilePicker();
+    let downloadFinishedPromise = promiseDownloadFinished(downloadList);
+
+    saveBrowser(gBrowser.selectedBrowser);
+
+    info("Waiting for a filename to be picked from the file picker");
+    let { destFile, filesFolder } = await filePickerShown;
+    let download = await downloadFinishedPromise;
+
+    is(mockCA.calls.length, 1, "Content Analysis called once");
+    is(download.target.path, destFile.path, "Download saved to picked file");
+    is(
+      download.target.filesFolderPath,
+      filesFolder.path,
+      "Download knows about its files folder"
+    );
+    is(
+      await IOUtils.exists(download.target.path),
+      allow,
+      "Target file existence"
+    );
+    is(await IOUtils.exists(filesFolder.path), allow, "Files folder existence");
+
+    if (allow) {
+      ok(!download.error, "Download should not have an error");
+      await IOUtils.remove(download.target.path);
+      await IOUtils.remove(filesFolder.path, { recursive: true });
+    } else {
+      ok(
+        download.error.becauseBlockedByContentAnalysis,
+        "Download blocked by content analysis"
+      );
+    }
+
+    await downloadList.removeFinished();
+  });
+
+  await SpecialPowers.popPrefEnv();
+}
+
+add_task(async function testSavePageCompleteAllow() {
+  await testSavePageComplete(true);
+});
+
+add_task(async function testSavePageCompleteBlock() {
+  await testSavePageComplete(false);
+});
@@ -0,0 +1,4 @@
+p {
+  /* stylelint-disable-next-line stylelint-plugin-mozilla/use-design-tokens */
+  color: red;
+}
@@ -0,0 +1,11 @@
+<!DOCTYPE html>
+<html>
+  <head>
+    <meta charset="utf-8">
+    <title>Test save page complete</title>
+    <link rel="stylesheet" href="save_page_complete.css">
+  </head>
+  <body>
+    <p>This page has an external stylesheet that is saved alongside it.</p>
+  </body>
+</html>
@@ -1883,6 +1883,14 @@ DownloadTarget.prototype = {
   */
  partFilePath: null,

+  /**
+   * String containing the path of the directory holding the additional files
+   * of a download that involves multiple files, like a complete web page saved
+   * to disk, or null if the download has no such directory. When the data of
+   * the download is removed, this directory is removed as well.
+   */
+  filesFolderPath: null,
+
  /**
   * Indicates whether the target file exists.
   *
@@ -1964,11 +1972,19 @@ DownloadTarget.prototype = {
   */
  toSerializable() {
    // Simplify the representation if we don't have other details.
-    if (!this.partFilePath && !this._unknownProperties) {
+    if (
+      !this.partFilePath &&
+      !this.filesFolderPath &&
+      !this._unknownProperties
+    ) {
      return this.path;
    }

-    let serializable = { path: this.path, partFilePath: this.partFilePath };
+    let serializable = {
+      path: this.path,
+      partFilePath: this.partFilePath,
+      filesFolderPath: this.filesFolderPath,
+    };
    serializeUnknownProperties(this, serializable);
    return serializable;
  },
@@ -1984,6 +2000,9 @@ DownloadTarget.prototype = {
 *        {
 *          path: String containing the path of the target file.
 *          partFilePath: optional string containing the part file path.
+ *          filesFolderPath: optional string containing the path of the
+ *                           directory holding the additional files of the
+ *                           download.
 *        }
 *
 * @return The newly created DownloadTarget object.
@@ -2003,11 +2022,17 @@ DownloadTarget.fromSerializable = function (aSerializable) {
    if ("partFilePath" in aSerializable) {
      target.partFilePath = aSerializable.partFilePath;
    }
+    if ("filesFolderPath" in aSerializable) {
+      target.filesFolderPath = aSerializable.filesFolderPath;
+    }

    deserializeUnknownProperties(
      target,
      aSerializable,
-      property => property != "path" && property != "partFilePath"
+      property =>
+        property != "path" &&
+        property != "partFilePath" &&
+        property != "filesFolderPath"
    );
  }
  return target;
@@ -2954,9 +2979,9 @@ DownloadCopySaver.prototype = {
   */
  async removeData(canRemoveFinalTarget = false) {
    // Defined inline so removeData can be shared with DownloadLegacySaver.
-    async function _tryToRemoveFile(path) {
+    async function _tryToRemoveFile(path, recursive = false) {
      try {
-        await IOUtils.remove(path);
+        await IOUtils.remove(path, { recursive });
      } catch (ex) {
        // On Windows we may get an access denied error instead of a no such
        // file error if the file existed before, and was recently deleted. This
@@ -2978,6 +3003,12 @@ DownloadCopySaver.prototype = {
        (await isPlaceholder(this.download.target.path))
      ) {
        await _tryToRemoveFile(this.download.target.path);
+        // A download that saved a complete web page also created a directory
+        // holding the additional files of the page, that has to be removed
+        // along with the main file.
+        if (this.download.target.filesFolderPath) {
+          await _tryToRemoveFile(this.download.target.filesFolderPath, true);
+        }
      }
      this.download.target.exists = false;
      this.download.target.size = 0;
@@ -265,7 +265,8 @@ DownloadLegacyTransfer.prototype = {
    aIsPrivate,
    aDownloadClassification,
    aReferrerInfo,
-    aOpenDownloadsListOnStart
+    aOpenDownloadsListOnStart,
+    aFilesFolder
  ) {
    return this._nsITransferInitInternal(
      aSource,
@@ -279,7 +280,8 @@ DownloadLegacyTransfer.prototype = {
      aIsPrivate,
      aDownloadClassification,
      aReferrerInfo,
-      aOpenDownloadsListOnStart
+      aOpenDownloadsListOnStart,
+      aFilesFolder
    );
  },

@@ -321,6 +323,7 @@ DownloadLegacyTransfer.prototype = {
      aDownloadClassification,
      aReferrerInfo,
      aOpenDownloadsListOnStart,
+      null,
      userContextId,
      browsingContextId,
      aHandleInternally,
@@ -340,7 +343,8 @@ DownloadLegacyTransfer.prototype = {
    isPrivate,
    aDownloadClassification,
    referrerInfo,
-    openDownloadsListOnStart = true,
+    openDownloadsListOnStart,
+    filesFolder,
    userContextId = 0,
    browsingContextId = 0,
    handleInternally = false,
@@ -388,6 +392,7 @@ DownloadLegacyTransfer.prototype = {
      target: {
        path: aTarget.QueryInterface(Ci.nsIFileURL).file.path,
        partFilePath: aTempFile && aTempFile.path,
+        filesFolderPath: filesFolder && filesFolder.path,
      },
      saver: "legacy",
      launchWhenSucceeded,
@@ -338,6 +338,8 @@ function promiseStartLegacyDownload(aSourceUrl, aOptions) {
          persist,
          isPrivate,
          classification,
+          null,
+          false,
          null
        );
        persist.progressListener = transfer;
@@ -491,6 +491,25 @@ function internalPersist(persistArgs) {
  // Find the URI associated with the target file
  var targetFileURL = makeFileURI(persistArgs.targetFile);

+  // The local directory into which to save the files associated with a
+  // document, or null when only a single file is written.
+  var filesFolder = null;
+  if (
+    persistArgs.sourceDocument &&
+    persistArgs.targetContentType != "text/plain"
+  ) {
+    filesFolder = persistArgs.targetFile.clone();
+
+    var nameWithoutExtension = getFileBaseName(filesFolder.leafName);
+    // Given the minimal benefits, the "_files" suffix is intentionally not
+    // localized. Localizing it introduces complexity in handling OS filename
+    // length limits (e.g. bug 1959738) and risks breaking the folder-linking
+    // feature if an unsupported suffix is used.
+    var filesFolderLeafName = nameWithoutExtension + "_files";
+
+    filesFolder.leafName = filesFolderLeafName;
+  }
+
  // Create download and initiate it (below)
  var tr = Cc["@mozilla.org/transfer;1"].createInstance(Ci.nsITransfer);
  tr.init(
@@ -504,7 +523,9 @@ function internalPersist(persistArgs) {
    persist,
    persistArgs.isPrivate,
    Ci.nsITransfer.DOWNLOAD_ACCEPTABLE,
-    persistArgs.sourceReferrerInfo
+    persistArgs.sourceReferrerInfo,
+    false /* aOpenDownloadsListOnStart */,
+    filesFolder
  );
  persist.progressListener = new DownloadListener(window, tr);
  const { saveCompleteCallback } = persistArgs;
@@ -517,21 +538,6 @@ function internalPersist(persistArgs) {

  if (persistArgs.sourceDocument) {
    // Saving a Document, not a URI:
-    var filesFolder = null;
-    if (persistArgs.targetContentType != "text/plain") {
-      // Create the local directory into which to save associated files.
-      filesFolder = persistArgs.targetFile.clone();
-
-      var nameWithoutExtension = getFileBaseName(filesFolder.leafName);
-      // Given the minimal benefits, the "_files" suffix is intentionally not
-      // localized. Localizing it introduces complexity in handling OS filename
-      // length limits (e.g. bug 1959738) and risks breaking the folder-linking
-      // feature if an unsupported suffix is used.
-      var filesFolderLeafName = nameWithoutExtension + "_files";
-
-      filesFolder.leafName = filesFolderLeafName;
-    }
-
    var encodingFlags = 0;
    if (persistArgs.targetContentType == "text/plain") {
      encodingFlags |= nsIWBP.ENCODE_FLAGS_FORMATTED;
@@ -64,9 +64,14 @@ interface nsITransfer : nsIWebProgressListener2 {
     *
     * @param aReferrerInfo The Referrer this download is started with
     *
-     * @param aOpenDownloadsListOnStart true (default) - Open downloads panel.
+     * @param aOpenDownloadsListOnStart true - Open downloads panel.
     *                                  false - Only show an icon indicator.
-     *                                  This parameter is optional.
+     *
+     * @param aFilesFolder The directory holding the additional files of a
+     *                     transfer that writes more than one file, like a
+     *                     complete web page saved to disk. It is removed along
+     *                     with the target file when the data of the download is
+     *                     removed. May be null.
     */
    void init(in nsIURI aSource,
              in nsIURI aSourceOriginalURI,
@@ -79,7 +84,8 @@ interface nsITransfer : nsIWebProgressListener2 {
              in boolean aIsPrivate,
              in long aDownloadClassification,
              in nsIReferrerInfo aReferrerInfo,
-              [optional] in boolean aOpenDownloadsListOnStart);
+              in boolean aOpenDownloadsListOnStart,
+              in nsIFile aFilesFolder);

    /**
     * Same as init, but allows for passing the browsingContext
@@ -2442,10 +2442,10 @@ nsresult nsExternalAppHandler::CreateTransfer() {
        mDownloadClassification, referrerInfo, !mDialogShowing,
        mBrowsingContext, mHandleInternally, nullptr);
  } else {
-    rv = transfer->Init(mSourceUrl, nullptr, target, u""_ns, mMimeInfo,
-                        mTimeDownloadStarted, mTempFile, this,
-                        channel && NS_UsePrivateBrowsing(channel),
-                        mDownloadClassification, referrerInfo, !mDialogShowing);
+    rv = transfer->Init(
+        mSourceUrl, nullptr, target, u""_ns, mMimeInfo, mTimeDownloadStarted,
+        mTempFile, this, channel && NS_UsePrivateBrowsing(channel),
+        mDownloadClassification, referrerInfo, !mDialogShowing, nullptr);
  }
  mDialogShowing = false;

@@ -2539,7 +2539,7 @@ nsresult nsExternalAppHandler::CreateFailedTransfer() {
    rv = transfer->Init(mSourceUrl, nullptr, pseudoTarget, u""_ns, mMimeInfo,
                        mTimeDownloadStarted, mTempFile, this,
                        channel && NS_UsePrivateBrowsing(channel),
-                        mDownloadClassification, referrerInfo, true);
+                        mDownloadClassification, referrerInfo, true, nullptr);
  }
  NS_ENSURE_SUCCESS(rv, rv);