Time Elapsed is currently not updated during download and will show as 0 seconds. The end time is not persisted in the jsdownloads api and needs to be calculated during cell text gets. Bug 888915 also introduced some nits and problems in the code especially wrt to module loading which should be fixed.

Cleans up the module getters for the files changes in bug 888915. Previously a mix out of defineLazyScriptGetter (not exactly correct for modules but harmless), defineModuleGetter and Cu.import. Mostly aligned with Firefox for these now.

Fixes the Time Elapsed column and removes the unused DownloadsViewPrototype. I wanted to get rid of DownloadProgressListener.js and use the DownloadsView originally in Bug 888915 but decided to shelve this for a later day. Forgot to take the unused code out in the final patch. Also a small typo fix :)

Comment on attachment 8991379 [details] [diff] [review] 1474946-modulegetter.patch >+++ b/suite/components/downloads/DownloadsCommon.jsm >+ChromeUtils.import("resource://gre/modules/XPCOMUtils.jsm"); >+ChromeUtils.import("resource://gre/modules/Services.jsm"); >+ChromeUtils.import("resource://gre/modules/AppConstants.jsm"); Firefox does this using a lazyModuleGetter rather than an import... https://dxr.mozilla.org/comm-central/source/mozilla/browser/components/downloads/DownloadsCommon.jsm#41 r/a=me with that addressed

Comment on attachment 8991380 [details] [diff] [review] 1474946-DownloadsViewPrototype.patch LGTM r/a=me

> Firefox does this using a lazyModuleGetter rather than an import... Yes, actually this was intentional. Importing AppConstants.jsm is different all over the place in browser so I opted for importing it via ChromeUtils to be consistent. Just means it is loaded immediately. https://dxr.mozilla.org/comm-central/search?q=resource%3A%2F%2Fgre%2Fmodules%2FAppConstants.jsm&redirect=false What do you think. Should we switch it to always use lazyModuleGetter in the future, decide on a case by case base and/or match Firefox when 1:1.

(In reply to Frank-Rainer Grahl (:frg) from comment #5) > > Firefox does this using a lazyModuleGetter rather than an import... > > Yes, actually this was intentional. Importing AppConstants.jsm is different > all over the place in browser so I opted for importing it via ChromeUtils to > be consistent. Just means it is loaded immediately. > > https://dxr.mozilla.org/comm-central/ > search?q=resource%3A%2F%2Fgre%2Fmodules%2FAppConstants.jsm&redirect=false > > What do you think. Should we switch it to always use lazyModuleGetter in the > future, decide on a case by case base and/or match Firefox when 1:1. I cannot see anything in AppConstants.jsm that would be slow in loading, so let's go for the import.

Comment on attachment 8991380 [details] [diff] [review] 1474946-DownloadsViewPrototype.patch > case "TimeElapsed": >- if (dl.endTime && dl.startTime && (dl.endTime > dl.startTime)) { >- var seconds = (dl.endTime - dl.startTime) / 1000; >- var [time1, unit1, time2, unit2] = >+ // With no end time persisted in the downloads backend this is >+ // utterly useless unless the download is progressing. I thought we manually set the end time on the download in onDownloadChanged(); does that not work, at least until the end of the session?

> I thought we manually set the end time on the download in onDownloadChanged(); does that not work, at least until the end of the session? Without looking in the follow-up patches in Bug 888915 I probably killed it when I removed the remnants of nsIDownloadManager and switched most of it to the DownloadCommon module. I thought about re-adding it there in the onDownloadChanged(). The TimeElapsed update seems to be called twice a second same as OnDownloadChanged. I didn't see a performance penalty but might have been better.

Pushed by frgrahl@gmx.net: https://hg.mozilla.org/comm-central/rev/80ecd5ae900d Fix time elapsed display in the Download Manager and removed unused DownloadsViewPrototype. r=IanN https://hg.mozilla.org/comm-central/rev/42629705a38c Use right loader instead of LazyScriptGetter for modules. r=IanN

https://hg.mozilla.org/releases/comm-esr60/rev/6a8a8e12b2602be78d38b6c0dd8edf99cf945960 Fix time elapsed display in the Download Manager and removed unused DownloadsViewPrototype. r=IanN a=IanN https://hg.mozilla.org/releases/comm-esr60/rev/26741ef333e1f7da626f70ce6cddc4b758cd42c9 Use right loader instead of LazyScriptGetter for modules. r=IanN a=IanN