Bug 1427419 - Part 12: Move inIDOMUtils.colorToRGBA to InspectorUtils. r=bz
authorCameron McCormack <cam@mcc.id.au>
Thu, 11 Jan 2018 12:38:00 +0800
changeset 452991 87e619f7e97c1bd5088fdb94091f31123d010f69
parent 452990 044e1b16df7658754e2fabfecf2adae9b3863550
child 452992 aec7e1a3fc907a4be845b8dc425ca2ee8cd8ba4c
push id1648
push usermtabara@mozilla.com
push dateThu, 01 Mar 2018 12:45:47 +0000
treeherdermozilla-release@cbb9688c2eeb [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 12: Move inIDOMUtils.colorToRGBA to InspectorUtils. r=bz MozReview-Commit-ID: 9EAdNibvZ4
browser/components/extensions/ext-browserAction.js
devtools/client/shared/test/unit/test_cssColor-01.js
devtools/client/shared/test/unit/test_cssColor-03.js
devtools/client/shared/test/unit/test_cssColorDatabase.js
dom/webidl/InspectorUtils.webidl
layout/inspector/InspectorUtils.h
layout/inspector/inDOMUtils.cpp
layout/inspector/inIDOMUtils.idl
layout/inspector/tests/test_color_to_rgba.html
--- a/browser/components/extensions/ext-browserAction.js
+++ b/browser/components/extensions/ext-browserAction.js
@@ -14,31 +14,29 @@ XPCOMUtils.defineLazyModuleGetter(this, 
                                   "resource://gre/modules/Timer.jsm");
 XPCOMUtils.defineLazyModuleGetter(this, "setTimeout",
                                   "resource://gre/modules/Timer.jsm");
 XPCOMUtils.defineLazyModuleGetter(this, "TelemetryStopwatch",
                                   "resource://gre/modules/TelemetryStopwatch.jsm");
 XPCOMUtils.defineLazyModuleGetter(this, "ViewPopup",
                                   "resource:///modules/ExtensionPopups.jsm");
 
-XPCOMUtils.defineLazyServiceGetter(this, "DOMUtils",
-                                   "@mozilla.org/inspector/dom-utils;1",
-                                   "inIDOMUtils");
-
 var {
   DefaultWeakMap,
 } = ExtensionUtils;
 
 Cu.import("resource://gre/modules/ExtensionParent.jsm");
 
 var {
   IconDetails,
   StartupCache,
 } = ExtensionParent;
 
+Cu.importGlobalProperties(["InspectorUtils"]);
+
 const POPUP_PRELOAD_TIMEOUT_MS = 200;
 const POPUP_OPEN_MS_HISTOGRAM = "WEBEXT_BROWSERACTION_POPUP_OPEN_MS";
 const POPUP_RESULT_HISTOGRAM = "WEBEXT_BROWSERACTION_POPUP_PRELOAD_RESULT_COUNT";
 
 var XUL_NS = "http://www.mozilla.org/keymaster/gatekeeper/there.is.only.xul";
 
 const isAncestorOrSelf = (target, node) => {
   for (; node; node = node.parentNode) {
@@ -668,17 +666,17 @@ this.browserAction = class extends Exten
           let popup = browserAction.getProperty(tab, "popup");
           return Promise.resolve(popup);
         },
 
         setBadgeBackgroundColor: function(details) {
           let tab = getTab(details.tabId);
           let color = details.color;
           if (!Array.isArray(color)) {
-            let col = DOMUtils.colorToRGBA(color);
+            let col = InspectorUtils.colorToRGBA(color);
             color = col && [col.r, col.g, col.b, Math.round(col.a * 255)];
           }
           browserAction.setProperty(tab, "badgeBackgroundColor", color);
         },
 
         getBadgeBackgroundColor: function(details, callback) {
           let tab = getTab(details.tabId);
 
--- a/devtools/client/shared/test/unit/test_cssColor-01.js
+++ b/devtools/client/shared/test/unit/test_cssColor-01.js
@@ -1,25 +1,20 @@
 /* Any copyright is dedicated to the Public Domain.
    http://creativecommons.org/publicdomain/zero/1.0/ */
 
 // Test classifyColor.
 
 "use strict";
 
 var Cu = Components.utils;
-var Ci = Components.interfaces;
-var Cc = Components.classes;
 
-var {require, loader} = Cu.import("resource://devtools/shared/Loader.jsm", {});
+var {require} = Cu.import("resource://devtools/shared/Loader.jsm", {});
 const {colorUtils} = require("devtools/shared/css/color");
-
-loader.lazyGetter(this, "DOMUtils", function () {
-  return Cc["@mozilla.org/inspector/dom-utils;1"].getService(Ci.inIDOMUtils);
-});
+const InspectorUtils = require("InspectorUtils");
 
 const CLASSIFY_TESTS = [
   { input: "rgb(255,0,192)", output: "rgb" },
   { input: "RGB(255,0,192)", output: "rgb" },
   { input: "RGB(100%,0%,83%)", output: "rgb" },
   { input: "rgba(255,0,192, 0.25)", output: "rgb" },
   { input: "hsl(5, 5%, 5%)", output: "hsl" },
   { input: "hsla(5, 5%, 5%, 0.25)", output: "hsl" },
@@ -29,20 +24,20 @@ const CLASSIFY_TESTS = [
   { input: "#fe01cb", output: "hex" },
   { input: "#fe01cb80", output: "hex" },
   { input: "#FE01CB", output: "hex" },
   { input: "#FE01CB80", output: "hex" },
   { input: "blue", output: "name" },
   { input: "orange", output: "name" }
 ];
 
-function compareWithDomutils(input, isColor) {
+function compareWithInspectorUtils(input, isColor) {
   let ours = colorUtils.colorToRGBA(input);
-  let platform = DOMUtils.colorToRGBA(input);
-  deepEqual(ours, platform, "color " + input + " matches DOMUtils");
+  let platform = InspectorUtils.colorToRGBA(input);
+  deepEqual(ours, platform, "color " + input + " matches InspectorUtils");
   if (isColor) {
     ok(ours !== null, "'" + input + "' is a color");
   } else {
     ok(ours === null, "'" + input + "' is not a color");
   }
 }
 
 function run_test() {
@@ -50,22 +45,22 @@ function run_test() {
     let result = colorUtils.classifyColor(test.input);
     equal(result, test.output, "test classifyColor(" + test.input + ")");
 
     let obj = new colorUtils.CssColor("purple");
     obj.setAuthoredUnitFromColor(test.input);
     equal(obj.colorUnit, test.output,
           "test setAuthoredUnitFromColor(" + test.input + ")");
 
-    // Check that our implementation matches DOMUtils.
-    compareWithDomutils(test.input, true);
+    // Check that our implementation matches InspectorUtils.
+    compareWithInspectorUtils(test.input, true);
 
     // And check some obvious errors.
-    compareWithDomutils("mumble" + test.input, false);
-    compareWithDomutils(test.input + "trailingstuff", false);
+    compareWithInspectorUtils("mumble" + test.input, false);
+    compareWithInspectorUtils(test.input + "trailingstuff", false);
   }
 
   // Regression test for bug 1303826.
   let black = new colorUtils.CssColor("#000");
   black.colorUnit = "name";
   equal(black.toString(), "black", "test non-upper-case color cycling");
 
   let upper = new colorUtils.CssColor("BLACK");
--- a/devtools/client/shared/test/unit/test_cssColor-03.js
+++ b/devtools/client/shared/test/unit/test_cssColor-03.js
@@ -1,25 +1,20 @@
 /* Any copyright is dedicated to the Public Domain.
    http://creativecommons.org/publicdomain/zero/1.0/ */
 
 // Test css-color-4 color function syntax and old-style syntax.
 
 "use strict";
 
 var Cu = Components.utils;
-var Ci = Components.interfaces;
-var Cc = Components.classes;
 
-var {require, loader} = Cu.import("resource://devtools/shared/Loader.jsm", {});
+var {require} = Cu.import("resource://devtools/shared/Loader.jsm", {});
 const {colorUtils} = require("devtools/shared/css/color");
-
-loader.lazyGetter(this, "DOMUtils", function () {
-  return Cc["@mozilla.org/inspector/dom-utils;1"].getService(Ci.inIDOMUtils);
-});
+const InspectorUtils = require("InspectorUtils");
 
 const OLD_STYLE_TESTS = [
   "rgb(255,0,192)",
   "RGB(255,0,192)",
   "RGB(100%,0%,83%)",
   "rgba(255,0,192,0.25)",
   "hsl(120, 100%, 40%)",
   "hsla(120, 100%, 40%, 0.25)",
@@ -38,24 +33,25 @@ const CSS_COLOR_4_TESTS = [
   "hsl(50deg 25% 33% / 0.25)",
   "hsl(60 120% 60% / 0.25)",
   "hSlA(5turn 40% 4%)",
 ];
 
 function run_test() {
   for (let test of OLD_STYLE_TESTS) {
     let ours = colorUtils.colorToRGBA(test, false);
-    let platform = DOMUtils.colorToRGBA(test);
-    deepEqual(ours, platform, "color " + test + " matches DOMUtils");
+    let platform = InspectorUtils.colorToRGBA(test);
+    deepEqual(ours, platform, "color " + test + " matches InspectorUtils");
     ok(ours !== null, "'" + test + "' is a color");
   }
 
   for (let test of CSS_COLOR_4_TESTS) {
     let oursOld = colorUtils.colorToRGBA(test, false);
     let oursNew = colorUtils.colorToRGBA(test, true);
-    let platform = DOMUtils.colorToRGBA(test);
+    let platform = InspectorUtils.colorToRGBA(test);
     notEqual(oursOld, platform, "old style parser for color " + test +
-             " should not match DOMUtils");
+             " should not match InspectorUtils");
     ok(oursOld === null, "'" + test + "' is not a color with old parser");
-    deepEqual(oursNew, platform, `css-color-4 parser for color ${test} matches DOMUtils`);
+    deepEqual(oursNew, platform,
+              `css-color-4 parser for color ${test} matches InspectorUtils`);
     ok(oursNew !== null, "'" + test + "' is a color with css-color-4 parser");
   }
 }
--- a/devtools/client/shared/test/unit/test_cssColorDatabase.js
+++ b/devtools/client/shared/test/unit/test_cssColorDatabase.js
@@ -21,32 +21,32 @@ function isValid(colorName) {
   ok(colorUtils.isValidCSSColor(colorName),
      colorName + " is valid in database");
   ok(DOMUtils.isValidCSSColor(colorName),
      colorName + " is valid in DOMUtils");
 }
 
 function checkOne(colorName, checkName) {
   let ours = colorUtils.colorToRGBA(colorName);
-  let fromDom = DOMUtils.colorToRGBA(colorName);
-  deepEqual(ours, fromDom, colorName + " agrees with DOMUtils");
+  let fromDom = InspectorUtils.colorToRGBA(colorName);
+  deepEqual(ours, fromDom, colorName + " agrees with InspectorUtils");
 
   isValid(colorName);
 
   if (checkName) {
     let {r, g, b} = ours;
 
     // The color we got might not map back to the same name; but our
-    // implementation should agree with DOMUtils about which name is
+    // implementation should agree with InspectorUtils about which name is
     // canonical.
     let ourName = colorUtils.rgbToColorName(r, g, b);
     let domName = InspectorUtils.rgbToColorName(r, g, b);
 
     equal(ourName, domName,
-          colorName + " canonical name agrees with DOMUtils");
+          colorName + " canonical name agrees with InspectorUtils");
   }
 }
 
 function run_test() {
   for (let name in cssColors) {
     checkOne(name, true);
   }
   checkOne("transparent", false);
--- a/dom/webidl/InspectorUtils.webidl
+++ b/dom/webidl/InspectorUtils.webidl
@@ -28,16 +28,17 @@ namespace InspectorUtils {
       Element element,
       CSSStyleRule rule,
       unsigned long selectorIndex,
       [TreatNullAs=EmptyString] optional DOMString pseudo = "");
   boolean isInheritedProperty(DOMString property);
   sequence<DOMString> getCSSPropertyNames(optional PropertyNamesOptions options);
   [Throws] sequence<DOMString> getCSSValuesForProperty(DOMString property);
   [Throws] DOMString rgbToColorName(octet r, octet g, octet b);
+  InspectorRGBATuple? colorToRGBA(DOMString colorString);
 };
 
 dictionary PropertyNamesOptions {
   boolean includeAliases = false;
 };
 
 dictionary InspectorRGBATuple {
   /*
--- a/layout/inspector/InspectorUtils.h
+++ b/layout/inspector/InspectorUtils.h
@@ -116,16 +116,25 @@ public:
                                       ErrorResult& aRv);
 
   // Utilities for working with CSS colors
   static void RgbToColorName(GlobalObject& aGlobal,
                              uint8_t aR, uint8_t aG, uint8_t aB,
                              nsAString& aResult,
                              ErrorResult& aRv);
 
+  // Convert a given CSS color string to rgba. Returns null on failure or an
+  // InspectorRGBATuple on success.
+  //
+  // NOTE: Converting a color to RGBA may be lossy when converting from some
+  // formats e.g. CMYK.
+  static void ColorToRGBA(GlobalObject& aGlobal,
+                          const nsAString& aColorString,
+                          Nullable<InspectorRGBATuple>& aResult);
+
 private:
   static already_AddRefed<nsStyleContext>
     GetCleanStyleContextForElement(Element* aElement, nsAtom* aPseudo);
 };
 
 } // namespace dom
 } // namespace mozilla
 
--- a/layout/inspector/inDOMUtils.cpp
+++ b/layout/inspector/inDOMUtils.cpp
@@ -866,55 +866,50 @@ InspectorUtils::RgbToColorName(GlobalObj
     aColorName.Truncate();
     aRv.Throw(NS_ERROR_INVALID_ARG);
     return;
   }
 
   aColorName.AssignASCII(color);
 }
 
-} // namespace dom
-} // namespace mozilla
-
-NS_IMETHODIMP
-inDOMUtils::ColorToRGBA(const nsAString& aColorString, JSContext* aCx,
-                        JS::MutableHandle<JS::Value> aValue)
+/* static */ void
+InspectorUtils::ColorToRGBA(GlobalObject& aGlobalObject,
+                            const nsAString& aColorString,
+                            Nullable<InspectorRGBATuple>& aResult)
 {
   nscolor color = NS_RGB(0, 0, 0);
 
 #ifdef MOZ_STYLO
   if (!ServoCSSParser::ComputeColor(nullptr, NS_RGB(0, 0, 0), aColorString,
                                     &color)) {
-    aValue.setNull();
-    return NS_OK;
+    aResult.SetNull();
+    return;
   }
 #else
   nsCSSParser cssParser;
   nsCSSValue cssValue;
 
   if (!cssParser.ParseColorString(aColorString, nullptr, 0, cssValue, true)) {
-    aValue.setNull();
-    return NS_OK;
+    aResult.SetNull();
+    return;
   }
 
   nsRuleNode::ComputeColor(cssValue, nullptr, nullptr, color);
 #endif
 
-  InspectorRGBATuple tuple;
+  InspectorRGBATuple& tuple = aResult.SetValue();
   tuple.mR = NS_GET_R(color);
   tuple.mG = NS_GET_G(color);
   tuple.mB = NS_GET_B(color);
   tuple.mA = nsStyleUtil::ColorComponentToFloat(NS_GET_A(color));
+}
 
-  if (!ToJSValue(aCx, tuple, aValue)) {
-    return NS_ERROR_FAILURE;
-  }
-
-  return NS_OK;
-}
+} // namespace dom
+} // namespace mozilla
 
 NS_IMETHODIMP
 inDOMUtils::IsValidCSSColor(const nsAString& aColorString, bool *_retval)
 {
 #ifdef MOZ_STYLO
   *_retval = ServoCSSParser::IsValidCSSColor(aColorString);
 #else
   nsCSSParser cssParser;
--- a/layout/inspector/inIDOMUtils.idl
+++ b/layout/inspector/inIDOMUtils.idl
@@ -15,24 +15,16 @@ interface nsIDOMNode;
 interface nsIDOMNodeList;
 interface nsIDOMFontFaceList;
 interface nsIDOMRange;
 interface nsIDOMCSSStyleSheet;
 
 [scriptable, uuid(362e98c3-82c2-4ad8-8dcb-00e8e4eab497)]
 interface inIDOMUtils : nsISupports
 {
-  // Convert a given CSS color string to rgba. Returns null on failure or an
-  // InspectorRGBATuple on success.
-  //
-  // NOTE: Converting a color to RGBA may be lossy when converting from some
-  // formats e.g. CMYK.
-  [implicit_jscontext]
-  jsval colorToRGBA(in DOMString aColorString);
-
   // Check whether a given color is a valid CSS color.
   bool isValidCSSColor(in AString aColorString);
 
   // Utilities for obtaining information about a CSS property.
 
   // Get a list of the longhands corresponding to the given CSS property.  If
   // the property is a longhand already, just returns the property itself.
   // Throws on unsupported property names.
--- a/layout/inspector/tests/test_color_to_rgba.html
+++ b/layout/inspector/tests/test_color_to_rgba.html
@@ -1,18 +1,17 @@
 <!DOCTYPE HTML>
 <html>
 <head>
   <meta charset="utf-8">
-  <title>Test inDOMUtils::ColorToRGBA</title>
+  <title>Test InspectorUtils::ColorToRGBA</title>
   <script type="application/javascript" src="/tests/SimpleTest/SimpleTest.js"></script>
   <link rel="stylesheet" type="text/css" href="/tests/SimpleTest/test.css"/>
   <script type="application/javascript">
-  let utils = SpecialPowers.Cc["@mozilla.org/inspector/dom-utils;1"]
-                           .getService(SpecialPowers.Ci.inIDOMUtils);
+  const InspectorUtils = SpecialPowers.InspectorUtils;
 
   testColor("red", {r:255, g:0, b:0, a:1});
   testColor("#f00", {r:255, g:0, b:0, a:1});
   testColor("#ff0000", {r:255, g:0, b:0, a:1});
   testColor("ff0000", null);
   testColor("rgb(255,0,0)", {r:255, g:0, b:0, a:1});
   testColor("rgba(255,0,0)", {r:255, g:0, b:0, a:1});
   testColor("rgb(255,0,0,0.7)", {r:255, g:0, b:0, a:0.7});
@@ -22,34 +21,34 @@
   testColor("rgb(100%,50%,25%,0.7)", {r:255, g:128, b:64, a:0.7});
   testColor("rgba(100%,50%,25%,0.7)", {r:255, g:128, b:64, a:0.7});
   testColor("hsl(320,30%,10%)", {r:33, g:18, b:28, a:1});
   testColor("hsla(320,30%,10%)", {r:33, g:18, b:28, a:1});
   testColor("hsl(170,60%,40%,0.9)", {r:41, g:163, b:143, a:0.9});
   testColor("hsla(170,60%,40%,0.9)", {r:41, g:163, b:143, a:0.9});
 
   function testColor(color, expected) {
-    let rgb = utils.colorToRGBA(color);
+    let rgb = InspectorUtils.colorToRGBA(color);
 
     if (rgb === null) {
       ok(expected === null, "color: " + color + " returns null");
       return;
     }
 
     let {r, g, b, a} = rgb;
 
     is(r, expected.r, "color: " + color + ", red component is converted correctly");
     is(g, expected.g, "color: " + color + ", green component is converted correctly");
     is(b, expected.b, "color: " + color + ", blue component is converted correctly");
     is(Math.round(a * 10) / 10, expected.a, "color: " + color + ", alpha component is a converted correctly");
   }
   </script>
 </head>
 <body>
-<h1>Test inDOMUtils::ColorToRGBA</h1>
+<h1>Test InspectorUtils::ColorToRGBA</h1>
 <p id="display"></p>
 <div id="content" style="display: none">
 
 </div>
 <pre id="test">
 </pre>
 </body>
 </html>