Deprecate chrome.extension.sendRequest in favor of sendMessage, with a safer

chrome/renderer/resources/extensions/miscellaneous_bindings.js

Index: chrome/renderer/resources/extensions/miscellaneous_bindings.js
diff --git a/chrome/renderer/resources/extensions/miscellaneous_bindings.js b/chrome/renderer/resources/extensions/miscellaneous_bindings.js
index 84556f3b14cc07c26058beae8140586bacdf948f..b20d34d14276eb1c20c03e7dcb3163c44a5acaf3 100644
--- a/chrome/renderer/resources/extensions/miscellaneous_bindings.js
+++ b/chrome/renderer/resources/extensions/miscellaneous_bindings.js
@@ -20,8 +20,10 @@
   var manifestVersion;
   var extensionId;
 
-  // The reserved channel name for the sendRequest API.
+  // The reserved channel name for the sendRequest/sendMessage APIs.
+  // Note: sendRequest is deprecated.
   chromeHidden.kRequestChannel = "chrome.extension.sendRequest";
+  chromeHidden.kMessageChannel = "chrome.extension.sendMessage";
 
   // Map of port IDs to port object.
   var ports = {};
@@ -95,6 +97,30 @@
     return port;
   };
 
+  // Helper function for dispatchOnConnect.
+  function handleSendRequestError(isSendMessage, responseCallbackAccessed,
+                                  sourceExtensionId, targetExtensionId) {
+    var errorMsg;
+    var eventName = (isSendMessage  ?
+        "chrome.extension.onMessage" : "chrome.extension.onRequest");
+    if (isSendMessage && !responseCallbackAccessed) {
+      errorMsg =
+          "The " + eventName + " listener must return true if you want to" +
+          " send a response after the listener returns ";
+    } else {
+      errorMsg =
+          "Cannot send a response more than once per " + eventName +
+          " listener per document";
+    }
+    errorMsg += " (message was sent by extension " + sourceExtensionId;
+    if (sourceExtensionId != targetExtensionId)
+      errorMsg += " for extension " + targetExtensionId;
+    errorMsg += ").";
+
+    chrome.extension.lastError = {"message": errorMsg};
+    console.error("Could not send response: " + errorMsg);
+  }
+
   // Called by native code when a channel has been opened to this context.
   chromeHidden.Port.dispatchOnConnect = function(portId, channelName, tab,
                                                  sourceExtensionId,
@@ -116,13 +142,19 @@
       tab = chromeHidden.JSON.parse(tab);
     var sender = {tab: tab, id: sourceExtensionId};
 
-    // Special case for sendRequest/onRequest.
-    if (channelName == chromeHidden.kRequestChannel) {
-      var requestEvent = (isExternal ?
-          chrome.extension.onRequestExternal : chrome.extension.onRequest);
+    // Special case for sendRequest/onRequest and sendMessage/onMessage.
+    var isSendMessage = channelName == chromeHidden.kMessageChannel;
+    if (channelName == chromeHidden.kRequestChannel ||
+        channelName == chromeHidden.kMessageChannel) {
+      var requestEvent = (isSendMessage ?

[Aaron Boodman, 2012/03/30 21:28:00]

O_o

[Matt Perry, 2012/03/30 23:02:48]

Yeah I went there.

+         (isExternal ?
+             chrome.extension.onMessageExternal : chrome.extension.onMessage) :
+         (isExternal ?
+             chrome.extension.onRequestExternal : chrome.extension.onRequest));
       if (requestEvent.hasListeners()) {
         var port = chromeHidden.Port.createPort(portId, channelName);
         port.onMessage.addListener(function(request) {
+          var responseCallbackAccessed = false;
           var responseCallback = function(response) {
             if (port) {
               port.postMessage(response);
@@ -130,16 +162,8 @@
             } else {
               // We nulled out port when sending the response, and now the page
               // is trying to send another response for the same request.
-              var errorMsg =
-                  "Cannot send a response more than once per " +
-                  "chrome.extension.onRequest listener per document (message " +
-                  "was sent by extension " + sourceExtensionId;
-              if (sourceExtensionId != targetExtensionId) {
-                errorMsg += " for extension " + targetExtensionId;
-              }
-              errorMsg += ").";
-              chrome.extension.lastError = {"message": errorMsg};
-              console.error("Could not send response: " + errorMsg);
+              handleSendRequestError(isSendMessage, responseCallbackAccessed,
+                                     sourceExtensionId, targetExtensionId);
             }
           };
           // In case the extension never invokes the responseCallback, and also
@@ -152,10 +176,26 @@
               port = null;
             }
           });
-          requestEvent.dispatch(request, sender, responseCallback);
+          if (!isSendMessage) {
+            requestEvent.dispatch(request, sender, responseCallback);
+          } else {
+            var retvals = requestEvent.dispatch(request, sender,
+                                                responseCallback);
+            for (var i in retvals) {
+              if (retvals[i] === true) {
+                responseCallbackAccessed = true;

[Aaron Boodman, 2012/03/30 21:28:00]

Were you originally trying to do some craziness where you detect whether they
use the callback variable? That is an interesting idea, but now I think the name
is out of date?

[Aaron Boodman, 2012/03/30 21:28:00]

responseCallback = retvals.indexOf(true) > -1;

[Matt Perry, 2012/03/30 23:02:48]

Yep. New name.

[Matt Perry, 2012/03/30 23:02:48]

Sweet, done.

+                break;
+              }
+            }
+            if (!responseCallbackAccessed) {
+              // If they didn't access the response callback, they're not
+              // going to send a response, so clean up the port immediately.
+              port.destroy_();
+              port = null;
+            }
+          }
         });
       }
-      return;
     }
 
     var connectEvent = (isExternal ?
@@ -203,6 +243,40 @@
     }
   };
 
+  // Shared implementation used by tabs.sendMessage and extension.sendMessage.
+  chromeHidden.Port.sendMessageImpl = function(port, request,
+                                               responseCallback) {
+    port.postMessage(request);
+
+    if (port.name == chromeHidden.kMessageChannel && !responseCallback) {
+      // Go ahead and disconnect immediately if the sender is not expecting
+      // a response.
+      port.disconnect();
+      return;
+    }
+
+    if (!responseCallback)

[Aaron Boodman, 2012/03/30 21:28:00]

Maybe add a comment that this is used by the older kRequestChannel?

[Matt Perry, 2012/03/30 23:02:48]

Done.

+      responseCallback = function() {};
+
+    port.onDisconnect.addListener(function() {
+      // For onDisconnects, we only notify the callback if there was an error
+      try {
+        if (chrome.extension.lastError)
+          responseCallback();
+      } finally {
+        port = null;
+      }
+    });
+    port.onMessage.addListener(function(response) {
+      try {
+        responseCallback(response);
+      } finally {
+        port.disconnect();
+        port = null;
+      }
+    });
+  }
+
   // This function is called on context initialization for both content scripts
   // and extension contexts.
   chromeHidden.onLoad.addListener(function(tempExtensionId,