Bug 1695435 - Part 2: Add more words to the chrome script heuristic in use-isInstance r=preferences-reviewers,Gijs,webdriver-reviewers,whimboo,sgalich

Differential Revision: https://phabricator.services.mozilla.com/D110932

accessible/tests/mochitest/events/test_focus_aria_activedescendant.html

@@ -26,7 +26,7 @@ https://bugzilla.mozilla.org/show_bug.cgi?id=429547
     // gA11yEventDumpToConsole = true; // debugging
 
     function changeARIAActiveDescendant(aContainer, aItem, aPrevItemId) {
-      let itemID = aItem instanceof Node ? aItem.id : aItem;
+      let itemID = Node.isInstance(aItem) ? aItem.id : aItem;
       this.eventSeq = [new focusChecker(aItem)];
 
       if (aPrevItemId) {

browser/base/content/browser-gestureSupport.js

@@ -501,7 +501,7 @@ var gGestureSupport = {
    *        The MozRotateGestureUpdate event triggering this call
    */
   rotate(aEvent) {
-    if (!(window.content.document instanceof ImageDocument)) {
+    if (!ImageDocument.isInstance(window.content.document)) {
       return;
     }
 
@@ -523,7 +523,7 @@ var gGestureSupport = {
    * Perform a rotation end for ImageDocuments
    */
   rotateEnd() {
-    if (!(window.content.document instanceof ImageDocument)) {
+    if (!ImageDocument.isInstance(window.content.document)) {
       return;
     }
 
@@ -607,7 +607,7 @@ var gGestureSupport = {
       return;
     }
 
-    if (!(window.content.document instanceof ImageDocument)) {
+    if (!ImageDocument.isInstance(window.content.document)) {
       return;
     }
 
@@ -637,7 +637,7 @@ var gGestureSupport = {
   _clearCompleteRotation() {
     let contentElement =
       window.content.document &&
-      window.content.document instanceof ImageDocument &&
+      ImageDocument.isInstance(window.content.document) &&
       window.content.document.body &&
       window.content.document.body.firstElementChild;
     if (!contentElement) {

browser/base/content/pageinfo/pageInfo.js

@@ -203,8 +203,8 @@ gImageView.getCellProperties = function(row, col) {
   var props = "";
   if (
     !checkProtocol(data) ||
-    item instanceof HTMLEmbedElement ||
+    HTMLEmbedElement.isInstance(item) ||
-    (item instanceof HTMLObjectElement && !item.type.startsWith("image/"))
+    (HTMLObjectElement.isInstance(item) && !item.type.startsWith("image/"))
   ) {
     props += "broken";
   }
@@ -720,9 +720,9 @@ function saveMedia() {
     if (url) {
       var titleKey = "SaveImageTitle";
 
-      if (item instanceof HTMLVideoElement) {
+      if (HTMLVideoElement.isInstance(item)) {
         titleKey = "SaveVideoTitle";
-      } else if (item instanceof HTMLAudioElement) {
+      } else if (HTMLAudioElement.isInstance(item)) {
         titleKey = "SaveAudioTitle";
       }
 

browser/base/content/tabbrowser-tabs.js

@@ -1998,7 +1998,7 @@
       if (isMovingTabs) {
         let sourceNode = dt.mozGetDataAt(TAB_DROP_TYPE, 0);
         if (
-          sourceNode instanceof XULElement &&
+          XULElement.isInstance(sourceNode) &&
           sourceNode.localName == "tab" &&
           sourceNode.ownerGlobal.isChromeWindow &&
           sourceNode.ownerDocument.documentElement.getAttribute("windowtype") ==

browser/base/content/test/general/browser_accesskeys.js

@@ -140,7 +140,7 @@ function performAccessKey(browser, key) {
       callback,
       { capture: true },
       event => {
-        if (!(event.target instanceof HTMLElement)) {
+        if (!HTMLElement.isInstance(event.target)) {
           return false; // ignore window and document focus events
         }
 

browser/base/content/test/general/browser_gestureSupport.js

@@ -972,7 +972,7 @@ async function test_rotateGesturesOnTab() {
     true
   );
 
-  if (!(content.document instanceof ImageDocument)) {
+  if (!ImageDocument.isInstance(content.document)) {
     ok(false, "Image document failed to open for rotation testing");
     gBrowser.removeTab(test_imageTab);
     BrowserTestUtils.removeTab(test_normalTab);

browser/components/preferences/findInPage.js

@@ -576,7 +576,7 @@ var gSearchResultsPane = {
       // add it to the list of subitems. The items that don't match the search term
       // will be hidden.
       if (
-        child instanceof Element &&
+        Element.isInstance(child) &&
         (child.classList.contains("featureGate") ||
           child.classList.contains("mozilla-product-item"))
       ) {

devtools/client/inspector/markup/markup-context-menu.js

@@ -54,7 +54,7 @@ class MarkupContextMenu {
 
   show(event) {
     if (
-      !(event.originalTarget instanceof Element) ||
+      !Element.isInstance(event.originalTarget) ||
       event.originalTarget.closest("input[type=text]") ||
       event.originalTarget.closest("input:not([type])") ||
       event.originalTarget.closest("textarea")

devtools/client/shared/widgets/TableWidget.js

@@ -1521,18 +1521,22 @@ Column.prototype = {
     // Only sort the array if we are sorting based on this column
     if (this.sorted == 1) {
       items.sort((a, b) => {
-        const val1 =
+        const val1 = Node.isInstance(a[this.id])
-          a[this.id] instanceof Node ? a[this.id].textContent : a[this.id];
+          ? a[this.id].textContent
-        const val2 =
+          : a[this.id];
-          b[this.id] instanceof Node ? b[this.id].textContent : b[this.id];
+        const val2 = Node.isInstance(b[this.id])
+          ? b[this.id].textContent
+          : b[this.id];
         return naturalSortCaseInsensitive(val1, val2);
       });
     } else if (this.sorted > 1) {
       items.sort((a, b) => {
-        const val1 =
+        const val1 = Node.isInstance(a[this.id])
-          a[this.id] instanceof Node ? a[this.id].textContent : a[this.id];
+          ? a[this.id].textContent
-        const val2 =
+          : a[this.id];
-          b[this.id] instanceof Node ? b[this.id].textContent : b[this.id];
+        const val2 = Node.isInstance(b[this.id])
+          ? b[this.id].textContent
+          : b[this.id];
         return naturalSortCaseInsensitive(val2, val1);
       });
     }
@@ -1683,13 +1687,13 @@ Cell.prototype = {
       return;
     }
 
-    if (this.wrapTextInElements && !(value instanceof Node)) {
+    if (this.wrapTextInElements && !Node.isInstance(value)) {
       const span = this.label.ownerDocument.createElementNS(HTML_NS, "span");
       span.textContent = value;
       value = span;
     }
 
-    if (value instanceof Node) {
+    if (Node.isInstance(value)) {
       this.label.removeAttribute("value");
 
       while (this.label.firstChild) {

devtools/client/webconsole/components/Output/LazyMessageList.js

@@ -130,7 +130,7 @@ class LazyMessageList extends Component {
     let element = this.#topBufferRef.current.nextSibling;
     let elementRect = element?.getBoundingClientRect();
     while (
-      element instanceof Element &&
+      Element.isInstance(element) &&
       index < this.#clampedEndIndex &&
       element !== this.#bottomBufferRef.current
     ) {

devtools/server/actors/inspector/node.js

@@ -427,7 +427,7 @@ const NodeActor = protocol.ActorClassWithSpec(nodeSpec, {
     // node with `name="attributes"` exists in the DOM we need to bail.
     if (
       !this.rawNode.attributes ||
-      !(this.rawNode.attributes instanceof NamedNodeMap)
+      !NamedNodeMap.isInstance(this.rawNode.attributes)
     ) {
       return undefined;
     }

docs/code-quality/lint/linters/eslint-plugin-mozilla/use-isInstance.rst

@@ -14,8 +14,14 @@ These files are covered:
 - ``*.js`` with a heuristic
 
 Since there is no straightforward way to detect chrome scripts, currently the
-linter assumes that any script including ``ChromeUtils`` is chrome privileged.
+linter assumes that any script including the following words are chrome
-This of course may not be sufficient and is open for change.
+privileged. This of course may not be sufficient and is open for change:
+
+- ``ChromeUtils``, but not ``SpecialPowers.ChromeUtils``
+- ``BrowserTestUtils``, ``PlacesUtils``
+- ``document.createXULElement``
+- ``loader.lazyRequireGetter``
+- ``Services.foo``, but not ``SpecialPowers.Services.foo``
 
 Examples of incorrect code for this rule:
 -----------------------------------------

docshell/test/browser/browser_browsing_context_attached.js

@@ -13,7 +13,7 @@ async function observeAttached(callback) {
 
   function observer(subject, topic, data) {
     is(topic, TOPIC, "observing correct topic");
-    ok(subject instanceof BrowsingContext, "subject to be a BrowsingContext");
+    ok(BrowsingContext.isInstance(subject), "subject to be a BrowsingContext");
     is(typeof data, "string", "data to be a String");
     info(`*** bc id=${subject.id}, why=${data}`);
     attached.set(subject.id, { browsingContext: subject, why: data });

docshell/test/browser/browser_browsing_context_discarded.js

@@ -6,7 +6,7 @@ async function observeDiscarded(browsingContexts, callback) {
   let discarded = [];
 
   let promise = BrowserUtils.promiseObserved(TOPIC, subject => {
-    ok(subject instanceof BrowsingContext, "subject to be a BrowsingContext");
+    ok(BrowsingContext.isInstance(subject), "subject to be a BrowsingContext");
     discarded.push(subject);
 
     return browsingContexts.every(item => discarded.includes(item));

dom/base/test/unit/test_structuredcloneholder.js

@@ -79,7 +79,7 @@ add_task(async function test_structuredCloneHolder() {
   res = await resultPromise;
 
   ok(
-    res.data instanceof StructuredCloneHolder,
+    StructuredCloneHolder.isInstance(res.data),
     "Sending structured clone holders through message managers works as expected"
   );
 

dom/events/test/test_bug328885.html

@@ -41,7 +41,7 @@ https://bugzilla.mozilla.org/show_bug.cgi?id=328885
     inputelement.addEventListener('click',
       function(event) {
         var evt = SpecialPowers.wrap(event);
-        ok(SpecialPowers.call_Instanceof(evt.originalTarget, HTMLDivElement),
+        ok(SpecialPowers.wrap(HTMLDivElement).isInstance(evt.originalTarget),
            "(1) Wrong originalTarget!");
         is(SpecialPowers.unwrap(evt.originalTarget.parentNode), inputelement,
            "(2) Wront parent node!");

dom/fetch/tests/browser_blobFromFile.js

@@ -51,7 +51,7 @@ add_task(async function test() {
     });
   });
 
-  ok(blob instanceof File, "We have a file");
+  ok(File.isInstance(blob), "We have a file");
 
   is(blob.size, file.fileSize, "The size matches");
   is(blob.name, file.leafName, "The name is correct");

dom/localstorage/test/unit/test_eviction.js

@@ -54,7 +54,7 @@ add_task(async function testSteps() {
       ok(false, "Should have thrown");
     } catch (ex) {
       ok(true, "Did throw");
-      ok(ex instanceof DOMException, "Threw DOMException");
+      ok(DOMException.isInstance(ex), "Threw DOMException");
       is(ex.name, "QuotaExceededError", "Threw right DOMException");
       is(ex.code, NS_ERROR_DOM_QUOTA_EXCEEDED_ERR, "Threw with right code");
     }

dom/localstorage/test/unit/test_groupLimit.js

@@ -63,7 +63,7 @@ add_task(async function testSteps() {
       ok(false, "Should have thrown");
     } catch (ex) {
       ok(true, "Did throw");
-      ok(ex instanceof DOMException, "Threw DOMException");
+      ok(DOMException.isInstance(ex), "Threw DOMException");
       is(ex.name, "QuotaExceededError", "Threw right DOMException");
       is(ex.code, NS_ERROR_DOM_QUOTA_EXCEEDED_ERR, "Threw with right code");
     }

dom/tests/browser/browser_persist_cross_origin_iframe.js

@@ -77,7 +77,7 @@ async function addFrame(browser, path, selector) {
   ) {
     let document = content.document;
     let target = document.querySelector(selector);
-    if (target instanceof content.HTMLIFrameElement) {
+    if (content.HTMLIFrameElement.isInstance(target)) {
       document = target.contentDocument;
       target = document.body;
     }

dom/xhr/tests/browser_blobFromFile.js

@@ -51,7 +51,7 @@ add_task(async function test() {
     });
   });
 
-  ok(blob instanceof File, "We have a file");
+  ok(File.isInstance(blob), "We have a file");
 
   is(blob.size, file.fileSize, "The size matches");
   is(blob.name, file.leafName, "The name is correct");

toolkit/components/extensions/test/xpcshell/test_ext_runtime_sendMessage.js

@@ -216,6 +216,7 @@ add_task(async function runtimeSendMessageReply() {
 add_task(async function runtimeSendMessageBlob() {
   function background() {
     browser.runtime.onMessage.addListener(msg => {
+      // eslint-disable-next-line mozilla/use-isInstance -- this function runs in an extension
       browser.test.assertTrue(msg.blob instanceof Blob, "Message is a blob");
       return Promise.resolve(msg);
     });
@@ -230,6 +231,7 @@ add_task(async function runtimeSendMessageBlob() {
       .sendMessage({ blob: new Blob(["hello"]) })
       .then(response => {
         browser.test.assertTrue(
+          // eslint-disable-next-line mozilla/use-isInstance -- this function runs in an extension
           response.blob instanceof Blob,
           "Response is a blob"
         );

toolkit/components/extensions/test/xpcshell/test_ext_xhr_cors.js

@@ -134,7 +134,7 @@ async function test_xhr({ manifest_version }) {
       let x = await runXHR("http://example.com/dummy", {
         responseType: "document",
       });
-      browser.test.assertTrue(x.response instanceof HTMLDocument, "is doc");
+      browser.test.assertTrue(HTMLDocument.isInstance(x.response), "is doc");
       browser.test.assertTrue(
         x.response.querySelector("#id_of_some_element"),
         "got parsed document"

toolkit/components/passwordmgr/test/unit/test_isProbablyANewPasswordField.js

@@ -137,13 +137,12 @@ add_task(async function test_returns_false_when_pref_disabled() {
   // Use registration form test case, where we know it should return true if enabled
   const testcase = TESTCASES[1];
   info("Starting testcase: " + testcase.description);
-  const document =
+  const document = Document.isInstance(testcase.document)
-    testcase.document instanceof Document
+    ? testcase.document
-      ? testcase.document
+    : MockDocument.createTestDocument(
-      : MockDocument.createTestDocument(
+        "http://localhost:8080/test/",
-          "http://localhost:8080/test/",
+        testcase.document
-          testcase.document
+      );
-        );
   for (let [i, input] of testcase.inputs ||
     document.querySelectorAll(`input[type="password"]`).entries()) {
     const result = LoginAutoComplete.isProbablyANewPasswordField(input);
@@ -164,13 +163,12 @@ for (let testcase of TESTCASES) {
   (function() {
     add_task(async function() {
       info("Starting testcase: " + testcase.description);
-      let document =
+      let document = Document.isInstance(testcase.document)
-        testcase.document instanceof Document
+        ? testcase.document
-          ? testcase.document
+        : MockDocument.createTestDocument(
-          : MockDocument.createTestDocument(
+            "http://localhost:8080/test/",
-              "http://localhost:8080/test/",
+            testcase.document
-              testcase.document
+          );
-            );
 
       document = makeDocumentVisibleToFathom(document);
 

toolkit/components/places/tests/history/test_fetchMany.js

@@ -48,7 +48,7 @@ add_task(async function test_fetchMany() {
   for (let key of keys) {
     let page = pages.find(p => p.guid == key || p.url == key);
     Assert.deepEqual(page, fetched.get(key));
-    Assert.ok(fetched.get(key).url instanceof URL);
+    Assert.ok(URL.isInstance(fetched.get(key).url));
   }
 });
 

toolkit/content/tests/widgets/test_ua_widget_sandbox.html

@@ -37,8 +37,8 @@ const sandboxScript = function(shadowRoot) {
   let doc = element.ownerDocument;
   let win = doc.defaultView;
 
-  ok(shadowRoot instanceof win.ShadowRoot, "shadowRoot is a ShadowRoot");
+  ok(win.ShadowRoot.isInstance(shadowRoot), "shadowRoot is a ShadowRoot");
-  ok(element instanceof win.HTMLDivElement, "Element is a <div>");
+  ok(win.HTMLDivElement.isInstance(element), "Element is a <div>");
 
   is("createElement" in doc, false, "No document.createElement");
   is("createElementNS" in doc, false, "No document.createElementNS");
@@ -67,7 +67,7 @@ const sandboxScript = function(shadowRoot) {
     `<div xmlns="http://www.w3.org/1999/xhtml">Hello from DOMParser!</div>`, "application/xml");
   shadowRoot.importNodeAndAppendChildAt(shadowRoot, parserDoc.documentElement, true);
 
-  ok(shadowRoot.lastChild instanceof win.HTMLDivElement, "<div> inserted");
+  ok(win.HTMLDivElement.isInstance(shadowRoot.lastChild), "<div> inserted");
   is(shadowRoot.lastChild.textContent, "Hello from DOMParser!", "Deep import node worked");
 
   info("UA Widget reflectors tests");
@@ -78,8 +78,8 @@ const sandboxScript = function(shadowRoot) {
 SpecialPowers.Cu.evalInSandbox("this.script = " + sandboxScript.toString(), sandbox);
 sandbox.script(div.shadowRoot);
 
-ok(SpecialPowers.call_Instanceof(window.spanElementFromUAWidget, HTMLSpanElement), "<span> exposed");
+ok(SpecialPowers.wrap(HTMLSpanElement).isInstance(window.spanElementFromUAWidget), "<span> exposed");
-ok(SpecialPowers.call_Instanceof(window.divElementFromUAWidget, HTMLDivElement), "<div> exposed");
+ok(SpecialPowers.wrap(HTMLDivElement).isInstance(window.divElementFromUAWidget), "<div> exposed");
 
 try {
   window.spanElementFromUAWidget.textContent;

tools/lint/eslint/eslint-plugin-mozilla/lib/rules/use-isInstance.js

@@ -95,9 +95,17 @@ function isChromeContext(context) {
     return fs.readFileSync(filename).includes("there.is.only.xul");
   }
 
-  // Treat scripts using ChromeUtils as chrome scripts, but not SpecialPowers.ChromeUtils
+  // Treat scripts as chrome privileged when using:
+  // 1. ChromeUtils, but not SpecialPowers.ChromeUtils
+  // 2. BrowserTestUtils, PlacesUtils
+  // 3. document.createXULElement
+  // 4. loader.lazyRequireGetter
+  // 5. Services.foo, but not SpecialPowers.Services.foo
+  // 6. evalInSandbox
   const source = context.getSourceCode().text;
-  return !!source.match(/(^|\s)ChromeUtils/);
+  return !!source.match(
+    /(^|\s)ChromeUtils|BrowserTestUtils|PlacesUtils|createXULElement|lazyRequireGetter|(^|\s)Services\.|evalInSandbox/
+  );
 }
 
 module.exports = {