From 5d75e8666654d7383ed230c11fb231fd7a3347fb Mon Sep 17 00:00:00 2001 From: "dtownsend@oxymoronical.com" Date: Fri, 19 Oct 2007 07:00:49 -0700 Subject: [PATCH] Bug 398080: EM should not broadcast to all XPIManagers. r=dveditz r=robstrong a=blocking-firefox3 M9 --- .../mozapps/extensions/content/extensions.js | 15 ++-- .../extensions/public/nsIExtensionManager.idl | 11 +-- .../extensions/src/nsExtensionManager.js.in | 72 ++++++------------- .../extensions/test/unit/test_bug299716.js | 4 +- xpinstall/src/nsXPInstallManager.cpp | 29 +++++--- 5 files changed, 57 insertions(+), 74 deletions(-) diff --git a/toolkit/mozapps/extensions/content/extensions.js b/toolkit/mozapps/extensions/content/extensions.js index acfda0a3c90..42b6fccf96a 100644 --- a/toolkit/mozapps/extensions/content/extensions.js +++ b/toolkit/mozapps/extensions/content/extensions.js @@ -726,8 +726,9 @@ function Startup() if ("arguments" in window) { try { var params = window.arguments[0].QueryInterface(Components.interfaces.nsIDialogParamBlock); + var manager = window.arguments[1].QueryInterface(Components.interfaces.nsIObserver); showView("installs"); - gDownloadManager.addDownloads(params); + gDownloadManager.addDownloads(params, manager); } catch (e) { if (window.arguments[0] == "updates-only") { @@ -815,12 +816,14 @@ XPInstallDownloadManager.prototype = { var params = aSubject.QueryInterface(Components.interfaces.nsISupportsArray); var paramBlock = params.GetElementAt(0).QueryInterface(Components.interfaces.nsISupportsInterfacePointer); paramBlock = paramBlock.data.QueryInterface(Components.interfaces.nsIDialogParamBlock); - this.addDownloads(paramBlock); + var manager = params.GetElementAt(1).QueryInterface(Components.interfaces.nsISupportsInterfacePointer); + manager = manager.data.QueryInterface(Components.interfaces.nsIObserver); + this.addDownloads(paramBlock, manager); break; } }, - addDownloads: function (aParams) + addDownloads: function (aParams, aManager) { var numXPInstallItems = aParams.GetInt(1); var items = []; @@ -846,7 +849,7 @@ XPInstallDownloadManager.prototype = { var certName = aParams.GetString(i++); } - gExtensionManager.addDownloads(items, items.length, false); + gExtensionManager.addDownloads(items, items.length, aManager); updateOptionalViews(); updateGlobalCommands(); }, @@ -1725,7 +1728,7 @@ function installUpdatesAll() { items.push(gExtensionManager.getItemForID(getIDFromResourceURI(children[i].id))); } if (items.length > 0) { - gExtensionManager.addDownloads(items, items.length, true); + gExtensionManager.addDownloads(items, items.length, null); showView("installs"); // Remove the updates view if there are no add-ons left to update updateOptionalViews(); @@ -2005,7 +2008,7 @@ var gExtensionsViewController = { showView("installs"); var item = gExtensionManager.getItemForID(getIDFromResourceURI(aSelectedItem.id)); - gExtensionManager.addDownloads([item], 1, true); + gExtensionManager.addDownloads([item], 1, null); // Remove the updates view if there are no add-ons left to update updateOptionalViews(); updateGlobalCommands(); diff --git a/toolkit/mozapps/extensions/public/nsIExtensionManager.idl b/toolkit/mozapps/extensions/public/nsIExtensionManager.idl index 7085d869a38..a58a89759da 100644 --- a/toolkit/mozapps/extensions/public/nsIExtensionManager.idl +++ b/toolkit/mozapps/extensions/public/nsIExtensionManager.idl @@ -48,6 +48,7 @@ interface nsIAddonUpdateCheckListener; interface nsICommandLine; interface nsISimpleEnumerator; interface nsIDirectoryEnumerator; +interface nsIObserver; /** * Interface representing a location where extensions, themes etc are @@ -198,7 +199,7 @@ interface nsIInstallLocation : nsISupports * XXXben - Some of this stuff should go into a management-ey interface, * some into an app-startup-ey interface. */ -[scriptable, uuid(feccf1ac-df58-43c1-bef0-b86dc768b906)] +[scriptable, uuid(6dd1c051-6322-40e7-8e70-1020a61a5f89)] interface nsIExtensionManager : nsISupports { /** @@ -376,14 +377,14 @@ interface nsIExtensionManager : nsISupports * A list of nsIUpdateItems to entries to add * @param itemCount * The length of |items| - * @param fromChrome - * true when called from chrome - * false when not called from chrome (e.g. web page) + * @param manager + * null when called from chrome + * the XPInstallManager when not called from chrome (e.g. web page) * * @throws NS_ERROR_ILLEGAL_VALUE if any item is invalid, or if itemCount == 0. */ void addDownloads([array, size_is(itemCount)] in nsIUpdateItem items, - in unsigned long itemCount, in boolean fromChrome); + in unsigned long itemCount, in nsIObserver manager); /** * Removes an active download from the UI diff --git a/toolkit/mozapps/extensions/src/nsExtensionManager.js.in b/toolkit/mozapps/extensions/src/nsExtensionManager.js.in index 3db073b2eba..d504c9ee488 100644 --- a/toolkit/mozapps/extensions/src/nsExtensionManager.js.in +++ b/toolkit/mozapps/extensions/src/nsExtensionManager.js.in @@ -5316,8 +5316,6 @@ ExtensionManager.prototype = { "quitCancelDownloadsAlertMsgMac", "dontQuitButtonMac"); #endif - if (!result) - this._cancelDownloads(); if (subject instanceof Ci.nsISupportsPRBool) subject.data = result; } @@ -5338,23 +5336,11 @@ ExtensionManager.prototype = { "offlineCancelDownloadsAlertMsgMultiple", "offlineCancelDownloadsAlertMsg", "dontGoOfflineButton"); - if (!result) - this._cancelDownloads(); if (subject instanceof Ci.nsISupportsPRBool) subject.data = result; } }, - /** - * Cancels all active downloads and removes them from the applicable UI. - */ - _cancelDownloads: function() { - for (var i = 0; i < this._transactions.length; ++i) - gOS.notifyObservers(this._transactions[i], "xpinstall-progress", "cancel"); - - this._removeAllDownloads(); - }, - /** * Ask the user whether or not they wish to cancel the Extension/Theme * downloads which are currently under way. @@ -5400,7 +5386,7 @@ ExtensionManager.prototype = { }, /* See nsIExtensionManager.idl */ - addDownloads: function(items, itemCount, fromChrome) { + addDownloads: function(items, itemCount, manager) { if (itemCount == 0) throw Cr.NS_ERROR_ILLEGAL_VALUE; @@ -5420,17 +5406,17 @@ ExtensionManager.prototype = { var urls = []; var hashes = []; - var txn = new ItemDownloadTransaction(this); + var txnID = Math.round(Math.random() * 100); + var txn = new ItemDownloadTransaction(this, txnID); for (var i = 0; i < itemCount; ++i) { var currItem = items[i]; - var txnID = Math.round(Math.random() * 100); - txn.addDownload(currItem, txnID); + txn.addDownload(currItem); urls.push(currItem.xpiURL); hashes.push(currItem.xpiHash ? currItem.xpiHash : null); // if this is an update remove the update metadata to prevent it from // being updated during an install. - if (fromChrome) { + if (!manager) { var id = currItem.id ds.setItemProperty(id, EM_R("availableUpdateURL"), null); ds.setItemProperty(id, EM_R("availableUpdateHash"), null); @@ -5439,41 +5425,21 @@ ExtensionManager.prototype = { ds.updateProperty(id, "availableUpdateURL"); ds.updateProperty(id, "updateable"); } - var id = fromChrome ? PREFIX_ITEM_URI + currItem.id : currItem.xpiURL; + var id = !manager ? PREFIX_ITEM_URI + currItem.id : currItem.xpiURL; ds.updateDownloadState(id, "waiting"); } this._transactions.push(txn); - if (fromChrome) { + if (manager) { + // XPIManager initiated -- let it know we're ready + manager.observe(txn, "xpinstall-progress", "open"); + } + else { // Initiate an install from chrome var xpimgr = Cc["@mozilla.org/xpinstall/install-manager;1"]. createInstance(Ci.nsIXPInstallManager); xpimgr.initManagerWithHashes(urls, hashes, urls.length, txn); } - else - gOS.notifyObservers(txn, "xpinstall-progress", "open"); - }, - - /** - * Removes a download of a URL. - * @param url - * The URL of the item being downloaded to remove. - */ - removeDownload: function(url) { - for (var i = 0; i < this._transactions.length; ++i) { - if (this._transactions[i].containsURL(url)) { - this._transactions[i].removeDownload(url); - return; - } - } - }, - - /** - * Remove all downloads from all transactions. - */ - _removeAllDownloads: function() { - for (var i = 0; i < this._transactions.length; ++i) - this._transactions[i].removeAllDownloads(); }, /** @@ -5521,7 +5487,7 @@ ExtensionManager.prototype = { ds.updateDownloadState(id, "failure"); ds.updateDownloadProgress(id, null); } - this.removeDownload(addon.xpiURL); + transaction.removeDownload(addon.xpiURL); break; case nsIXPIProgressDialog.DIALOG_CLOSE: for (var i = 0; i < this._transactions.length; ++i) { @@ -5535,6 +5501,8 @@ ExtensionManager.prototype = { break; } } + // Remove any remaining downloads from this transaction + transaction.removeAllDownloads(); break; } }, @@ -5626,10 +5594,15 @@ ExtensionManager.prototype = { * track each operation independently. * * @constructor + * @param manager + * The extension manager creating this transaction + * @param id + * The integer identifier of this transaction */ -function ItemDownloadTransaction(manager) { +function ItemDownloadTransaction(manager, id) { this._manager = manager; this._downloads = []; + this.id = id; } ItemDownloadTransaction.prototype = { _manager : null, @@ -5640,13 +5613,10 @@ ItemDownloadTransaction.prototype = { * Add a download to this transaction * @param addon * An object implementing nsIUpdateItem for the item to be downloaded - * @param id - * The integer identifier of this transaction */ - addDownload: function(addon, id) { + addDownload: function(addon) { this._downloads.push({ addon: addon, waiting: true }); this._manager.datasource.addDownload(addon); - this.id = id; }, /** diff --git a/toolkit/mozapps/extensions/test/unit/test_bug299716.js b/toolkit/mozapps/extensions/test/unit/test_bug299716.js index 34162a33dbb..150b1cecbf6 100644 --- a/toolkit/mozapps/extensions/test/unit/test_bug299716.js +++ b/toolkit/mozapps/extensions/test/unit/test_bug299716.js @@ -415,7 +415,7 @@ function run_test_pt3() { // Here, we have some bad items that try to update. Pepto-Bismol time. try { - gEM.addDownloads(addonsArray, addonsArray.length, true); + gEM.addDownloads(addonsArray, addonsArray.length, null); do_throw("Shouldn't reach here!"); } catch (e if (e instanceof Components.interfaces.nsIException && e.result == Components.results.NS_ERROR_ILLEGAL_VALUE)) { @@ -429,7 +429,7 @@ function run_test_pt3() { } do_check_true(addonsArray.length > 0); - gEM.addDownloads(addonsArray, addonsArray.length, true); + gEM.addDownloads(addonsArray, addonsArray.length, null); } /** diff --git a/xpinstall/src/nsXPInstallManager.cpp b/xpinstall/src/nsXPInstallManager.cpp index b7f023bbf6b..df39140393d 100644 --- a/xpinstall/src/nsXPInstallManager.cpp +++ b/xpinstall/src/nsXPInstallManager.cpp @@ -173,10 +173,6 @@ nsXPInstallManager::InitManagerWithHashes(const PRUnichar **aURLs, mNeedsShutdown = PR_TRUE; - nsCOMPtr os(do_GetService("@mozilla.org/observer-service;1")); - if (os) - os->AddObserver(this, XPI_PROGRESS_TOPIC, PR_TRUE); - for (PRUint32 i = 0; i < aURLCount; ++i) { nsXPITriggerItem* item = new nsXPITriggerItem(0, aURLs[i], nsnull, @@ -340,10 +336,6 @@ nsXPInstallManager::InitManagerInternal() if (OKtoInstall) { - nsCOMPtr os(do_GetService("@mozilla.org/observer-service;1")); - if (os) - os->AddObserver(this, XPI_PROGRESS_TOPIC, PR_TRUE); - //----------------------------------------------------- // Open the progress dialog //----------------------------------------------------- @@ -576,7 +568,8 @@ NS_IMETHODIMP nsXPInstallManager::Observe( nsISupports *aSubject, if ( !aTopic || !aData ) return rv; - if ( nsDependentCString( aTopic ).Equals( XPI_PROGRESS_TOPIC ) ) + nsDependentCString topic( aTopic ); + if ( topic.Equals( XPI_PROGRESS_TOPIC ) ) { //------------------------------------------------------ // Communication from the XPInstall Progress Dialog @@ -593,6 +586,13 @@ NS_IMETHODIMP nsXPInstallManager::Observe( nsISupports *aSubject, mDialogOpen = PR_TRUE; rv = NS_OK; + nsCOMPtr os(do_GetService("@mozilla.org/observer-service;1")); + if (os) + { + os->AddObserver(this, NS_IOSERVICE_GOING_OFFLINE_TOPIC, PR_TRUE); + os->AddObserver(this, "quit-application", PR_TRUE); + } + nsCOMPtr dlg( do_QueryInterface(aSubject) ); if (dlg) { @@ -621,6 +621,12 @@ NS_IMETHODIMP nsXPInstallManager::Observe( nsISupports *aSubject, rv = NS_OK; } } + else if ( topic.Equals( NS_IOSERVICE_GOING_OFFLINE_TOPIC ) || + topic.Equals( "quit-application" ) ) + { + mCancelled = PR_TRUE; + rv = NS_OK; + } return rv; } @@ -912,7 +918,10 @@ void nsXPInstallManager::Shutdown() NS_PROXY_SYNC | NS_PROXY_ALWAYS, getter_AddRefs(pos) ); if (NS_SUCCEEDED(rv)) - pos->RemoveObserver(this, XPI_PROGRESS_TOPIC); + { + pos->RemoveObserver(this, NS_IOSERVICE_GOING_OFFLINE_TOPIC); + pos->RemoveObserver(this, "quit-application"); + } } if (mTriggers)