Backed out changeset 6f0f765b2e1d (bug 913110) for leaking

This commit is contained in:
Wes Kocher 2013-09-13 14:52:40 -07:00
parent 4047a88cf2
commit 83927f1292
7 changed files with 70 additions and 432 deletions

View File

@ -13,9 +13,6 @@
*
* DownloadCombinedList
* Provides a unified, unordered list combining public and private downloads.
*
* DownloadSummary
* Provides an aggregated view on the contents of a DownloadList.
*/
"use strict";
@ -23,7 +20,6 @@
this.EXPORTED_SYMBOLS = [
"DownloadList",
"DownloadCombinedList",
"DownloadSummary",
];
////////////////////////////////////////////////////////////////////////////////
@ -336,185 +332,3 @@ DownloadCombinedList.prototype = {
this._notifyAllViews("onDownloadRemoved", aDownload);
},
};
////////////////////////////////////////////////////////////////////////////////
//// DownloadSummary
/**
* Provides an aggregated view on the contents of a DownloadList.
*/
function DownloadSummary() {
this._downloads = [];
this._views = new Set();
}
DownloadSummary.prototype = {
/**
* Array of Download objects that are currently part of the summary.
*/
_downloads: null,
/**
* Underlying DownloadList whose contents should be summarized.
*/
_list: null,
/**
* This method may be called once to bind this object to a DownloadList.
*
* Views on the summarized data can be registered before this object is bound
* to an actual list. This allows the summary to be used without requiring
* the initialization of the DownloadList first.
*
* @param aList
* Underlying DownloadList whose contents should be summarized.
*/
bindToList: function (aList)
{
if (this._list) {
throw new Error("bindToList may be called only once.");
}
aList.addView(this);
// Set the list reference only after addView has returned, so that we don't
// send a notification to our views for each download that is added.
this._list = aList;
this._onListChanged();
},
/**
* Set of currently registered views.
*/
_views: null,
/**
* Adds a view that will be notified of changes to the summary. The newly
* added view will receive an initial onSummaryChanged notification.
*
* @param aView
* The view object to add. The following methods may be defined:
* {
* onSummaryChanged: function () {
* // Called after any property of the summary has changed.
* },
* }
*/
addView: function (aView)
{
this._views.add(aView);
if ("onSummaryChanged" in aView) {
try {
aView.onSummaryChanged();
} catch (ex) {
Cu.reportError(ex);
}
}
},
/**
* Removes a view that was previously added using addView. The removed view
* will not receive any more notifications after this method returns.
*
* @param aView
* The view object to remove.
*/
removeView: function (aView)
{
this._views.delete(aView);
},
/**
* Indicates whether all the downloads are currently stopped.
*/
allHaveStopped: true,
/**
* Indicates the total number of bytes to be transferred before completing all
* the downloads that are currently in progress.
*
* For downloads that do not have a known final size, the number of bytes
* currently transferred is reported as part of this property.
*
* This is zero if no downloads are currently in progress.
*/
progressTotalBytes: 0,
/**
* Number of bytes currently transferred as part of all the downloads that are
* currently in progress.
*
* This is zero if no downloads are currently in progress.
*/
progressCurrentBytes: 0,
/**
* This function is called when any change in the list of downloads occurs,
* and will recalculate the summary and notify the views in case the
* aggregated properties are different.
*/
_onListChanged: function () {
let allHaveStopped = true;
let progressTotalBytes = 0;
let progressCurrentBytes = 0;
// Recalculate the aggregated state. See the description of the individual
// properties for an explanation of the summarization logic.
for (let download of this._downloads) {
if (!download.stopped) {
allHaveStopped = false;
progressTotalBytes += download.hasProgress ? download.totalBytes
: download.currentBytes;
progressCurrentBytes += download.currentBytes;
}
}
// Exit now if the properties did not change.
if (this.allHaveStopped == allHaveStopped &&
this.progressTotalBytes == progressTotalBytes &&
this.progressCurrentBytes == progressCurrentBytes) {
return;
}
this.allHaveStopped = allHaveStopped;
this.progressTotalBytes = progressTotalBytes;
this.progressCurrentBytes = progressCurrentBytes;
// Notify all the views that our properties changed.
for (let view of this._views) {
try {
if ("onSummaryChanged" in view) {
view.onSummaryChanged();
}
} catch (ex) {
Cu.reportError(ex);
}
}
},
//////////////////////////////////////////////////////////////////////////////
//// DownloadList view
onDownloadAdded: function (aDownload)
{
this._downloads.push(aDownload);
if (this._list) {
this._onListChanged();
}
},
onDownloadChanged: function (aDownload)
{
this._onListChanged();
},
onDownloadRemoved: function (aDownload)
{
let index = this._downloads.indexOf(aDownload);
if (index != -1) {
this._downloads.splice(index, 1);
}
this._onListChanged();
},
};

View File

@ -31,8 +31,6 @@ XPCOMUtils.defineLazyModuleGetter(this, "DownloadIntegration",
"resource://gre/modules/DownloadIntegration.jsm");
XPCOMUtils.defineLazyModuleGetter(this, "DownloadList",
"resource://gre/modules/DownloadList.jsm");
XPCOMUtils.defineLazyModuleGetter(this, "DownloadSummary",
"resource://gre/modules/DownloadList.jsm");
XPCOMUtils.defineLazyModuleGetter(this, "DownloadUIHelper",
"resource://gre/modules/DownloadUIHelper.jsm");
XPCOMUtils.defineLazyModuleGetter(this, "Promise",
@ -164,87 +162,44 @@ this.Downloads = {
* @rejects JavaScript exception.
*/
getList: function (aType)
{
if (!this._promiseListsInitialized) {
this._promiseListsInitialized = Task.spawn(function () {
let publicList = new DownloadList();
let privateList = new DownloadList();
let combinedList = new DownloadCombinedList(publicList, privateList);
try {
yield DownloadIntegration.addListObservers(publicList, false);
yield DownloadIntegration.addListObservers(privateList, true);
yield DownloadIntegration.initializePublicDownloadList(publicList);
} catch (ex) {
Cu.reportError(ex);
}
let publicSummary = yield this.getSummary(Downloads.PUBLIC);
let privateSummary = yield this.getSummary(Downloads.PRIVATE);
let combinedSummary = yield this.getSummary(Downloads.ALL);
publicSummary.bindToList(publicList);
privateSummary.bindToList(privateList);
combinedSummary.bindToList(combinedList);
this._lists[Downloads.PUBLIC] = publicList;
this._lists[Downloads.PRIVATE] = privateList;
this._lists[Downloads.ALL] = combinedList;
}.bind(this));
}
return this._promiseListsInitialized.then(() => this._lists[aType]);
},
/**
* Promise resolved when the initialization of the download lists has
* completed, or null if initialization has never been requested.
*/
_promiseListsInitialized: null,
/**
* After initialization, this object is populated with one key for each type
* of download list that can be returned (Downloads.PUBLIC, Downloads.PRIVATE,
* or Downloads.ALL). The values are the DownloadList objects.
*/
_lists: {},
/**
* Retrieves the specified type of DownloadSummary object. There is one
* download summary for each type, and this method always retrieves a
* reference to the same download summary when called with the same argument.
*
* Calling this function does not cause the list of public downloads to be
* reloaded from the previous session. The summary will behave as if no
* downloads are present until the getList method is called.
*
* @param aType
* This can be Downloads.PUBLIC, Downloads.PRIVATE, or Downloads.ALL.
*
* @return {Promise}
* @resolves The requested DownloadList or DownloadCombinedList object.
* @rejects JavaScript exception.
*/
getSummary: function (aType)
{
if (aType != Downloads.PUBLIC && aType != Downloads.PRIVATE &&
aType != Downloads.ALL) {
throw new Error("Invalid aType argument.");
}
if (!(aType in this._summaries)) {
this._summaries[aType] = new DownloadSummary();
if (!(aType in this._listPromises)) {
this._listPromises[aType] = Task.spawn(function () {
let list;
if (aType == Downloads.ALL) {
list = new DownloadCombinedList(
(yield this.getList(Downloads.PUBLIC)),
(yield this.getList(Downloads.PRIVATE)));
} else {
list = new DownloadList();
try {
yield DownloadIntegration.addListObservers(
list, aType == Downloads.PRIVATE);
if (aType == Downloads.PUBLIC) {
yield DownloadIntegration.initializePublicDownloadList(list);
}
} catch (ex) {
Cu.reportError(ex);
}
}
throw new Task.Result(list);
}.bind(this));
}
return Promise.resolve(this._summaries[aType]);
return this._listPromises[aType];
},
/**
* This object is populated by the getSummary method with one key for each
* type of object that can be returned (Downloads.PUBLIC, Downloads.PRIVATE,
* or Downloads.ALL). The values are the DownloadSummary objects.
* This object is populated by the getList method with one key for each type
* of object that can be returned (Downloads.PUBLIC, Downloads.PRIVATE, or
* Downloads.ALL). The values are the promises returned by the method.
*/
_summaries: {},
_listPromises: {},
/**
* Returns the system downloads directory asynchronously.

View File

@ -35,6 +35,36 @@ function promiseStartDownload(aSourceUrl) {
});
}
/**
* Waits for a download to reach half of its progress, in case it has not
* reached the expected progress already.
*
* @param aDownload
* The Download object to wait upon.
*
* @return {Promise}
* @resolves When the download has reached half of its progress.
* @rejects Never.
*/
function promiseDownloadMidway(aDownload) {
let deferred = Promise.defer();
// Wait for the download to reach half of its progress.
let onchange = function () {
if (!aDownload.stopped && !aDownload.canceled && aDownload.progress == 50) {
aDownload.onchange = null;
deferred.resolve();
}
};
// Register for the notification, but also call the function directly in
// case the download already reached the expected progress.
aDownload.onchange = onchange;
onchange();
return deferred.promise;
}
/**
* Waits for a download to finish, in case it has not finished already.
*

View File

@ -463,36 +463,6 @@ function promiseStartExternalHelperAppServiceDownload(aSourceUrl) {
return deferred.promise;
}
/**
* Waits for a download to reach half of its progress, in case it has not
* reached the expected progress already.
*
* @param aDownload
* The Download object to wait upon.
*
* @return {Promise}
* @resolves When the download has reached half of its progress.
* @rejects Never.
*/
function promiseDownloadMidway(aDownload) {
let deferred = Promise.defer();
// Wait for the download to reach half of its progress.
let onchange = function () {
if (!aDownload.stopped && !aDownload.canceled && aDownload.progress == 50) {
aDownload.onchange = null;
deferred.resolve();
}
};
// Register for the notification, but also call the function directly in
// case the download already reached the expected progress.
aDownload.onchange = onchange;
onchange();
return deferred.promise;
}
/**
* Returns a new public or private DownloadList object.
*
@ -505,13 +475,19 @@ function promiseDownloadMidway(aDownload) {
*/
function promiseNewList(aIsPrivate)
{
// We need to clear all the internal state for the list and summary objects,
// since all the objects are interdependent internally.
Downloads._promiseListsInitialized = null;
Downloads._lists = {};
Downloads._summaries = {};
let type = aIsPrivate ? Downloads.PRIVATE : Downloads.PUBLIC;
return Downloads.getList(aIsPrivate ? Downloads.PRIVATE : Downloads.PUBLIC);
// Force the creation of a new list.
if (type in Downloads._listPromises) {
delete Downloads._listPromises[type];
}
// Invalidate the combined list, if any.
if (Downloads.ALL in Downloads._listPromises) {
delete Downloads._listPromises[Downloads.ALL];
}
return Downloads.getList(type);
}
/**

View File

@ -278,7 +278,7 @@ add_task(function test_mix_notifications()
mustInterruptResponses();
let publicList = yield promiseNewList();
let privateList = yield Downloads.getList(Downloads.PRIVATE);
let privateList = yield promiseNewList(true);
let download1 = yield promiseNewDownload(httpUrl("interruptible.txt"));
let download2 = yield promiseNewDownload(httpUrl("interruptible.txt"));
let promiseAttempt1 = download1.start();

View File

@ -137,7 +137,7 @@ add_task(function test_remove()
add_task(function test_DownloadCombinedList_add_remove_getAll()
{
let publicList = yield promiseNewList();
let privateList = yield Downloads.getList(Downloads.PRIVATE);
let privateList = yield promiseNewList(true);
let combinedList = yield Downloads.getList(Downloads.ALL);
let publicDownload = yield promiseNewDownload();
@ -448,120 +448,3 @@ add_task(function test_removeFinished()
let downloads = yield list.getAll()
do_check_eq(downloads.length, 1);
});
/**
* Tests the global DownloadSummary objects for the public, private, and
* combined download lists.
*/
add_task(function test_DownloadSummary()
{
mustInterruptResponses();
let publicList = yield promiseNewList();
let privateList = yield Downloads.getList(Downloads.PRIVATE);
let publicSummary = yield Downloads.getSummary(Downloads.PUBLIC);
let privateSummary = yield Downloads.getSummary(Downloads.PRIVATE);
let combinedSummary = yield Downloads.getSummary(Downloads.ALL);
// Add a public download that has succeeded.
let succeededPublicDownload = yield promiseNewDownload();
yield succeededPublicDownload.start();
publicList.add(succeededPublicDownload);
// Add a public download that has been canceled midway.
let canceledPublicDownload =
yield promiseNewDownload(httpUrl("interruptible.txt"));
canceledPublicDownload.start();
yield promiseDownloadMidway(canceledPublicDownload);
yield canceledPublicDownload.cancel();
publicList.add(canceledPublicDownload);
// Add a public download that is in progress.
let inProgressPublicDownload =
yield promiseNewDownload(httpUrl("interruptible.txt"));
inProgressPublicDownload.start();
yield promiseDownloadMidway(inProgressPublicDownload);
publicList.add(inProgressPublicDownload);
// Add a private download that is in progress.
let inProgressPrivateDownload = yield Downloads.createDownload({
source: { url: httpUrl("interruptible.txt"), isPrivate: true },
target: getTempFile(TEST_TARGET_FILE_NAME).path,
});
inProgressPrivateDownload.start();
yield promiseDownloadMidway(inProgressPrivateDownload);
privateList.add(inProgressPrivateDownload);
// Verify that the summary includes the total number of bytes and the
// currently transferred bytes only for the downloads that are not stopped.
// For simplicity, we assume that after a download is added to the list, its
// current state is immediately propagated to the summary object, which is
// true in the current implementation, though it is not guaranteed as all the
// download operations may happen asynchronously.
do_check_false(publicSummary.allHaveStopped);
do_check_eq(publicSummary.progressTotalBytes, TEST_DATA_SHORT.length * 2);
do_check_eq(publicSummary.progressCurrentBytes, TEST_DATA_SHORT.length);
do_check_false(privateSummary.allHaveStopped);
do_check_eq(privateSummary.progressTotalBytes, TEST_DATA_SHORT.length * 2);
do_check_eq(privateSummary.progressCurrentBytes, TEST_DATA_SHORT.length);
do_check_false(combinedSummary.allHaveStopped);
do_check_eq(combinedSummary.progressTotalBytes, TEST_DATA_SHORT.length * 4);
do_check_eq(combinedSummary.progressCurrentBytes, TEST_DATA_SHORT.length * 2);
yield inProgressPublicDownload.cancel();
// Stopping the download should have excluded it from the summary.
do_check_true(publicSummary.allHaveStopped);
do_check_eq(publicSummary.progressTotalBytes, 0);
do_check_eq(publicSummary.progressCurrentBytes, 0);
do_check_false(privateSummary.allHaveStopped);
do_check_eq(privateSummary.progressTotalBytes, TEST_DATA_SHORT.length * 2);
do_check_eq(privateSummary.progressCurrentBytes, TEST_DATA_SHORT.length);
do_check_false(combinedSummary.allHaveStopped);
do_check_eq(combinedSummary.progressTotalBytes, TEST_DATA_SHORT.length * 2);
do_check_eq(combinedSummary.progressCurrentBytes, TEST_DATA_SHORT.length);
yield inProgressPrivateDownload.cancel();
// All the downloads should be stopped now.
do_check_true(publicSummary.allHaveStopped);
do_check_eq(publicSummary.progressTotalBytes, 0);
do_check_eq(publicSummary.progressCurrentBytes, 0);
do_check_true(privateSummary.allHaveStopped);
do_check_eq(privateSummary.progressTotalBytes, 0);
do_check_eq(privateSummary.progressCurrentBytes, 0);
do_check_true(combinedSummary.allHaveStopped);
do_check_eq(combinedSummary.progressTotalBytes, 0);
do_check_eq(combinedSummary.progressCurrentBytes, 0);
});
/**
* Checks that views receive the summary change notification. This is tested on
* the combined summary when adding a public download, as we assume that if we
* pass the test in this case we will also pass it in the others.
*/
add_task(function test_DownloadSummary_notifications()
{
let list = yield promiseNewList();
let summary = yield Downloads.getSummary(Downloads.ALL);
let download = yield promiseNewDownload();
list.add(download);
// Check that we receive change notifications.
let receivedOnSummaryChanged = false;
summary.addView({
onSummaryChanged: function () {
receivedOnSummaryChanged = true;
},
});
yield download.start();
do_check_true(receivedOnSummaryChanged);
});

View File

@ -118,26 +118,6 @@ add_task(function test_getList()
do_check_neq(publicListOne, privateListOne);
});
/**
* Tests that the getSummary function returns the same summary when called
* multiple times with the same argument, but returns different summaries when
* called with different arguments. More detailed tests are implemented
* separately for the DownloadSummary object in the DownloadList module.
*/
add_task(function test_getSummary()
{
let publicSummaryOne = yield Downloads.getSummary(Downloads.PUBLIC);
let privateSummaryOne = yield Downloads.getSummary(Downloads.PRIVATE);
let publicSummaryTwo = yield Downloads.getSummary(Downloads.PUBLIC);
let privateSummaryTwo = yield Downloads.getSummary(Downloads.PRIVATE);
do_check_eq(publicSummaryOne, publicSummaryTwo);
do_check_eq(privateSummaryOne, privateSummaryTwo);
do_check_neq(publicSummaryOne, privateSummaryOne);
});
/**
* Tests that the getSystemDownloadsDirectory returns a valid nsFile
* download directory object.