Issue 1724 - Fixed memory leak in messaging code, when no response is sent, on Safari

safari/ext/common.js

Index: safari/ext/common.js
===================================================================
--- a/safari/ext/common.js
+++ b/safari/ext/common.js
@@ -37,13 +37,28 @@
     },
     handleRequest: function(request, sender)
     {
-      var sendResponse;
       if ("callbackId" in request)
-        sendResponse = this._sendResponse.bind(this, request);
+      {
+        var sent = false;
+        var sendResponse = function(message)
+        {
+          this._sendResponse(request, message);
+          sent = true;
+        }.bind(this);
+
+        var results = ext.onMessage._dispatch(request.payload, sender, sendResponse);
+
+        // The onMessage listener has to return true to indicate that a response
+        // is sent later asynchronously. Otherwise if no response were sent yet,

[Sebastian Noack, 2015/01/08 14:36:13]

Done.

+        // we sent a response indicating that there is no response, that
+        // the other end, can remove the callback and doesn't leak memory.
+        if (!sent && results.indexOf(true) != -1)

[Wladimir Palant, 2015/01/08 14:28:19]

Shouldn't this be .indexOf(true) == -1?

+          this._sendResponse(request, null);

[Wladimir Palant, 2015/01/08 14:28:19]

I'd rather go with undefined here, that's what the Firefox implementation does.
There might be valid reasons to send null as a response.

I assume that undefined cannot be sent as a property value. That's why the
Firefox implementation of _sendResponse() explicitly sets |responseSent: typeof
response != "undefined"| property in the response.

+      }
       else
-        sendResponse = function() {};
-
-      ext.onMessage._dispatch(request.payload, sender, sendResponse);
+      {
+        ext.onMessage._dispatch(request.payload, sender, function() {});
+      }
     },
     handleResponse: function(response)
     {
@@ -52,7 +67,9 @@
       if (callback)
       {
         delete this._responseCallbacks[callbackId];
-        callback(response.payload);
+
+        if (response.payload !== null)
+          callback(response.payload);
       }
     },
     sendMessage: function(message, responseCallback, extra)