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..6dff62ac951810c1e9f084c4cdbc437d77195af4 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,29 @@ chrome.test.getConfig(function(config) {
       chrome.test.log("sendMessageFromTab: sent first message to tab");
     },
 
+    // Tests that a message from a child frame has a non-negative frameId.
+    function sendMessageFromFrameInTab() {
+      listenOnce(
+        chrome.runtime.onMessage,
+        function(request, sender, sendResponse) {
+          chrome.test.log("sendMessageFromFrameInTab: got frameId " +

[not at google - send to devlin, 2014/11/10 18:36:42]

Remove logging before submitting.

[robwu, 2014/11/10 21:43:07]

Other tests in this file also log this information. When these test fail, this
information will aid in diagnosing the problem, so I think that they'd better be
kept.

[not at google - send to devlin, 2014/11/11 00:24:03]

Oh right, and I meant to respond to this. I agree that it's good to know why
tests fail, when they fail, but ideally that should mean a meaningful failure
message, not logging stuff every time a test is run.

[robwu, 2014/11/11 00:34:26]

So, what should this be?
- Removed?
- if (...) test.fail() ?

[not at google - send to devlin, 2014/11/11 00:37:08]

Like,

chrome.test.assertTrue(sender.frameId >= 1,
  'Illegal frame ID ' + sender.frameId + ', must be >= 1');

that sort of thing.

[robwu, 2014/11/11 00:41:03]

Done.

+              sender.frameId);
+          chrome.test.assertTrue(sender.frameId >= 1);

[not at google - send to devlin, 2014/11/10 18:36:42]

Add comment like "frameId of 0 means the main frame, and we know this request
doesn't come from the main frame".

Also note that you could actually test this using the webNavigation API though
it frightens me to imagine how to make that not racy.

[robwu, 2014/11/10 21:43:07]

Done.
chrome.webNavigation.getAllFrames seems non-racy for this purpose. I've added a
webNavigation call because it gives more confidence that the frameId is correct.

+          delete sender.frameId;
+          chrome.test.assertEq({
+            tab: testTab,
+            url: request.frameUrl,
+            id: chrome.runtime.id
+          }, sender);
+        }
+      );
+
+      var port = chrome.tabs.connect(testTab.id);
+      port.postMessage({testSendMessageFromFrame: true});
+      port.disconnect();
+      chrome.test.log("sendMessageFromFrameInTab: send 1st message to tab");

[not at google - send to devlin, 2014/11/10 18:36:42]

Remove logging before submitting.

[robwu, 2014/11/10 21:43:07]

Same note as before: This file is full of chrome.test.log calls before opening
the tab.

+    },
+
     // Tests error handling when sending a request from a content script to an
     // invalid extension.
     function sendMessageFromTabError() {