Bug 1417017 - Check for content. usage at global and within add_task for no-cpows-in-tests. r?Standard8
MozReview-Commit-ID: Asz6dM29uRJ
--- a/browser/base/content/test/general/browser_bug581253.js
+++ b/browser/base/content/test/general/browser_bug581253.js
@@ -19,16 +19,17 @@ add_task(async function test_remove_book
await PlacesUtils.bookmarks.insert({
parentGuid: PlacesUtils.bookmarks.unfiledGuid,
title: "",
url: testURL,
});
Assert.ok(await PlacesUtils.bookmarks.fetch({url: testURL}), "the test url is bookmarked");
+ // eslint-disable-next-line mozilla/no-cpows-in-tests
content.location = testURL;
await BrowserTestUtils.waitForCondition(
() => BookmarkingUI.status == BookmarkingUI.STATUS_STARRED,
"star button indicates that the page is bookmarked");
PlacesUtils.tagging.tagURI(makeURI(testURL), [testTag]);
--- a/browser/components/preferences/in-content/tests/browser_advanced_update.js
+++ b/browser/components/preferences/in-content/tests/browser_advanced_update.js
@@ -109,16 +109,17 @@ add_task(async function() {
gBrowser.removeCurrentTab();
});
add_task(async function() {
await openPreferencesViaOpenPreferencesAPI("general", { leaveOpen: true });
let doc = gBrowser.selectedBrowser.contentDocument;
let showBtn = doc.getElementById("showUpdateHistory");
+ // eslint-disable-next-line mozilla/no-cpows-in-tests
let dialogOverlay = content.gSubDialog._preloadDialog._overlay;
// XXX: For unknown reasons, this mock cannot be loaded by
// XPCOMUtils.defineLazyServiceGetter() called in aboutDialog-appUpdater.js.
// It is registered here so that we could assert update history subdialog
// without stopping the preferences advanced pane from loading.
// See bug 1361929.
mockUpdateManager.register();
--- a/browser/components/preferences/in-content/tests/browser_cookies_dialog.js
+++ b/browser/components/preferences/in-content/tests/browser_cookies_dialog.js
@@ -35,16 +35,17 @@ add_task(async function testDeleteCookie
if (AppConstants.platform == "macosx") {
EventUtils.synthesizeKey("VK_BACK_SPACE", {});
} else {
EventUtils.synthesizeKey("VK_DELETE", {});
}
await waitForCondition(() => tree.view.rowCount == 0);
+ // eslint-disable-next-line mozilla/no-cpows-in-tests
is_element_visible(content.gSubDialog._dialogs[0]._box,
"Subdialog is visible after deleting an element");
});
add_task(async function removeTab() {
gBrowser.removeCurrentTab();
});
--- a/browser/components/preferences/in-content/tests/browser_password_management.js
+++ b/browser/components/preferences/in-content/tests/browser_password_management.js
@@ -61,11 +61,12 @@ add_task(async function test_deletePassw
if (AppConstants.platform == "macosx") {
EventUtils.synthesizeKey("VK_BACK_SPACE", {});
} else {
EventUtils.synthesizeKey("VK_DELETE", {});
}
await waitForCondition(() => tree.view.rowCount == 0);
+ // eslint-disable-next-line mozilla/no-cpows-in-tests
is_element_visible(content.gSubDialog._dialogs[0]._box,
"Subdialog is visible after deleting an element");
});
--- a/browser/components/preferences/in-content/tests/browser_siteData.js
+++ b/browser/components/preferences/in-content/tests/browser_siteData.js
@@ -142,16 +142,17 @@ add_task(async function() {
// eslint-disable-next-line mozilla/no-cpows-in-tests
await waitForEvent(gBrowser.selectedBrowser.contentWindow, "test-indexedDB-done");
await BrowserTestUtils.removeTab(gBrowser.selectedTab);
let updatedPromise = promiseSiteDataManagerSitesUpdated();
await openPreferencesViaOpenPreferencesAPI("privacy", { leaveOpen: true });
await updatedPromise;
await openSiteDataSettingsDialog();
+ // eslint-disable-next-line mozilla/no-cpows-in-tests
let dialog = content.gSubDialog._topDialog;
let dialogFrame = dialog._frame;
let frameDoc = dialogFrame.contentDocument;
let siteItems = frameDoc.getElementsByTagName("richlistitem");
is(siteItems.length, 2, "Should list sites using quota usage or appcache");
let appcacheSite = frameDoc.querySelector(`richlistitem[host="${TEST_OFFLINE_HOST}"]`);
--- a/browser/components/preferences/in-content/tests/browser_siteData3.js
+++ b/browser/components/preferences/in-content/tests/browser_siteData3.js
@@ -132,16 +132,17 @@ add_task(async function() {
},
];
let updatePromise = promiseSiteDataManagerSitesUpdated();
await openPreferencesViaOpenPreferencesAPI("privacy", { leaveOpen: true });
await updatePromise;
await openSiteDataSettingsDialog();
+ // eslint-disable-next-line mozilla/no-cpows-in-tests
let dialog = content.gSubDialog._topDialog;
let dialogFrame = dialog._frame;
let frameDoc = dialogFrame.contentDocument;
let hostCol = frameDoc.getElementById("hostCol");
let usageCol = frameDoc.getElementById("usageCol");
let statusCol = frameDoc.getElementById("statusCol");
let sitesList = frameDoc.getElementById("sitesList");
--- a/browser/components/preferences/in-content/tests/browser_site_login_exceptions.js
+++ b/browser/components/preferences/in-content/tests/browser_site_login_exceptions.js
@@ -66,11 +66,12 @@ add_task(async function deleteALoginExce
if (AppConstants.platform == "macosx") {
EventUtils.synthesizeKey("VK_BACK_SPACE", {});
} else {
EventUtils.synthesizeKey("VK_DELETE", {});
}
await waitForCondition(() => tree.view.rowCount == 0);
+ // eslint-disable-next-line mozilla/no-cpows-in-tests
is_element_visible(content.gSubDialog._dialogs[0]._box,
"Subdialog is visible after deleting an element");
});
--- a/browser/components/preferences/in-content/tests/browser_subdialogs.js
+++ b/browser/components/preferences/in-content/tests/browser_subdialogs.js
@@ -174,16 +174,17 @@ add_task(async function check_reopening_
await close_subdialog_and_test_generic_end_state(tab.linkedBrowser,
function() { content.window.gSubDialog._topDialog._frame.contentDocument.documentElement.acceptDialog(); },
"accept", 1);
});
add_task(async function check_opening_while_closing() {
await open_subdialog_and_test_generic_start_state(tab.linkedBrowser);
info("closing");
+ // eslint-disable-next-line mozilla/no-cpows-in-tests
content.window.gSubDialog._topDialog.close();
info("reopening immediately after calling .close()");
await open_subdialog_and_test_generic_start_state(tab.linkedBrowser);
await close_subdialog_and_test_generic_end_state(tab.linkedBrowser,
function() { content.window.gSubDialog._topDialog._frame.contentDocument.documentElement.acceptDialog(); },
"accept", 1);
});
--- a/browser/components/search/test/browser_abouthome_behavior.js
+++ b/browser/components/search/test/browser_abouthome_behavior.js
@@ -34,16 +34,17 @@ function test() {
if (event.originalTarget != tab.linkedBrowser.contentDocumentAsCPOW ||
event.target.location.href == "about:blank") {
info("skipping spurious load event");
return;
}
tab.linkedBrowser.removeEventListener("load", load, true);
// Observe page setup
+ // eslint-disable-next-line mozilla/no-cpows-in-tests
let doc = gBrowser.contentDocumentAsCPOW;
gMutationObserver = new MutationObserver(function(mutations) {
for (let mutation of mutations) {
if (mutation.attributeName == "searchEngineName") {
// Re-add the listener, and perform a search
gBrowser.addProgressListener(listener);
gMutationObserver.disconnect();
gMutationObserver = null;
--- a/browser/extensions/onboarding/test/browser/browser_onboarding_skip_tour.js
+++ b/browser/extensions/onboarding/test/browser/browser_onboarding_skip_tour.js
@@ -31,12 +31,13 @@ add_task(async function test_hide_skip_b
resetOnboardingDefaultState();
Preferences.set("browser.onboarding.skip-tour-button.hide", true);
let tab = await openTab(ABOUT_NEWTAB_URL);
await promiseOnboardingOverlayLoaded(tab.linkedBrowser);
await BrowserTestUtils.synthesizeMouseAtCenter("#onboarding-overlay-button", {}, tab.linkedBrowser);
await promiseOnboardingOverlayOpened(tab.linkedBrowser);
+ // eslint-disable-next-line mozilla/no-cpows-in-tests
ok(!content.document.querySelector("#onboarding-skip-tour-button"), "should not render the skip button");
await BrowserTestUtils.removeTab(tab);
});
--- a/toolkit/content/tests/browser/browser_datetime_datepicker.js
+++ b/toolkit/content/tests/browser/browser_datetime_datepicker.js
@@ -162,16 +162,17 @@ add_task(async function test_datepicker_
await helper.openPicker(`data:text/html, <input type="date" value="${inputValue}">`);
// Click the first item (top-left corner) of the calendar
helper.click(helper.getElement(DAYS_VIEW).children[0]);
await ContentTask.spawn(helper.tab.linkedBrowser, {}, async function() {
let inputEl = content.document.querySelector("input");
await ContentTaskUtils.waitForEvent(inputEl, "input");
});
+ // eslint-disable-next-line mozilla/no-cpows-in-tests
Assert.equal(content.document.querySelector("input").value, firstDayOnCalendar);
await helper.tearDown();
});
/**
* Make sure picker is in correct state when it is reopened.
*/
--- a/tools/lint/eslint/eslint-plugin-mozilla/lib/helpers.js
+++ b/tools/lint/eslint/eslint-plugin-mozilla/lib/helpers.js
@@ -372,29 +372,46 @@ module.exports = {
ecmaFeatures: {
experimentalObjectRestSpread: true,
globalReturn: true
}
};
},
/**
+ * Check whether a node is a function.
+ *
+ * @param {Object} node
+ * The AST node to check
+ *
+ * @return {Boolean}
+ * True or false
+ */
+ getIsFunctionNode(node) {
+ switch (node.type) {
+ case "ArrowFunctionExpression":
+ case "FunctionDeclaration":
+ case "FunctionExpression":
+ return true;
+ }
+ return false;
+ },
+
+ /**
* Check whether the context is the global scope.
*
* @param {Array} ancestors
* The parents of the current node.
*
* @return {Boolean}
* True or false
*/
getIsGlobalScope(ancestors) {
for (let parent of ancestors) {
- if (parent.type == "FunctionExpression" ||
- parent.type == "FunctionDeclaration" ||
- parent.type == "ArrowFunctionExpression") {
+ if (this.getIsFunctionNode(parent)) {
return false;
}
}
return true;
},
/**
* Check whether we might be in a test head file.
--- a/tools/lint/eslint/eslint-plugin-mozilla/lib/rules/no-cpows-in-tests.js
+++ b/tools/lint/eslint/eslint-plugin-mozilla/lib/rules/no-cpows-in-tests.js
@@ -9,45 +9,81 @@
"use strict";
// -----------------------------------------------------------------------------
// Rule Definition
// -----------------------------------------------------------------------------
var helpers = require("../helpers");
-// Note: we match to the end of the string as well as the beginning, to avoid
-// multiple reports from MemberExpression statements.
var cpows = [
- /^gBrowser\.contentWindow$/,
- /^gBrowser\.contentDocument$/,
- /^gBrowser\.selectedBrowser\.contentWindow$/,
- /^browser\.contentDocument$/,
- /^window\.content$/
+ /^gBrowser\.contentWindow/,
+ /^gBrowser\.contentDocument/,
+ /^gBrowser\.selectedBrowser\.contentWindow/,
+ /^browser\.contentDocument/,
+ /^window\.content/
];
+// Keep track of where the last error is reported so to avoid reporting the same
+// expression (e.g., window.content.X vs window.content). Resets for each file.
+var lastErrorStart;
+
+// Keep track of whether we've entered a ContentTask.spawn call where accesses
+// to "content" are *not* CPOW. Resets when exiting the call.
var isInContentTask = false;
module.exports = function(context) {
// ---------------------------------------------------------------------------
// Helpers
// ---------------------------------------------------------------------------
function showError(node, identifier) {
if (isInContentTask) {
return;
}
+ // Avoid showing partial expressions errors when one already errored.
+ if (node.start === lastErrorStart) {
+ return;
+ }
+ lastErrorStart = node.start;
+
context.report({
node,
message: identifier +
" is a possible Cross Process Object Wrapper (CPOW)."
});
}
+ function hasLocalContentVariable(node) {
+ // Walk up the parents, see if we can find if "content" is a local variable.
+ let parent = node;
+ do {
+ parent = parent.parent;
+
+ // Don't error if 'content' is one of the function parameters.
+ if (helpers.getIsFunctionNode(parent) &&
+ context.getDeclaredVariables(parent).some(variable => variable.name === "content")) {
+ return true;
+ } else if (parent.type === "BlockStatement" || parent.type === "Program") {
+ // Don't error if the block or program includes their own definition of content.
+ for (let item of parent.body) {
+ if (item.type === "VariableDeclaration" && item.declarations.length) {
+ for (let declaration of item.declarations) {
+ if (declaration.id && declaration.id.name === "content") {
+ return true;
+ }
+ }
+ }
+ }
+ }
+ } while (parent.parent);
+ return false;
+ }
+
function isContentTask(node) {
return node &&
node.type === "MemberExpression" &&
node.property.type === "Identifier" &&
node.property.name === "spawn" &&
node.object.type === "Identifier" &&
node.object.name === "ContentTask";
}
@@ -64,36 +100,57 @@ module.exports = function(context) {
},
"CallExpression:exit": function(node) {
if (isContentTask(node.callee)) {
isInContentTask = false;
}
},
+ Program() {
+ lastErrorStart = undefined;
+ },
+
MemberExpression(node) {
if (helpers.getTestType(context) != "browser") {
return;
}
var expression = context.getSource(node);
// Only report a single CPOW error per node -- so if checking
// |cpows| reports one, don't report another below.
var someCpowFound = cpows.some(function(cpow) {
if (cpow.test(expression)) {
showError(node, expression);
return true;
}
return false;
});
- if (!someCpowFound && helpers.getIsGlobalScope(context.getAncestors())) {
- if (/^content\./.test(expression)) {
- showError(node, expression);
+
+ // Specially scope checks for "context." to avoid false positives.
+ if (!someCpowFound && /^content\./.test(expression)) {
+ // Don't error if we're multiple scopes deep. For now we only care about
+ // 2 scopes: global (length = 0) or immediately called by add_task.
+ // Ideally, we would care about any scope, but figuring out whether
+ // "context" is CPOW-or-not depends on how functions are defined and
+ // called -- both aspects are especially complex with head.js helpers.
+ // https://bugzilla.mozilla.org/show_bug.cgi?id=1417017#c9
+ const scopes = context.getAncestors().filter(helpers.getIsFunctionNode);
+ if (scopes.length > 1 || scopes.length === 1 &&
+ (!scopes[0].parent.callee || scopes[0].parent.callee.name !== "add_task")) {
+ return;
}
+
+ // Don't error if there's a locally scoped "content"
+ if (hasLocalContentVariable(node)) {
+ return;
+ }
+
+ showError(node, expression);
}
},
Identifier(node) {
if (helpers.getTestType(context) != "browser") {
return;
}
@@ -107,36 +164,18 @@ module.exports = function(context) {
}
// Don't error in the case of `let content = foo`.
if (node.parent.type === "VariableDeclarator" &&
node.parent.id && node.parent.id.name === "content") {
return;
}
- // Walk up the parents, see if we can find if this is a local variable.
- let parent = node;
- do {
- parent = parent.parent;
-
- // Don't error if 'content' is one of the function parameters.
- if (parent.type === "FunctionDeclaration" &&
- context.getDeclaredVariables(parent).some(variable => variable.name === "content")) {
- return;
- } else if (parent.type === "BlockStatement" || parent.type === "Program") {
- // Don't error if the block or program includes their own definition of content.
- for (let item of parent.body) {
- if (item.type === "VariableDeclaration" && item.declarations.length) {
- for (let declaration of item.declarations) {
- if (declaration.id && declaration.id.name === "content") {
- return;
- }
- }
- }
- }
- }
- } while (parent.parent);
+ // Don't error if there's a locally scoped "content"
+ if (hasLocalContentVariable(node)) {
+ return;
+ }
var expression = context.getSource(node);
showError(node, expression);
}
};
};
--- a/tools/lint/eslint/eslint-plugin-mozilla/tests/no-cpows-in-tests.js
+++ b/tools/lint/eslint/eslint-plugin-mozilla/tests/no-cpows-in-tests.js
@@ -28,21 +28,28 @@ function invalidCode(code, item, type =
}
ruleTester.run("no-cpows-in-tests", rule, {
valid: [
"window.document",
wrapCode("ContentTask.spawn(browser, null, () => { content.document; });"),
wrapCode("let x = cssDocs.tooltip.content.contentDocument;"),
wrapCode("function test(content) { let x = content; }"),
- wrapCode('let content = "content"; foo(content);')
+ wrapCode('let content = "content"; foo(content);'),
+ wrapCode("helper(() => content.document);"),
+ wrapCode("add_task(() => helper(() => content.document));"),
+ wrapCode("add_task(() => { let content = {}; content.document; })"),
+ wrapCode("add_task(content => content.document)"),
+ wrapCode("add_task(function(content) { content.document; })"),
+ wrapCode("add_task(() => ContentTask.spawn(browser, null, () => content.document));")
],
invalid: [
invalidCode("let x = gBrowser.contentWindow;", "gBrowser.contentWindow"),
invalidCode("let x = gBrowser.contentDocument;", "gBrowser.contentDocument"),
invalidCode("let x = gBrowser.selectedBrowser.contentWindow;", "gBrowser.selectedBrowser.contentWindow"),
invalidCode("let x = browser.contentDocument;", "browser.contentDocument"),
invalidCode("let x = window.content;", "window.content"),
invalidCode("content.document;", "content.document"),
+ invalidCode("add_task(() => content.document.querySelectorAll())", "content.document.querySelectorAll"),
invalidCode("let x = content;", "content", "Identifier"),
- invalidCode("let x = gBrowser.contentWindow.wrappedJSObject", "gBrowser.contentWindow")
+ invalidCode("let x = gBrowser.contentWindow.wrappedJSObject", "gBrowser.contentWindow.wrappedJSObject")
]
});