Bug 398080: EM should not broadcast to all XPIManagers. r=dveditz r=robstrong a=blocking-firefox3 M9
authordtownsend@oxymoronical.com
Fri, 19 Oct 2007 07:00:49 -0700
changeset 7037 f8a74f092ae2219d88cef7e59dd4e7eeaa6a5b22
parent 7036 559588a5c74d74776e5918f55d99d89dd41b3d2f
child 7038 b9610d97dd90d26769bf28854192c81cac7d85a4
push idunknown
push userunknown
push dateunknown
reviewersdveditz, robstrong, blocking-firefox3
bugs398080
milestone1.9a9pre
Bug 398080: EM should not broadcast to all XPIManagers. r=dveditz r=robstrong a=blocking-firefox3 M9
toolkit/mozapps/extensions/content/extensions.js
toolkit/mozapps/extensions/public/nsIExtensionManager.idl
toolkit/mozapps/extensions/src/nsExtensionManager.js.in
toolkit/mozapps/extensions/test/unit/test_bug299716.js
xpinstall/src/nsXPInstallManager.cpp
--- a/toolkit/mozapps/extensions/content/extensions.js
+++ b/toolkit/mozapps/extensions/content/extensions.js
@@ -721,18 +721,19 @@ function Startup()
     showMessage("chrome://mozapps/skin/extensions/question.png",
                 getExtensionString("safeModeMsg"),
                 null, null, true, null);
   }
 
   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") {
         gUpdatesOnly = true;
 #ifdef MOZ_PHOENIX
         // If we are Firefox when updating on startup don't display context
         // menuitems that can open a browser window.
         gUpdateContextMenus = gUpdateContextMenusNoBrowser;
@@ -810,22 +811,24 @@ XPInstallDownloadManager.prototype = {
   observe: function (aSubject, aTopic, aData)
   {
     switch (aTopic) {
       case "xpinstall-download-started":
         showView("installs");
         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 = [];
     for (var i = 0; i < numXPInstallItems;) {
       var displayName = aParams.GetString(i++);
       var url = aParams.GetString(i++);
       var iconURL = aParams.GetString(i++);
       var uri = Components.classes["@mozilla.org/network/io-service;1"]
@@ -841,17 +844,17 @@ XPInstallDownloadManager.prototype = {
                            .createInstance(Components.interfaces.nsIUpdateItem);
       item.init(url, " ", "app-profile", "", "", displayName, url, "", iconURL, "", "", type, "");
       items.push(item);
 
       // Advance the enumerator
       var certName = aParams.GetString(i++);
     }
 
-    gExtensionManager.addDownloads(items, items.length, false);
+    gExtensionManager.addDownloads(items, items.length, aManager);
     updateOptionalViews();
     updateGlobalCommands();
   },
 
   getElementForAddon: function(aAddon)
   {
     var element = document.getElementById(PREFIX_ITEM_URI + aAddon.id);
     if (aAddon.id == aAddon.xpiURL)
@@ -1720,17 +1723,17 @@ function installUpdatesAll() {
   var items = [];
   var children = gExtensionsView.children;
   for (var i = 0; i < children.length; ++i) {
     var includeUpdate = document.getAnonymousElementByAttribute(children[i], "anonid", "includeUpdate");
     if (includeUpdate && includeUpdate.checked)
       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();
     updateGlobalCommands();
   }
 }
 
 function restartApp() {
@@ -2000,17 +2003,17 @@ var gExtensionsViewController = {
       if (isOffline("offlineUpdateMsg"))
         return;
 
       if (!isXPInstallEnabled())
         return;
 
       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();
     },
 
     cmd_includeUpdate: function (aSelectedItem)
     {
       var includeUpdate = document.getAnonymousElementByAttribute(aSelectedItem, "anonid", "includeUpdate");
--- a/toolkit/mozapps/extensions/public/nsIExtensionManager.idl
+++ b/toolkit/mozapps/extensions/public/nsIExtensionManager.idl
@@ -43,16 +43,17 @@
 interface nsIFile;
 interface nsIRDFDataSource;
 interface nsIUpdateItem;
 interface nsIAddonUpdateListener;
 interface nsIAddonUpdateCheckListener;
 interface nsICommandLine;
 interface nsISimpleEnumerator;
 interface nsIDirectoryEnumerator;
+interface nsIObserver;
 
 /**
  * Interface representing a location where extensions, themes etc are
  * installed.
  */
 [scriptable, uuid(32a74707-ec7c-af19-f4d8-d0cd8cb6a948)]
 interface nsIInstallLocation : nsISupports
 {
@@ -193,17 +194,17 @@ interface nsIInstallLocation : nsISuppor
 
 /**
  * Interface representing a system for the installation and management of
  * Extensions, Themes etc.
  *
  * 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
 {
   /**
    * Constants representing types of update checks.
    */
   const unsigned long UPDATE_CHECK_NEWVERSION    = 0;
   const unsigned long UPDATE_CHECK_COMPATIBILITY = 1;
   const unsigned long UPDATE_SYNC_COMPATIBILITY  = 2;
@@ -371,24 +372,24 @@ interface nsIExtensionManager : nsISuppo
   readonly attribute nsIRDFDataSource datasource;
 
   /**
    * Adds active download entries to the UI
    * @param   items
    *          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
    * @param   url
    *          The URL of the active download to remove
    */
   void removeDownload(in AString url);
 
--- a/toolkit/mozapps/extensions/src/nsExtensionManager.js.in
+++ b/toolkit/mozapps/extensions/src/nsExtensionManager.js.in
@@ -5311,18 +5311,16 @@ ExtensionManager.prototype = {
                                             "dontQuitButtonWin");
 #else
       result = this._confirmCancelDownloads(this._downloadCount,
                                             "quitCancelDownloadsAlertTitle",
                                             "quitCancelDownloadsAlertMsgMacMultiple",
                                             "quitCancelDownloadsAlertMsgMac",
                                             "dontQuitButtonMac");
 #endif
-      if (!result)
-        this._cancelDownloads();
       if (subject instanceof Ci.nsISupportsPRBool)
         subject.data = result;
     }
   },
 
   /**
    * Ask the user if they really want to go offline, since this will cancel
    * one or more Extension/Theme downloads.
@@ -5333,34 +5331,22 @@ ExtensionManager.prototype = {
    */
   _confirmCancelDownloadsOnOffline: function(subject) {
     if (this._downloadCount > 0) {
       result = this._confirmCancelDownloads(this._downloadCount,
                                             "offlineCancelDownloadsAlertTitle",
                                             "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.
    * @param   count
    *          The number of active downloads.
    * @param   title
    *          The key of the title for the message box to be displayed
    * @param   cancelMessageMultiple
    *          The key of the message to be displayed in the message box
@@ -5395,17 +5381,17 @@ ExtensionManager.prototype = {
              getService(nsIPromptService);
     var flags = (nsIPromptService.BUTTON_TITLE_IS_STRING * nsIPromptService.BUTTON_POS_0) +
                 (nsIPromptService.BUTTON_TITLE_IS_STRING * nsIPromptService.BUTTON_POS_1);
     var rv = ps.confirmEx(win, title, message, flags, quitButton, dontQuitButton, null, null, { });
     return rv == 1;
   },
 
   /* See nsIExtensionManager.idl */
-  addDownloads: function(items, itemCount, fromChrome) {
+  addDownloads: function(items, itemCount, manager) {
     if (itemCount == 0)
       throw Cr.NS_ERROR_ILLEGAL_VALUE;
 
     for (i = 0; i < itemCount; ++i) {
       var currItem = items[i];
       if (!currItem)
         throw Cr.NS_ERROR_ILLEGAL_VALUE;
     }
@@ -5415,70 +5401,50 @@ ExtensionManager.prototype = {
     if (this._downloadCount == 0) {
       gOS.addObserver(this, "offline-requested", false);
       gOS.addObserver(this, "quit-application-requested", false);
     }
     this._downloadCount += itemCount;
 
     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);
         ds.setItemProperty(id, EM_R("availableUpdateVersion"), null);
         ds.setItemProperty(id, EM_R("availableUpdateInfo"), null);
         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();
   },
 
   /**
    * Download Operation State has changed from one to another.
    *
    * The nsIXPIProgressDialog implementation in the download transaction object
    * forwards notifications through these methods which we then pass on to any
    * front end objects implementing nsIExtensionDownloadListener that
@@ -5516,30 +5482,32 @@ ExtensionManager.prototype = {
       // From nsInstall.h
       // SUCCESS        = 0
       // REBOOT_NEEDED  = 999
       // USER_CANCELLED = -210
       if (value != 0 && value != 999 && value != -210 && id != addon.xpiURL) {
         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) {
         if (this._transactions[i].id == transaction.id) {
           this._transactions.splice(i, 1);
           // Remove the observers when all transactions have completed.
           if (this._transactions.length == 0) {
             gOS.removeObserver(this, "offline-requested");
             gOS.removeObserver(this, "quit-application-requested");
           }
           break;
         }
       }
+      // Remove any remaining downloads from this transaction
+      transaction.removeAllDownloads();
       break;
     }
   },
 
   onProgress: function(addon, value, maxValue) {
     for (var i = 0; i < this._updateListeners.length; ++i)
       this._updateListeners[i].onProgress(addon, value, maxValue);
 
@@ -5621,37 +5589,39 @@ ExtensionManager.prototype = {
  * a collection of separate transaction objects because it's possible to have
  * multiple separate XPInstall download/install operations going on
  * simultaneously, each with its own XPInstallManager instance. For instance
  * you could start downloading two extensions and then download a theme. Each
  * of these operations would open the appropriate FE and have to be able to
  * 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,
   _downloads  : [],
   id          : -1,
 
   /**
    * 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;
   },
 
   /**
    * Removes a download from this transaction
    * @param   url
    *          The URL to remove
    */
   removeDownload: function(url) {
--- a/toolkit/mozapps/extensions/test/unit/test_bug299716.js
+++ b/toolkit/mozapps/extensions/test/unit/test_bug299716.js
@@ -410,31 +410,31 @@ function run_test_pt3() {
   }
   dump("\n\n*** UPDATING " + addonsArray.length + " ITEMS\n\n");
 
   // updateListener will call run_test_pt4().
   next_test = run_test_pt4;
 
   // 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)) {
     // do nothing, this is good
   }
 
   for (i = addonsArray.length - 1; i >= 0; i--) {
     if (!addonsArray[i]) {
       addonsArray.splice(i, 1);
     }
   }
 
   do_check_true(addonsArray.length > 0);
-  gEM.addDownloads(addonsArray, addonsArray.length, true);
+  gEM.addDownloads(addonsArray, addonsArray.length, null);
 }
 
 /**
  * Check the final version of each extension.
  */
 function run_test_pt4() {
   dump("\n\n*** RESTARTING EXTENSION MANAGER\n\n");
   restartEM();
--- a/xpinstall/src/nsXPInstallManager.cpp
+++ b/xpinstall/src/nsXPInstallManager.cpp
@@ -168,20 +168,16 @@ nsXPInstallManager::InitManagerWithHashe
         return NS_OK;
   
     mTriggers = new nsXPITriggerInfo();
     if (!mTriggers)
         return NS_ERROR_OUT_OF_MEMORY;
 
     mNeedsShutdown = PR_TRUE;
 
-    nsCOMPtr<nsIObserverService> 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,
                                                       aHashes ? aHashes[i] : nsnull);
         if (!item)
         {
             delete mTriggers; // nsXPITriggerInfo frees any alloc'ed nsXPITriggerItems
             mTriggers = nsnull;
@@ -335,20 +331,16 @@ nsXPInstallManager::InitManagerInternal(
             if (NS_FAILED(rv))
                 OKtoInstall = PR_FALSE;
 #ifdef ENABLE_SKIN_SIMPLE_INSTALLATION_UI
         }
 #endif
 
         if (OKtoInstall)
         {
-            nsCOMPtr<nsIObserverService> os(do_GetService("@mozilla.org/observer-service;1"));
-            if (os)
-                os->AddObserver(this, XPI_PROGRESS_TOPIC, PR_TRUE);
-
             //-----------------------------------------------------
             // Open the progress dialog
             //-----------------------------------------------------
 
             rv = dlgSvc->OpenProgressDialog( packageList, numStrings, this );
         }
     }
     else
@@ -571,33 +563,41 @@ NS_IMETHODIMP nsXPInstallManager::Observ
                                            const char *aTopic,
                                            const PRUnichar *aData )
 {
     nsresult rv = NS_ERROR_ILLEGAL_VALUE;
 
     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
         //------------------------------------------------------
 
         nsDependentString data( aData );
 
         if ( data.Equals( NS_LITERAL_STRING("open") ) )
         {
             // -- The dialog has been opened
             if (mDialogOpen)
                 return NS_OK; // We've already been opened, nothing more to do
 
             mDialogOpen = PR_TRUE;
             rv = NS_OK;
 
+            nsCOMPtr<nsIObserverService> 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<nsIXPIProgressDialog> dlg( do_QueryInterface(aSubject) );
             if (dlg)
             {
                 // --- create and save a proxy for the dialog
                 NS_GetProxyForObject( NS_PROXY_TO_MAIN_THREAD,
                                       NS_GET_IID(nsIXPIProgressDialog),
                                       dlg,
                                       NS_PROXY_SYNC | NS_PROXY_ALWAYS,
@@ -616,16 +616,22 @@ NS_IMETHODIMP nsXPInstallManager::Observ
             {
                 // if we've never been opened then we can shutdown right here,
                 // otherwise we need to let mCancelled get discovered elsewhere
                 Shutdown();
             }
             rv = NS_OK;
         }
     }
+    else if ( topic.Equals( NS_IOSERVICE_GOING_OFFLINE_TOPIC ) ||
+              topic.Equals( "quit-application" ) )
+    {
+        mCancelled = PR_TRUE;
+        rv = NS_OK;
+    }
 
     return rv;
 }
 
 
 NS_IMETHODIMP nsXPInstallManager::DownloadNext()
 {
     nsresult rv;
@@ -907,17 +913,20 @@ void nsXPInstallManager::Shutdown()
             nsresult rv;
             nsCOMPtr<nsIObserverService> pos;
             rv = NS_GetProxyForObject( NS_PROXY_TO_MAIN_THREAD,
                                        NS_GET_IID(nsIObserverService),
                                        os,
                                        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)
         {
             delete mTriggers;
             mTriggers = nsnull;
         }