Bug 1367138 fix webrequest frameId and parentFrameId, r=aswan
authorShane Caraveo <scaraveo@mozilla.com>
Fri, 02 Jun 2017 12:05:13 -0700
changeset 412563 71d8ec8f89d49c2242970902630617bc2be9f603
parent 412562 d62f2d63e674eb78ba995bec230c3c0a3b8d7c19
child 412564 0ff4ad45f99c531c6760e9540864c4b6597852d0
push id1490
push usermtabara@mozilla.com
push dateMon, 31 Jul 2017 14:08:16 +0000
treeherdermozilla-release@70e32e6bf15e [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersaswan
bugs1367138
milestone55.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1367138 fix webrequest frameId and parentFrameId, r=aswan MozReview-Commit-ID: F8lD8vvfSQ5
toolkit/components/extensions/ext-webRequest.js
toolkit/components/extensions/test/mochitest/file_simple_xhr.html
toolkit/components/extensions/test/mochitest/file_simple_xhr_frame.html
toolkit/components/extensions/test/mochitest/file_simple_xhr_frame2.html
toolkit/components/extensions/test/mochitest/mochitest-common.ini
toolkit/components/extensions/test/mochitest/test_ext_webrequest_frameId.html
toolkit/modules/addons/WebRequest.jsm
--- a/toolkit/components/extensions/ext-webRequest.js
+++ b/toolkit/components/extensions/ext-webRequest.js
@@ -47,18 +47,18 @@ function WebRequestEventManager(context,
         requestId: data.requestId,
         url: data.url,
         originUrl: data.originUrl,
         documentUrl: data.documentUrl,
         method: data.method,
         tabId: browserData.tabId,
         type: data.type,
         timeStamp: Date.now(),
-        frameId: data.type == "main_frame" ? 0 : data.windowId,
-        parentFrameId: data.type == "main_frame" ? -1 : data.parentWindowId,
+        frameId: data.windowId,
+        parentFrameId: data.parentWindowId,
       };
 
       const maybeCached = ["onResponseStarted", "onBeforeRedirect", "onCompleted", "onErrorOccurred"];
       if (maybeCached.includes(eventName)) {
         data2.fromCache = !!data.fromCache;
       }
 
       if ("ip" in data) {
new file mode 100644
--- /dev/null
+++ b/toolkit/components/extensions/test/mochitest/file_simple_xhr.html
@@ -0,0 +1,19 @@
+<!DOCTYPE HTML>
+
+<html>
+<head>
+<meta charset="utf-8">
+</head>
+<body>
+
+<script>
+"use strict";
+
+let req = new XMLHttpRequest();
+req.open("GET", "http://example.org/example.txt");
+req.send();
+</script>
+<img src="file_image_good.png"/>
+<iframe src="file_simple_xhr_frame.html"/>
+</body>
+</html>
new file mode 100644
--- /dev/null
+++ b/toolkit/components/extensions/test/mochitest/file_simple_xhr_frame.html
@@ -0,0 +1,19 @@
+<!DOCTYPE HTML>
+
+<html>
+<head>
+<meta charset="utf-8">
+</head>
+<body>
+
+<script>
+"use strict";
+
+let req = new XMLHttpRequest();
+req.open("GET", "/xhr_resource");
+req.send();
+</script>
+<img src="file_image_bad.png"/>
+<iframe src="file_simple_xhr_frame2.html"/>
+</body>
+</html>
new file mode 100644
--- /dev/null
+++ b/toolkit/components/extensions/test/mochitest/file_simple_xhr_frame2.html
@@ -0,0 +1,18 @@
+<!DOCTYPE HTML>
+
+<html>
+<head>
+<meta charset="utf-8">
+</head>
+<body>
+
+<script>
+"use strict";
+
+let req = new XMLHttpRequest();
+req.open("GET", "/xhr_resource_2");
+req.send();
+</script>
+<img src="file_image_bad.png#2"/>
+</body>
+</html>
--- a/toolkit/components/extensions/test/mochitest/mochitest-common.ini
+++ b/toolkit/components/extensions/test/mochitest/mochitest-common.ini
@@ -31,16 +31,19 @@ support-files =
   file_style_bad.css
   file_style_redirect.css
   file_script_good.js
   file_script_bad.js
   file_script_redirect.js
   file_script_xhr.js
   file_remote_frame.html
   file_sample.html
+  file_simple_xhr.html
+  file_simple_xhr_frame.html
+  file_simple_xhr_frame2.html
   redirection.sjs
   file_privilege_escalation.html
   file_ext_test_api_injection.js
   file_permission_xhr.html
   file_teardown_test.js
   return_headers.sjs
   webrequest_worker.js
   !/toolkit/components/passwordmgr/test/authenticate.sjs
@@ -101,16 +104,17 @@ skip-if = os == 'android' # Bug 1258975 
 [test_ext_unload_frame.html]
 [test_ext_listener_proxies.html]
 [test_ext_web_accessible_resources.html]
 [test_ext_webrequest_auth.html]
 skip-if = os == 'android'
 [test_ext_webrequest_background_events.html]
 [test_ext_webrequest_basic.html]
 [test_ext_webrequest_filter.html]
+[test_ext_webrequest_frameId.html]
 [test_ext_webrequest_suspend.html]
 [test_ext_webrequest_upload.html]
 skip-if = os == 'android' # Currently fails in emulator tests
 [test_ext_webrequest_permission.html]
 [test_ext_webnavigation.html]
 [test_ext_webnavigation_filters.html]
 [test_ext_window_postMessage.html]
 [test_ext_subframes_privileges.html]
new file mode 100644
--- /dev/null
+++ b/toolkit/components/extensions/test/mochitest/test_ext_webrequest_frameId.html
@@ -0,0 +1,126 @@
+<!DOCTYPE HTML>
+
+<html>
+<head>
+<meta charset="utf-8">
+  <script type="text/javascript" src="/tests/SimpleTest/SimpleTest.js"></script>
+  <script type="text/javascript" src="/tests/SimpleTest/SpawnTask.js"></script>
+  <script type="text/javascript" src="/tests/SimpleTest/ExtensionTestUtils.js"></script>
+  <script type="text/javascript" src="head_webrequest.js"></script>
+  <script type="text/javascript" src="head.js"></script>
+  <link rel="stylesheet" type="text/css" href="/tests/SimpleTest/test.css"/>
+<script>
+"use strict";
+
+function background() {
+  browser.webRequest.onBeforeRequest.addListener(details => {
+    browser.test.sendMessage("onBeforeRequest", details);
+  }, {urls: ["<all_urls>"]}, ["blocking"]);
+
+  let tab;
+  browser.tabs.onCreated.addListener(newTab => {
+    tab = newTab;
+  });
+
+  browser.test.onMessage.addListener(msg => {
+    if (msg === "close-tab") {
+      browser.tabs.remove(tab.id);
+      browser.test.sendMessage("tab-closed");
+    }
+  });
+}
+
+let extensionData = {
+  manifest: {
+    permissions: ["webRequest", "webRequestBlocking", "<all_urls>", "tabs"],
+  },
+  background,
+};
+
+let expected = {
+  "file_simple_xhr.html": {
+    type: "main_frame",
+    toplevel: true,
+  },
+  "file_image_good.png": {
+    type: "image",
+    toplevel: true,
+  },
+  "example.txt": {
+    type: "xmlhttprequest",
+    toplevel: true,
+  },
+  "file_simple_xhr_frame.html": {
+    type: "sub_frame",
+    toplevelParent: true,
+  },
+  "file_image_bad.png": {
+    type: "image",
+  },
+  "xhr_resource": {
+    type: "xmlhttprequest",
+  },
+  "file_simple_xhr_frame2.html": {
+    type: "sub_frame",
+  },
+  "file_image_bad.png#2": {
+    type: "image",
+  },
+  "xhr_resource_2": {
+    type: "xmlhttprequest",
+  },
+};
+
+let subframeId, parentId;
+function checkDetails(details) {
+  let url = new URL(details.url);
+  let filename = url.pathname.split("/").pop();
+  let expect = expected[filename];
+  is(expect.type, details.type, `${details.type} type matches`);
+  if (expect.toplevel) {
+    is(0, details.frameId, "expect load at top level");
+    is(-1, details.parentFrameId, "expect top level frame to have no parent");
+  } else if (details.type == "sub_frame") {
+    ok(details.frameId > 0, "expect sub_frame to load into a new frame");
+    if (expect.toplevelParent) {
+      is(0, details.parentFrameId, "expect sub_frame to have top level parent");
+    } else {
+      ok(details.parentFrameId > 0, "expect sub_frame to have parent");
+    }
+    subframeId = details.frameId;
+    parentId = details.parentFrameId;
+  } else {
+    is(subframeId, details.frameId, "expect load in subframe");
+    is(parentId, details.parentFrameId, "expect subframe parent");
+  }
+}
+
+add_task(async function test_webRequest_main_frame() {
+  // Clear the image cache, since it gets in the way otherwise.
+  let imgTools = SpecialPowers.Cc["@mozilla.org/image/tools;1"].getService(SpecialPowers.Ci.imgITools);
+  let cache = imgTools.getImgCacheForDocument(document);
+  cache.clearCache(false);
+
+  let extension = ExtensionTestUtils.loadExtension(extensionData);
+  await extension.startup();
+
+  let a = addLink(`file_simple_xhr.html?nocache=${Math.random()}`);
+  a.click();
+
+  for (let i = 0; i < Object.keys(expected).length; i++) {
+    checkDetails(await extension.awaitMessage("onBeforeRequest"));
+  }
+
+  let closed = extension.awaitMessage("tab-closed");
+  extension.sendMessage("close-tab");
+  await closed;
+  await extension.unload();
+});
+
+</script>
+</head>
+<body>
+<div id="test">Sample text</div>
+
+</body>
+</html>
--- a/toolkit/modules/addons/WebRequest.jsm
+++ b/toolkit/modules/addons/WebRequest.jsm
@@ -732,18 +732,19 @@ HttpObserverManager = {
 
     let data = {
       requestId: RequestId.get(channel),
       url: channel.URI.spec,
       method: channel.requestMethod,
       browser: loadContext && loadContext.topFrameElement,
       type: WebRequestCommon.typeForPolicyType(policyType),
       fromCache: getData(channel).fromCache,
+      // Defaults for a top level request
       windowId: 0,
-      parentWindowId: 0,
+      parentWindowId: -1,
     };
 
     if (loadInfo) {
       let originPrincipal = loadInfo.triggeringPrincipal;
       if (originPrincipal.URI) {
         data.originUrl = originPrincipal.URI.spec;
       }
       let docPrincipal = loadInfo.loadingPrincipal;
@@ -757,25 +758,37 @@ HttpObserverManager = {
       // request results in, only rely on that if no other principal is available.
       let {isSystemPrincipal} = Services.scriptSecurityManager;
       let isTopLevel = !loadInfo.loadingPrincipal && !!data.browser;
       data.isSystemPrincipal = !isTopLevel &&
                                isSystemPrincipal(loadInfo.loadingPrincipal ||
                                                  loadInfo.principalToInherit ||
                                                  loadInfo.triggeringPrincipal);
 
-      if (loadInfo.frameOuterWindowID) {
+      // Handle window and parent id values for sub_frame requests or requests
+      // inside a sub_frame.
+      if (loadInfo.frameOuterWindowID != 0) {
+        // This is a sub_frame.  Only frames (ie. iframe; a request with a frameloader)
+        // have a non-zero frameOuterWindowID.  For a sub_frame, outerWindowID
+        // points at the frames parent.  The parent frame is the main_frame if
+        // outerWindowID == parentOuterWindowID, in which case set parentWindowId
+        // to zero.
         Object.assign(data, {
           windowId: loadInfo.frameOuterWindowID,
-          parentWindowId: loadInfo.outerWindowID,
+          parentWindowId: loadInfo.outerWindowID == loadInfo.parentOuterWindowID ? 0 : loadInfo.outerWindowID,
         });
-      } else {
+      } else if (loadInfo.outerWindowID != loadInfo.parentOuterWindowID) {
+        // This is a non-frame (e.g. script, image, etc) request within a
+        // sub_frame.  We have to check parentOuterWindowID against the browser
+        // to see if it is the main_frame in which case the parenteWindowId
+        // available to the caller must be set to zero.
+        let parentMainFrame = data.browser && data.browser.outerWindowID == loadInfo.parentOuterWindowID;
         Object.assign(data, {
           windowId: loadInfo.outerWindowID,
-          parentWindowId: loadInfo.parentOuterWindowID,
+          parentWindowId: parentMainFrame ? 0 : loadInfo.parentOuterWindowID,
         });
       }
     }
 
     if (channel instanceof Ci.nsIHttpChannelInternal) {
       try {
         data.ip = channel.remoteAddress;
       } catch (e) {