Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code

Unified Diff: safari/ext/common.js

Issue 6751260996796416: Issue 1724 - Fixed memory leak in messaging code, when no response is sent, on Safari (Closed)
Patch Set: Addressed comments Created Jan. 8, 2015, 2:35 p.m.
Use n/p to move between diff chunks; N/P to move between comments.
Jump to:
View side-by-side diff with in-line comments
Download patch
« no previous file with comments | « safari/ext/background.js ('k') | no next file » | no next file with comments »
Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
Index: safari/ext/common.js
===================================================================
--- a/safari/ext/common.js
+++ b/safari/ext/common.js
@@ -31,19 +31,35 @@
var response = {};
for (var prop in request)
response[prop] = request[prop];
- response.payload = message;
+ if (typeof message != "undefined")
+ response.payload = message;
this._messageDispatcher.dispatchMessage("response", response);
},
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 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)
+ this._sendResponse(request, undefined);
+ }
else
- sendResponse = function() {};
-
- ext.onMessage._dispatch(request.payload, sender, sendResponse);
+ {
+ ext.onMessage._dispatch(request.payload, sender, function() {});
+ }
},
handleResponse: function(response)
{
@@ -52,7 +68,9 @@
if (callback)
{
delete this._responseCallbacks[callbackId];
- callback(response.payload);
+
+ if ("payload" in response)
+ callback(response.payload);
}
},
sendMessage: function(message, responseCallback, extra)
« no previous file with comments | « safari/ext/background.js ('k') | no next file » | no next file with comments »

Powered by Google App Engine
This is Rietveld