Bug 514823. r=Mardak, r=sdwilsh

toolkit/components/downloads/src/nsDownloadManager.cpp

@@ -2111,10 +2111,6 @@ nsDownload::SetState(DownloadState aState)
   PRInt16 oldState = mDownloadState;
   mDownloadState = aState;
 
-  nsresult rv;
-
-  nsCOMPtr<nsIPrefBranch> pref = do_GetService(NS_PREFSERVICE_CONTRACTID);
-
   // We don't want to lose access to our member variables
   nsRefPtr<nsDownload> kungFuDeathGrip = this;
 
@@ -2146,11 +2142,19 @@ nsDownload::SetState(DownloadState aState)
     case nsIDownloadManager::DOWNLOAD_FINISHED:
     {
       // Do what exthandler would have done if necessary
-      (void)ExecuteDesiredAction();
+      nsresult rv = ExecuteDesiredAction();
+      if (NS_FAILED(rv)) {
+        // We've failed to execute the desired action.  As a result, we should
+        // fail the download so the user can try again.
+        (void)FailDownload(rv, nsnull);
+        return rv;
+      }
 
       // Now that we're done with handling the download, clean it up
       Finalize();
 
+      nsCOMPtr<nsIPrefBranch> pref(do_GetService(NS_PREFSERVICE_CONTRACTID));
+
       // Master pref to control this function.
       PRBool showTaskbarAlert = PR_TRUE;
       if (pref)
@@ -2243,7 +2247,7 @@ nsDownload::SetState(DownloadState aState)
 
   // Before notifying the listener, we must update the database so that calls
   // to it work out properly.
-  rv = UpdateDB();
+  nsresult rv = UpdateDB();
   NS_ENSURE_SUCCESS(rv, rv);
 
   mDownloadManager->NotifyListenersOnDownloadStateChange(oldState, this);

uriloader/exthandler/nsExternalHelperAppService.cpp

@@ -1887,6 +1887,19 @@ nsresult nsExternalAppHandler::ExecuteDesiredAction()
         if (NS_SUCCEEDED(rv))
           rv = OpenWithApplication();
       }
+      else
+      {
+        // Cancel the download and report an error.  We do not want to end up in
+        // a state where it appears that we have a normal download that is
+        // pointing to a file that we did not actually create.
+        nsAutoString path;
+        mTempFile->GetPath(path);
+        SendStatusChange(kWriteError, rv, nsnull, path);
+        Cancel(rv);
+
+        // We still need to notify if we have a progress listener, so we cannot
+        // return at this point.
+      }
     }
     else // Various unknown actions go here too
     {
@@ -1899,7 +1912,7 @@ nsresult nsExternalAppHandler::ExecuteDesiredAction()
         gExtProtSvc->FixFilePermissions(destfile);
       }
     }
-    
+
     // Notify dialog that download is complete.
     // By waiting till this point, it ensures that the progress dialog doesn't indicate
     // success until we're really done.
@@ -1915,7 +1928,7 @@ nsresult nsExternalAppHandler::ExecuteDesiredAction()
         nsIWebProgressListener::STATE_IS_NETWORK, NS_OK);
     }
   }
-  
+
   return rv;
 }