Bug 1258645 - Split marker-utils.js into two modules (one DOM-only and one blueprint-only) and cleanup code rot, r=jsantell
authorVictor Porof <vporof@mozilla.com>
Mon, 28 Mar 2016 13:50:02 +0200
changeset 290530 ae98422622f7f75e608621c7291b27b94e2eddce
parent 290529 678ddeb8eb8483540c58ce4ad54248a2429cb6b7
child 290531 46fb0301d5ca4ca1f76087e7a182ff05811c4c88
push id19656
push usergwagner@mozilla.com
push dateMon, 04 Apr 2016 13:43:23 +0000
treeherderb2g-inbound@e99061fde28a [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersjsantell
bugs1258645
milestone48.0a1
Bug 1258645 - Split marker-utils.js into two modules (one DOM-only and one blueprint-only) and cleanup code rot, r=jsantell
devtools/client/performance/modules/logic/marker-utils.js
devtools/client/performance/modules/logic/moz.build
devtools/client/performance/modules/marker-blueprint-utils.js
devtools/client/performance/modules/marker-dom-utils.js
devtools/client/performance/modules/moz.build
devtools/client/themes/performance.css
--- a/devtools/client/performance/modules/logic/moz.build
+++ b/devtools/client/performance/modules/logic/moz.build
@@ -1,13 +1,12 @@
 # vim: set filetype=python:
 # This Source Code Form is subject to the terms of the Mozilla Public
 # License, v. 2.0. If a copy of the MPL was not distributed with this
 # file, You can obtain one at http://mozilla.org/MPL/2.0/.
 
 DevToolsModules(
     'frame-utils.js',
     'jit.js',
-    'marker-utils.js',
     'telemetry.js',
     'tree-model.js',
     'waterfall-utils.js',
 )
new file mode 100644
--- /dev/null
+++ b/devtools/client/performance/modules/marker-blueprint-utils.js
@@ -0,0 +1,112 @@
+/* This Source Code Form is subject to the terms of the Mozilla Public
+ * License, v. 2.0. If a copy of the MPL was not distributed with this file,
+ * You can obtain one at http://mozilla.org/MPL/2.0/. */
+"use strict";
+
+const { TIMELINE_BLUEPRINT } = require("devtools/client/performance/modules/markers");
+
+/**
+ * This file contains utilities for parsing out the markers blueprint
+ * to generate strings to be displayed in the UI.
+ */
+
+exports.MarkerBlueprintUtils = {
+  /**
+   * Takes a marker and a list of marker names that should be hidden, and
+   * determines if this marker should be filtered or not.
+   *
+   * @param object marker
+   * @return boolean
+   */
+  shouldDisplayMarker: function(marker, hiddenMarkerNames) {
+    if (!hiddenMarkerNames || hiddenMarkerNames.length == 0) {
+      return true;
+    }
+
+    // If this marker isn't yet defined in the blueprint, simply check if the
+    // entire category of "UNKNOWN" markers are supposed to be visible or not.
+    let isUnknown = !(marker.name in TIMELINE_BLUEPRINT);
+    if (isUnknown) {
+      return hiddenMarkerNames.indexOf("UNKNOWN") == -1;
+    }
+
+    return hiddenMarkerNames.indexOf(marker.name) == -1;
+  },
+
+  /**
+   * Takes a marker and returns the blueprint definition for that marker type,
+   * falling back to the UNKNOWN blueprint definition if undefined.
+   *
+   * @param object marker
+   * @return object
+   */
+  getBlueprintFor: function(marker) {
+    return TIMELINE_BLUEPRINT[marker.name] || TIMELINE_BLUEPRINT.UNKNOWN;
+  },
+
+  /**
+   * Returns the label to display for a marker, based off the blueprints.
+   *
+   * @param object marker
+   * @return string
+   */
+  getMarkerLabel: function(marker) {
+    let blueprint = this.getBlueprintFor(marker);
+    let label = typeof blueprint.label === "function" ? blueprint.label(marker) : blueprint.label;
+    return label;
+  },
+
+  /**
+   * Returns the generic label to display for a marker name.
+   * (e.g. "Function Call" for JS markers, rather than "setTimeout", etc.)
+   *
+   * @param string type
+   * @return string
+   */
+  getMarkerGenericName: function(markerName) {
+    let blueprint = this.getBlueprintFor({ name: markerName });
+    let generic = typeof blueprint.label === "function" ? blueprint.label() : blueprint.label;
+
+    // If no class name found, attempt to throw a descriptive error as to
+    // how the marker implementor can fix this.
+    if (!generic) {
+      let message = `Could not find marker generic name for "${markerName}".`;
+      if (typeof blueprint.label === "function") {
+        message += ` The following function must return a generic name string when no marker passed: ${blueprint.label}`;
+      } else {
+        message += ` ${markerName}.label must be defined in the marker blueprint.`;
+      }
+      throw new Error(message);
+    }
+
+    return generic;
+  },
+
+  /**
+   * Returns an array of objects with key/value pairs of what should be rendered
+   * in the marker details view.
+   *
+   * @param object marker
+   * @return array<object>
+   */
+  getMarkerFields: function(marker) {
+    let blueprint = this.getBlueprintFor(marker);
+
+    // If blueprint.fields is a function, use that.
+    if (typeof blueprint.fields === "function") {
+      let fields = blueprint.fields(marker) || {};
+      return Object.keys(fields).map(label => ({ label, value: fields[label] }));
+    }
+
+    // Otherwise, iterate over the array.
+    return (blueprint.fields || []).reduce((fields, field) => {
+      // Ensure this marker has this field present.
+      if (field.property in marker) {
+        let label = field.label;
+        let value = marker[field.property];
+        fields.push({ label, value });
+      }
+      return fields;
+    }, []);
+  },
+};
rename from devtools/client/performance/modules/logic/marker-utils.js
rename to devtools/client/performance/modules/marker-dom-utils.js
--- a/devtools/client/performance/modules/logic/marker-utils.js
+++ b/devtools/client/performance/modules/marker-dom-utils.js
@@ -1,221 +1,131 @@
 /* This Source Code Form is subject to the terms of the Mozilla Public
  * License, v. 2.0. If a copy of the MPL was not distributed with this file,
  * You can obtain one at http://mozilla.org/MPL/2.0/. */
 "use strict";
 
 /**
- * This file contains utilities for creating elements for markers to be displayed,
- * and parsing out the blueprint to generate correct values for markers.
+ * This file contains utilities for creating DOM nodes for markers
+ * to be displayed in the UI.
  */
 
-const { Cu, Ci } = require("chrome");
-
-const Services = require("Services");
-const { L10N } = require("devtools/client/performance/modules/global");
-const { TIMELINE_BLUEPRINT } = require("devtools/client/performance/modules/markers");
+const { L10N, PREFS } = require("devtools/client/performance/modules/global");
+const { MarkerBlueprintUtils } = require("devtools/client/performance/modules/marker-blueprint-utils");
 const { getSourceNames } = require("devtools/client/shared/source-utils");
-const SHOW_TRIGGER_FOR_GC_TYPES_PREF = "devtools.performance.ui.show-triggers-for-gc-types";
-
-/**
- * Takes a marker, blueprint, and filter list and
- * determines if this marker should be filtered or not.
- */
-function isMarkerValid (marker, filter) {
-  if (!filter || filter.length === 0) {
-    return true;
-  }
-
-  let isUnknown = !(marker.name in TIMELINE_BLUEPRINT);
-  if (isUnknown) {
-    return filter.indexOf("UNKNOWN") === -1;
-  }
-  return filter.indexOf(marker.name) === -1;
-}
-
-/**
- * Returns the correct label to display for passed in marker, based
- * off of the blueprints.
- *
- * @param {ProfileTimelineMarker} marker
- * @return {string}
- */
-function getMarkerLabel (marker) {
-  let blueprint = getBlueprintFor(marker);
-  // Either use the label function in the blueprint, or use it directly
-  // as a string.
-  return typeof blueprint.label === "function" ? blueprint.label(marker) : blueprint.label;
-}
-
-/**
- * Returns the correct generic name for a marker class, like "Function Call"
- * being the general class for JS markers, rather than "setTimeout", etc.
- *
- * @param {string} type
- * @return {string}
- */
-function getMarkerClassName (type) {
-  let blueprint = getBlueprintFor({ name: type });
-  // Either use the label function in the blueprint, or use it directly
-  // as a string.
-  let className = typeof blueprint.label === "function" ? blueprint.label() : blueprint.label;
-
-  // If no class name found, attempt to throw a descriptive error how the marker
-  // implementor can fix this.
-  if (!className) {
-    let message = `Could not find marker class name for "${type}".`;
-    if (typeof blueprint.label === "function") {
-      message += ` The following function must return a class name string when no marker passed: ${blueprint.label}`;
-    } else {
-      message += ` ${type}.label must be defined in the marker blueprint.`;
-    }
-    throw new Error(message);
-  }
-
-  return className;
-}
-
-/**
- * Returns an array of objects with key/value pairs of what should be rendered
- * in the marker details view.
- *
- * @param {ProfileTimelineMarker} marker
- * @return {Array<object>}
- */
-function getMarkerFields (marker) {
-  let blueprint = getBlueprintFor(marker);
-
-  // If blueprint.fields is a function, use that
-  if (typeof blueprint.fields === "function") {
-    let fields = blueprint.fields(marker);
-    return Object.keys(fields || []).map(label => {
-      return { label, value: fields[label] };
-    });
-  }
-
-  // Otherwise, iterate over the array
-  return (blueprint.fields || []).reduce((fields, field) => {
-    // Ensure this marker has this field present
-    if (field.property in marker) {
-      let label = field.label;
-      let value = marker[field.property];
-      fields.push({ label, value });
-    }
-    return fields;
-  }, []);
-}
 
 /**
  * Utilites for creating elements for markers.
  */
-const DOM = {
+exports.MarkerDOMUtils = {
   /**
    * Builds all the fields possible for the given marker. Returns an
    * array of elements to be appended to a parent element.
    *
-   * @param {Document} doc
-   * @param {ProfileTimelineMarker} marker
-   * @return {Array<Element>}
+   * @param document doc
+   * @param object marker
+   * @return array<nsIDOMNode>
    */
   buildFields: function (doc, marker) {
-    let blueprint = getBlueprintFor(marker);
-    let fields = getMarkerFields(marker);
-
-    return fields.map(({ label, value }) => DOM.buildNameValueLabel(doc, label, value));
+    let fields = MarkerBlueprintUtils.getMarkerFields(marker);
+    return fields.map(({ label, value }) => this.buildNameValueLabel(doc, label, value));
   },
 
   /**
-   * Builds the label representing marker's type.
+   * Builds the label representing the marker's type.
    *
-   * @param {Document} doc
-   * @param {ProfileTimelineMarker}
-   * @return {Element}
+   * @param document doc
+   * @param object marker
+   * @return nsIDOMNode
    */
   buildTitle: function (doc, marker) {
-    let blueprint = getBlueprintFor(marker);
+    let blueprint = MarkerBlueprintUtils.getBlueprintFor(marker);
 
     let hbox = doc.createElement("hbox");
     hbox.setAttribute("align", "center");
 
     let bullet = doc.createElement("hbox");
     bullet.className = `marker-details-bullet marker-color-${blueprint.colorName}`;
 
-    let title = getMarkerLabel(marker);
+    let title = MarkerBlueprintUtils.getMarkerLabel(marker);
     let label = doc.createElement("label");
     label.className = "marker-details-type";
     label.setAttribute("value", title);
 
     hbox.appendChild(bullet);
     hbox.appendChild(label);
 
     return hbox;
   },
 
   /**
-   * Builds the duration element, like "Duration: 200ms".
+   * Builds the label representing the marker's duration.
    *
-   * @param {Document} doc
-   * @param {ProfileTimelineMarker} marker
-   * @return {Element}
+   * @param document doc
+   * @param object marker
+   * @return nsIDOMNode
    */
   buildDuration: function (doc, marker) {
     let label = L10N.getStr("marker.field.duration");
     let start = L10N.getFormatStrWithNumbers("timeline.tick", marker.start);
     let end = L10N.getFormatStrWithNumbers("timeline.tick", marker.end);
     let duration = L10N.getFormatStrWithNumbers("timeline.tick", marker.end - marker.start);
-    let el = DOM.buildNameValueLabel(doc, label, duration);
+
+    let el = this.buildNameValueLabel(doc, label, duration);
     el.classList.add("marker-details-duration");
     el.setAttribute("tooltiptext", `${start} → ${end}`);
+
     return el;
   },
 
   /**
-   * Builds labels for name:value pairs. Like "Start: 100ms",
-   * "Duration: 200ms", ...
+   * Builds labels for name:value pairs.
+   * E.g. "Start: 100ms", "Duration: 200ms", ...
    *
-   * @param {Document} doc
+   * @param document doc
    * @param string field
-   *        String identifier for label's name.
    * @param string value
-   *        Label's value.
-   * @return {Element}
+   * @return nsIDOMNode
    */
   buildNameValueLabel: function (doc, field, value) {
     let hbox = doc.createElement("hbox");
     hbox.className = "marker-details-labelcontainer";
-    let labelName = doc.createElement("label");
-    let labelValue = doc.createElement("label");
-    labelName.className = "plain marker-details-labelname";
-    labelValue.className = "plain marker-details-labelvalue";
-    labelName.setAttribute("value", field);
-    labelValue.setAttribute("value", value);
-    hbox.appendChild(labelName);
-    hbox.appendChild(labelValue);
+
+    let nameLabel = doc.createElement("label");
+    nameLabel.className = "plain marker-details-name-label";
+    nameLabel.setAttribute("value", field);
+    hbox.appendChild(nameLabel);
+
+    let valueLabel = doc.createElement("label");
+    valueLabel.className = "plain marker-details-value-label";
+    valueLabel.setAttribute("value", value);
+    hbox.appendChild(valueLabel);
+
     return hbox;
   },
 
   /**
    * Builds a stack trace in an element.
    *
-   * @param {Document} doc
+   * @param document doc
    * @param object params
    *        An options object with the following members:
-   *        string type - String identifier for type of stack ("stack", "startStack" or "endStack")
-   *        number frameIndex - The index of the topmost stack frame.
-   *        array frames - Array of stack frames.
+   *          - string type: string identifier for type of stack ("stack", "startStack" or "endStack"
+   *          - number frameIndex: the index of the topmost stack frame
+   *          - array frames: array of stack frames
    */
   buildStackTrace: function(doc, { type, frameIndex, frames }) {
     let container = doc.createElement("vbox");
-    let labelName = doc.createElement("label");
-    labelName.className = "plain marker-details-labelname";
-    labelName.setAttribute("value", L10N.getStr(`marker.field.${type}`));
+    container.className = "marker-details-stack";
     container.setAttribute("type", type);
-    container.className = "marker-details-stack";
-    container.appendChild(labelName);
+
+    let nameLabel = doc.createElement("label");
+    nameLabel.className = "plain marker-details-name-label";
+    nameLabel.setAttribute("value", L10N.getStr(`marker.field.${type}`));
+    container.appendChild(nameLabel);
 
     // Workaround for profiles that have looping stack traces.  See
     // bug 1246555.
     let wasAsyncParent = false;
     let seen = new Set();
 
     while (frameIndex > 0) {
       if (seen.has(frameIndex)) {
@@ -226,64 +136,67 @@ const DOM = {
       let frame = frames[frameIndex];
       let url = frame.source;
       let displayName = frame.functionDisplayName;
       let line = frame.line;
 
       // If the previous frame had an async parent, then the async
       // cause is in this frame and should be displayed.
       if (wasAsyncParent) {
+        let asyncStr = L10N.getFormatStr("marker.field.asyncStack", frame.asyncCause);
         let asyncBox = doc.createElement("hbox");
         let asyncLabel = doc.createElement("label");
         asyncLabel.className = "devtools-monospace";
-        asyncLabel.setAttribute("value", L10N.getFormatStr("marker.field.asyncStack",
-                                                           frame.asyncCause));
+        asyncLabel.setAttribute("value", asyncStr);
         asyncBox.appendChild(asyncLabel);
         container.appendChild(asyncBox);
         wasAsyncParent = false;
       }
 
       let hbox = doc.createElement("hbox");
 
       if (displayName) {
         let functionLabel = doc.createElement("label");
         functionLabel.className = "devtools-monospace";
         functionLabel.setAttribute("value", displayName);
         hbox.appendChild(functionLabel);
       }
 
       if (url) {
-        let aNode = doc.createElement("a");
-        aNode.className = "waterfall-marker-location devtools-source-link";
-        aNode.href = url;
-        aNode.draggable = false;
-        aNode.setAttribute("title", url);
+        let linkNode = doc.createElement("a");
+        linkNode.className = "waterfall-marker-location devtools-source-link";
+        linkNode.href = url;
+        linkNode.draggable = false;
+        linkNode.setAttribute("title", url);
 
-        let urlNode = doc.createElement("label");
-        urlNode.className = "filename";
-        urlNode.setAttribute("value", getSourceNames(url).short);
-        let lineNode = doc.createElement("label");
-        lineNode.className = "line-number";
-        lineNode.setAttribute("value", `:${line}`);
+        let urlLabel = doc.createElement("label");
+        urlLabel.className = "filename";
+        urlLabel.setAttribute("value", getSourceNames(url).short);
+        linkNode.appendChild(urlLabel);
 
-        aNode.appendChild(urlNode);
-        aNode.appendChild(lineNode);
-        hbox.appendChild(aNode);
+        let lineLabel = doc.createElement("label");
+        lineLabel.className = "line-number";
+        lineLabel.setAttribute("value", `:${line}`);
+        linkNode.appendChild(lineLabel);
+
+        hbox.appendChild(linkNode);
 
         // Clicking here will bubble up to the parent,
         // which handles the view source.
-        aNode.setAttribute("data-action", JSON.stringify({
-          url, line, action: "view-source"
+        linkNode.setAttribute("data-action", JSON.stringify({
+          url: url,
+          line: line,
+          action: "view-source"
         }));
       }
 
       if (!displayName && !url) {
-        let label = doc.createElement("label");
-        label.setAttribute("value", L10N.getStr("marker.value.unknownFrame"));
-        hbox.appendChild(label);
+        let unknownLabel = doc.createElement("label");
+        unknownLabel.setAttribute("value", L10N.getStr("marker.value.unknownFrame"));
+        hbox.appendChild(unknownLabel);
       }
 
       container.appendChild(hbox);
 
       if (frame.asyncParent) {
         frameIndex = frame.asyncParent;
         wasAsyncParent = true;
       } else {
@@ -292,66 +205,51 @@ const DOM = {
     }
 
     return container;
   },
 
   /**
    * Builds any custom fields specific to the marker.
    *
-   * @param {Document} doc
-   * @param {ProfileTimelineMarker} marker
-   * @param {object} options
-   * @return {Array<Element>}
+   * @param document doc
+   * @param object marker
+   * @param object options
+   * @return array<nsIDOMNode>
    */
   buildCustom: function (doc, marker, options) {
     let elements = [];
 
-    if (options.allocations && showAllocationsTrigger(marker)) {
+    if (options.allocations && shouldShowAllocationsTrigger(marker)) {
       let hbox = doc.createElement("hbox");
       hbox.className = "marker-details-customcontainer";
 
       let label = doc.createElement("label");
       label.className = "custom-button devtools-button";
       label.setAttribute("value", "Show allocation triggers");
       label.setAttribute("type", "show-allocations");
       label.setAttribute("data-action", JSON.stringify({
-        endTime: marker.start, action: "show-allocations"
+        endTime: marker.start,
+        action: "show-allocations"
       }));
 
       hbox.appendChild(label);
       elements.push(hbox);
     }
 
     return elements;
   },
 };
 
 /**
- * Takes a marker and returns the definition for that marker type,
- * falling back to the UNKNOWN definition if undefined.
- *
- * @param {Marker} marker
- * @return {object}
- */
-function getBlueprintFor (marker) {
-  return TIMELINE_BLUEPRINT[marker.name] || TIMELINE_BLUEPRINT.UNKNOWN;
-}
-
-/**
  * Takes a marker and determines if this marker should display
  * the allocations trigger button.
  *
- * @param {Marker} marker
- * @return {boolean}
+ * @param object marker
+ * @return boolean
  */
-function showAllocationsTrigger (marker) {
-  return marker.name === "GarbageCollection" &&
-         Services.prefs.getCharPref(SHOW_TRIGGER_FOR_GC_TYPES_PREF)
-         .split(" ").indexOf(marker.causeName) !== -1;
+function shouldShowAllocationsTrigger (marker) {
+  if (marker.name == "GarbageCollection") {
+    let showTriggers = PREFS["show-triggers-for-gc-types"];
+    return showTriggers.split(" ").indexOf(marker.causeName) !== -1;
+  }
+  return false;
 }
-
-exports.isMarkerValid = isMarkerValid;
-exports.getMarkerLabel = getMarkerLabel;
-exports.getMarkerClassName = getMarkerClassName;
-exports.getMarkerFields = getMarkerFields;
-exports.DOM = DOM;
-exports.getBlueprintFor = getBlueprintFor;
--- a/devtools/client/performance/modules/moz.build
+++ b/devtools/client/performance/modules/moz.build
@@ -8,11 +8,13 @@ DIRS += [
     'widgets',
 ]
 
 DevToolsModules(
     'categories.js',
     'constants.js',
     'global.js',
     'io.js',
+    'marker-blueprint-utils.js',
+    'marker-dom-utils.js',
     'marker-formatters.js',
     'markers.js',
 )
--- a/devtools/client/themes/performance.css
+++ b/devtools/client/themes/performance.css
@@ -546,17 +546,17 @@
 }
 
 .marker-details-bullet {
   width: 8px;
   height: 8px;
   border-radius: 1px;
 }
 
-.marker-details-labelname {
+.marker-details-name-label {
   -moz-padding-end: 4px;
 }
 
 .marker-details-type {
   font-size: 1.2em;
   font-weight: bold;
 }