Bug 1553194 - Remove unnecessary isInherited from CssLogic constructor. r=pbro
authorRazvan Caliman <rcaliman@mozilla.com>
Wed, 22 May 2019 07:34:03 +0000
changeset 474940 e24c48157bf155e4f92db2e8e33e487fcbfa98db
parent 474939 0a9f9f076b93f3d456a3f11a330aae3d42ed6f78
child 474941 e61c315b780db1def991c721606ad947d6848a47
push id113181
push usershindli@mozilla.com
push dateWed, 22 May 2019 15:39:08 +0000
treeherdermozilla-inbound@bdd76368ddc1 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerspbro
bugs1553194
milestone69.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 1553194 - Remove unnecessary isInherited from CssLogic constructor. r=pbro `isInherited` is a callback function that checks if a given CSS property is inherited. It is misleadingly commented as a cache of inherited properties (which perhaps it is on the InspectorUtils implementation, but on the consumer side it is just a function). The actual call is done by InspectorUtils.isPropertyInherited. There is no need to pass the handler to CssLogic or to CssPropertyInfo since InspectorUtils is available in the same context as the definition of the consumers. There is no other use case where a custom handler is passed to check for inherited properties in so it is safe to remove this as an argument and just use InspectorUtils.isPropertyInherited where needed. This cleans up the code slightly. Differential Revision: https://phabricator.services.mozilla.com/D32016
devtools/server/actors/inspector/css-logic.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
--- a/devtools/server/actors/inspector/css-logic.js
+++ b/devtools/server/actors/inspector/css-logic.js
@@ -44,23 +44,17 @@ const {
 } = require("devtools/shared/inspector/css-logic");
 const InspectorUtils = require("InspectorUtils");
 
 const COMPAREMODE = {
   "BOOLEAN": "bool",
   "INTEGER": "int",
 };
 
-/**
- * @param {function} isInherited A function that determines if the CSS property
- *                   is inherited.
- */
-function CssLogic(isInherited) {
-  // The cache of examined CSS properties.
-  this._isInherited = isInherited;
+function CssLogic() {
   this._propertyInfos = {};
 }
 
 exports.CssLogic = CssLogic;
 
 CssLogic.prototype = {
   // Both setup by highlight().
   viewedElement: null,
@@ -211,17 +205,17 @@ CssLogic.prototype = {
    */
   getPropertyInfo: function(property) {
     if (!this.viewedElement) {
       return {};
     }
 
     let info = this._propertyInfos[property];
     if (!info) {
-      info = new CssPropertyInfo(this, property, this._isInherited);
+      info = new CssPropertyInfo(this, property);
       this._propertyInfos[property] = info;
     }
 
     return info;
   },
 
   /**
    * Cache all the stylesheets in the inspected document
@@ -504,17 +498,17 @@ CssLogic.prototype = {
       const rule = value[0];
       const status = value[1];
       properties = properties.filter((property) => {
         // We just need to find if a rule has this property while it matches
         // the viewedElement (or its parents).
         if (rule.getPropertyValue(property) &&
             (status == STATUS.MATCHED ||
              (status == STATUS.PARENT_MATCH &&
-              this._isInherited(property)))) {
+              InspectorUtils.isInheritedProperty(property)))) {
           result[property] = true;
           return false;
         }
         // Keep the property for the next rule.
         return true;
       });
       return properties.length == 0;
     }, this);
@@ -1208,25 +1202,22 @@ CssSelector.prototype = {
  *
  * The heart of the CssPropertyInfo object is the _findMatchedSelectors()
  * method. This are invoked when the PropertyView tries to access the
  * .matchedSelectors array.
  * Results are cached, for later reuse.
  *
  * @param {CssLogic} cssLogic Reference to the parent CssLogic instance
  * @param {string} property The CSS property we are gathering information for
- * @param {function} isInherited A function that determines if the CSS property
- *                   is inherited.
  * @constructor
  */
-function CssPropertyInfo(cssLogic, property, isInherited) {
+function CssPropertyInfo(cssLogic, property) {
   this._cssLogic = cssLogic;
   this.property = property;
   this._value = "";
-  this._isInherited = isInherited;
 
   // An array holding CssSelectorInfo objects for each of the matched selectors
   // that are inside a CSS rule. Only rules that hold the this.property are
   // counted. This includes rules that come from filtered stylesheets (those
   // that have sheetAllowed = false).
   this._matchedSelectors = null;
 }
 
@@ -1302,17 +1293,17 @@ CssPropertyInfo.prototype = {
    * @param {STATUS} status the CssSelector match status.
    */
   _processMatchedSelector: function(selector, status, distance) {
     const cssRule = selector.cssRule;
     const value = cssRule.getPropertyValue(this.property);
     if (value &&
         (status == STATUS.MATCHED ||
          (status == STATUS.PARENT_MATCH &&
-          this._isInherited(this.property)))) {
+          InspectorUtils.isInheritedProperty(this.property)))) {
       const selectorInfo = new CssSelectorInfo(selector, this.property, value,
           status, distance);
       this._matchedSelectors.push(selectorInfo);
     }
   },
 
   /**
    * Refilter the matched selectors array when the CssLogic.sourceFilter
--- a/devtools/server/actors/styles.js
+++ b/devtools/server/actors/styles.js
@@ -68,17 +68,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(InspectorUtils.isInheritedProperty);
+    this.cssLogic = new CssLogic();
 
     // 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);
--- a/devtools/server/tests/mochitest/test_css-logic-media-queries.html
+++ b/devtools/server/tests/mochitest/test_css-logic-media-queries.html
@@ -26,22 +26,21 @@ Test that css-logic handles media-querie
   <div></div>
   <script type="application/javascript">
   "use strict";
 
   window.onload = function() {
     const {require} = ChromeUtils.import("resource://devtools/shared/Loader.jsm");
     const Services = require("Services");
     const {CssLogic} = require("devtools/server/actors/inspector/css-logic");
-    const InspectorUtils = require("InspectorUtils");
 
     SimpleTest.waitForExplicitFinish();
 
     const div = document.querySelector("div");
-    const cssLogic = new CssLogic(InspectorUtils.isInheritedProperty);
+    const cssLogic = new CssLogic();
     cssLogic.highlight(div);
     cssLogic.processMatchedSelectors();
 
     const _strings = Services.strings
       .createBundle("chrome://devtools-shared/locale/styleinspector.properties");
 
     const 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
@@ -45,17 +45,17 @@ Test that css-logic calculates CSS speci
 
     function getExpectedSpecificity(selectorText) {
       return TEST_DATA.filter(i => i.text === selectorText)[0].expected;
     }
 
     SimpleTest.waitForExplicitFinish();
 
     createDocument();
-    const cssLogic = new CssLogic(InspectorUtils.isInheritedProperty);
+    const cssLogic = new CssLogic();
 
     cssLogic.highlight(document.body);
     const cssSheet = cssLogic.sheets[0];
     const cssRule = cssSheet.domSheet.cssRules[0];
     const selectors = CssLogic.getSelectors(cssRule);
 
     info("Iterating over the test selectors");
     for (let i = 0; i < selectors.length; i++) {