http://tinderbox.mozilla.org/showlog.cgi?log=Firefox/1274727181.1274727881.26342.gz Linux mozilla-central opt test mochitest-other on 2010/05/24 11:53:01 s: mv-moz2-linux-ix-slave08 Running chrome://mochikit/content/browser/browser/components/privatebrowsing/test/browser/browser_privatebrowsing_downloadmonitor.js... TEST-PASS | chrome://mochikit/content/browser/browser/components/privatebrowsing/test/browser/browser_privatebrowsing_downloadmonitor.js | The download panel should be successfully added initially TEST-INFO | chrome://mochikit/content/browser/browser/components/privatebrowsing/test/browser/browser_privatebrowsing_downloadmonitor.js | Console message: TEST-UNEXPECTED-FAIL | automation.py | application timed out after 330 seconds with no output

This is the same issue as bug 542928, only on a different test.

Actually, it seems that this was caused by bug 541739.

This patch makes the test handle the dialog that is now correctly displayed, and adds the necessary cleanup code for the downloads, and makes sure that the downloads are non-resumable, so that we won't have to deal with them resuming after leaving the private browsing mode.

Comment on attachment 447333 [details] [diff] [review] Patch (v1) >diff --git a/browser/components/privatebrowsing/test/browser/browser_privatebrowsing_downloadmonitor.js b/browser/components/privatebrowsing/test/browser/browser_privatebrowsing_downloadmonitor.js > setTimeout(function () { > setTimeout(function () { These should probably be changed to executeSoon(). > // cleanup > let dls = dm.activeDownloads; > while (dls.hasMoreElements()) { > let dl = dls.getNext().QueryInterface(Ci.nsIDownload); >+ dm.cancelDownload(dl.id); > dm.removeDownload(dl.id); > let file = dl.targetFile; > if (file.exists()) > file.remove(false); > } >+ persist.cancel(0); >+ persist2.cancel(0); > if (file.exists()) > file.remove(false); >+ if (file2.exists()) >+ file2.remove(false); Hrm, is this just belts-and-braces? I would have thought the while() loop would be sufficient for cleaning up. (The nsICancelable documentation frowns on passing in success codes, so perhaps you should use Cr.NS_ERROR_FAILURE.)

(In reply to comment #4) > (From update of attachment 447333 [details] [diff] [review]) > >diff --git a/browser/components/privatebrowsing/test/browser/browser_privatebrowsing_downloadmonitor.js b/browser/components/privatebrowsing/test/browser/browser_privatebrowsing_downloadmonitor.js > > > setTimeout(function () { > > > setTimeout(function () { > > These should probably be changed to executeSoon(). Done. > > // cleanup > > let dls = dm.activeDownloads; > > while (dls.hasMoreElements()) { > > let dl = dls.getNext().QueryInterface(Ci.nsIDownload); > >+ dm.cancelDownload(dl.id); > > dm.removeDownload(dl.id); > > let file = dl.targetFile; > > if (file.exists()) > > file.remove(false); > > } > >+ persist.cancel(0); > >+ persist2.cancel(0); > > if (file.exists()) > > file.remove(false); > >+ if (file2.exists()) > >+ file2.remove(false); > > Hrm, is this just belts-and-braces? I would have thought the while() loop would > be sufficient for cleaning up. (The nsICancelable documentation frowns on > passing in success codes, so perhaps you should use Cr.NS_ERROR_FAILURE.) Heh, these were added in another incarnation of this patch, but they're no longer necessary. I removed them all.