Bug 1427419 - Part 7: Move inIDOMUtils.isInheritedProperty to InspectorUtils. r=bz draft
authorCameron McCormack <cam@mcc.id.au>
Sat, 06 Jan 2018 15:08:13 +0800
changeset 716752 da596c2c8eabe9bb358041c35b06ca8b0bb3d243
parent 716751 2316ed35d61347f60a2c1582855b9f059f89542a
child 716753 6b30d752eddbd61ec5a53e01a72e66e4b596cff6
push id94496
push userbmo:cam@mcc.id.au
push dateSat, 06 Jan 2018 07:08:40 +0000
reviewersbz
bugs1427419
milestone59.0a1
Bug 1427419 - Part 7: Move inIDOMUtils.isInheritedProperty to InspectorUtils. r=bz MozReview-Commit-ID: AwILrjGiJ3L
devtools/server/actors/css-properties.js
devtools/server/actors/styles.js
devtools/server/tests/mochitest/test_css-logic-media-queries.html
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_isinheritableproperty.html
--- a/devtools/server/actors/css-properties.js
+++ b/devtools/server/actors/css-properties.js
@@ -8,16 +8,17 @@ loader.lazyServiceGetter(this, "DOMUtils
   "@mozilla.org/inspector/dom-utils;1", "inIDOMUtils");
 loader.lazyRequireGetter(this, "CSS_TYPES",
   "devtools/shared/css/properties-db", true);
 
 const protocol = require("devtools/shared/protocol");
 const { ActorClassWithSpec, Actor } = protocol;
 const { cssPropertiesSpec } = require("devtools/shared/specs/css-properties");
 const { cssColors } = require("devtools/shared/css/color-db");
+const InspectorUtils = require("InspectorUtils");
 
 exports.CssPropertiesActor = ActorClassWithSpec(cssPropertiesSpec, {
   typeName: "cssProperties",
 
   initialize(conn) {
     Actor.prototype.initialize.call(this, conn);
   },
 
@@ -62,17 +63,17 @@ function generateCssProperties() {
     if (values.includes("aliceblue")) {
       values = values.filter(x => !colors.includes(x));
       values.unshift("COLOR");
     }
 
     let subproperties = DOMUtils.getSubpropertiesForCSSProperty(name);
 
     properties[name] = {
-      isInherited: DOMUtils.isInheritedProperty(name),
+      isInherited: InspectorUtils.isInheritedProperty(name),
       values,
       supports,
       subproperties,
     };
   });
 
   return properties;
 }
--- a/devtools/server/actors/styles.js
+++ b/devtools/server/actors/styles.js
@@ -59,17 +59,17 @@ var PageStyleActor = protocol.ActorClass
   initialize: function (inspector) {
     protocol.Actor.prototype.initialize.call(this, null);
     this.inspector = inspector;
     if (!this.inspector.walker) {
       throw Error("The inspector's WalkerActor must be created before " +
                    "creating a PageStyleActor.");
     }
     this.walker = inspector.walker;
-    this.cssLogic = new CssLogic(DOMUtils.isInheritedProperty);
+    this.cssLogic = new CssLogic(InspectorUtils.isInheritedProperty);
 
     // Stores the association of DOM objects -> actors
     this.refMap = new Map();
 
     // Maps document elements to style elements, used to add new rules.
     this.styleElements = new WeakMap();
 
     this.onFrameUnload = this.onFrameUnload.bind(this);
@@ -462,17 +462,17 @@ var PageStyleActor = protocol.ActorClass
       // See the comment in |form| to understand this.
       yield rule.getAuthoredCssText();
     }
     return result;
   }),
 
   _hasInheritedProps: function (style) {
     return Array.prototype.some.call(style, prop => {
-      return DOMUtils.isInheritedProperty(prop);
+      return InspectorUtils.isInheritedProperty(prop);
     });
   },
 
   isPositionEditable: Task.async(function* (node) {
     if (!node || node.rawNode.nodeType !== node.rawNode.ELEMENT_NODE) {
       return false;
     }
 
@@ -586,17 +586,17 @@ var PageStyleActor = protocol.ActorClass
       if (isSystem && options.filter != SharedCssLogic.FILTER.UA) {
         continue;
       }
 
       if (inherited) {
         // Don't include inherited rules if none of its properties
         // are inheritable.
         let hasInherited = [...domRule.style].some(
-          prop => DOMUtils.isInheritedProperty(prop)
+          prop => InspectorUtils.isInheritedProperty(prop)
         );
         if (!hasInherited) {
           continue;
         }
       }
 
       let ruleActor = this._styleRef(domRule);
       rules.push({
--- a/devtools/server/tests/mochitest/test_css-logic-media-queries.html
+++ b/devtools/server/tests/mochitest/test_css-logic-media-queries.html
@@ -23,28 +23,26 @@ Test that css-logic handles media-querie
   </style>
 </head>
 <body>
   <div></div>
   <script type="application/javascript">
   "use strict";
 
   window.onload = function () {
-    const { classes: Cc, utils: Cu, interfaces: Ci } = Components;
-    const DOMUtils = Cc["@mozilla.org/inspector/dom-utils;1"]
-      .getService(Ci.inIDOMUtils);
+    const { utils: Cu } = Components;
 
     const {require} = Cu.import("resource://devtools/shared/Loader.jsm", {});
     const Services = require("Services");
     const {CssLogic} = require("devtools/server/css-logic");
 
     SimpleTest.waitForExplicitFinish();
 
     let div = document.querySelector("div");
-    let cssLogic = new CssLogic(DOMUtils.isInheritedProperty);
+    let cssLogic = new CssLogic(InspectorUtils.isInheritedProperty);
     cssLogic.highlight(div);
     cssLogic.processMatchedSelectors();
 
     let _strings = Services.strings
       .createBundle("chrome://devtools-shared/locale/styleinspector.properties");
 
     let inline = _strings.GetStringFromName("rule.sourceInline");
 
--- a/devtools/server/tests/mochitest/test_css-logic-specificity.html
+++ b/devtools/server/tests/mochitest/test_css-logic-specificity.html
@@ -8,22 +8,20 @@ Test that css-logic calculates CSS speci
   <title>Test css-logic specificity</title>
   <script type="application/javascript" src="chrome://mochikit/content/tests/SimpleTest/SimpleTest.js"></script>
 </head>
 <body style="background:blue;">
   <script type="application/javascript">
   "use strict";
 
   window.onload = function () {
-    const {utils: Cu, classes: Cc, interfaces: Ci} = Components;
+    const {utils: Cu} = 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},
@@ -49,17 +47,17 @@ Test that css-logic calculates CSS speci
 
     function getExpectedSpecificity(selectorText) {
       return TEST_DATA.filter(i => i.text === selectorText)[0].expected;
     }
 
     SimpleTest.waitForExplicitFinish();
 
     createDocument();
-    let cssLogic = new CssLogic(DOMUtils.isInheritedProperty);
+    let cssLogic = new CssLogic(InspectorUtils.isInheritedProperty);
 
     cssLogic.highlight(document.body);
     let cssSheet = cssLogic.sheets[0];
     let cssRule = cssSheet.domSheet.cssRules[0];
     let selectors = CssLogic.getSelectors(cssRule);
 
     info("Iterating over the test selectors");
     for (let i = 0; i < selectors.length; i++) {
--- a/dom/webidl/InspectorUtils.webidl
+++ b/dom/webidl/InspectorUtils.webidl
@@ -24,16 +24,17 @@ namespace InspectorUtils {
                                      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 = "");
+  boolean isInheritedProperty(DOMString property);
 };
 
 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
@@ -92,16 +92,22 @@ public:
   // idea what the right scope is.
   static bool SelectorMatchesElement(GlobalObject& aGlobal,
                                      Element& aElement,
                                      BindingStyleRule& aRule,
                                      uint32_t aSelectorIndex,
                                      const nsAString& aPseudo,
                                      ErrorResult& aRv);
 
+  // Utilities for working with CSS properties
+  //
+  // Returns true if the string names a property that is inherited by default.
+  static bool IsInheritedProperty(GlobalObject& aGlobal,
+                                  const nsAString& aPropertyName);
+
 private:
   static already_AddRefed<nsStyleContext>
     GetCleanStyleContextForElement(Element* aElement, nsAtom* aPseudo);
 };
 
 } // namespace dom
 } // namespace mozilla
 
--- a/layout/inspector/inDOMUtils.cpp
+++ b/layout/inspector/inDOMUtils.cpp
@@ -379,43 +379,41 @@ InspectorUtils::SelectorMatchesElement(G
                                        ErrorResult& aRv)
 {
   bool result = false;
   aRv = aRule.SelectorMatchesElement(&aElement, aSelectorIndex, aPseudo,
                                      &result);
   return result;
 }
 
-} // namespace dom
-} // namespace mozilla
-
-NS_IMETHODIMP
-inDOMUtils::IsInheritedProperty(const nsAString &aPropertyName, bool *_retval)
+/* static */ bool
+InspectorUtils::IsInheritedProperty(GlobalObject& aGlobalObject,
+                                    const nsAString& aPropertyName)
 {
   nsCSSPropertyID prop = nsCSSProps::
     LookupProperty(aPropertyName, CSSEnabledState::eIgnoreEnabledState);
   if (prop == eCSSProperty_UNKNOWN) {
-    *_retval = false;
-    return NS_OK;
+    return false;
   }
 
   if (prop == eCSSPropertyExtra_variable) {
-    *_retval = true;
-    return NS_OK;
+    return true;
   }
 
   if (nsCSSProps::IsShorthand(prop)) {
     prop = nsCSSProps::SubpropertyEntryFor(prop)[0];
   }
 
   nsStyleStructID sid = nsCSSProps::kSIDTable[prop];
-  *_retval = !nsStyleContext::IsReset(sid);
-  return NS_OK;
+  return !nsStyleContext::IsReset(sid);
 }
 
+} // namespace dom
+} // namespace mozilla
+
 extern const char* const kCSSRawProperties[];
 
 NS_IMETHODIMP
 inDOMUtils::GetCSSPropertyNames(uint32_t aFlags, uint32_t* aCount,
                                 char16_t*** aProps)
 {
   // maxCount is the largest number of properties we could have; our actual
   // number might be smaller because properties might be disabled.
--- a/layout/inspector/inIDOMUtils.idl
+++ b/layout/inspector/inIDOMUtils.idl
@@ -15,21 +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 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);
   const unsigned long INCLUDE_ALIASES = (1<<1);
   void getCSSPropertyNames([optional] in unsigned long aFlags,
 			   [optional] out unsigned long aCount,
 			   [retval, array, size_is(aCount)] out wstring aProps);
 
--- a/layout/inspector/tests/test_isinheritableproperty.html
+++ b/layout/inspector/tests/test_isinheritableproperty.html
@@ -1,34 +1,33 @@
 <!DOCTYPE HTML>
 <html>
 <!--
 https://bugzilla.mozilla.org/show_bug.cgi?id=699592
 -->
 <head>
-  <title>Test for nsIDOMUtils::isInheritedProperty</title>
+  <title>Test for InspectorUtils::isInheritedProperty</title>
   <script type="application/javascript" src="/tests/SimpleTest/SimpleTest.js"></script>
   <link rel="stylesheet" type="text/css" href="/tests/SimpleTest/test.css"/>
 </head>
 <body>
 <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);
-
-  is(utils.isInheritedProperty("font-size"), true, "font-size is inherited.");
+  is(InspectorUtils.isInheritedProperty("font-size"), true, "font-size is inherited.");
 
-  is(utils.isInheritedProperty("min-width"), false, "min-width is not inherited.");
+  is(InspectorUtils.isInheritedProperty("min-width"), false, "min-width is not inherited.");
 
-  is(utils.isInheritedProperty("font"), true, "shorthand font property is inherited.");
+  is(InspectorUtils.isInheritedProperty("font"), true, "shorthand font property is inherited.");
 
-  is(utils.isInheritedProperty("border"), false, "shorthand border property not inherited.");
-  is(utils.isInheritedProperty("garbage"), false, "Unknown property isn't inherited.");
+  is(InspectorUtils.isInheritedProperty("border"), false, "shorthand border property not inherited.");
+  is(InspectorUtils.isInheritedProperty("garbage"), false, "Unknown property isn't inherited.");
 
   SimpleTest.finish();
 }
 
 SimpleTest.waitForExplicitFinish();
 addLoadEvent(do_test);