From 0c306ff01d6b173e9a49edf31aae3b9a947f8306 Mon Sep 17 00:00:00 2001 From: "sdwilsh@shawnwilsher.com" Date: Mon, 21 Jan 2008 19:43:02 -0800 Subject: [PATCH] Bug 410289 - Do not allow the pausing of downloads that cannot actually be resumed. r=Mardak --- .../downloads/public/nsIDownload.idl | 9 +- .../downloads/src/nsDownloadManager.cpp | 49 ++---- .../downloads/src/nsDownloadManager.h | 12 +- .../mozapps/downloads/downloads.properties | 2 + .../mozapps/downloads/content/download.xml | 6 +- .../mozapps/downloads/content/downloads.js | 17 +- .../downloads/tests/browser/Makefile.in | 1 + .../tests/browser/browser_bug_410289.js | 151 ++++++++++++++++++ 8 files changed, 198 insertions(+), 49 deletions(-) create mode 100644 toolkit/mozapps/downloads/tests/browser/browser_bug_410289.js diff --git a/toolkit/components/downloads/public/nsIDownload.idl b/toolkit/components/downloads/public/nsIDownload.idl index 771564f0a9c..d88060d3ea7 100644 --- a/toolkit/components/downloads/public/nsIDownload.idl +++ b/toolkit/components/downloads/public/nsIDownload.idl @@ -54,7 +54,7 @@ interface nsIMIMEInfo; * nsIDownloadManager::DOWNLOAD_FAILED * nsIDownloadManager::DOWNLOAD_CANCELED */ -[scriptable, uuid(4ee8befe-b05a-4ca2-9b30-6c47a9c4a622)] +[scriptable, uuid(c891111e-92a6-47b8-bc46-874ebb61ac9d)] interface nsIDownload : nsITransfer { /** @@ -133,6 +133,13 @@ interface nsIDownload : nsITransfer { * and can be null. */ readonly attribute nsIURI referrer; + + /** + * Indicates if the download can be resumed after being paused or not. This + * is only the case if the download is over HTTP/1.1 or FTP and if the + * server supports it. + */ + readonly attribute boolean resumable; }; %{C++ diff --git a/toolkit/components/downloads/src/nsDownloadManager.cpp b/toolkit/components/downloads/src/nsDownloadManager.cpp index 915a58f98e4..047d0baf391 100644 --- a/toolkit/components/downloads/src/nsDownloadManager.cpp +++ b/toolkit/components/downloads/src/nsDownloadManager.cpp @@ -214,7 +214,7 @@ nsDownloadManager::RemoveAllDownloads() nsRefPtr dl = mCurrentDownloads[0]; nsresult result; - if (dl->IsRealPaused()) + if (dl->IsPaused()) result = mCurrentDownloads.RemoveObject(dl); else result = CancelDownload(dl->mID); @@ -1617,10 +1617,7 @@ nsDownloadManager::Observe(nsISupports *aSubject, const PRUnichar *aData) { // Count active downloads that aren't real-paused - PRInt32 currDownloadCount = 0; - for (PRInt32 i = mCurrentDownloads.Count() - 1; i >= 0; --i) - if (!mCurrentDownloads[i]->IsRealPaused()) - currDownloadCount++; + PRInt32 currDownloadCount = mCurrentDownloads.Count(); nsresult rv; if (strcmp(aTopic, "oncancel") == 0) { @@ -2234,6 +2231,13 @@ nsDownload::GetReferrer(nsIURI **referrer) return NS_OK; } +NS_IMETHODIMP +nsDownload::GetResumable(PRBool *resumable) +{ + *resumable = IsResumable(); + return NS_OK; +} + //////////////////////////////////////////////////////////////////////////////// //// nsDownload Helper Functions @@ -2401,13 +2405,10 @@ nsDownload::SetProgressBytes(PRInt64 aCurrBytes, PRInt64 aMaxBytes) nsresult nsDownload::Pause() { - nsresult rv = NS_ERROR_FAILURE; - if (IsResumable()) - rv = Cancel(); - else if (mRequest) - rv = mRequest->Suspend(); - else - NS_NOTREACHED("We don't have a resumable download or a request to suspend??"); + if (!IsResumable()) + return NS_ERROR_UNEXPECTED; + + nsresult rv = Cancel(); NS_ENSURE_SUCCESS(rv, rv); return SetState(nsIDownloadManager::DOWNLOAD_PAUSED); @@ -2429,21 +2430,9 @@ nsDownload::Cancel() nsresult nsDownload::Resume() { - nsresult rv = NS_ERROR_FAILURE; - if (IsResumable()) - rv = RealResume(); - else if (mRequest) - rv = mRequest->Resume(); - else - NS_NOTREACHED("We don't have a resumable download or a request to resume??"); - NS_ENSURE_SUCCESS(rv, rv); + if (!IsPaused() || !IsResumable()) + return NS_ERROR_UNEXPECTED; - return SetState(nsIDownloadManager::DOWNLOAD_DOWNLOADING); -} - -nsresult -nsDownload::RealResume() -{ nsresult rv; nsCOMPtr wbp = do_CreateInstance("@mozilla.org/embedding/browser/nsWebBrowserPersist;1", &rv); @@ -2511,7 +2500,7 @@ nsDownload::RealResume() return rv; } - return NS_OK; + return SetState(nsIDownloadManager::DOWNLOAD_DOWNLOADING); } PRBool @@ -2532,12 +2521,6 @@ nsDownload::WasResumed() return mResumedAt != -1; } -PRBool -nsDownload::IsRealPaused() -{ - return IsPaused() && IsResumable(); -} - PRBool nsDownload::ShouldAutoResume() { diff --git a/toolkit/components/downloads/src/nsDownloadManager.h b/toolkit/components/downloads/src/nsDownloadManager.h index 0388c5cbd10..85ae4e096cb 100644 --- a/toolkit/components/downloads/src/nsDownloadManager.h +++ b/toolkit/components/downloads/src/nsDownloadManager.h @@ -276,15 +276,10 @@ protected: nsresult Cancel(); /** - * Resume the download. Works for both real-paused and fake-paused. + * Resume the download. */ nsresult Resume(); - /** - * Resume the real-paused download. Let Resume decide if this should get used. - */ - nsresult RealResume(); - /** * Download is not transferring? */ @@ -300,11 +295,6 @@ protected: */ PRBool WasResumed(); - /** - * Download is real-paused? (not fake-paused by stalling the channel) - */ - PRBool IsRealPaused(); - /** * Indicates if the download should try to automatically resume or not. */ diff --git a/toolkit/locales/en-US/chrome/mozapps/downloads/downloads.properties b/toolkit/locales/en-US/chrome/mozapps/downloads/downloads.properties index c352d8489d8..91fe93264a0 100644 --- a/toolkit/locales/en-US/chrome/mozapps/downloads/downloads.properties +++ b/toolkit/locales/en-US/chrome/mozapps/downloads/downloads.properties @@ -6,6 +6,8 @@ failed=Failed finished=Finished canceled=Canceled +cannotPause=This download cannot be paused + downloadErrorAlertTitle=Download Error downloadErrorGeneric=The download cannot be saved because an unknown error occurred.\n\nPlease try again. diff --git a/toolkit/mozapps/downloads/content/download.xml b/toolkit/mozapps/downloads/content/download.xml index c8a6278ddb6..b60577bcaff 100644 --- a/toolkit/mozapps/downloads/content/download.xml +++ b/toolkit/mozapps/downloads/content/download.xml @@ -99,8 +99,12 @@ diff --git a/toolkit/mozapps/downloads/content/downloads.js b/toolkit/mozapps/downloads/content/downloads.js index 1c5c17d6232..cb294086c29 100644 --- a/toolkit/mozapps/downloads/content/downloads.js +++ b/toolkit/mozapps/downloads/content/downloads.js @@ -78,6 +78,7 @@ var gUserInteracted = false; // bundle on startup. let gStr = { paused: "paused", + cannotPause: "cannotPause", statusFormat: "statusFormat2", transferSameUnits: "transferSameUnits", transferDiffUnits: "transferDiffUnits", @@ -613,6 +614,7 @@ var gDownloadViewController = { } let dl = aItem; + let download = null; // used for getting an nsIDownload object switch (aCommand) { case "cmd_cancel": @@ -621,11 +623,14 @@ var gDownloadViewController = { let file = getLocalFileFromNativePathOrUrl(dl.getAttribute("file")); return dl.openable && file.exists(); case "cmd_pause": - return dl.inProgress && !dl.paused; + download = gDownloadManager.getDownload(dl.getAttribute("dlid")); + return dl.inProgress && !dl.paused && download.resumable; case "cmd_pauseResume": - return dl.inProgress || dl.paused; + download = gDownloadManager.getDownload(dl.getAttribute("dlid")); + return (dl.inProgress || dl.paused) && download.resumable; case "cmd_resume": - return dl.paused; + download = gDownloadManager.getDownload(dl.getAttribute("dlid")); + return dl.paused && download.resumable; case "cmd_openReferrer": return dl.hasAttribute("referrer"); case "cmd_removeFromList": @@ -784,6 +789,12 @@ function updateButtons(aItem) let cmd = buttons[i].getAttribute("cmd"); let enabled = gDownloadViewController.isCommandEnabled(cmd, aItem); buttons[i].disabled = !enabled; + + if ("cmd_pause" == cmd && !enabled) { + // We need to add the tooltip indicating that the download cannot be + // paused now. + buttons[i].setAttribute("tooltiptext", gStr.cannotPause); + } } } diff --git a/toolkit/mozapps/downloads/tests/browser/Makefile.in b/toolkit/mozapps/downloads/tests/browser/Makefile.in index da50e7203ee..67681c87432 100644 --- a/toolkit/mozapps/downloads/tests/browser/Makefile.in +++ b/toolkit/mozapps/downloads/tests/browser/Makefile.in @@ -49,6 +49,7 @@ _BROWSER_FILES = \ browser_basic_functionality.js \ browser_bug_411172.js \ browser_bug_394039.js \ + browser_bug_410289.js \ $(NULL) ifneq (,$(filter cocoa, $(MOZ_WIDGET_TOOLKIT))) diff --git a/toolkit/mozapps/downloads/tests/browser/browser_bug_410289.js b/toolkit/mozapps/downloads/tests/browser/browser_bug_410289.js new file mode 100644 index 00000000000..3d8c8dd4794 --- /dev/null +++ b/toolkit/mozapps/downloads/tests/browser/browser_bug_410289.js @@ -0,0 +1,151 @@ +/* ***** BEGIN LICENSE BLOCK ***** + * Version: MPL 1.1/GPL 2.0/LGPL 2.1 + * + * The contents of this file are subject to the Mozilla Public License Version + * 1.1 (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * http://www.mozilla.org/MPL/ + * + * Software distributed under the License is distributed on an "AS IS" basis, + * WITHOUT WARRANTY OF ANY KIND, either express or implied. See the License + * for the specific language governing rights and limitations under the + * License. + * + * The Original Code is mozilla.org code. + * + * The Initial Developer of the Original Code is + * Mozilla Corporation. + * Portions created by the Initial Developer are Copyright (C) 2008 + * the Initial Developer. All Rights Reserved. + * + * Contributor(s): + * Shawn Wilsher (Original Author) + * + * Alternatively, the contents of this file may be used under the terms of + * either the GNU General Public License Version 2 or later (the "GPL"), or + * the GNU Lesser General Public License Version 2.1 or later (the "LGPL"), + * in which case the provisions of the GPL or the LGPL are applicable instead + * of those above. If you wish to allow use of your version of this file only + * under the terms of either the GPL or the LGPL, and not to allow others to + * use your version of this file under the terms of the MPL, indicate your + * decision by deleting the provisions above and replace them with the notice + * and other provisions required by the GPL or the LGPL. If you do not delete + * the provisions above, a recipient may use your version of this file under + * the terms of any one of the MPL, the GPL or the LGPL. + * + * ***** END LICENSE BLOCK ***** */ + +var ios = Cc["@mozilla.org/network/io-service;1"].getService(Ci.nsIIOService); +var dmFile = Cc["@mozilla.org/file/directory_service;1"]. + getService(Ci.nsIProperties).get("TmpD", Ci.nsIFile); +dmFile.append("dmuitest.file"); +dmFile.createUnique(Ci.nsIFile.NORMAL_FILE_TYPE, 0666); +var gTestPath = ios.newFileURI(dmFile).spec; + +const DownloadData = [ + { name: "Firefox 2.0.0.11.dmg", + source: "http://mozilla-mirror.naist.jp//firefox/releases/2.0.0.11/mac/en-US/Firefox%202.0.0.11.dmg", + target: gTestPath, + startTime: 1200185939538521, + endTime: 1200185939538521, + entityID: "%22216c12-1116bd8-440070d5d2700%22/17918936/Thu, 29 Nov 2007 01:15:40 GMT", + state: Ci.nsIDownloadManager.DOWNLOAD_DOWNLOADING, + currBytes: 0, maxBytes: -1, preferredAction: 0, autoResume: 0 }, + { name: "Firefox 2.0.0.11.dmg", + source: "http://mozilla-mirror.naist.jp//firefox/releases/2.0.0.11/mac/en-US/Firefox%202.0.0.11.dmg", + target: gTestPath, + startTime: 1200185939538520, + endTime: 1200185939538520, + entityID: "", + state: Ci.nsIDownloadManager.DOWNLOAD_DOWNLOADING, + currBytes: 0, maxBytes: -1, preferredAction: 0, autoResume: 0 } +]; + +function test_pauseButtonCorrect(aWin) +{ + // This also tests the ordering of the display + var doc = aWin.document; + + var dm = Cc["@mozilla.org/download-manager;1"]. + getService(Ci.nsIDownloadManager); + + var richlistbox = doc.getElementById("downloadView"); + for (var i = 0; i < DownloadData.length; i++) { + var dl = richlistbox.children[i]; + var buttons = dl.buttons; + for (var j = 0; j < buttons.length; j++) { + var button = buttons[j]; + if ("cmd_pause" == button.getAttribute("cmd")) { + var id = dl.getAttribute("dlid"); + + // check if it should be disabled or not + var resumable = dm.getDownload(id).resumable; + is(DownloadData[i].entityID ? true : false, resumable, + "Pause button is properly disabled"); + + // also check if tooltip text was updated + if (!resumable) { + var sb = doc.getElementById("downloadStrings"); + is(button.getAttribute("tooltiptext"), sb.getString("cannotPause"), + "Pause button has proper text"); + } + } + } + } +} + +var testFuncs = [ + test_pauseButtonCorrect +]; + +function test() +{ + var dm = Cc["@mozilla.org/download-manager;1"]. + getService(Ci.nsIDownloadManager); + var db = dm.DBConnection; + + // First, we populate the database with some fake data + db.executeSimpleSQL("DELETE FROM moz_downloads"); + var rawStmt = db.createStatement( + "INSERT INTO moz_downloads (name, source, target, startTime, endTime, " + + "state, currBytes, maxBytes, preferredAction, autoResume, entityID) " + + "VALUES (:name, :source, :target, :startTime, :endTime, :state, " + + ":currBytes, :maxBytes, :preferredAction, :autoResume, :entityID)"); + var stmt = Cc["@mozilla.org/storage/statement-wrapper;1"]. + createInstance(Ci.mozIStorageStatementWrapper) + stmt.initialize(rawStmt); + for each (var dl in DownloadData) { + for (var prop in dl) + stmt.params[prop] = dl[prop]; + + stmt.execute(); + } + stmt.statement.finalize(); + + // See if the DM is already open, and if it is, close it! + var wm = Cc["@mozilla.org/appshell/window-mediator;1"]. + getService(Ci.nsIWindowMediator); + var win = wm.getMostRecentWindow("Download:Manager"); + if (win) + win.close(); + + // OK, now that all the data is in, let's pull up the UI + Cc["@mozilla.org/download-manager-ui;1"]. + getService(Ci.nsIDownloadManagerUI).show(); + + // The window doesn't open once we call show, so we need to wait a little bit + function finishUp() { + var win = wm.getMostRecentWindow("Download:Manager"); + + // Now we can run our tests + for each (var t in testFuncs) + t(win); + + win.close(); + finish(); + } + + waitForExplicitFinish(); + // We also need to allow enough time for the DM to build up the whole list + window.setTimeout(finishUp, 2000); +}