Bug 1016330 - (Part 1) Make CSS Coverage work with B2G; r=paul
authorJoe Walker <jwalker@mozilla.com>
Sun, 08 Jun 2014 22:33:36 +0100
changeset 206636 776897298340eda860b5f9574c0c4b2587e82ae5
parent 206634 17402b39be2440bd46c75fbb65c56a7ff4a2a5f3
child 206637 f2f51353b58ff935c46eba07e7921730ab1da9c7
push id3741
push userasasaki@mozilla.com
push dateMon, 21 Jul 2014 20:25:18 +0000
treeherdermozilla-beta@4d6f46f5af68 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerspaul
bugs1016330
milestone32.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 1016330 - (Part 1) Make CSS Coverage work with B2G; r=paul https://bugzilla.mozilla.org/show_bug.cgi?id=1016330#c3 > prettifyCSS needs to be used client side or moved at the toolkit level. Done https://bugzilla.mozilla.org/show_bug.cgi?id=1016330#c4 > Move the call to makeRemote to DeveloperToolbar.jsm Not done. See https://bugzilla.mozilla.org/show_bug.cgi?id=1016330#c6 https://bugzilla.mozilla.org/show_bug.cgi?id=1016330#c5 > We need to test first there's a cssusage actor. > A toolbox might be debugging gecko < 32. Done https://bugzilla.mozilla.org/show_bug.cgi?id=1016330#c2 > Kill reference to this._tabActor.browser. This might create issues > with sub documents ("load" is sent to browser even for inner documents. > It's not the case with tabActor.window). Future work.
browser/devtools/shared/DeveloperToolbar.jsm
browser/devtools/styleeditor/StyleEditorUI.jsm
browser/devtools/styleeditor/StyleSheetEditor.jsm
toolkit/devtools/gcli/commands/csscoverage.js
toolkit/devtools/server/actors/csscoverage.js
toolkit/devtools/styleinspector/css-logic.js
toolkit/locales/en-US/chrome/global/devtools/csscoverage.properties
--- a/browser/devtools/shared/DeveloperToolbar.jsm
+++ b/browser/devtools/shared/DeveloperToolbar.jsm
@@ -260,16 +260,19 @@ const NOTIFICATIONS = {
 };
 
 /**
  * Attach notification constants to the object prototype so tests etc can
  * use them without needing to import anything
  */
 DeveloperToolbar.prototype.NOTIFICATIONS = NOTIFICATIONS;
 
+/**
+ * target is dynamic because the selectedTab changes
+ */
 Object.defineProperty(DeveloperToolbar.prototype, "target", {
   get: function() {
     return TargetFactory.forTab(this._chromeWindow.getBrowser().selectedTab);
   },
   enumerable: true
 });
 
 /**
--- a/browser/devtools/styleeditor/StyleEditorUI.jsm
+++ b/browser/devtools/styleeditor/StyleEditorUI.jsm
@@ -485,16 +485,20 @@ StyleEditorUI.prototype = {
           }
 
           editor.onShow();
 
           this.emit("editor-selected", editor);
 
           // Is there any CSS coverage markup to include?
           csscoverage.getUsage(this._target).then(usage => {
+            if (usage == null) {
+              return;
+            }
+
             let href = editor.styleSheet.href || editor.styleSheet.nodeHref;
             usage.createEditorReport(href).then(data => {
               editor.removeAllUnusedRegions();
 
               if (data.reports.length > 0) {
                 // So there is some coverage markup, but can we apply it?
                 let text = editor.sourceEditor.getText() + "\r";
                 // If the CSS text contains a '}' with some non-whitespace
--- a/browser/devtools/styleeditor/StyleSheetEditor.jsm
+++ b/browser/devtools/styleeditor/StyleSheetEditor.jsm
@@ -221,17 +221,17 @@ StyleSheetEditor.prototype = {
   },
 
   /**
    * Start fetching the full text source for this editor's sheet.
    */
   fetchSource: function(callback) {
     return this.styleSheet.getText().then((longStr) => {
       longStr.string().then((source) => {
-        this._state.text = prettifyCSS(source);
+        this._state.text = CssLogic.prettifyCSS(source);
         this.sourceLoaded = true;
 
         if (callback) {
           callback(source);
         }
         return source;
       });
     }, e => {
@@ -635,83 +635,16 @@ StyleSheetEditor.prototype = {
       this.sourceEditor.destroy();
     }
     this.cssSheet.off("property-change", this._onPropertyChange);
     this.cssSheet.off("media-rules-changed", this._onMediaRulesChanged);
     this.styleSheet.off("error", this._onError);
   }
 }
 
-
-const TAB_CHARS = "\t";
-
-const CURRENT_OS = Cc["@mozilla.org/xre/app-info;1"].getService(Ci.nsIXULRuntime).OS;
-const LINE_SEPARATOR = CURRENT_OS === "WINNT" ? "\r\n" : "\n";
-
-/**
- * Prettify minified CSS text.
- * This prettifies CSS code where there is no indentation in usual places while
- * keeping original indentation as-is elsewhere.
- *
- * @param string text
- *        The CSS source to prettify.
- * @return string
- *         Prettified CSS source
- */
-function prettifyCSS(text)
-{
-  // remove initial and terminating HTML comments and surrounding whitespace
-  text = text.replace(/(?:^\s*<!--[\r\n]*)|(?:\s*-->\s*$)/g, "");
-
-  let parts = [];    // indented parts
-  let partStart = 0; // start offset of currently parsed part
-  let indent = "";
-  let indentLevel = 0;
-
-  for (let i = 0; i < text.length; i++) {
-    let c = text[i];
-    let shouldIndent = false;
-
-    switch (c) {
-      case "}":
-        if (i - partStart > 1) {
-          // there's more than just } on the line, add line
-          parts.push(indent + text.substring(partStart, i));
-          partStart = i;
-        }
-        indent = TAB_CHARS.repeat(--indentLevel);
-        /* fallthrough */
-      case ";":
-      case "{":
-        shouldIndent = true;
-        break;
-    }
-
-    if (shouldIndent) {
-      let la = text[i+1]; // one-character lookahead
-      if (!/\n/.test(la) || /^\s+$/.test(text.substring(i+1, text.length))) {
-        // following character should be a new line, but isn't,
-        // or it's whitespace at the end of the file
-        parts.push(indent + text.substring(partStart, i + 1));
-        if (c == "}") {
-          parts.push(""); // for extra line separator
-        }
-        partStart = i + 1;
-      } else {
-        return text; // assume it is not minified, early exit
-      }
-    }
-
-    if (c == "{") {
-      indent = TAB_CHARS.repeat(++indentLevel);
-    }
-  }
-  return parts.join(LINE_SEPARATOR);
-}
-
 /**
  * Find a path on disk for a file given it's hosted uri, the uri of the
  * original resource that generated it (e.g. Sass file), and the location of the
  * local file for that source.
  *
  * @param {nsIURI} uri
  *        The uri of the resource
  * @param {nsIURI} origUri
--- a/toolkit/devtools/gcli/commands/csscoverage.js
+++ b/toolkit/devtools/gcli/commands/csscoverage.js
@@ -27,65 +27,81 @@ exports.items = [
     description: l10n.lookup("csscoverageDesc"),
   },
   {
     name: "csscoverage start",
     hidden: true,
     description: l10n.lookup("csscoverageStartDesc"),
     exec: function*(args, context) {
       let usage = yield csscoverage.getUsage(context.environment.target);
+      if (usage == null) {
+        throw new Error(l10n.lookup("csscoverageNoRemoteError"));
+      }
       yield usage.start(context.environment.chromeWindow,
                         context.environment.target);
     }
   },
   {
     name: "csscoverage stop",
     hidden: true,
     description: l10n.lookup("csscoverageStopDesc"),
     exec: function*(args, context) {
       let target = context.environment.target;
       let usage = yield csscoverage.getUsage(target);
+      if (usage == null) {
+        throw new Error(l10n.lookup("csscoverageNoRemoteError"));
+      }
       yield usage.stop();
       yield gDevTools.showToolbox(target, "styleeditor");
     }
   },
   {
     name: "csscoverage oneshot",
     hidden: true,
     description: l10n.lookup("csscoverageOneShotDesc"),
     exec: function*(args, context) {
       let target = context.environment.target;
       let usage = yield csscoverage.getUsage(target);
+      if (usage == null) {
+        throw new Error(l10n.lookup("csscoverageNoRemoteError"));
+      }
       yield usage.oneshot();
       yield gDevTools.showToolbox(target, "styleeditor");
     }
   },
   {
     name: "csscoverage toggle",
     hidden: true,
     description: l10n.lookup("csscoverageToggleDesc"),
     exec: function*(args, context) {
       let target = context.environment.target;
       let usage = yield csscoverage.getUsage(target);
+      if (usage == null) {
+        throw new Error(l10n.lookup("csscoverageNoRemoteError"));
+      }
 
       let running = yield usage.toggle();
       if (running) {
         return l10n.lookup("csscoverageRunningReply");
       }
 
       yield usage.stop();
       yield gDevTools.showToolbox(target, "styleeditor");
     }
   },
   {
     name: "csscoverage report",
     hidden: true,
     description: l10n.lookup("csscoverageReportDesc"),
     exec: function*(args, context) {
       let usage = yield csscoverage.getUsage(context.environment.target);
+      if (usage == null) {
+        throw new Error(l10n.lookup("csscoverageNoRemoteError"));
+      }
+
       return {
         isTypedData: true,
         type: "csscoveragePageReport",
         data: yield usage.createPageReport()
       };
     }
   },
   {
--- a/toolkit/devtools/server/actors/csscoverage.js
+++ b/toolkit/devtools/server/actors/csscoverage.js
@@ -16,19 +16,16 @@ const protocol = require("devtools/serve
 const { method, custom, RetVal, Arg } = protocol;
 
 loader.lazyGetter(this, "gDevTools", () => {
   return require("resource:///modules/devtools/gDevTools.jsm").gDevTools;
 });
 loader.lazyGetter(this, "DOMUtils", () => {
   return Cc["@mozilla.org/inspector/dom-utils;1"].getService(Ci.inIDOMUtils)
 });
-loader.lazyGetter(this, "prettifyCSS", () => {
-  return require("resource:///modules/devtools/StyleSheetEditor.jsm").prettifyCSS;
-});
 
 const CSSRule = Ci.nsIDOMCSSRule;
 
 const MAX_UNUSED_RULES = 10000;
 
 /**
  * Allow: let foo = l10n.lookup("csscoverageFoo");
  */
@@ -362,17 +359,17 @@ let UsageReportActor = protocol.ActorCla
 
     // Helper function to create a JSONable data structure representing a rule
     const ruleToRuleReport = function(rule, ruleData) {
       return {
         url: rule.url,
         shortUrl: rule.url.split("/").slice(-1),
         start: { line: rule.line, column: rule.column },
         selectorText: ruleData.selectorText,
-        formattedCssText: prettifyCSS(ruleData.cssText)
+        formattedCssText: CssLogic.prettifyCSS(ruleData.cssText)
       };
     }
 
     // A count of each type of rule for the bar chart
     let summary = { used: 0, unused: 0, preload: 0 };
 
     // Create the set of the unused rules
     let unusedMap = new Map();
@@ -752,19 +749,21 @@ exports.unregister = function(handle) {
   handle.removeGlobalActor(UsageReportActor, "usageReportActor");
   handle.removeTabActor(UsageReportActor, "usageReportActor");
 };
 
 const knownFronts = new WeakMap();
 
 /**
  * Create a UsageReportFront only when needed (returns a promise)
+ * For notes on target.makeRemote(), see
+ * https://bugzilla.mozilla.org/show_bug.cgi?id=1016330#c7
  */
 const getUsage = exports.getUsage = function(target) {
   return target.makeRemote().then(() => {
     let front = knownFronts.get(target.client)
-    if (front == null) {
+    if (front == null && target.form.usageReportActor != null) {
       front = new UsageReportFront(target.client, target.form);
       knownFronts.set(target.client, front);
     }
     return front;
   });
 };
--- a/toolkit/devtools/styleinspector/css-logic.js
+++ b/toolkit/devtools/styleinspector/css-logic.js
@@ -923,16 +923,80 @@ CssLogic.findCssSelector = function CssL
     index = positionInNodeList(ele, ele.parentNode.children) + 1;
     selector = CssLogic_findCssSelector(ele.parentNode) + ' > ' +
             tagName + ':nth-child(' + index + ')';
   }
 
   return selector;
 };
 
+const TAB_CHARS = "\t";
+
+/**
+ * Prettify minified CSS text.
+ * This prettifies CSS code where there is no indentation in usual places while
+ * keeping original indentation as-is elsewhere.
+ * @param string text The CSS source to prettify.
+ * @return string Prettified CSS source
+ */
+CssLogic.prettifyCSS = function(text) {
+  if (CssLogic.LINE_SEPARATOR == null) {
+    let os = Cc["@mozilla.org/xre/app-info;1"].getService(Ci.nsIXULRuntime).OS;
+    CssLogic.LINE_SEPARATOR = (os === "WINNT" ? "\r\n" : "\n");
+  }
+
+  // remove initial and terminating HTML comments and surrounding whitespace
+  text = text.replace(/(?:^\s*<!--[\r\n]*)|(?:\s*-->\s*$)/g, "");
+
+  let parts = [];    // indented parts
+  let partStart = 0; // start offset of currently parsed part
+  let indent = "";
+  let indentLevel = 0;
+
+  for (let i = 0; i < text.length; i++) {
+    let c = text[i];
+    let shouldIndent = false;
+
+    switch (c) {
+      case "}":
+        if (i - partStart > 1) {
+          // there's more than just } on the line, add line
+          parts.push(indent + text.substring(partStart, i));
+          partStart = i;
+        }
+        indent = TAB_CHARS.repeat(--indentLevel);
+        /* fallthrough */
+      case ";":
+      case "{":
+        shouldIndent = true;
+        break;
+    }
+
+    if (shouldIndent) {
+      let la = text[i+1]; // one-character lookahead
+      if (!/\n/.test(la) || /^\s+$/.test(text.substring(i+1, text.length))) {
+        // following character should be a new line, but isn't,
+        // or it's whitespace at the end of the file
+        parts.push(indent + text.substring(partStart, i + 1));
+        if (c == "}") {
+          parts.push(""); // for extra line separator
+        }
+        partStart = i + 1;
+      } else {
+        return text; // assume it is not minified, early exit
+      }
+    }
+
+    if (c == "{") {
+      indent = TAB_CHARS.repeat(++indentLevel);
+    }
+  }
+  return parts.join(CssLogic.LINE_SEPARATOR);
+};
+
 /**
  * A safe way to access cached bits of information about a stylesheet.
  *
  * @constructor
  * @param {CssLogic} aCssLogic pointer to the CssLogic instance working with
  * this CssSheet object.
  * @param {CSSStyleSheet} aDomSheet reference to a DOM CSSStyleSheet object.
  * @param {number} aIndex tells the index/position of the stylesheet within the
--- a/toolkit/locales/en-US/chrome/global/devtools/csscoverage.properties
+++ b/toolkit/locales/en-US/chrome/global/devtools/csscoverage.properties
@@ -21,8 +21,9 @@ csscoverageRunningReply=Running CSS cove
 csscoverageDoneReply=CSS Coverage analysis completed
 
 # LOCALIZATION NOTE (csscoverageRunningError, csscoverageNotRunningError,
 # csscoverageNotRunError): Error message that describe things that can go wrong
 # with the css coverage system
 csscoverageRunningError=CSS coverage analysis already running
 csscoverageNotRunningError=CSS coverage analysis not running
 csscoverageNotRunError=CSS coverage analysis has not been run
+csscoverageNoRemoteError=Target does not support CSS Coverage