From 0b15a2b89e6615c5e68f75c2515fe0cdf49178e4 Mon Sep 17 00:00:00 2001 From: janekptacijarabaci Date: Sun, 29 Jul 2018 12:58:23 +0200 Subject: [PALEMOON] Bug 1129896 - Part 1 of 2 - Keep cached metadata for history downloads indefinitely --- .../downloads/content/allDownloadsViewOverlay.js | 145 ++++++++++----------- 1 file changed, 69 insertions(+), 76 deletions(-) (limited to 'application/palemoon/components/downloads') diff --git a/application/palemoon/components/downloads/content/allDownloadsViewOverlay.js b/application/palemoon/components/downloads/content/allDownloadsViewOverlay.js index 980913cfe..c86df36df 100644 --- a/application/palemoon/components/downloads/content/allDownloadsViewOverlay.js +++ b/application/palemoon/components/downloads/content/allDownloadsViewOverlay.js @@ -46,18 +46,16 @@ HistoryDownload.prototype = { /** * Pushes information from Places metadata into this object. */ - updateFromMetaData(aPlacesMetaData) { + updateFromMetaData(metaData) { try { this.target.path = Cc["@mozilla.org/network/protocol;1?name=file"] .getService(Ci.nsIFileProtocolHandler) - .getFileFromURLSpec(aPlacesMetaData. - targetFileURISpec).path; + .getFileFromURLSpec(metaData.targetFileSpec).path; } catch (ex) { this.target.path = undefined; } - try { - let metaData = JSON.parse(aPlacesMetaData.jsonDetails); + if ("state" in metaData) { this.succeeded = metaData.state == nsIDM.DOWNLOAD_FINISHED; this.error = metaData.state == nsIDM.DOWNLOAD_FAILED ? { message: "History download failed." } @@ -74,7 +72,7 @@ HistoryDownload.prototype = { // is refreshed, at which point these values may be updated. this.target.exists = true; this.target.size = metaData.fileSize; - } catch (ex) { + } else { // Metadata might be missing from a download that has started but hasn't // stopped already. Normally, this state is overridden with the one from // the corresponding in-progress session download. But if the browser is @@ -517,94 +515,82 @@ DownloadsPlacesView.prototype = { }, /** - * Map containing a metadata object for each download source URI found in - * Places annotations, or null if this cache must be rebuilt. - */ - _cachedPlacesMetaData: null, - - /** - * This method exists in order to optimize the first load of the Downloads - * View, when Places annotations for history downloads must be read. In fact, - * annotations are stored in a single table, and reading all of them at once - * is much more efficient than an individual query. + * This cache exists in order to optimize the load of the Downloads View, when + * Places annotations for history downloads must be read. In fact, annotations + * are stored in a single table, and reading all of them at once is much more + * efficient than an individual query. * - * When this metod is first called, after the Places result container has been - * invalidated, it reads the annotations for all the history downloads. + * When this property is first requested, it reads the annotations for all the + * history downloads and stores them indefinitely. * - * At this point, as metadata for the various elements is requested by the - * view, it is pulled from the cache and returned as an object. + * The historical annotations are not expected to change for the duration of + * the session, except in the case where a session download is running for the + * same URI as a history download. To ensure we don't use stale data, URIs + * corresponding to session downloads are permanently removed from the cache. + * This is a very small mumber compared to history downloads. * - * This works efficiently because the metadata is requested only for newly - * created element shells that contain a history download but don't have a - * matching session download, and at most one such element shell is created - * for each source URI. + * This property returns a Map from each download source URI found in Places + * annotations to an object with the format: * - * @param url - * URI string of the Places node for which metadata is requested. + * { targetFileSpec, state, endTime, fileSize, ... } * - * @return Object of the form { targetFileURISpec, jsonDetails }. - * Any of the properties may be missing from the object. + * The targetFileSpec property is the value of "downloads/destinationFileURI", + * while the other properties are taken from "downloads/metaData". Any of the + * properties may be missing from the object. */ - _getCachedPlacesMetaDataFor(url) { - if (!this._cachedPlacesMetaData) { - this._cachedPlacesMetaData = new Map(); + get _cachedPlacesMetaData() { + if (!this.__cachedPlacesMetaData) { + this.__cachedPlacesMetaData = new Map(); - // Use the destination file annotation to build the initial Map of items. + // Read the metadata annotations first, but ignore invalid JSON. for (let result of PlacesUtils.annotations.getAnnotationsWithName( - DESTINATION_FILE_URI_ANNO)) { - this._cachedPlacesMetaData.set(result.uri.spec, { - targetFileURISpec: result.annotationValue, - }); + DOWNLOAD_META_DATA_ANNO)) { + try { + this.__cachedPlacesMetaData.set(result.uri.spec, + JSON.parse(result.annotationValue)); + } catch (ex) {} } - // Add the string with the JSON details from the metadata annotation. + // Add the target file annotations to the metadata. for (let result of PlacesUtils.annotations.getAnnotationsWithName( - DOWNLOAD_META_DATA_ANNO)) { - let metadata = this._cachedPlacesMetaData.get(result.uri.spec); - - // The destination file annotation is expected to be present for all - // the downloads with the metadata annotation. If this is not the case, - // we simply ignore the item, as if it has no JSON details at all. - if (metadata) { - metadata.jsonDetails = result.annotationValue; + DESTINATION_FILE_URI_ANNO)) { + let metaData = this.__cachedPlacesMetaData.get(result.uri.spec); + if (!metaData) { + metaData = {}; + this.__cachedPlacesMetaData.set(result.uri.spec, metaData); } + metaData.targetFileSpec = result.annotationValue; } } - let metadata = this._cachedPlacesMetaData.get(url); - this._cachedPlacesMetaData.delete(url); - - // In the rare case where a history download is added again, but there is no - // corresponding session download, read the metadata from the database. This - // could potentially cause an inefficient database access for very old - // downloads with no metadata to begin with, but these should have been - // already deleted from history on a recent browser profile. - return metadata || this._getPlacesMetaDataFor(url); + return this.__cachedPlacesMetaData; }, + __cachedPlacesMetaData: null, /** - * Read the latest version of the Places metadata for the given URI. + * Reads current metadata from Places annotations for the specified URI, and + * returns an object with the format: * - * @param url - * URI string of the Places node for which metadata is requested. + * { targetFileSpec, state, endTime, fileSize, ... } * - * @return Object of the form { targetFileURISpec, jsonDetails }. - * Any of the properties may be missing from the object. + * The targetFileSpec property is the value of "downloads/destinationFileURI", + * while the other properties are taken from "downloads/metaData". Any of the + * properties may be missing from the object. */ - _getPlacesMetaDataFor(url) { - let metadata = {}; + _getPlacesMetaDataFor(spec) { + let metaData = {}; try { - let uri = NetUtil.newURI(url); - metadata = { - targetFileURISpec: PlacesUtils.annotations.getPageAnnotation( - uri, DESTINATION_FILE_URI_ANNO), - }; - metadata.jsonDetails = PlacesUtils.annotations.getPageAnnotation( - uri, DOWNLOAD_META_DATA_ANNO); + let uri = NetUtil.newURI(spec); + try { + metaData = JSON.parse(PlacesUtils.annotations.getPageAnnotation( + uri, DOWNLOAD_META_DATA_ANNO)); + } catch (ex) {} + metaData.targetFileSpec = PlacesUtils.annotations.getPageAnnotation( + uri, DESTINATION_FILE_URI_ANNO); } catch (ex) {} - return metadata; + return metaData; }, /** @@ -642,6 +628,18 @@ DownloadsPlacesView.prototype = { this._downloadElementsShellsForURI.set(downloadURI, shellsForURI); } + // When a session download is attached to a shell, we ensure not to keep + // stale metadata around for the corresponding history download. This + // prevents stale state from being used if the view is rebuilt. + // + // Note that we will eagerly load the data in the cache at this point, even + // if we have seen no history download. The case where no history download + // will appear at all is rare enough in normal usage, so we can apply this + // simpler solution rather than keeping a list of cache items to ignore. + if (sessionDownload) { + this._cachedPlacesMetaData.delete(sessionDownload.source.url); + } + let newOrUpdatedShell = null; // Trivial: if there are no shells for this download URI, we always @@ -685,7 +683,8 @@ DownloadsPlacesView.prototype = { // because it will not be obscured by the session download. let historyDownload = null; if (aPlacesNode) { - let metaData = this._getCachedPlacesMetaDataFor(aPlacesNode.uri); + let metaData = this._cachedPlacesMetaData.get(aPlacesNode.uri) || + this._getPlacesMetaDataFor(aPlacesNode.uri); historyDownload = new HistoryDownload(aPlacesNode); historyDownload.updateFromMetaData(metaData); } @@ -955,12 +954,6 @@ DownloadsPlacesView.prototype = { if (!aContainer.containerOpen) throw new Error("Root container for the downloads query cannot be closed"); - // When the container is invalidated, it means we are about to re-read all - // the information about history downloads from the database, discarding the - // download element shells, thus we fully rebuild the Places annotation - // cache with the latest values. - this._cachedPlacesMetaData = null; - let suppressOnSelect = this._richlistbox.suppressOnSelect; this._richlistbox.suppressOnSelect = true; try { -- cgit v1.2.3