Bug 1391353: Try to avoid keeping message data alive while waiting for responses. r?zombie draft
authorKris Maglione <maglione.k@gmail.com>
Thu, 17 Aug 2017 11:41:55 -0700
changeset 648390 058c0cd70643bba47be392146224a0856d09dc64
parent 648362 df895f38e74e70debb9e9a679c3c52d46035a3ae
child 648554 42126ce3427c591ad5c5983491059814f83ba2b2
child 649170 0dcec4c601fdfe0d654f0310ec15813350fe9bb3
push id74746
push usermaglione.k@gmail.com
push dateThu, 17 Aug 2017 18:42:17 +0000
reviewerszombie
bugs1391353
milestone57.0a1
Bug 1391353: Try to avoid keeping message data alive while waiting for responses. r?zombie MozReview-Commit-ID: 5JAUBWufpsf
toolkit/components/extensions/ExtensionChild.jsm
toolkit/components/extensions/MessageChannel.jsm
--- a/toolkit/components/extensions/ExtensionChild.jsm
+++ b/toolkit/components/extensions/ExtensionChild.jsm
@@ -333,16 +333,17 @@ class Messenger {
     let promise = this._sendMessage(messageManager, "Extension:Message", holder, recipient)
       .catch(error => {
         if (error.result == MessageChannel.RESULT_NO_HANDLER) {
           return Promise.reject({message: "Could not establish connection. Receiving end does not exist."});
         } else if (error.result != MessageChannel.RESULT_NO_RESPONSE) {
           return Promise.reject({message: error.message});
         }
       });
+    holder = null;
 
     return this.context.wrapPromise(promise, responseCallback);
   }
 
   sendNativeMessage(messageManager, msg, recipient, responseCallback) {
     msg = NativeApp.encodeMessage(this.context, msg);
     return this.sendMessage(messageManager, msg, recipient, responseCallback);
   }
@@ -369,22 +370,26 @@ class Messenger {
           let promise = new Promise(resolve => {
             sendResponse = value => {
               resolve(value);
               response = promise;
             };
           });
 
           let message = holder.deserialize(this.context.cloneScope);
+          holder = null;
+
           sender = Cu.cloneInto(sender, this.context.cloneScope);
           sendResponse = Cu.exportFunction(sendResponse, this.context.cloneScope);
 
           // Note: We intentionally do not use runSafe here so that any
           // errors are propagated to the message sender.
           let result = fire.raw(message, sender, sendResponse);
+          message = null;
+
           if (result instanceof this.context.cloneScope.Promise) {
             return result;
           } else if (result === true) {
             return promise;
           }
           return response;
         },
       };
--- a/toolkit/components/extensions/MessageChannel.jsm
+++ b/toolkit/components/extensions/MessageChannel.jsm
@@ -517,16 +517,17 @@ this.MessageChannel = {
    */
   sendMessage(target, messageName, data, options = {}) {
     let sender = options.sender || {};
     let recipient = options.recipient || {};
     let responseType = options.responseType || this.RESPONSE_SINGLE;
 
     let channelId = ExtensionUtils.getUniqueId();
     let message = {messageName, channelId, sender, recipient, data, responseType};
+    data = null;
 
     if (responseType == this.RESPONSE_NONE) {
       try {
         target.sendAsyncMessage(MESSAGE_MESSAGE, message);
       } catch (e) {
         // Caller is not expecting a reply, so dump the error to the console.
         Cu.reportError(e);
         return Promise.reject(e);
@@ -556,16 +557,17 @@ this.MessageChannel = {
     };
     deferred.promise.then(cleanup, cleanup);
 
     try {
       target.sendAsyncMessage(MESSAGE_MESSAGE, message);
     } catch (e) {
       deferred.reject(e);
     }
+    message = null;
     return deferred.promise;
   },
 
   _callHandlers(handlers, data) {
     let responseType = data.responseType;
 
     // At least one handler is required for all response types but
     // RESPONSE_ALL.
@@ -590,16 +592,17 @@ this.MessageChannel = {
 
     let responses = handlers.map(handler => {
       try {
         return handler.receiveMessage(data);
       } catch (e) {
         return Promise.reject(e);
       }
     });
+    data = null;
     responses = responses.filter(response => response !== undefined);
 
     switch (responseType) {
       case this.RESPONSE_FIRST:
         if (responses.length == 0) {
           return Promise.reject({result: MessageChannel.RESULT_NO_RESPONSE,
                                  message: "No handler returned a response"});
         }
@@ -629,36 +632,38 @@ this.MessageChannel = {
       handlers.forEach(handler => {
         // The sender expects no reply, so dump any errors to the console.
         new Promise(resolve => {
           resolve(handler.receiveMessage(data));
         }).catch(e => {
           Cu.reportError(e.stack ? `${e}\n${e.stack}` : e.message || e);
         });
       });
+      data = null;
       // Note: Unhandled messages are silently dropped.
       return;
     }
 
     let target = new MessageManagerProxy(data.target);
 
     let deferred = {
       sender: data.sender,
       messageManager: target,
       channelId: data.channelId,
     };
     deferred.promise = new Promise((resolve, reject) => {
       deferred.reject = reject;
 
       this._callHandlers(handlers, data).then(resolve, reject);
+      data = null;
     }).then(
       value => {
         let response = {
           result: this.RESULT_SUCCESS,
-          messageName: data.channelId,
+          messageName: deferred.channelId,
           recipient: {},
           value,
         };
 
         target.sendAsyncMessage(MESSAGE_RESPONSE, response);
       },
       error => {
         if (target.isDisconnected) {
@@ -668,17 +673,17 @@ this.MessageChannel = {
               error.result !== this.RESULT_NO_RESPONSE) {
             Cu.reportError(Cu.getClassName(error, false) === "Object" ? error.message : error);
           }
           return;
         }
 
         let response = {
           result: this.RESULT_ERROR,
-          messageName: data.channelId,
+          messageName: deferred.channelId,
           recipient: {},
           error: {},
         };
 
         if (error && typeof(error) == "object") {
           if (error.result) {
             response.result = error.result;
           }