This is an automated email from the git hooks/post-receive script.
pierov pushed a commit to branch geckoview-99.0.1-11.0-1 in repository tor-browser.
commit 0669f3eca7b1b37bb6190e97389e280df7d7ca0a Author: Rob Wu rob@robwu.nl AuthorDate: Thu Jan 6 14:15:47 2022 +0000
Bug 1748158 - Omit sender.frameId if sender.tab is unset r=rpl,geckoview-reviewers,jonalmeida, a=dsmith
`sender.frameId` should be set iff `sender.tab` is set, as documented at https://developer.mozilla.org/en-US/docs/Mozilla/Add-ons/WebExtensions/API/r... The removal of the `viewType == "tab"` condition broke this in https://hg.mozilla.org/mozilla-central/rev/2dc4f1baccc8
This patch makes the presence of `frameId` conditional on `tab`, and fixes several tests that relied on the incorrect behavior:
- Move the runtime.onConnect test from test_ext_contentscript_in_background.js to a new mochitest at test_ext_runtime_connect_iframe.html.
- Simplify test_ext_contentscript_in_background.js to continue to provide test coverage for contentScripts.register + allFrames.
- Replace runtime.onConnect with runtime.getFrameId in test_ext_contentscript_xorigin_frame.js, since sender.frameId is no longer available in xpcshell tests (because internals to support the tabs extension API are not available in xpcshell tests). The test cannot be moved to a mochitest because its purpose is to provide test coverage for process switching in a xpcshell test (bug 1580811).
Differential Revision: https://phabricator.services.mozilla.com/D135057 --- .../browser_ext_contentscript_nontab_connect.js | 5 +- .../test_ext_native_messaging_permissions.js | 4 +- toolkit/components/extensions/ExtensionParent.jsm | 3 +- .../extensions/test/mochitest/mochitest-common.ini | 1 + .../test/mochitest/test_ext_runtime_connect.html | 1 + .../mochitest/test_ext_runtime_connect_iframe.html | 136 +++++++++++++++++++++ .../test_ext_contentscript_in_background.js | 24 +--- .../test_ext_contentscript_xorigin_frame.js | 29 +---- 8 files changed, 149 insertions(+), 54 deletions(-)
diff --git a/browser/components/extensions/test/browser/browser_ext_contentscript_nontab_connect.js b/browser/components/extensions/test/browser/browser_ext_contentscript_nontab_connect.js index edcb1c768574f..011f89c14ec2b 100644 --- a/browser/components/extensions/test/browser/browser_ext_contentscript_nontab_connect.js +++ b/browser/components/extensions/test/browser/browser_ext_contentscript_nontab_connect.js @@ -11,12 +11,9 @@ function extensionScript() {
browser.runtime.onConnect.addListener(port => { browser.test.assertEq(port.sender.tab, undefined, "Sender is not a tab"); + browser.test.assertEq(port.sender.frameId, undefined, "frameId unset"); browser.test.assertEq(port.sender.url, FRAME_URL, "Expected sender URL");
- let { frameId } = port.sender; - browser.test.assertEq(typeof frameId, "number", "frameId is a number"); - browser.test.assertTrue(frameId > 0, "frameId greater than 0"); - port.onMessage.addListener(msg => { browser.test.assertEq("pong", msg, "Reply from content script"); port.disconnect(); diff --git a/mobile/android/components/extensions/test/xpcshell/test_ext_native_messaging_permissions.js b/mobile/android/components/extensions/test/xpcshell/test_ext_native_messaging_permissions.js index 50e1cc93cf4d8..2c7ac3cc2f47b 100644 --- a/mobile/android/components/extensions/test/xpcshell/test_ext_native_messaging_permissions.js +++ b/mobile/android/components/extensions/test/xpcshell/test_ext_native_messaging_permissions.js @@ -96,7 +96,7 @@ add_task(async function test_nativeMessaging_unprivileged() { add_task(async function test_geckoViewAddons_missing() { const ERROR_NATIVE_MESSAGE_FROM_BACKGROUND = "Native manifests are not supported on android"; - const ERROR_NATIVE_MESSAGE_FROM_CONTENT = /^Native messaging not allowed: {.*"envType":"content_child","frameId":0,"url":"http://example.com/dummy"}$/; + const ERROR_NATIVE_MESSAGE_FROM_CONTENT = /^Native messaging not allowed: {.*"envType":"content_child","url":"http://example.com/dummy"}$/;
async function testBackground() { await browser.test.assertRejects( @@ -134,7 +134,7 @@ add_task(async function test_geckoViewAddons_missing() { // Checks that privileged extensions cannot use native messaging from content // without the nativeMessagingFromContent permission. add_task(async function test_nativeMessagingFromContent_missing() { - const ERROR_NATIVE_MESSAGE_FROM_CONTENT_NO_PERM = /^Unexpected messaging sender: {.*"envType":"content_child","frameId":0,"url":"http://example.com/dummy"}$/; + const ERROR_NATIVE_MESSAGE_FROM_CONTENT_NO_PERM = /^Unexpected messaging sender: {.*"envType":"content_child","url":"http://example.com/dummy"}$/; function testBackground() { // sendNativeMessage / connectNative are expected to succeed, but we // are not testing that here because XpcshellTestRunnerService does not diff --git a/toolkit/components/extensions/ExtensionParent.jsm b/toolkit/components/extensions/ExtensionParent.jsm index 4934b6cea5cbc..b67ac00fb1357 100644 --- a/toolkit/components/extensions/ExtensionParent.jsm +++ b/toolkit/components/extensions/ExtensionParent.jsm @@ -298,7 +298,6 @@ const ProxyMessenger = { contextId: source.id, id: source.extensionId, envType: source.envType, - frameId: source.frameId, url: source.url, };
@@ -308,6 +307,8 @@ const ProxyMessenger = { browser && apiManager.global.tabTracker.getBrowserData(browser); if (data?.tabId > 0) { sender.tab = extension.tabManager.get(data.tabId, null)?.convert(); + // frameId is documented to only be set if sender.tab is set. + sender.frameId = source.frameId; } }
diff --git a/toolkit/components/extensions/test/mochitest/mochitest-common.ini b/toolkit/components/extensions/test/mochitest/mochitest-common.ini index f109cc05d2e1a..a9c7625c284d1 100644 --- a/toolkit/components/extensions/test/mochitest/mochitest-common.ini +++ b/toolkit/components/extensions/test/mochitest/mochitest-common.ini @@ -143,6 +143,7 @@ skip-if = os == 'win' && (debug || asan) # Bug 1563440 [test_ext_request_urlClassification.html] skip-if = os == 'android' # Bug 1615427 [test_ext_runtime_connect.html] +[test_ext_runtime_connect_iframe.html] [test_ext_runtime_connect_twoway.html] [test_ext_runtime_connect2.html] [test_ext_runtime_disconnect.html] diff --git a/toolkit/components/extensions/test/mochitest/test_ext_runtime_connect.html b/toolkit/components/extensions/test/mochitest/test_ext_runtime_connect.html index c4726092ecb46..85f98d5034114 100644 --- a/toolkit/components/extensions/test/mochitest/test_ext_runtime_connect.html +++ b/toolkit/components/extensions/test/mochitest/test_ext_runtime_connect.html @@ -17,6 +17,7 @@ function background() { browser.test.assertEq(port.name, "ernie", "port name correct"); browser.test.assertTrue(port.sender.url.endsWith("file_sample.html"), "URL correct"); browser.test.assertTrue(port.sender.tab.url.endsWith("file_sample.html"), "tab URL correct"); + browser.test.assertEq(port.sender.frameId, 0, "frameId of top frame");
let expected = "message 1"; port.onMessage.addListener(msg => { diff --git a/toolkit/components/extensions/test/mochitest/test_ext_runtime_connect_iframe.html b/toolkit/components/extensions/test/mochitest/test_ext_runtime_connect_iframe.html new file mode 100644 index 0000000000000..9c64635063f85 --- /dev/null +++ b/toolkit/components/extensions/test/mochitest/test_ext_runtime_connect_iframe.html @@ -0,0 +1,136 @@ +<!DOCTYPE HTML> +<html> +<head> + <title>WebExtension test</title> + <script src="/tests/SimpleTest/SimpleTest.js"></script> + <script src="/tests/SimpleTest/ExtensionTestUtils.js"></script> + <script type="text/javascript" src="head.js"></script> + <link rel="stylesheet" type="text/css" href="/tests/SimpleTest/test.css"/> +</head> +<body> + +<script type="text/javascript"> +"use strict"; + +// The purpose of this test is to verify that the port.sender properties are +// not set for messages from iframes in background scripts. This is the toolkit +// version of the browser_ext_contentscript_nontab_connect.js test, and exists +// to provide test coverage for non-toolkit builds (e.g. Android). +// +// This used to be a xpcshell test (from bug 1488105), but became a mochitest +// because port.sender.tab and port.sender.frameId do not represent the real +// values in xpcshell tests. +// Specifically, ProxyMessenger.prototype.getSender uses the tabTracker, which +// expects real tabs instead of browsers from the ContentPage API in xpcshell +// tests. +add_task(async function connect_from_background_frame() { + if (!SpecialPowers.getBoolPref("extensions.webextensions.remote", true)) { + info("Cannot load remote content in parent process; skipping test task"); + return; + } + async function background() { + const FRAME_URL = "https://example.com/tests/toolkit/components/extensions/test/mochitest/file_..."; + browser.runtime.onConnect.addListener(port => { + // The next two assertions are the reason for this being a mochitest + // instead of a xpcshell test. + browser.test.assertEq(port.sender.tab, undefined, "Sender is not a tab"); + browser.test.assertEq(port.sender.frameId, undefined, "frameId unset"); + browser.test.assertEq(port.sender.url, FRAME_URL, "Expected sender URL"); + port.onMessage.addListener(msg => { + browser.test.assertEq("pong", msg, "Reply from content script"); + port.disconnect(); + }); + port.postMessage("ping"); + }); + + await browser.contentScripts.register({ + matches: [FRAME_URL], + js: [{ file: "contentscript.js" }], + allFrames: true, + }); + + let f = document.createElement("iframe"); + f.src = FRAME_URL; + document.body.appendChild(f); + } + + function contentScript() { + browser.test.log(`Running content script at ${document.URL}`); + + let port = browser.runtime.connect(); + port.onMessage.addListener(msg => { + browser.test.assertEq("ping", msg, "Expected message to content script"); + port.postMessage("pong"); + }); + port.onDisconnect.addListener(() => { + browser.test.sendMessage("disconnected_in_content_script"); + }); + } + + let extension = ExtensionTestUtils.loadExtension({ + manifest: { + permissions: ["https://example.com/*"], + }, + files: { + "contentscript.js": contentScript, + }, + background, + }); + await extension.startup(); + await extension.awaitMessage("disconnected_in_content_script"); + await extension.unload(); +}); + +// The test_ext_contentscript_fission_frame.html test already checks the +// behavior of onConnect in cross-origin frames, so here we just limit the test +// to checking that the port.sender properties are sensible. +add_task(async function connect_from_content_script_in_frame() { + async function background() { + const TAB_URL = "https://example.org/tests/toolkit/components/extensions/test/mochitest/file_..."; + const FRAME_URL = "https://example.org/tests/toolkit/components/extensions/test/mochitest/file_..."; + let createdTab; + browser.runtime.onConnect.addListener(port => { + // The next two assertions are the reason for this being a mochitest + // instead of a xpcshell test. + browser.test.assertEq(port.sender.tab.url, TAB_URL, "Sender is the tab"); + browser.test.assertTrue(port.sender.frameId > 0, "frameId is set"); + browser.test.assertEq(port.sender.url, FRAME_URL, "Expected sender URL"); + + browser.test.assertEq(createdTab.id, port.sender.tab.id, "Tab to close"); + browser.tabs.remove(port.sender.tab.id).then(() => { + browser.test.sendMessage("tab_port_checked_and_tab_closed"); + }); + }); + + await browser.contentScripts.register({ + matches: [FRAME_URL], + js: [{ file: "contentscript.js" }], + allFrames: true, + }); + + createdTab = await browser.tabs.create({ url: TAB_URL }); + } + + function contentScript() { + browser.test.log(`Running content script at ${document.URL}`); + + browser.runtime.connect(); + } + + let extension = ExtensionTestUtils.loadExtension({ + manifest: { + permissions: ["https://example.org/*"], + }, + files: { + "contentscript.js": contentScript, + }, + background, + }); + await extension.startup(); + await extension.awaitMessage("tab_port_checked_and_tab_closed"); + await extension.unload(); +}); +</script> + +</body> +</html> diff --git a/toolkit/components/extensions/test/xpcshell/test_ext_contentscript_in_background.js b/toolkit/components/extensions/test/xpcshell/test_ext_contentscript_in_background.js index 1a8aa6d7060d6..e813b46ca0cc5 100644 --- a/toolkit/components/extensions/test/xpcshell/test_ext_contentscript_in_background.js +++ b/toolkit/components/extensions/test/xpcshell/test_ext_contentscript_in_background.js @@ -9,19 +9,9 @@ server.registerPathHandler("/dummyFrame", (request, response) => { response.write(""); });
-add_task(async function connect_from_background_frame() { +add_task(async function content_script_in_background_frame() { async function background() { const FRAME_URL = "http://example.com:8888/dummyFrame"; - browser.runtime.onConnect.addListener(port => { - browser.test.assertEq(port.sender.tab, undefined, "Sender is not a tab"); - browser.test.assertEq(port.sender.url, FRAME_URL, "Expected sender URL"); - port.onMessage.addListener(msg => { - browser.test.assertEq("pong", msg, "Reply from content script"); - port.disconnect(); - }); - port.postMessage("ping"); - }); - await browser.contentScripts.register({ matches: ["http://example.com/dummyFrame"], js: [{ file: "contentscript.js" }], @@ -35,15 +25,7 @@ add_task(async function connect_from_background_frame() {
function contentScript() { browser.test.log(`Running content script at ${document.URL}`); - - let port = browser.runtime.connect(); - port.onMessage.addListener(msg => { - browser.test.assertEq("ping", msg, "Expected message to content script"); - port.postMessage("pong"); - }); - port.onDisconnect.addListener(() => { - browser.test.sendMessage("disconnected_in_content_script"); - }); + browser.test.sendMessage("done_in_content_script"); }
let extension = ExtensionTestUtils.loadExtension({ @@ -56,6 +38,6 @@ add_task(async function connect_from_background_frame() { background, }); await extension.startup(); - await extension.awaitMessage("disconnected_in_content_script"); + await extension.awaitMessage("done_in_content_script"); await extension.unload(); }); diff --git a/toolkit/components/extensions/test/xpcshell/test_ext_contentscript_xorigin_frame.js b/toolkit/components/extensions/test/xpcshell/test_ext_contentscript_xorigin_frame.js index cab508b040086..8a58b2475c68c 100644 --- a/toolkit/components/extensions/test/xpcshell/test_ext_contentscript_xorigin_frame.js +++ b/toolkit/components/extensions/test/xpcshell/test_ext_contentscript_xorigin_frame.js @@ -17,23 +17,6 @@ add_task(async function test_process_switch_cross_origin_frame() { ], },
- background() { - browser.runtime.onConnect.addListener(port => { - port.onMessage.addListener(async () => { - let { url, frameId } = port.sender; - - browser.test.assertTrue(frameId > 0, "sender frameId is ok"); - browser.test.assertTrue( - url.endsWith("file_iframe.html"), - "url is ok" - ); - - port.postMessage(frameId); - port.disconnect(); - }); - }); - }, - files: { "cs.js"() { browser.test.assertEq( @@ -42,15 +25,9 @@ add_task(async function test_process_switch_cross_origin_frame() { "url is ok" );
- let frameId; - let port = browser.runtime.connect(); - port.onMessage.addListener(response => { - frameId = response; - }); - port.onDisconnect.addListener(() => { - browser.test.sendMessage("content-script-loaded", frameId); - }); - port.postMessage("hello"); + // frameId is the BrowsingContext ID in practice. + let frameId = browser.runtime.getFrameId(window); + browser.test.sendMessage("content-script-loaded", frameId); }, }, });