Bug 1427419 - Part 6: Move selector methods from inIDOMUtils to InspectorUtils. r=bz
authorCameron McCormack <cam@mcc.id.au>
Thu, 11 Jan 2018 12:37:59 +0800
changeset 450468 cd91519d16340ec87bc026f90bf05b25d35c09d9
parent 450467 c890abebe25b67fd9471fc91ab49ea2bd204e206
child 450469 dae8bd22a766f5fcfeac5c4e44be0a103c2a8e37
push id8527
push userCallek@gmail.com
push dateThu, 11 Jan 2018 21:05:50 +0000
treeherdermozilla-beta@95342d212a7a [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbz
bugs1427419
milestone59.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 1427419 - Part 6: Move selector methods from inIDOMUtils to InspectorUtils. r=bz MozReview-Commit-ID: 8FKRPeIijkC
devtools/server/actors/styles.js
devtools/server/css-logic.js
devtools/server/tests/mochitest/test_css-logic-specificity.html
dom/webidl/InspectorUtils.webidl
layout/inspector/InspectorUtils.h
layout/inspector/inDOMUtils.cpp
layout/inspector/inIDOMUtils.idl
layout/inspector/tests/test_selectormatcheselement.html
--- a/devtools/server/actors/styles.js
+++ b/devtools/server/actors/styles.js
@@ -674,18 +674,18 @@ var PageStyleActor = protocol.ActorClass
         let domRule = entry.rule.rawRule;
         let selectors = CssLogic.getSelectors(domRule);
         let element = entry.inherited ? entry.inherited.rawNode : node.rawNode;
 
         let {bindingElement, pseudo} =
             CssLogic.getBindingElementAndPseudo(element);
         entry.matchedSelectors = [];
         for (let i = 0; i < selectors.length; i++) {
-          if (DOMUtils.selectorMatchesElement(bindingElement, domRule, i,
-                                              pseudo)) {
+          if (InspectorUtils.selectorMatchesElement(bindingElement, domRule, i,
+                                                    pseudo)) {
             entry.matchedSelectors.push(selectors[i]);
           }
         }
       }
     }
 
     // Add all the keyframes rule associated with the element
     let computedStyle = this.cssLogic.computedStyle;
--- a/devtools/server/css-logic.js
+++ b/devtools/server/css-logic.js
@@ -24,18 +24,17 @@
  * - CssPropertyInfo contains style information for a single property for the
  *   highlighted element.
  * - CssSelectorInfo is a wrapper around CssSelector, which adds sorting with
  *   reference to the selected element.
  */
 
 "use strict";
 
-const { Cc, Ci, Cu } = require("chrome");
-const DevToolsUtils = require("devtools/shared/DevToolsUtils");
+const { Cu } = require("chrome");
 const nodeConstants = require("devtools/shared/dom-node-constants");
 const {
   getBindingElementAndPseudo,
   getCSSStyleRules,
   l10n,
   isContentStylesheet,
   shortSource,
   FILTER,
@@ -470,17 +469,17 @@ CssLogic.prototype = {
    *        The index of the selector within the DOMRule.
    * @return {boolean}
    *         true if the given selector matches the highlighted element or any
    *         of its parents, otherwise false is returned.
    */
   selectorMatchesElement: function (domRule, idx) {
     let element = this.viewedElement;
     do {
-      if (domUtils.selectorMatchesElement(element, domRule, idx)) {
+      if (InspectorUtils.selectorMatchesElement(element, domRule, idx)) {
         return true;
       }
     } while ((element = element.parentNode) &&
              element.nodeType === nodeConstants.ELEMENT_NODE);
 
     return false;
   },
 
@@ -636,19 +635,19 @@ CssLogic.getShortName = function (elemen
  * @param {DOMRule} domRule
  *        The DOMRule to parse.
  * @return {Array}
  *         An array of string selectors.
  */
 CssLogic.getSelectors = function (domRule) {
   let selectors = [];
 
-  let len = domUtils.getSelectorCount(domRule);
+  let len = InspectorUtils.getSelectorCount(domRule);
   for (let i = 0; i < len; i++) {
-    let text = domUtils.getSelectorText(domRule, i);
+    let text = InspectorUtils.getSelectorText(domRule, i);
     selectors.push(text);
   }
   return selectors;
 };
 
 /**
  * Given a node, check to see if it is a ::before or ::after element.
  * If so, return the node that is accessible from within the document
@@ -1125,18 +1124,18 @@ CssSelector.prototype = {
       // directly. @see http://www.w3.org/TR/CSS2/cascade.html#specificity
       return 0x40000000;
     }
 
     if (this._specificity) {
       return this._specificity;
     }
 
-    this._specificity = domUtils.getSpecificity(this.cssRule.domRule,
-                                                this.selectorIndex);
+    this._specificity = InspectorUtils.getSpecificity(this.cssRule.domRule,
+                                                      this.selectorIndex);
 
     return this._specificity;
   },
 
   toString: function () {
     return this.text;
   },
 };
@@ -1479,12 +1478,8 @@ CssSelectorInfo.prototype = {
 
     return 0;
   },
 
   toString: function () {
     return this.selector + " -> " + this.value;
   },
 };
-
-DevToolsUtils.defineLazyGetter(this, "domUtils", function () {
-  return Cc["@mozilla.org/inspector/dom-utils;1"].getService(Ci.inIDOMUtils);
-});
--- a/devtools/server/tests/mochitest/test_css-logic-specificity.html
+++ b/devtools/server/tests/mochitest/test_css-logic-specificity.html
@@ -14,16 +14,17 @@ Test that css-logic calculates CSS speci
 
   window.onload = function () {
     const {utils: Cu, classes: Cc, interfaces: Ci} = Components;
 
     const {require} = Cu.import("resource://devtools/shared/Loader.jsm", {});
     const {CssLogic, CssSelector} = require("devtools/server/css-logic");
     const DOMUtils = Cc["@mozilla.org/inspector/dom-utils;1"]
                        .getService(Ci.inIDOMUtils);
+    const InspectorUtils = SpecialPowers.InspectorUtils;
 
     const TEST_DATA = [
       {text: "*", expected: 0},
       {text: "LI", expected: 1},
       {text: "UL LI", expected: 2},
       {text: "UL OL + LI", expected: 3},
       {text: "H1 + [REL=\"up\"]", expected: 1025},
       {text: "UL OL LI.red", expected: 1027},
@@ -62,18 +63,18 @@ Test that css-logic calculates CSS speci
 
     info("Iterating over the test selectors");
     for (let i = 0; i < selectors.length; i++) {
       let selectorText = selectors[i];
       info("Testing selector " + selectorText);
 
       let selector = new CssSelector(cssRule, selectorText, i);
       let expected = getExpectedSpecificity(selectorText);
-      let specificity = DOMUtils.getSpecificity(selector.cssRule,
-                                                selector.selectorIndex);
+      let specificity = InspectorUtils.getSpecificity(selector.cssRule,
+                                                      selector.selectorIndex);
       is(specificity, expected,
         'Selector "' + selectorText + '" has a specificity of ' + expected);
     }
 
     info("Testing specificity of element.style");
     let colorProp = cssLogic.getPropertyInfo("background");
     is(colorProp.matchedSelectors[0].specificity, 0x40000000,
        "Element styles have specificity of 0x40000000 (1073741824).");
--- a/dom/webidl/InspectorUtils.webidl
+++ b/dom/webidl/InspectorUtils.webidl
@@ -14,16 +14,26 @@ namespace InspectorUtils {
   sequence<StyleSheet> getAllStyleSheets(Document document);
   sequence<CSSRule> getCSSStyleRules(
     Element element,
     [TreatNullAs=EmptyString] optional DOMString pseudo = "");
   unsigned long getRuleLine(CSSRule rule);
   unsigned long getRuleColumn(CSSRule rule);
   unsigned long getRelativeRuleLine(CSSRule rule);
   [NewObject] CSSLexer getCSSLexer(DOMString text);
+  unsigned long getSelectorCount(CSSStyleRule rule);
+  [Throws] DOMString getSelectorText(CSSStyleRule rule,
+                                     unsigned long selectorIndex);
+  [Throws] unsigned long long getSpecificity(CSSStyleRule rule,
+                                             unsigned long selectorIndex);
+  [Throws] boolean selectorMatchesElement(
+      Element element,
+      CSSStyleRule rule,
+      unsigned long selectorIndex,
+      [TreatNullAs=EmptyString] optional DOMString pseudo = "");
 };
 
 dictionary InspectorRGBTriple {
   /*
    * NOTE: Using octet for RGB components is not generally OK, because
    * they can be outside the 0-255 range, but for backwards-compatible
    * named colors (which is what we use this dictionary for) the 0-255
    * assumption is fine.
--- a/layout/inspector/InspectorUtils.h
+++ b/layout/inspector/InspectorUtils.h
@@ -64,16 +64,44 @@ public:
    * @param aRule the rule to examine
    * @return the line number of the rule, possibly relative to the
    *         <style> element
    */
   static uint32_t GetRelativeRuleLine(GlobalObject& aGlobal, css::Rule& aRule);
 
   static CSSLexer* GetCSSLexer(GlobalObject& aGlobal, const nsAString& aText);
 
+  // Utilities for working with selectors.  We don't have a JS OM representation
+  // of a single selector or a selector list yet, but given a rule we can index
+  // into the selector list.
+  //
+  // These methods would probably make more sense being [ChromeOnly] APIs on
+  // CSSStyleRule itself (bug 1428245).
+  static uint32_t GetSelectorCount(GlobalObject& aGlobal,
+                                   BindingStyleRule& aRule);
+
+  // For all three functions below, aSelectorIndex is 0-based
+  static void GetSelectorText(GlobalObject& aGlobal,
+                              BindingStyleRule& aRule,
+                              uint32_t aSelectorIndex,
+                              nsString& aText,
+                              ErrorResult& aRv);
+  static uint64_t GetSpecificity(GlobalObject& aGlobal,
+                                 BindingStyleRule& aRule,
+                                 uint32_t aSelectorIndex,
+                                 ErrorResult& aRv);
+  // Note: This does not handle scoped selectors correctly, because it has no
+  // idea what the right scope is.
+  static bool SelectorMatchesElement(GlobalObject& aGlobal,
+                                     Element& aElement,
+                                     BindingStyleRule& aRule,
+                                     uint32_t aSelectorIndex,
+                                     const nsAString& aPseudo,
+                                     ErrorResult& aRv);
+
 private:
   static already_AddRefed<nsStyleContext>
     GetCleanStyleContextForElement(Element* aElement, nsAtom* aPseudo);
 };
 
 } // namespace dom
 } // namespace mozilla
 
--- a/layout/inspector/inDOMUtils.cpp
+++ b/layout/inspector/inDOMUtils.cpp
@@ -300,43 +300,16 @@ InspectorUtils::GetCSSStyleRules(GlobalO
         aResult.AppendElement(rule);
       } else {
         MOZ_ASSERT_UNREACHABLE("We should be able to map a raw rule to a rule");
       }
     }
   }
 }
 
-} // namespace dom
-} // namespace mozilla
-
-static already_AddRefed<BindingStyleRule>
-GetRuleFromDOMRule(nsIDOMCSSStyleRule *aRule, ErrorResult& rv)
-{
-  nsCOMPtr<nsICSSStyleRuleDOMWrapper> rule = do_QueryInterface(aRule);
-  if (!rule) {
-    rv.Throw(NS_ERROR_INVALID_POINTER);
-    return nullptr;
-  }
-
-  RefPtr<BindingStyleRule> cssrule;
-  rv = rule->GetCSSStyleRule(getter_AddRefs(cssrule));
-  if (rv.Failed()) {
-    return nullptr;
-  }
-
-  if (!cssrule) {
-    rv.Throw(NS_ERROR_FAILURE);
-  }
-  return cssrule.forget();
-}
-
-namespace mozilla {
-namespace dom {
-
 /* static */ uint32_t
 InspectorUtils::GetRuleLine(GlobalObject& aGlobal, css::Rule& aRule)
 {
   return aRule.GetLineNumber();
 }
 
 /* static */ uint32_t
 InspectorUtils::GetRuleColumn(GlobalObject& aGlobal, css::Rule& aRule)
@@ -364,78 +337,60 @@ InspectorUtils::GetRelativeRuleLine(Glob
 }
 
 /* static */ CSSLexer*
 InspectorUtils::GetCSSLexer(GlobalObject& aGlobal, const nsAString& aText)
 {
   return new CSSLexer(aText);
 }
 
-} // namespace dom
-} // namespace mozilla
-
-NS_IMETHODIMP
-inDOMUtils::GetSelectorCount(nsIDOMCSSStyleRule* aRule, uint32_t *aCount)
+/* static */ uint32_t
+InspectorUtils::GetSelectorCount(GlobalObject& aGlobal,
+                                 BindingStyleRule& aRule)
 {
-  ErrorResult rv;
-  RefPtr<BindingStyleRule> rule = GetRuleFromDOMRule(aRule, rv);
-  if (rv.Failed()) {
-    return rv.StealNSResult();
-  }
-
-  *aCount = rule->GetSelectorCount();
-
-  return NS_OK;
+  return aRule.GetSelectorCount();
 }
 
-NS_IMETHODIMP
-inDOMUtils::GetSelectorText(nsIDOMCSSStyleRule* aRule,
-                            uint32_t aSelectorIndex,
-                            nsAString& aText)
+/* static */ void
+InspectorUtils::GetSelectorText(GlobalObject& aGlobal,
+                                BindingStyleRule& aRule,
+                                uint32_t aSelectorIndex,
+                                nsString& aText,
+                                ErrorResult& aRv)
 {
-  ErrorResult rv;
-  RefPtr<BindingStyleRule> rule = GetRuleFromDOMRule(aRule, rv);
-  MOZ_ASSERT(!rv.Failed(), "How could we get a selector but not a rule?");
-
-  return rule->GetSelectorText(aSelectorIndex, aText);
+  aRv = aRule.GetSelectorText(aSelectorIndex, aText);
 }
 
-NS_IMETHODIMP
-inDOMUtils::GetSpecificity(nsIDOMCSSStyleRule* aRule,
-                           uint32_t aSelectorIndex,
-                           uint64_t* aSpecificity)
+/* static */ uint64_t
+InspectorUtils::GetSpecificity(GlobalObject& aGlobal,
+                               BindingStyleRule& aRule,
+                               uint32_t aSelectorIndex,
+                               ErrorResult& aRv)
 {
-  ErrorResult rv;
-  RefPtr<BindingStyleRule> rule = GetRuleFromDOMRule(aRule, rv);
-  if (rv.Failed()) {
-    return rv.StealNSResult();
-  }
-
-  return rule->GetSpecificity(aSelectorIndex, aSpecificity);
+  uint64_t s;
+  aRv = aRule.GetSpecificity(aSelectorIndex, &s);
+  return s;
 }
 
-NS_IMETHODIMP
-inDOMUtils::SelectorMatchesElement(nsIDOMElement* aElement,
-                                   nsIDOMCSSStyleRule* aRule,
-                                   uint32_t aSelectorIndex,
-                                   const nsAString& aPseudo,
-                                   bool* aMatches)
+/* static */ bool
+InspectorUtils::SelectorMatchesElement(GlobalObject& aGlobalObject,
+                                       Element& aElement,
+                                       BindingStyleRule& aRule,
+                                       uint32_t aSelectorIndex,
+                                       const nsAString& aPseudo,
+                                       ErrorResult& aRv)
 {
-  nsCOMPtr<Element> element = do_QueryInterface(aElement);
-  NS_ENSURE_ARG_POINTER(element);
+  bool result = false;
+  aRv = aRule.SelectorMatchesElement(&aElement, aSelectorIndex, aPseudo,
+                                     &result);
+  return result;
+}
 
-  ErrorResult rv;
-  RefPtr<BindingStyleRule> rule = GetRuleFromDOMRule(aRule, rv);
-  if (rv.Failed()) {
-    return rv.StealNSResult();
-  }
-
-  return rule->SelectorMatchesElement(element, aSelectorIndex, aPseudo,
-                                      aMatches);
-}
+} // namespace dom
+} // namespace mozilla
 
 NS_IMETHODIMP
 inDOMUtils::IsInheritedProperty(const nsAString &aPropertyName, bool *_retval)
 {
   nsCSSPropertyID prop = nsCSSProps::
     LookupProperty(aPropertyName, CSSEnabledState::eIgnoreEnabledState);
   if (prop == eCSSProperty_UNKNOWN) {
     *_retval = false;
--- a/layout/inspector/inIDOMUtils.idl
+++ b/layout/inspector/inIDOMUtils.idl
@@ -15,35 +15,16 @@ interface nsIDOMNode;
 interface nsIDOMNodeList;
 interface nsIDOMFontFaceList;
 interface nsIDOMRange;
 interface nsIDOMCSSStyleSheet;
 
 [scriptable, uuid(362e98c3-82c2-4ad8-8dcb-00e8e4eab497)]
 interface inIDOMUtils : nsISupports
 {
-  // Utilities for working with selectors.  We don't have a JS OM representation
-  // of a single selector or a selector list yet, but given a rule we can index
-  // into the selector list.
-  //
-  // This is a somewhat backwards API; once we move StyleRule to WebIDL we
-  // should consider using [ChromeOnly] APIs on that.
-  unsigned long getSelectorCount(in nsIDOMCSSStyleRule aRule);
-  // For all three functions below, aSelectorIndex is 0-based
-  AString getSelectorText(in nsIDOMCSSStyleRule aRule,
-                          in unsigned long aSelectorIndex);
-  unsigned long long getSpecificity(in nsIDOMCSSStyleRule aRule,
-                                    in unsigned long aSelectorIndex);
-  // Note: This does not handle scoped selectors correctly, because it has no
-  // idea what the right scope is.
-  bool selectorMatchesElement(in nsIDOMElement aElement,
-                              in nsIDOMCSSStyleRule aRule,
-                              in unsigned long aSelectorIndex,
-                              [optional] in DOMString aPseudo);
-
   // Utilities for working with CSS properties
   //
   // Returns true if the string names a property that is inherited by default.
   bool isInheritedProperty(in AString aPropertyName);
 
   // Get a list of all our supported property names.  Optionally
   // shorthands can be excluded or property aliases included.
   const unsigned long EXCLUDE_SHORTHANDS = (1<<0);
--- a/layout/inspector/tests/test_selectormatcheselement.html
+++ b/layout/inspector/tests/test_selectormatcheselement.html
@@ -36,54 +36,51 @@ https://bugzilla.mozilla.org/show_bug.cg
 <body>
 <div id="foo">foo content</div>
 <pre id="test">
 <script type="application/javascript">
 
 const InspectorUtils = SpecialPowers.InspectorUtils;
 
 function do_test() {
-  var utils = SpecialPowers.Cc["@mozilla.org/inspector/dom-utils;1"]
-    .getService(SpecialPowers.Ci.inIDOMUtils);
-
   var element = document.querySelector("#foo");
 
   var elementRules = InspectorUtils.getCSSStyleRules(element);
   var elementRule = elementRules[elementRules.length - 1];
 
-  is (utils.selectorMatchesElement(element, elementRule, 0), true,
+  is (InspectorUtils.selectorMatchesElement(element, elementRule, 0), true,
     "Matches #foo");
-  is (utils.selectorMatchesElement(element, elementRule, 1), false,
+  is (InspectorUtils.selectorMatchesElement(element, elementRule, 1), false,
     "Doesn't match #bar");
-  is (utils.selectorMatchesElement(element, elementRule, 0, ":bogus"), false,
+  is (InspectorUtils.selectorMatchesElement(element, elementRule, 0, ":bogus"), false,
     "Doesn't match #foo with a bogus pseudo");
-  is (utils.selectorMatchesElement(element, elementRule, 2, ":bogus"), false,
+  is (InspectorUtils.selectorMatchesElement(element, elementRule, 2, ":bogus"), false,
     "Doesn't match #foo::before with bogus pseudo");
-  is (utils.selectorMatchesElement(element, elementRule, 0, ":after"), false,
+  is (InspectorUtils.selectorMatchesElement(element, elementRule, 0, ":after"), false,
     "Does match #foo::before with the :after pseudo");
 
   checkPseudo(":before");
   checkPseudo(":after");
   checkPseudo(":first-letter");
   checkPseudo(":first-line");
 
   SimpleTest.finish();
 
   function checkPseudo(pseudo) {
     var rules = InspectorUtils.getCSSStyleRules(element, pseudo);
     var rule = rules[rules.length - 1];
 
-    is (utils.selectorMatchesElement(element, rule, 0), false,
+    is (InspectorUtils.selectorMatchesElement(element, rule, 0), false,
       "Doesn't match without " + pseudo);
-    is (utils.selectorMatchesElement(element, rule, 1), false,
+    is (InspectorUtils.selectorMatchesElement(element, rule, 1), false,
       "Doesn't match without " + pseudo);
 
-    is (utils.selectorMatchesElement(element, rule, 0, pseudo), true,
+    is (InspectorUtils.selectorMatchesElement(element, rule, 0, pseudo), true,
       "Matches on #foo" + pseudo);
-    is (utils.selectorMatchesElement(element, rule, 1, pseudo), false,
+    is (InspectorUtils.selectorMatchesElement(element, rule, 1, pseudo), false,
       "Doesn't match on #bar" + pseudo);
   }
 }
 
 SimpleTest.waitForExplicitFinish();
 addLoadEvent(do_test);
 
 </script>