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

Side by Side 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: Created Jan. 8, 2015, 2:14 p.m.
Left:
Right:
Use n/p to move between diff chunks; N/P to move between comments.
Jump to:
View unified diff | Download patch
« no previous file with comments | « safari/ext/background.js ('k') | no next file » | no next file with comments »
Toggle Intra-line Diffs ('i') | Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
OLDNEW
1 /* 1 /*
2 * This file is part of Adblock Plus <http://adblockplus.org/>, 2 * This file is part of Adblock Plus <http://adblockplus.org/>,
3 * Copyright (C) 2006-2014 Eyeo GmbH 3 * Copyright (C) 2006-2014 Eyeo GmbH
4 * 4 *
5 * Adblock Plus is free software: you can redistribute it and/or modify 5 * Adblock Plus is free software: you can redistribute it and/or modify
6 * it under the terms of the GNU General Public License version 3 as 6 * it under the terms of the GNU General Public License version 3 as
7 * published by the Free Software Foundation. 7 * published by the Free Software Foundation.
8 * 8 *
9 * Adblock Plus is distributed in the hope that it will be useful, 9 * Adblock Plus is distributed in the hope that it will be useful,
10 * but WITHOUT ANY WARRANTY; without even the implied warranty of 10 * but WITHOUT ANY WARRANTY; without even the implied warranty of
(...skipping 19 matching lines...) Expand all
30 { 30 {
31 var response = {}; 31 var response = {};
32 for (var prop in request) 32 for (var prop in request)
33 response[prop] = request[prop]; 33 response[prop] = request[prop];
34 response.payload = message; 34 response.payload = message;
35 35
36 this._messageDispatcher.dispatchMessage("response", response); 36 this._messageDispatcher.dispatchMessage("response", response);
37 }, 37 },
38 handleRequest: function(request, sender) 38 handleRequest: function(request, sender)
39 { 39 {
40 var sendResponse;
41 if ("callbackId" in request) 40 if ("callbackId" in request)
42 sendResponse = this._sendResponse.bind(this, request); 41 {
42 var sent = false;
43 var sendResponse = function(message)
44 {
45 this._sendResponse(request, message);
46 sent = true;
47 }.bind(this);
48
49 var results = ext.onMessage._dispatch(request.payload, sender, sendRespo nse);
50
51 // The onMessage listener has to return true to indicate that a response
52 // is sent later asynchronously. Otherwise if no response were sent yet,
Sebastian Noack 2015/01/08 14:36:13 Done.
53 // we sent a response indicating that there is no response, that
54 // the other end, can remove the callback and doesn't leak memory.
55 if (!sent && results.indexOf(true) != -1)
Wladimir Palant 2015/01/08 14:28:19 Shouldn't this be .indexOf(true) == -1?
56 this._sendResponse(request, null);
Wladimir Palant 2015/01/08 14:28:19 I'd rather go with undefined here, that's what the
57 }
43 else 58 else
44 sendResponse = function() {}; 59 {
45 60 ext.onMessage._dispatch(request.payload, sender, function() {});
46 ext.onMessage._dispatch(request.payload, sender, sendResponse); 61 }
47 }, 62 },
48 handleResponse: function(response) 63 handleResponse: function(response)
49 { 64 {
50 var callbackId = response.callbackId; 65 var callbackId = response.callbackId;
51 var callback = this._responseCallbacks[callbackId]; 66 var callback = this._responseCallbacks[callbackId];
52 if (callback) 67 if (callback)
53 { 68 {
54 delete this._responseCallbacks[callbackId]; 69 delete this._responseCallbacks[callbackId];
55 callback(response.payload); 70
71 if (response.payload !== null)
72 callback(response.payload);
56 } 73 }
57 }, 74 },
58 sendMessage: function(message, responseCallback, extra) 75 sendMessage: function(message, responseCallback, extra)
59 { 76 {
60 var request = {payload: message}; 77 var request = {payload: message};
61 78
62 if (responseCallback) 79 if (responseCallback)
63 { 80 {
64 request.callbackId = ++this._responseCallbackCounter; 81 request.callbackId = ++this._responseCallbackCounter;
65 this._responseCallbacks[request.callbackId] = responseCallback; 82 this._responseCallbacks[request.callbackId] = responseCallback;
(...skipping 128 matching lines...) Expand 10 before | Expand all | Expand 10 after
194 }; 211 };
195 212
196 213
197 /* Utils */ 214 /* Utils */
198 215
199 ext.getURL = function(path) 216 ext.getURL = function(path)
200 { 217 {
201 return safari.extension.baseURI + path; 218 return safari.extension.baseURI + path;
202 }; 219 };
203 })(); 220 })();
OLDNEW
« 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