Make sure ports are closed when they're no longer used.

Make sure ports are closed when they're no longer used.

It appears to be a common pattern that extensions don't invoke the
responseCallback, and so the port objects leak. Register a callback that
triggers when the responseCallback is finalized to free up these resources.

BUG=114738, 93566
TEST=manual

Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=126884

[jochen (gone - plz use gerrit), 2012-03-13 16:41:22]

plz review

[Matt Perry, 2012-03-13 19:54:39]

Awesome, thanks! Can you also update the docs here
(http://code.google.com/chrome/extensions/trunk/messaging.html )? Currently it
reads "The request will remain open until you call sendResponse, so it is good
practice to call sendResponse with an empty object to allow the request to be
cleaned up."

This is no longer true, and due to http://crbug.com/93566 is a bad idea anyway.
I think you can just remove it (and update the sample code to not call
sendResponse with empty args.

http://codereview.chromium.org/9693048/diff/1/chrome/renderer/extensions/misc...
File chrome/renderer/extensions/miscellaneous_bindings.cc (right):

http://codereview.chromium.org/9693048/diff/1/chrome/renderer/extensions/misc...
chrome/renderer/extensions/miscellaneous_bindings.cc:178: }
nit: can you add an "else NOTREACHED()" here so we know we're invoking it
correctly? It should be guaranteed to be called with the right args.

http://codereview.chromium.org/9693048/diff/1/chrome/renderer/resources/exten...
File chrome/renderer/resources/extensions/miscellaneous_bindings.js (right):

http://codereview.chromium.org/9693048/diff/1/chrome/renderer/resources/exten...
chrome/renderer/resources/extensions/miscellaneous_bindings.js:152:
port.postMessage(null);
Change this to port.disconnect(); We shouldn't send a response on the
extension's behalf. See http://crbug.com/93566 for more info.

BTW, please add that bug to the CL description, since this change will fix it.

[jochen (gone - plz use gerrit), 2012-03-14 09:20:05]

all done

[Matt Perry, 2012-03-14 23:22:47]

lgtm

[commit-bot: I haz the power, 2012-03-15 08:25:38]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/jochen@chromium.org/9693048/6001

[commit-bot: I haz the power, 2012-03-15 10:43:23]

Change committed as 126884