Bug 1276418 - Remove getNode and all references to it in inspector tests; r=gl draft
authorPatrick Brosset <pbrosset@mozilla.com>
Fri, 16 Sep 2016 15:58:50 +0200
changeset 415072 f2c63499f8ee0837f385eb18d9a1b9aa7ead40a2
parent 415070 a3c71bbbe5505815168e075a0e913453d94196d8
child 531537 c1e9b1bf43b6417f57c4f38a896a3f742255ff40
push id29787
push userbmo:pbrosset@mozilla.com
push dateMon, 19 Sep 2016 12:21:14 +0000
reviewersgl
bugs1276418
milestone51.0a1
Bug 1276418 - Remove getNode and all references to it in inspector tests; r=gl MozReview-Commit-ID: HIhOHZvtUl
devtools/client/inspector/components/test/head.js
devtools/client/inspector/rules/test/doc_frame_script.js
devtools/client/inspector/shared/test/browser_styleinspector_csslogic-content-stylesheets.js
devtools/client/inspector/shared/test/doc_frame_script.js
devtools/client/inspector/test/browser_inspector_initialization.js
devtools/client/inspector/test/head.js
devtools/client/shared/test/test-actor.js
--- a/devtools/client/inspector/components/test/head.js
+++ b/devtools/client/inspector/components/test/head.js
@@ -12,45 +12,32 @@ Services.scriptloader.loadSubScript(
   this);
 
 Services.prefs.setIntPref("devtools.toolbox.footer.height", 350);
 registerCleanupFunction(() => {
   Services.prefs.clearUserPref("devtools.toolbox.footer.height");
 });
 
 /**
- * Simple DOM node accesor function that takes either a node or a string css
- * selector as argument and returns the corresponding node
- * FIXME: Delete this function and use inspector/test/head.js' getNode instead,
- * and fix all box model view tests to use nodeFronts instead of CPOWs.
- * @param {String|DOMNode} nodeOrSelector
- * @return {DOMNode}
- */
-function getNode(nodeOrSelector) {
-  return typeof nodeOrSelector === "string" ?
-    content.document.querySelector(nodeOrSelector) :
-    nodeOrSelector;
-}
-
-/**
  * Highlight a node and set the inspector's current selection to the node or
  * the first match of the given css selector.
- * @param {String|DOMNode} nodeOrSelector
+ * @param {String|NodeFront} selectorOrNodeFront
+ *        The selector for the node to be set, or the nodeFront
  * @param {InspectorPanel} inspector
  *        The instance of InspectorPanel currently loaded in the toolbox
  * @return a promise that resolves when the inspector is updated with the new
  * node
  */
-function selectAndHighlightNode(nodeOrSelector, inspector) {
-  info("Highlighting and selecting the node " + nodeOrSelector);
+function* selectAndHighlightNode(selectorOrNodeFront, inspector) {
+  info("Highlighting and selecting the node " + selectorOrNodeFront);
 
-  let node = getNode(nodeOrSelector);
+  let nodeFront = yield getNodeFront(selectorOrNodeFront, inspector);
   let updated = inspector.toolbox.once("highlighter-ready");
-  inspector.selection.setNode(node, "test-highlight");
-  return updated;
+  inspector.selection.setNodeFront(nodeFront, "test-highlight");
+  yield updated;
 }
 
 /**
  * Open the toolbox, with the inspector tool visible, and the computed view
  * sidebar tab selected to display the box model view.
  * @return a promise that resolves when the inspector is ready and the box model
  * view is visible and ready
  */
--- a/devtools/client/inspector/rules/test/doc_frame_script.js
+++ b/devtools/client/inspector/rules/test/doc_frame_script.js
@@ -11,20 +11,19 @@
 // then execute code upon receiving, and immediately send back a message.
 // This is so that chrome test code can execute code in content and wait for a
 // response this way:
 // let response = yield executeInContent(browser, "Test:msgName", data, true);
 // The response message should have the same name "Test:msgName"
 //
 // Some listeners do not send a response message back.
 
-var {classes: Cc, interfaces: Ci, utils: Cu} = Components;
+var {utils: Cu} = Components;
 
 var {require} = Cu.import("resource://devtools/shared/Loader.jsm", {});
-var {isContentStylesheet} = require("devtools/shared/inspector/css-logic");
 var defer = require("devtools/shared/defer");
 
 /**
  * Get a value for a given property name in a css rule in a stylesheet, given
  * their indexes
  * @param {Object} data Expects a data object with the following properties
  * - {Number} styleSheetIndex
  * - {Number} ruleIndex
@@ -45,42 +44,16 @@ addMessageListener("Test:GetRuleProperty
       value = rule.style.getPropertyValue(name);
     }
   }
 
   sendAsyncMessage("Test:GetRulePropertyValue", value);
 });
 
 /**
- * Get information about all the stylesheets that contain rules that apply to
- * a given node. The information contains the sheet href and whether or not the
- * sheet is a content sheet or not
- * @param {Object} objects Expects a 'target' CPOW object
- * @return {Array} A list of stylesheet info objects
- */
-addMessageListener("Test:GetStyleSheetsInfoForNode", function (msg) {
-  let target = msg.objects.target;
-  let sheets = [];
-
-  let domUtils = Cc["@mozilla.org/inspector/dom-utils;1"]
-    .getService(Ci.inIDOMUtils);
-  let domRules = domUtils.getCSSStyleRules(target);
-
-  for (let i = 0, n = domRules.Count(); i < n; i++) {
-    let sheet = domRules.GetElementAt(i).parentStyleSheet;
-    sheets.push({
-      href: sheet.href,
-      isContentSheet: isContentStylesheet(sheet)
-    });
-  }
-
-  sendAsyncMessage("Test:GetStyleSheetsInfoForNode", sheets);
-});
-
-/**
  * Get the property value from the computed style for an element.
  * @param {Object} data Expects a data object with the following properties
  * - {String} selector: The selector used to obtain the element.
  * - {String} pseudo: pseudo id to query, or null.
  * - {String} name: name of the property
  * @return {String} The value, if found, null otherwise
  */
 addMessageListener("Test:GetComputedStylePropertyValue", function (msg) {
--- a/devtools/client/inspector/shared/test/browser_styleinspector_csslogic-content-stylesheets.js
+++ b/devtools/client/inspector/shared/test/browser_styleinspector_csslogic-content-stylesheets.js
@@ -20,61 +20,57 @@ var ssm = Components.classes["@mozilla.o
                             .getService(Ci.nsIScriptSecurityManager);
 const XUL_PRINCIPAL = ssm.createCodebasePrincipal(XUL_URI, {});
 
 add_task(function* () {
   requestLongerTimeout(2);
 
   info("Checking stylesheets on HTML document");
   yield addTab(TEST_URI_HTML);
-  let target = getNode("#target");
 
-  let {inspector} = yield openInspector();
+  let {inspector, testActor} = yield openInspector();
   yield selectNode("#target", inspector);
 
   info("Checking stylesheets");
-  yield checkSheets(target);
+  yield checkSheets("#target", testActor);
 
   info("Checking authored stylesheets");
   yield addTab(TEST_URI_AUTHOR);
 
   ({inspector} = yield openInspector());
-  target = getNode("#target");
   yield selectNode("#target", inspector);
-  yield checkSheets(target);
+  yield checkSheets("#target", testActor);
 
   info("Checking stylesheets on XUL document");
   info("Allowing XUL content");
   allowXUL();
   yield addTab(TEST_URI_XUL);
 
   ({inspector} = yield openInspector());
-  target = getNode("#target");
   yield selectNode("#target", inspector);
 
-  yield checkSheets(target);
+  yield checkSheets("#target", testActor);
   info("Disallowing XUL content");
   disallowXUL();
 });
 
 function allowXUL() {
   Cc["@mozilla.org/permissionmanager;1"].getService(Ci.nsIPermissionManager)
     .addFromPrincipal(XUL_PRINCIPAL, "allowXULXBL",
       Ci.nsIPermissionManager.ALLOW_ACTION);
 }
 
 function disallowXUL() {
   Cc["@mozilla.org/permissionmanager;1"].getService(Ci.nsIPermissionManager)
     .addFromPrincipal(XUL_PRINCIPAL, "allowXULXBL",
       Ci.nsIPermissionManager.DENY_ACTION);
 }
 
-function* checkSheets(target) {
-  let sheets = yield executeInContent("Test:GetStyleSheetsInfoForNode", {},
-    {target});
+function* checkSheets(targetSelector, testActor) {
+  let sheets = yield testActor.getStyleSheetsInfoForNode(targetSelector);
 
   for (let sheet of sheets) {
     if (!sheet.href ||
         /doc_content_stylesheet_/.test(sheet.href) ||
         // For the "authored" case.
         /^data:.*seagreen/.test(sheet.href)) {
       ok(sheet.isContentSheet,
         sheet.href + " identified as content stylesheet");
--- a/devtools/client/inspector/shared/test/doc_frame_script.js
+++ b/devtools/client/inspector/shared/test/doc_frame_script.js
@@ -45,42 +45,16 @@ addMessageListener("Test:GetRuleProperty
       value = rule.style.getPropertyValue(name);
     }
   }
 
   sendAsyncMessage("Test:GetRulePropertyValue", value);
 });
 
 /**
- * Get information about all the stylesheets that contain rules that apply to
- * a given node. The information contains the sheet href and whether or not the
- * sheet is a content sheet or not
- * @param {Object} objects Expects a 'target' CPOW object
- * @return {Array} A list of stylesheet info objects
- */
-addMessageListener("Test:GetStyleSheetsInfoForNode", function (msg) {
-  let target = msg.objects.target;
-  let sheets = [];
-
-  let domUtils = Cc["@mozilla.org/inspector/dom-utils;1"]
-    .getService(Ci.inIDOMUtils);
-  let domRules = domUtils.getCSSStyleRules(target);
-
-  for (let i = 0, n = domRules.Count(); i < n; i++) {
-    let sheet = domRules.GetElementAt(i).parentStyleSheet;
-    sheets.push({
-      href: sheet.href,
-      isContentSheet: isContentStylesheet(sheet)
-    });
-  }
-
-  sendAsyncMessage("Test:GetStyleSheetsInfoForNode", sheets);
-});
-
-/**
  * Get the property value from the computed style for an element.
  * @param {Object} data Expects a data object with the following properties
  * - {String} selector: The selector used to obtain the element.
  * - {String} pseudo: pseudo id to query, or null.
  * - {String} name: name of the property
  * @return {String} The value, if found, null otherwise
  */
 addMessageListener("Test:GetComputedStylePropertyValue", function (msg) {
--- a/devtools/client/inspector/test/browser_inspector_initialization.js
+++ b/devtools/client/inspector/test/browser_inspector_initialization.js
@@ -24,39 +24,38 @@ const HTML = `
 `;
 
 const TEST_URI = "data:text/html;charset=utf-8," + encodeURI(HTML);
 
 add_task(function* () {
   let tab = yield addTab(TEST_URI);
   let testActor = yield getTestActorWithoutToolbox(tab);
 
-  yield testToolboxInitialization(tab);
+  yield testToolboxInitialization(testActor, tab);
   yield testContextMenuInitialization(testActor);
   yield testContextMenuInspectorAlreadyOpen(testActor);
 });
 
-function* testToolboxInitialization(tab) {
+function* testToolboxInitialization(testActor, tab) {
   let target = TargetFactory.forTab(tab);
 
   info("Opening inspector with gDevTools.");
   let toolbox = yield gDevTools.showToolbox(target, "inspector");
   let inspector = toolbox.getCurrentPanel();
 
   ok(true, "Inspector started, and notification received.");
   ok(inspector, "Inspector instance is accessible.");
   ok(inspector.isReady, "Inspector instance is ready.");
   is(inspector.target.tab, tab, "Valid target.");
 
   yield selectNode("p", inspector);
   yield testMarkupView("p", inspector);
   yield testBreadcrumbs("p", inspector);
 
-  let span = getNode("span");
-  span.scrollIntoView();
+  yield testActor.scrollIntoView("span");
 
   yield selectNode("span", inspector);
   yield testMarkupView("span", inspector);
   yield testBreadcrumbs("span", inspector);
 
   info("Destroying toolbox");
   let destroyed = toolbox.once("destroyed");
   toolbox.destroy();
--- a/devtools/client/inspector/test/head.js
+++ b/devtools/client/inspector/test/head.js
@@ -62,51 +62,16 @@ registerCleanupFunction(function* () {
 });
 
 var navigateTo = function (toolbox, url) {
   let activeTab = toolbox.target.activeTab;
   return activeTab.navigateTo(url);
 };
 
 /**
- * Simple DOM node accesor function that takes either a node or a string css
- * selector as argument and returns the corresponding node
- * @param {String|DOMNode} nodeOrSelector
- * @param {Object} options
- *        An object containing any of the following options:
- *        - document: HTMLDocument that should be queried for the selector.
- *                    Default: content.document.
- *        - expectNoMatch: If true and a node matches the given selector, a
- *                         failure is logged for an unexpected match.
- *                         If false and nothing matches the given selector, a
- *                         failure is logged for a missing match.
- *                         Default: false.
- * @return {DOMNode}
- */
-function getNode(nodeOrSelector, options = {}) {
-  let document = options.document || content.document;
-  let noMatches = !!options.expectNoMatch;
-
-  if (typeof nodeOrSelector === "string") {
-    info("Looking for a node that matches selector " + nodeOrSelector);
-    let node = document.querySelector(nodeOrSelector);
-    if (noMatches) {
-      ok(!node, "Selector " + nodeOrSelector + " didn't match any nodes.");
-    } else {
-      ok(node, "Selector " + nodeOrSelector + " matched a node.");
-    }
-
-    return node;
-  }
-
-  info("Looking for a node but selector was not a string.");
-  return nodeOrSelector;
-}
-
-/**
  * Start the element picker and focus the content window.
  * @param {Toolbox} toolbox
  */
 var startPicker = Task.async(function* (toolbox) {
   info("Start the element picker");
   yield toolbox.highlighterUtils.startPicker();
   // Make sure the content window is focused since the picker does not focus
   // the content window by default.
--- a/devtools/client/shared/test/test-actor.js
+++ b/devtools/client/shared/test/test-actor.js
@@ -5,16 +5,17 @@
 "use strict";
 
 // A helper actor for inspector and markupview tests.
 
 var { Cc, Ci, Cu, Cr } = require("chrome");
 const {getRect, getElementFromPoint, getAdjustedQuads} = require("devtools/shared/layout/utils");
 const defer = require("devtools/shared/defer");
 const {Task} = require("devtools/shared/task");
+const {isContentStylesheet} = require("devtools/shared/inspector/css-logic");
 var DOMUtils = Cc["@mozilla.org/inspector/dom-utils;1"].getService(Ci.inIDOMUtils);
 var loader = Cc["@mozilla.org/moz/jssubscript-loader;1"]
             .getService(Ci.mozIJSSubScriptLoader);
 
 // Set up a dummy environment so that EventUtils works. We need to be careful to
 // pass a window object into each EventUtils method we call rather than having
 // it rely on the |window| global.
 let EventUtils = {};
@@ -139,16 +140,22 @@ var testSpec = protocol.generateActorSpe
       response: {}
     },
     synthesizeKey: {
       request: {
         args: Arg(0, "json")
       },
       response: {}
     },
+    scrollIntoView: {
+      request: {
+        args: Arg(0, "string")
+      },
+      response: {}
+    },
     hasPseudoClassLock: {
       request: {
         selector: Arg(0, "string"),
         pseudo: Arg(1, "string")
       },
       response: {
         value: RetVal("boolean")
       }
@@ -255,16 +262,24 @@ var testSpec = protocol.generateActorSpe
     },
     getNodeInfo: {
       request: {
         selector: Arg(0, "string")
       },
       response: {
         value: RetVal("json")
       }
+    },
+    getStyleSheetsInfoForNode: {
+      request: {
+        selector: Arg(0, "string")
+      },
+      response: {
+        value: RetVal("json")
+      }
     }
   }
 });
 
 var TestActor = exports.TestActor = protocol.ActorClassWithSpec(testSpec, {
   initialize: function (conn, tabActor, options) {
     this.conn = conn;
     this.tabActor = tabActor;
@@ -494,16 +509,25 @@ var TestActor = exports.TestActor = prot
   * back. Consumers should listen to specific events on the inspector/highlighter
   * to know when the event got synthesized.
   */
   synthesizeKey: function ({key, options, content}) {
     EventUtils.synthesizeKey(key, options, this.content);
   },
 
   /**
+   * Scroll an element into view.
+   * @param {String} selector The selector for the node to scroll into view.
+   */
+  scrollIntoView: function (selector) {
+    let node = this._querySelector(selector);
+    node.scrollIntoView();
+  },
+
+  /**
    * Check that an element currently has a pseudo-class lock.
    * @param {String} selector The node selector to get the pseudo-class from
    * @param {String} pseudo The pseudoclass to check for
    * @return {Boolean}
    */
   hasPseudoClassLock: function (selector, pseudo) {
     let node = this._querySelector(selector);
     return DOMUtils.hasPseudoClassLock(node, pseudo);
@@ -720,16 +744,42 @@ var TestActor = exports.TestActor = prot
         }),
         outerHTML: node.outerHTML,
         innerHTML: node.innerHTML,
         textContent: node.textContent
       };
     }
 
     return info;
+  },
+
+  /**
+   * Get information about the stylesheets which have CSS rules that apply to a given DOM
+   * element, identified by a selector.
+   * @param {String} selector The CSS selector to get the node (can be an array
+   * of selectors to get elements in an iframe).
+   * @return {Array} A list of stylesheet objects, each having the following properties:
+   * - {String} href.
+   * - {Boolean} isContentSheet.
+   */
+  getStyleSheetsInfoForNode: function (selector) {
+    let node = this._querySelector(selector);
+    let domRules = DOMUtils.getCSSStyleRules(node);
+
+    let sheets = [];
+
+    for (let i = 0, n = domRules.Count(); i < n; i++) {
+      let sheet = domRules.GetElementAt(i).parentStyleSheet;
+      sheets.push({
+        href: sheet.href,
+        isContentSheet: isContentStylesheet(sheet)
+      });
+    }
+
+    return sheets;
   }
 });
 
 var TestActorFront = exports.TestActorFront = protocol.FrontClassWithSpec(testSpec, {
   initialize: function (client, { testActor }, toolbox) {
     protocol.Front.prototype.initialize.call(this, client, { actor: testActor });
     this.manage(this);
     this.toolbox = toolbox;