Bug 1116629: Cancelling an in-progress download fails to remove the temporary file on windows. r=Unfocused
authorDave Townsend <dtownsend@oxymoronical.com>
Tue, 30 Dec 2014 16:25:44 -0800
changeset 221770 355be9b24453e69c64f8b35900cd170af777fc0d
parent 221731 4a7b2b24738a75a4d0a06f28a7cf9473f6295157
child 221771 d3c44062ebbb2606978acdf898b407dfbe9146ff
push id28041
push userkwierso@gmail.com
push dateThu, 01 Jan 2015 00:53:12 +0000
treeherdermozilla-central@3c296aa11c51 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersUnfocused
bugs1116629
milestone37.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1116629: Cancelling an in-progress download fails to remove the temporary file on windows. r=Unfocused When attempting to cancel the install we must wait for the channel to close before attempting to delete the temporary file or it will be locked on windows.
toolkit/mozapps/extensions/internal/XPIProvider.jsm
toolkit/mozapps/extensions/test/xpcshell/test_install.js
toolkit/mozapps/extensions/test/xpcshell/test_install_strictcompat.js
--- a/toolkit/mozapps/extensions/internal/XPIProvider.jsm
+++ b/toolkit/mozapps/extensions/internal/XPIProvider.jsm
@@ -5006,18 +5006,21 @@ AddonInstall.prototype = {
   /**
    * Cancels installation of this add-on.
    *
    * @throws if installation cannot be cancelled from the current state
    */
   cancel: function AI_cancel() {
     switch (this.state) {
     case AddonManager.STATE_DOWNLOADING:
-      if (this.channel)
+      if (this.channel) {
+        logger.debug("Cancelling download of " + this.sourceURI.spec);
         this.channel.cancel(Cr.NS_BINDING_ABORTED);
+      }
+      break;
     case AddonManager.STATE_AVAILABLE:
     case AddonManager.STATE_DOWNLOADED:
       logger.debug("Cancelling download of " + this.sourceURI.spec);
       this.state = AddonManager.STATE_CANCELLED;
       XPIProvider.removeActiveInstall(this);
       AddonManagerPrivate.callInstallListeners("onDownloadCancelled",
                                                this.listeners, this.wrapper);
       this.removeTemporaryFile();
@@ -5494,18 +5497,30 @@ AddonInstall.prototype = {
    * @see nsIStreamListener
    */
   onStopRequest: function AI_onStopRequest(aRequest, aContext, aStatus) {
     this.stream.close();
     this.channel = null;
     this.badCerthandler = null;
     Services.obs.removeObserver(this, "network:offline-about-to-go-offline");
 
-    // If the download was cancelled then all events will have already been sent
+    // If the download was cancelled then update the state and send events
     if (aStatus == Cr.NS_BINDING_ABORTED) {
+      if (this.state == AddonManager.STATE_DOWNLOADING) {
+        logger.debug("Cancelled download of " + this.sourceURI.spec);
+        this.state = AddonManager.STATE_CANCELLED;
+        XPIProvider.removeActiveInstall(this);
+        AddonManagerPrivate.callInstallListeners("onDownloadCancelled",
+                                                 this.listeners, this.wrapper);
+        // If a listener restarted the download then there is no need to
+        // remove the temporary file
+        if (this.state != AddonManager.STATE_CANCELLED)
+          return;
+      }
+
       this.removeTemporaryFile();
       if (this.restartDownload)
         this.openChannel();
       return;
     }
 
     logger.debug("Download of " + this.sourceURI.spec + " completed.");
 
--- a/toolkit/mozapps/extensions/test/xpcshell/test_install.js
+++ b/toolkit/mozapps/extensions/test/xpcshell/test_install.js
@@ -559,27 +559,31 @@ function run_test_9() {
       install.install();
     });
   }, "application/x-xpinstall", null, "Real Test 4", null, "1.0");
 }
 
 function check_test_9(install) {
   prepare_test({}, [
     "onDownloadCancelled"
-  ]);
+  ], function() {
+    let file = install.file;
+
+    // Allow the file removal to complete
+    do_execute_soon(function() {
+      AddonManager.getAllInstalls(function(activeInstalls) {
+        do_check_eq(activeInstalls.length, 0);
+        do_check_false(file.exists());
+
+        run_test_10();
+      });
+    });
+  });
 
   install.cancel();
-
-  ensure_test_completed();
-
-  AddonManager.getAllInstalls(function(activeInstalls) {
-    do_check_eq(activeInstalls.length, 0);
-
-    run_test_10();
-  });
 }
 
 // Tests that after cancelling a pending install it is removed from the active
 // installs
 function run_test_10() {
   prepare_test({ }, [
     "onNewInstall"
   ]);
@@ -1008,38 +1012,41 @@ function run_test_14() {
     ], check_test_14);
     install.install();
   }, "application/x-xpinstall");
 }
 
 function check_test_14(install) {
   prepare_test({ }, [
     "onDownloadCancelled"
-  ]);
-
-  install.cancel();
-
-  ensure_test_completed();
+  ], function() {
+    let file = install.file;
 
-  install.addListener({
-    onDownloadProgress: function() {
-      do_throw("Download should not have continued");
-    },
-    onDownloadEnded: function() {
-      do_throw("Download should not have continued");
-    }
+    install.addListener({
+      onDownloadProgress: function() {
+        do_throw("Download should not have continued");
+      },
+      onDownloadEnded: function() {
+        do_throw("Download should not have continued");
+      }
+    });
+
+    // Allow the listener to return to see if it continues downloading. The
+    // The listener only really tests if we give it time to see progress, the
+    // file check isn't ideal either
+    do_execute_soon(function() {
+      do_check_false(file.exists());
+
+      run_test_15();
+    });
   });
 
-  // Allow the listener to return to see if it continues downloading. The
-  // The listener only really tests if we give it time to see progress, the
-  // file check isn't ideal either
+  // Wait for the channel to be ready to cancel
   do_execute_soon(function() {
-    do_check_eq(install.file, null);
-
-    run_test_15();
+    install.cancel();
   });
 }
 
 // Checks that cancelling the install from onDownloadEnded actually cancels it
 function run_test_15() {
   prepare_test({ }, [
     "onNewInstall"
   ]);
@@ -1628,17 +1635,20 @@ function check_test_27(aInstall) {
     ]
   }, [
     "onDownloadStarted",
     "onDownloadEnded",
     "onInstallStarted",
     "onInstallEnded"
   ], finish_test_27);
 
+  let file = aInstall.file;
   aInstall.install();
+  do_check_neq(file.path, aInstall.file.path);
+  do_check_false(file.exists());
 }
 
 function finish_test_27(aInstall) {
   prepare_test({
     "addon3@tests.mozilla.org": [
       "onOperationCancelled"
     ]
   }, [
--- a/toolkit/mozapps/extensions/test/xpcshell/test_install_strictcompat.js
+++ b/toolkit/mozapps/extensions/test/xpcshell/test_install_strictcompat.js
@@ -548,27 +548,31 @@ function run_test_9() {
       install.install();
     });
   }, "application/x-xpinstall", null, "Real Test 4", null, "1.0");
 }
 
 function check_test_9(install) {
   prepare_test({}, [
     "onDownloadCancelled"
-  ]);
+  ], function() {
+    let file = install.file;
+
+    // Allow the file removal to complete
+    do_execute_soon(function() {
+      AddonManager.getAllInstalls(function(activeInstalls) {
+        do_check_eq(activeInstalls.length, 0);
+        do_check_false(file.exists());
+
+        run_test_10();
+      });
+    });
+  });
 
   install.cancel();
-
-  ensure_test_completed();
-
-  AddonManager.getAllInstalls(function(activeInstalls) {
-    do_check_eq(activeInstalls.length, 0);
-
-    run_test_10();
-  });
 }
 
 // Tests that after cancelling a pending install it is removed from the active
 // installs
 function run_test_10() {
   prepare_test({ }, [
     "onNewInstall"
   ]);
@@ -999,38 +1003,41 @@ function run_test_14() {
     ], check_test_14);
     install.install();
   }, "application/x-xpinstall");
 }
 
 function check_test_14(install) {
   prepare_test({ }, [
     "onDownloadCancelled"
-  ]);
-
-  install.cancel();
-
-  ensure_test_completed();
+  ], function() {
+    let file = install.file;
 
-  install.addListener({
-    onDownloadProgress: function() {
-      do_throw("Download should not have continued");
-    },
-    onDownloadEnded: function() {
-      do_throw("Download should not have continued");
-    }
+    install.addListener({
+      onDownloadProgress: function() {
+        do_throw("Download should not have continued");
+      },
+      onDownloadEnded: function() {
+        do_throw("Download should not have continued");
+      }
+    });
+
+    // Allow the listener to return to see if it continues downloading. The
+    // The listener only really tests if we give it time to see progress, the
+    // file check isn't ideal either
+    do_execute_soon(function() {
+      do_check_false(file.exists());
+
+      run_test_15();
+    });
   });
 
-  // Allow the listener to return to see if it continues downloading. The
-  // The listener only really tests if we give it time to see progress, the
-  // file check isn't ideal either
+  // Wait for the channel to be ready to cancel
   do_execute_soon(function() {
-    do_check_eq(install.file, null);
-
-    run_test_15();
+    install.cancel();
   });
 }
 
 // Checks that cancelling the install from onDownloadEnded actually cancels it
 function run_test_15() {
   prepare_test({ }, [
     "onNewInstall"
   ]);
@@ -1619,17 +1626,20 @@ function check_test_27(aInstall) {
     ]
   }, [
     "onDownloadStarted",
     "onDownloadEnded",
     "onInstallStarted",
     "onInstallEnded"
   ], finish_test_27);
 
+  let file = aInstall.file;
   aInstall.install();
+  do_check_neq(file.path, aInstall.file.path);
+  do_check_false(file.exists());
 }
 
 function finish_test_27(aInstall) {
   prepare_test({
     "addon3@tests.mozilla.org": [
       "onOperationCancelled"
     ]
   }, [