From 779d96d10f705a39ea4b18a1ff9bdc7e10ed84f1 Mon Sep 17 00:00:00 2001 From: Justin Lebar Date: Mon, 8 Jul 2013 17:55:42 -0400 Subject: [PATCH] Bug 889984 - Don't leak objects which "inherit" from DOMRequestIpcHelper the associated window is closed. r=fabrice --- dom/alarm/AlarmsManager.js | 6 +- dom/apps/src/Webapps.js | 14 ++- dom/base/DOMRequestHelper.jsm | 155 ++++++++++++++++++------- dom/contacts/ContactManager.js | 6 +- dom/fm/DOMFMRadioChild.js | 5 +- dom/messages/SystemMessageManager.js | 5 +- dom/network/src/NetworkStatsManager.js | 5 +- dom/payment/Payment.js | 5 +- dom/push/src/Push.js | 7 +- dom/system/gonk/RILContentHelper.js | 8 +- dom/wifi/DOMWifiManager.js | 5 +- 11 files changed, 152 insertions(+), 69 deletions(-) diff --git a/dom/alarm/AlarmsManager.js b/dom/alarm/AlarmsManager.js index fa3283a468c4..9e5ff0becf3f 100644 --- a/dom/alarm/AlarmsManager.js +++ b/dom/alarm/AlarmsManager.js @@ -35,7 +35,9 @@ AlarmsManager.prototype = { classID : ALARMSMANAGER_CID, - QueryInterface : XPCOMUtils.generateQI([nsIDOMMozAlarmsManager, Ci.nsIDOMGlobalPropertyInitializer]), + QueryInterface : XPCOMUtils.generateQI([nsIDOMMozAlarmsManager, + Ci.nsIDOMGlobalPropertyInitializer, + Ci.nsISupportsWeakReference]), classInfo : XPCOMUtils.generateCI({ classID: ALARMSMANAGER_CID, contractID: ALARMSMANAGER_CONTRACTID, @@ -162,7 +164,7 @@ AlarmsManager.prototype = { this._cpmm = Cc["@mozilla.org/childprocessmessagemanager;1"].getService(Ci.nsISyncMessageSender); // Add the valid messages to be listened. - this.initHelper(aWindow, ["AlarmsManager:Add:Return:OK", "AlarmsManager:Add:Return:KO", + this.initDOMRequestHelper(aWindow, ["AlarmsManager:Add:Return:OK", "AlarmsManager:Add:Return:KO", "AlarmsManager:GetAll:Return:OK", "AlarmsManager:GetAll:Return:KO"]); // Get the manifest URL if this is an installed app diff --git a/dom/apps/src/Webapps.js b/dom/apps/src/Webapps.js index 17b0083d2bf4..3c1854660e51 100644 --- a/dom/apps/src/Webapps.js +++ b/dom/apps/src/Webapps.js @@ -242,7 +242,7 @@ WebappsRegistry.prototype = { // nsIDOMGlobalPropertyInitializer implementation init: function(aWindow) { - this.initHelper(aWindow, ["Webapps:Install:Return:OK", "Webapps:Install:Return:KO", + this.initDOMRequestHelper(aWindow, ["Webapps:Install:Return:OK", "Webapps:Install:Return:KO", "Webapps:GetInstalled:Return:OK", "Webapps:GetSelf:Return:OK", "Webapps:CheckInstalled:Return:OK" ]); @@ -264,7 +264,8 @@ WebappsRegistry.prototype = { classID: Components.ID("{fff440b3-fae2-45c1-bf03-3b5a2e432270}"), - QueryInterface: XPCOMUtils.generateQI([Ci.mozIDOMApplicationRegistry, + QueryInterface: XPCOMUtils.generateQI([Ci.nsISupportsWeakReference, + Ci.mozIDOMApplicationRegistry, #ifdef MOZ_B2G Ci.mozIDOMApplicationRegistry2, #elifdef MOZ_WIDGET_ANDROID @@ -364,7 +365,7 @@ WebappsApplication.prototype = { this._downloadError = null; - this.initHelper(aWindow, ["Webapps:OfflineCache", + this.initDOMRequestHelper(aWindow, ["Webapps:OfflineCache", "Webapps:CheckForUpdate:Return:OK", "Webapps:CheckForUpdate:Return:KO", "Webapps:Launch:Return:OK", @@ -636,7 +637,8 @@ WebappsApplication.prototype = { classID: Components.ID("{723ed303-7757-4fb0-b261-4f78b1f6bd22}"), - QueryInterface: XPCOMUtils.generateQI([Ci.mozIDOMApplication]), + QueryInterface: XPCOMUtils.generateQI([Ci.mozIDOMApplication, + Ci.nsISupportsWeakReference]), classInfo: XPCOMUtils.generateCI({classID: Components.ID("{723ed303-7757-4fb0-b261-4f78b1f6bd22}"), contractID: "@mozilla.org/webapps/application;1", @@ -649,7 +651,7 @@ WebappsApplication.prototype = { * mozIDOMApplicationMgmt object */ function WebappsApplicationMgmt(aWindow) { - this.initHelper(aWindow, ["Webapps:GetAll:Return:OK", + this.initDOMRequestHelper(aWindow, ["Webapps:GetAll:Return:OK", "Webapps:GetAll:Return:KO", "Webapps:Uninstall:Return:OK", "Webapps:Uninstall:Broadcast:Return:OK", @@ -787,7 +789,7 @@ WebappsApplicationMgmt.prototype = { classID: Components.ID("{8c1bca96-266f-493a-8d57-ec7a95098c15}"), - QueryInterface: XPCOMUtils.generateQI([Ci.mozIDOMApplicationMgmt]), + QueryInterface: XPCOMUtils.generateQI([Ci.mozIDOMApplicationMgmt, Ci.nsISupportsWeakReference]), classInfo: XPCOMUtils.generateCI({classID: Components.ID("{8c1bca96-266f-493a-8d57-ec7a95098c15}"), contractID: "@mozilla.org/webapps/application-mgmt;1", diff --git a/dom/base/DOMRequestHelper.jsm b/dom/base/DOMRequestHelper.jsm index 3eaa1690219b..b1101af80343 100644 --- a/dom/base/DOMRequestHelper.jsm +++ b/dom/base/DOMRequestHelper.jsm @@ -3,10 +3,10 @@ * You can obtain one at http://mozilla.org/MPL/2.0/. */ /** - * helper object for APIs that deal with DOMRequest and need to release them properly - * when the window goes out of scope - */ -const Cu = Components.utils; + * Helper object for APIs that deal with DOMRequests and need to release them + * when the window goes out of scope. + */ +const Cu = Components.utils; const Cc = Components.classes; const Ci = Components.interfaces; @@ -19,10 +19,109 @@ XPCOMUtils.defineLazyServiceGetter(this, "cpmm", "@mozilla.org/childprocessmessagemanager;1", "nsIMessageListenerManager"); +/** + * We use DOMRequestIpcHelperMessageListener to avoid leaking objects which + * "inherit" from DOMRequestIpcHelper. + * + * The issue is that the message manager will hold a strong ref to the message + * listener we register with it. But we don't want to hold a strong ref to the + * DOMRequestIpcHelper object, because that object may be arbitrarily large. + * + * So instead the message manager holds a strong ref to the + * DOMRequestIpcHelperMessageListener, and that holds a /weak/ ref to its + * DOMRequestIpcHelper. + * + * Additionally, we want to unhook all of these message listeners when the + * appropriate window is destroyed. We use DOMRequestIpcHelperMessageListener + * for this, too. + */ +this.DOMRequestIpcHelperMessageListener = function(aHelper, aWindow, aMessages) { + this._weakHelper = Cu.getWeakReference(aHelper); + + this._messages = aMessages; + this._messages.forEach(function(msgName) { + cpmm.addMessageListener(msgName, this); + }, this); + + Services.obs.addObserver(this, "inner-window-destroyed", /* weakRef */ true); + + // aWindow may be null; in that case, the DOMRequestIpcHelperMessageListener + // is not tied to a particular window and lives forever. + if (aWindow) { + let util = aWindow.QueryInterface(Ci.nsIInterfaceRequestor) + .getInterface(Ci.nsIDOMWindowUtils); + this._innerWindowID = util.currentInnerWindowID; + } +} + +DOMRequestIpcHelperMessageListener.prototype = { + QueryInterface: XPCOMUtils.generateQI([Ci.nsIMessageListener, + Ci.nsIObserver, + Ci.nsISupportsWeakReference]), + + observe: function(aSubject, aTopic, aData) { + if (aTopic !== "inner-window-destroyed") { + return; + } + + let wId = aSubject.QueryInterface(Ci.nsISupportsPRUint64).data; + if (wId != this._innerWindowID) { + return; + } + + this.destroy(); + }, + + receiveMessage: function(aMsg) { + let helper = this._weakHelper.get(); + if (helper) { + helper.receiveMessage(aMsg); + } else { + this.destroy(); + } + }, + + destroy: function() { + Services.obs.removeObserver(this, "inner-window-destroyed"); + + this._messages.forEach(function(msgName) { + cpmm.removeMessageListener(msgName, this); + }, this); + this._messages = null; + + let helper = this._weakHelper.get(); + if (helper) { + helper.destroyDOMRequestHelper(); + } + } +} + this.DOMRequestIpcHelper = function DOMRequestIpcHelper() { } DOMRequestIpcHelper.prototype = { + /** + * An object which "inherits" from DOMRequestIpcHelper and declares its own + * queryInterface method MUST implement Ci.nsISupportsWeakReference. + */ + QueryInterface: XPCOMUtils.generateQI([Ci.nsISupportsWeakReference]), + + initDOMRequestHelper: function(aWindow, aMessages) { + this._DOMRequestIpcHelperMessageListener = + new DOMRequestIpcHelperMessageListener(this, aWindow, aMessages); + + this._window = aWindow; + this._requests = []; + this._id = this._getRandomId(); + + if (this._window) { + // We don't use this.innerWindowID, but other classes rely on it. + let util = this._window.QueryInterface(Ci.nsIInterfaceRequestor) + .getInterface(Ci.nsIDOMWindowUtils); + this.innerWindowID = util.currentInnerWindowID; + } + }, + getRequestId: function(aRequest) { let id = "id" + this._getRandomId(); this._requests[id] = aRequest; @@ -51,48 +150,22 @@ DOMRequestIpcHelper.prototype = { return Cc["@mozilla.org/uuid-generator;1"].getService(Ci.nsIUUIDGenerator).generateUUID().toString(); }, - observe: function(aSubject, aTopic, aData) { - if (aTopic !== "inner-window-destroyed") { + destroyDOMRequestHelper: function() { + // This function is re-entrant -- + // DOMRequestIpcHelperMessageListener.destroy() calls back into this + // function, and this.uninit() may also call it. + if (this._destroyed) { return; } + this._destroyed = true; - let wId = aSubject.QueryInterface(Ci.nsISupportsPRUint64).data; - if (wId == this.innerWindowID) { - Services.obs.removeObserver(this, "inner-window-destroyed"); - this._requests = []; - this._window = null; - this.removeMessageListener(); - if(this.uninit) - this.uninit(); - } - }, - - initRequests: function initRequests() { + this._DOMRequestIpcHelperMessageListener.destroy(); this._requests = []; - }, + this._window = null; - initMessageListener: function initMessageListener(aMessages) { - this._messages = aMessages; - this._messages.forEach(function(msgName) { - cpmm.addMessageListener(msgName, this); - }, this); - }, - - initHelper: function(aWindow, aMessages) { - this.initMessageListener(aMessages); - this.initRequests(); - this._id = this._getRandomId(); - Services.obs.addObserver(this, "inner-window-destroyed", false); - this._window = aWindow; - let util = this._window.QueryInterface(Ci.nsIInterfaceRequestor).getInterface(Ci.nsIDOMWindowUtils); - this.innerWindowID = util.currentInnerWindowID; - }, - - removeMessageListener: function removeMessageListener() { - this._messages.forEach(function(msgName) { - cpmm.removeMessageListener(msgName, this); - }, this); - this._messages = null; + if(this.uninit) { + this.uninit(); + } }, createRequest: function() { diff --git a/dom/contacts/ContactManager.js b/dom/contacts/ContactManager.js index d641f088108a..57728369871e 100644 --- a/dom/contacts/ContactManager.js +++ b/dom/contacts/ContactManager.js @@ -911,7 +911,7 @@ ContactManager.prototype = { }, init: function(aWindow) { - this.initHelper(aWindow, ["Contacts:Find:Return:OK", "Contacts:Find:Return:KO", + this.initDOMRequestHelper(aWindow, ["Contacts:Find:Return:OK", "Contacts:Find:Return:KO", "Contacts:Clear:Return:OK", "Contacts:Clear:Return:KO", "Contact:Save:Return:OK", "Contact:Save:Return:KO", "Contact:Remove:Return:OK", "Contact:Remove:Return:KO", @@ -929,7 +929,9 @@ ContactManager.prototype = { }, classID : CONTACTMANAGER_CID, - QueryInterface : XPCOMUtils.generateQI([nsIDOMContactManager, Ci.nsIDOMGlobalPropertyInitializer]), + QueryInterface : XPCOMUtils.generateQI([nsIDOMContactManager, + Ci.nsIDOMGlobalPropertyInitializer, + Ci.nsISupportsWeakReference]), classInfo : XPCOMUtils.generateCI({classID: CONTACTMANAGER_CID, contractID: CONTACTMANAGER_CONTRACTID, diff --git a/dom/fm/DOMFMRadioChild.js b/dom/fm/DOMFMRadioChild.js index a8163b92da22..af9699046ee2 100644 --- a/dom/fm/DOMFMRadioChild.js +++ b/dom/fm/DOMFMRadioChild.js @@ -42,7 +42,8 @@ DOMFMRadioChild.prototype = { }), QueryInterface: XPCOMUtils.generateQI([Ci.nsIDOMFMRadio, - Ci.nsIDOMGlobalPropertyInitializer]), + Ci.nsIDOMGlobalPropertyInitializer, + Ci.nsISupportsWeakReference]), // nsIDOMGlobalPropertyInitializer implementation init: function(aWindow) { @@ -72,7 +73,7 @@ DOMFMRadioChild.prototype = { "DOMFMRadio:frequencyChange", "DOMFMRadio:powerStateChange", "DOMFMRadio:antennaChange"]; - this.initHelper(aWindow, messages); + this.initDOMRequestHelper(aWindow, messages); let els = Cc["@mozilla.org/eventlistenerservice;1"] .getService(Ci.nsIEventListenerService); diff --git a/dom/messages/SystemMessageManager.js b/dom/messages/SystemMessageManager.js index ee215dedbea5..bd37a421b881 100644 --- a/dom/messages/SystemMessageManager.js +++ b/dom/messages/SystemMessageManager.js @@ -248,7 +248,7 @@ SystemMessageManager.prototype = { // nsIDOMGlobalPropertyInitializer implementation. init: function sysMessMgr_init(aWindow) { debug("init"); - this.initHelper(aWindow, ["SystemMessageManager:Message", + this.initDOMRequestHelper(aWindow, ["SystemMessageManager:Message", "SystemMessageManager:GetPendingMessages:Return"]); let principal = aWindow.document.nodePrincipal; @@ -307,7 +307,8 @@ SystemMessageManager.prototype = { QueryInterface: XPCOMUtils.generateQI([Ci.nsIDOMNavigatorSystemMessages, Ci.nsIDOMGlobalPropertyInitializer, - Ci.nsIObserver]), + Ci.nsIObserver, + Ci.nsISupportsWeakReference]), classInfo: XPCOMUtils.generateCI({ classID: Components.ID("{bc076ea0-609b-4d8f-83d7-5af7cbdc3bb2}"), diff --git a/dom/network/src/NetworkStatsManager.js b/dom/network/src/NetworkStatsManager.js index 9912d2fbcf82..22f7845fd069 100644 --- a/dom/network/src/NetworkStatsManager.js +++ b/dom/network/src/NetworkStatsManager.js @@ -219,7 +219,7 @@ NetworkStatsManager.prototype = { return null; } - this.initHelper(aWindow, ["NetworkStats:Get:Return", + this.initDOMRequestHelper(aWindow, ["NetworkStats:Get:Return", "NetworkStats:Clear:Return"]); }, @@ -232,7 +232,8 @@ NetworkStatsManager.prototype = { classID : NETWORKSTATSMANAGER_CID, QueryInterface : XPCOMUtils.generateQI([nsIDOMMozNetworkStatsManager, - Ci.nsIDOMGlobalPropertyInitializer]), + Ci.nsIDOMGlobalPropertyInitializer, + Ci.nsISupportsWeakReference]), classInfo : XPCOMUtils.generateCI({classID: NETWORKSTATSMANAGER_CID, contractID: NETWORKSTATSMANAGER_CONTRACTID, diff --git a/dom/payment/Payment.js b/dom/payment/Payment.js index 6102cc1bf950..da2a065f0923 100644 --- a/dom/payment/Payment.js +++ b/dom/payment/Payment.js @@ -31,7 +31,8 @@ PaymentContentHelper.prototype = { __proto__: DOMRequestIpcHelper.prototype, QueryInterface: XPCOMUtils.generateQI([Ci.nsINavigatorPayment, - Ci.nsIDOMGlobalPropertyInitializer]), + Ci.nsIDOMGlobalPropertyInitializer, + Ci.nsISupportsWeakReference]), classID: PAYMENTCONTENTHELPER_CID, classInfo: XPCOMUtils.generateCI({ classID: PAYMENTCONTENTHELPER_CID, @@ -78,7 +79,7 @@ PaymentContentHelper.prototype = { init: function(aWindow) { this._window = aWindow; - this.initHelper(aWindow, PAYMENT_IPC_MSG_NAMES); + this.initDOMRequestHelper(aWindow, PAYMENT_IPC_MSG_NAMES); return this.pay.bind(this); }, diff --git a/dom/push/src/Push.js b/dom/push/src/Push.js index 3ae2a6ac573a..0334763578ea 100644 --- a/dom/push/src/Push.js +++ b/dom/push/src/Push.js @@ -34,7 +34,8 @@ Push.prototype = { classID : PUSH_CID, - QueryInterface : XPCOMUtils.generateQI([Ci.nsIDOMGlobalPropertyInitializer]), + QueryInterface : XPCOMUtils.generateQI([Ci.nsIDOMGlobalPropertyInitializer, + Ci.nsISupportsWeakReference]), init: function(aWindow) { debug("init()"); @@ -58,9 +59,7 @@ Push.prototype = { if (perm != Ci.nsIPermissionManager.ALLOW_ACTION) return null; - this.initHelper(aWindow, []); - - this.initMessageListener([ + this.initDOMRequestHelper(aWindow, [ "PushService:Register:OK", "PushService:Register:KO", "PushService:Unregister:OK", diff --git a/dom/system/gonk/RILContentHelper.js b/dom/system/gonk/RILContentHelper.js index b720cc7f5c97..5e2bbc6cd5d6 100644 --- a/dom/system/gonk/RILContentHelper.js +++ b/dom/system/gonk/RILContentHelper.js @@ -375,8 +375,7 @@ function RILContentHelper() { }; this.voicemailInfo = new VoicemailInfo(); - this.initRequests(); - this.initMessageListener(RIL_IPC_MSG_NAMES); + this.initDOMRequestHelper(/* aWindow */ null, RIL_IPC_MSG_NAMES); this._windowsMap = []; Services.obs.addObserver(this, "xpcom-shutdown", false); } @@ -389,7 +388,8 @@ RILContentHelper.prototype = { Ci.nsIVoicemailProvider, Ci.nsITelephonyProvider, Ci.nsIIccProvider, - Ci.nsIObserver]), + Ci.nsIObserver, + Ci.nsISupportsWeakReference]), classID: RILCONTENTHELPER_CID, classInfo: XPCOMUtils.generateCI({classID: RILCONTENTHELPER_CID, classDescription: "RILContentHelper", @@ -1236,7 +1236,7 @@ RILContentHelper.prototype = { observe: function observe(subject, topic, data) { if (topic == "xpcom-shutdown") { - this.removeMessageListener(); + this.destroyDOMRequestHelper(); Services.obs.removeObserver(this, "xpcom-shutdown"); } }, diff --git a/dom/wifi/DOMWifiManager.js b/dom/wifi/DOMWifiManager.js index e22d148ba182..b4703fc3be0c 100644 --- a/dom/wifi/DOMWifiManager.js +++ b/dom/wifi/DOMWifiManager.js @@ -56,7 +56,8 @@ DOMWifiManager.prototype = { flags: Ci.nsIClassInfo.DOM_OBJECT}), QueryInterface: XPCOMUtils.generateQI([Ci.nsIDOMWifiManager, - Ci.nsIDOMGlobalPropertyInitializer]), + Ci.nsIDOMGlobalPropertyInitializer, + Ci.nsISupportsWeakReference]), // nsIDOMGlobalPropertyInitializer implementation init: function(aWindow) { @@ -93,7 +94,7 @@ DOMWifiManager.prototype = { "WifiManager:onwpstimeout", "WifiManager:onwpsfail", "WifiManager:onwpsoverlap", "WifiManager:connectionInfoUpdate", "WifiManager:onconnectingfailed"]; - this.initHelper(aWindow, messages); + this.initDOMRequestHelper(aWindow, messages); this._mm = Cc["@mozilla.org/childprocessmessagemanager;1"].getService(Ci.nsISyncMessageSender); var state = this._mm.sendSyncMessage("WifiManager:getState")[0];