Add frameId to MessageSender (extension messaging API)

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

 // Copyright (c) 2011 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 var listenOnce = chrome.test.listenOnce;
 var listenForever = chrome.test.listenForever;
 
 JSON.parse = function() {
   return "JSON.parse clobbered by extension.";
 };
@@ skipping 52 matching lines @@
         chrome.test.assertEq(msg.portName, portName);
         port.disconnect();
       });
     },
 
     // Tests that postMessage from the tab and its response works.
     function postMessageFromTab() {
       listenOnce(chrome.runtime.onConnect, function(port) {
         chrome.test.assertEq({
           tab: testTab,
+          frameId: 0, // Main frame
           url: testTab.url,
            id: chrome.runtime.id
         }, port.sender);
         listenOnce(port.onMessage, function(msg) {
           chrome.test.assertTrue(msg.testPostMessageFromTab);
           port.postMessage({success: true, portName: port.name});
           chrome.test.log("postMessageFromTab: got message from tab");
         });
       });
 
       var port = chrome.tabs.connect(testTab.id);
       port.postMessage({testPostMessageFromTab: true});
       chrome.test.log("postMessageFromTab: sent first message to tab");
       listenOnce(port.onMessage, function(msg) {
         port.disconnect();
       });
     },
 
     // Tests receiving a request from a content script and responding.
     function sendMessageFromTab() {
       var doneListening = listenForever(
         chrome.runtime.onMessage,
         function(request, sender, sendResponse) {
           chrome.test.assertEq({
             tab: testTab,
+            frameId: 0, // Main frame
             url: testTab.url,
              id: chrome.runtime.id
           }, sender);
           if (request.step == 1) {
             // Step 1: Page should send another request for step 2.
             chrome.test.log("sendMessageFromTab: got step 1");
             sendResponse({nextStep: true});
           } else {
             // Step 2.
             chrome.test.assertEq(request.step, 2);
             sendResponse();
             doneListening();
           }
       });
 
       var port = chrome.tabs.connect(testTab.id);
       port.postMessage({testSendMessageFromTab: true});
       port.disconnect();
       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() {
       listenOnce(
         chrome.runtime.onMessage,
         function(request, sender, sendResponse) {
           if (!request.success)
             chrome.test.fail();
         }
       );
@@ skipping 76 matching lines @@
       } catch(e) {
         error = e;
       }
       chrome.test.assertTrue(error != undefined);
 
       chrome.test.succeed();
     },
 
   ]);
 });