Closed Bug 814700 Opened 11 years ago Closed 11 years ago
_private _resume .js fails on Birch
https://tbpl.mozilla.org/php/getParsedLog.php?id=17298431&tree=Birch&full=1 TEST-UNEXPECTED-FAIL | (xpcshell/head.js) | [Exception... "Component returned failure code: 0x80040111 (NS_ERROR_NOT_AVAILABLE) [nsIDownload.id]" nsresult: "0x80040111 (NS_ERROR_NOT_AVAILABLE)" location: "JS frame :: /home/cltbld/talos-slave/test/build/xpcshell/tests/toolkit/components/downloads/test/unit/head_download_manager.js :: addDownload :: line 122" data: no] Josh, is this a bug in the download manager patch?
Not a bug in the patch, just a change that is required in the implementation of adddDownload in head_download_manager.js. The id property of private downloads is not available when per-window is enabled, but the fix is harder than just obtaining the guid instead, since getDownloadByGUID is asynchronous: >120 // This will throw if it isn't found, and that would mean test failure, so no >121 // try catch block >122 var test = dm.getDownload(dl.id); I'm not certain what to do here. Maybe we can just eliminate this check. Marco?
> >120 // This will throw if it isn't found, and that would mean test failure, so no > >121 // try catch block > >122 var test = dm.getDownload(dl.id); > > I'm not certain what to do here. Maybe we can just eliminate this check. > Marco? You can add a check before, to do it only if not private. > if (!aParams.isPrivate) > var test = dm.getDownload(dl.id);
Yeah, makes sense.
(Now, this is the only failing xpcshell test on birch!)
I migrated the test, however with the current per-window PB mode the private downloads can't be resume, paused or cancel, so this test checks only that. There is already a test that checks the resume on a public window: http://mxr.mozilla.org/mozilla-central/source/toolkit/components/downloads/test/unit/test_resume.js
Why do you say that we cannot pause/resume/cancel private downloads in per-window mode? That should definitely not be happening!
(In reply to Ehsan Akhgari [:ehsan] from comment #6) > Why do you say that we cannot pause/resume/cancel private downloads in > per-window mode? That should definitely not be happening! My mistake, I checked that I was using Services.downloads.cancelDownload(download.id) and it's not possible to get the id from a private download: http://mxr.mozilla.org/mozilla-central/source/toolkit/components/downloads/nsDownloadManager.cpp#3262, so that fails. And also that I was trying to access cancel() and pause() before the download begins, so it fails too. I'll update the patch! Thanks!
Comment on attachment 686782 [details] [diff] [review] Patch v1 Cool! Clearing the request for now, then!
Fixed the patch, now it test pause and resume correctly. Also, I updated the head and related tests to use a common method.
Attachment #687142 - Flags: review?(ehsan) → review+
Landed and screwed up the commit message meanwhile! https://hg.mozilla.org/integration/mozilla-inbound/rev/31f4c1cb0e91
Status: NEW → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla20
You need to log in before you can comment on or make changes to this bug.