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

safari/ext/common.js

 /*
  * This file is part of Adblock Plus <http://adblockplus.org/>,
  * Copyright (C) 2006-2014 Eyeo GmbH
  *
  * Adblock Plus is free software: you can redistribute it and/or modify
  * it under the terms of the GNU General Public License version 3 as
  * published by the Free Software Foundation.
  *
  * Adblock Plus is distributed in the hope that it will be useful,
  * but WITHOUT ANY WARRANTY; without even the implied warranty of
@@ skipping 31 matching lines @@
         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.

+        // is sent later asynchronously. Otherwise if no response was sent yet,
         // 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.

+        // the other end can remove the callback and doesn't leak memory.
+        if (!sent && results.indexOf(true) == -1)
+          this._sendResponse(request, undefined);
       }
       else
       {
         ext.onMessage._dispatch(request.payload, sender, function() {});
       }
     },
     handleResponse: function(response)
     {
       var callbackId = response.callbackId;
       var callback = this._responseCallbacks[callbackId];
       if (callback)
       {
         delete this._responseCallbacks[callbackId];
 
-        if (response.payload !== null)
+        if (typeof response.payload != "undefined")
           callback(response.payload);
       }
     },
     sendMessage: function(message, responseCallback, extra)
     {
       var request = {payload: message};
 
       if (responseCallback)
       {
         request.callbackId = ++this._responseCallbackCounter;
@@ skipping 129 matching lines @@
   };
 
 
   /* Utils */
 
   ext.getURL = function(path)
   {
     return safari.extension.baseURI + path;
   };
 })();