Add frameId to MessageSender (extension messaging API)

chrome/test/data/extensions/api_test/messaging/connect/test.js

Index: chrome/test/data/extensions/api_test/messaging/connect/test.js
diff --git a/chrome/test/data/extensions/api_test/messaging/connect/test.js b/chrome/test/data/extensions/api_test/messaging/connect/test.js
index d76997fd34bccc78feb735d7c6a85a1ca5f24eed..29a870e65f0d226d010700fbb73708ed50350405 100644
--- a/chrome/test/data/extensions/api_test/messaging/connect/test.js
+++ b/chrome/test/data/extensions/api_test/messaging/connect/test.js
@@ -70,6 +70,7 @@ chrome.test.getConfig(function(config) {
       listenOnce(chrome.runtime.onConnect, function(port) {
         chrome.test.assertEq({
           tab: testTab,
+          frameId: 0, // Main frame
           url: testTab.url,
            id: chrome.runtime.id
         }, port.sender);
@@ -95,6 +96,7 @@ chrome.test.getConfig(function(config) {
         function(request, sender, sendResponse) {
           chrome.test.assertEq({
             tab: testTab,
+            frameId: 0, // Main frame
             url: testTab.url,
              id: chrome.runtime.id
           }, sender);
@@ -116,6 +118,51 @@ chrome.test.getConfig(function(config) {
       chrome.test.log("sendMessageFromTab: sent first message to tab");
     },
 
+    // Tests that a message from a child frame has a correct frameId.
+    function sendMessageFromFrameInTab() {
+      var senderFrameUrl = {};
+      listenForever(
+        chrome.runtime.onMessage,
+        function(request, sender, sendResponse) {
+          // Child frames have a positive frameId.
+          chrome.test.assertTrue(sender.frameId >= 1,
+              'frameId must be positive, but it is ' + sender.frameId);
+          chrome.test.assertFalse(sender.frameId in messageSenders,
+              'frameId ' + sender.frameId + ' was already seen before!');
+          senderFrameUrl[sender.frameId] = request.frameUrl;
+          delete sender.frameId;
+          chrome.test.assertEq({

[not at google - send to devlin, 2014/11/11 22:23:00]

Test is better now, but I think you can make it even clearer. I just don't like
this "delete sender.frameId" part, and the "frameId >= 1" assertion seems
redundant if your assertions are useful (like I mentioned).

Basically:

1. Remove lines 127-131 and 134-138 (leaving just 132).
2. Rename senderFrameUrl to "senders" and just make it a list of senders, then
line 132 becomes just appending the sender to it.
3. When you're done and you fetch the frames, rather than iterating over the
details, transform it into a list of what the senders *should* have been. Call
it expectedSenders or something.
4. Sort each list so you can do just assertEq(expectedSenders, senders).

Does that make sense?

Sorry to keep on banging on about the test, but I'd like this to be as solid as
possible.

[robwu, 2014/11/11 22:44:38]

The list of frames *could* potentially contain more frames (due to the way
navigations are handled). I've applied your changes for now, but if the test
fail or become flaky, then I'll update the test to explicitly filter frames.
Yes. Done.
No problem, I'm not offended at all.

+            tab: testTab,
+            url: request.frameUrl,
+            id: chrome.runtime.id
+          }, sender);
+
+          if (Object.keys(senderFrameUrl).length == 2) {
+            // Now test whether the frameId that we got back makes sense...
+            chrome.webNavigation.getAllFrames({
+              tabId: testTab.id
+            }, function(details) {
+              var framesFound = 0;
+              details.forEach(function(frame) {
+                var url = senderFrameUrl[frame.frameId];
+                if (url) {
+                  chrome.test.assertEq(frame.url, url);
+                  ++framesFound;
+                }
+              });
+              chrome.test.assertEq(framesFound, 2);
+              doneListening();
+            });
+          }
+        }
+      );
+
+      var port = chrome.tabs.connect(testTab.id);
+      port.postMessage({testSendMessageFromFrame: true});
+      port.disconnect();
+      chrome.test.log("sendMessageFromFrameInTab: send 1st message to tab");
+    },
+
     // Tests error handling when sending a request from a content script to an
     // invalid extension.
     function sendMessageFromTabError() {