Bug 1258416 - Cleanup marker.js and marker-formatters.js by adding missing l10n strings, removing dead code and updating comments, r=jsantell
authorVictor Porof <vporof@mozilla.com>
Tue, 22 Mar 2016 10:17:21 +0100
changeset 289879 0c6bacea23968480ebb71482f6bedf54aaf23d55
parent 289878 539fe6a355f19ee7b9f32d66adcc849b82fb5fe1
child 289880 5e3ef0ac174e1053b2854e0d51ae6ac4a7cf88b1
push id74027
push userkwierso@gmail.com
push dateTue, 22 Mar 2016 23:57:42 +0000
treeherdermozilla-inbound@eb528d042c85 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersjsantell
bugs1258416
milestone48.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 1258416 - Cleanup marker.js and marker-formatters.js by adding missing l10n strings, removing dead code and updating comments, r=jsantell
devtools/client/locales/en-US/markers.properties
devtools/client/performance/modules/logic/marker-formatters.js
devtools/client/performance/modules/logic/moz.build
devtools/client/performance/modules/marker-formatters.js
devtools/client/performance/modules/markers.js
devtools/client/performance/modules/moz.build
--- a/devtools/client/locales/en-US/markers.properties
+++ b/devtools/client/locales/en-US/markers.properties
@@ -40,54 +40,59 @@ marker.label.unknown=Unknown
 # reasons that can be translated.
 marker.label.javascript.scriptElement=Script Tag
 marker.label.javascript.promiseCallback=Promise Callback
 marker.label.javascript.promiseInit=Promise Init
 marker.label.javascript.workerRunnable=Worker
 marker.label.javascript.jsURI=JavaScript URI
 marker.label.javascript.eventHandler=Event Handler
 
-# LOCALIZATION NOTE (marker.fieldFormat):
-# Some timeline markers come with details, like a size, a name, a js function.
-# %1$S is replaced with one of the above label (marker.label.*) and %2$S
-# with the details. For examples: Paint (200x100), or console.time (FOO)
-marker.fieldFormat=%1$S (%2$S)
-
 # LOCALIZATION NOTE (marker.field.*):
 # Strings used in the waterfall sidebar as property names.
 
 # General marker fields
 marker.field.start=Start:
 marker.field.end=End:
 marker.field.duration=Duration:
+
+# General "reason" for a marker (JavaScript, Garbage Collection)
+marker.field.causeName=Cause:
+# General "type" for a marker (Cycle Collection, Garbage Collection)
+marker.field.type=Type:
+# General "label" for a marker (user defined)
+marker.field.label=Label:
+
 # Field names for stack values
 marker.field.stack=Stack:
 marker.field.startStack=Stack at start:
 marker.field.endStack=Stack at end:
+
 # %S is the "Async Cause" of a marker, and this signifies that the cause
 # was an asynchronous one in a displayed stack.
 marker.field.asyncStack=(Async: %S)
+
 # For console.time markers
 marker.field.consoleTimerName=Timer Name:
+
 # For DOM Event markers
 marker.field.DOMEventType=Event Type:
 marker.field.DOMEventPhase=Phase:
+
 # Non-incremental cause for a Garbage Collection marker
 marker.field.nonIncrementalCause=Non-incremental Cause:
+
 # For "Recalculate Style" markers
 marker.field.restyleHint=Restyle Hint:
-# General "reason" for a marker (JavaScript, Garbage Collection)
-marker.field.causeName=Cause:
-# General "type" for a marker (Cycle Collection, Garbage Collection)
-marker.field.type=Type:
+
 # The type of operation performed by a Worker.
 marker.worker.serializeDataOffMainThread=Serialize data in Worker
 marker.worker.serializeDataOnMainThread=Serialize data on the main thread
 marker.worker.deserializeDataOffMainThread=Deserialize data in Worker
 marker.worker.deserializeDataOnMainThread=Deserialize data on the main thread
+
 # The type of operation performed by a MessagePort
 marker.messagePort.serializeData=Serialize data
 marker.messagePort.deserializeData=Deserialize data
 
 # Strings used in the waterfall sidebar as values.
 marker.value.unknownFrame=<unknown location>
 marker.value.DOMEventTargetPhase=Target
 marker.value.DOMEventCapturingPhase=Capture
--- a/devtools/client/performance/modules/logic/moz.build
+++ b/devtools/client/performance/modules/logic/moz.build
@@ -1,14 +1,13 @@
 # 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-formatters.js',
     'marker-utils.js',
     'telemetry.js',
     'tree-model.js',
     'waterfall-utils.js',
 )
rename from devtools/client/performance/modules/logic/marker-formatters.js
rename to devtools/client/performance/modules/marker-formatters.js
--- a/devtools/client/performance/modules/logic/marker-formatters.js
+++ b/devtools/client/performance/modules/marker-formatters.js
@@ -3,19 +3,17 @@
  * 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.
  */
 const { Ci } = require("chrome");
-const Services = require("Services");
-const { L10N } = require("devtools/client/performance/modules/global");
-const PLATFORM_DATA_PREF = "devtools.performance.ui.show-platform-data";
+const { L10N, PREFS } = require("devtools/client/performance/modules/global");
 
 // String used to fill in platform data when it should be hidden.
 const GECKO_SYMBOL = "(Gecko)";
 
 /**
  * Mapping of JS marker causes to a friendlier form. Only
  * markers that are considered "from content" should be labeled here.
  */
@@ -33,128 +31,166 @@ const JS_MARKER_MAP = {
   "setInterval handler":       "setInterval",
   "setTimeout handler":        "setTimeout",
   "FrameRequestCallback":      "requestAnimationFrame",
 };
 
 /**
  * A series of formatters used by the blueprint.
  */
-const Formatters = {
+exports.Formatters = {
   /**
    * Uses the marker name as the label for markers that do not have
    * a blueprint entry. Uses "Other" in the marker filter menu.
    */
-  UnknownLabel: function (marker={}) {
+  UnknownLabel: function (marker = {}) {
     return marker.name || L10N.getStr("marker.label.unknown");
   },
 
+  /* Group 0 - Reflow and Rendering pipeline */
+
+  StylesFields: function (marker) {
+    if ("restyleHint" in marker) {
+      let label = marker.restyleHint.replace(/eRestyle_/g, "");
+      return {
+        [L10N.getStr("marker.field.restyleHint")]: label
+      };
+    }
+  },
+
+  /* Group 1 - JS */
+
+  DOMEventFields: function (marker) {
+    let fields = Object.create(null);
+
+    if ("type" in marker) {
+      fields[L10N.getStr("marker.field.DOMEventType")] = marker.type;
+    }
+
+    if ("eventPhase" in marker) {
+      let label;
+      switch (marker.eventPhase) {
+        case Ci.nsIDOMEvent.AT_TARGET:
+          label = L10N.getStr("marker.value.DOMEventTargetPhase");
+          break;
+        case Ci.nsIDOMEvent.CAPTURING_PHASE:
+          label = L10N.getStr("marker.value.DOMEventCapturingPhase");
+          break;
+        case Ci.nsIDOMEvent.BUBBLING_PHASE:
+          label = L10N.getStr("marker.value.DOMEventBubblingPhase");
+          break;
+      }
+      fields[L10N.getStr("marker.field.DOMEventPhase")] = label;
+    }
+
+    return fields;
+  },
+
+  JSLabel: function (marker = {}) {
+    let generic = L10N.getStr("marker.label.javascript");
+    if ("causeName" in marker) {
+      return JS_MARKER_MAP[marker.causeName] || generic;
+    }
+    return generic;
+  },
+
+  JSFields: function (marker) {
+    if ("causeName" in marker && !JS_MARKER_MAP[marker.causeName]) {
+      let label = PREFS["show-platform-data"] ? marker.causeName : GECKO_SYMBOL;
+      return {
+        [L10N.getStr("marker.field.causeName")]: label
+      };
+    }
+  },
+
   GCLabel: function (marker) {
     if (!marker) {
       return L10N.getStr("marker.label.garbageCollection2");
     }
     // Only if a `nonincrementalReason` exists, do we want to label
     // this as a non incremental GC event.
     if ("nonincrementalReason" in marker) {
       return L10N.getStr("marker.label.garbageCollection.nonIncremental");
-    }
-    return L10N.getStr("marker.label.garbageCollection.incremental");
-  },
-
-  JSLabel: function (marker={}) {
-    let generic = L10N.getStr("marker.label.javascript");
-    if ("causeName" in marker) {
-      return JS_MARKER_MAP[marker.causeName] || generic;
-    }
-    return generic;
-  },
-
-  DOMJSLabel: function (marker={}) {
-    return `Event (${marker.type})`;
-  },
-
-  /**
-   * Returns a hash for computing a fields object for a JS marker. If the cause
-   * is considered content (so an entry exists in the JS_MARKER_MAP), do not display it
-   * since it's redundant with the label. Otherwise for Gecko code, either display
-   * the cause, or "(Gecko)", depending on if "show-platform-data" is set.
-   */
-  JSFields: function (marker) {
-    if ("causeName" in marker && !JS_MARKER_MAP[marker.causeName]) {
-      let cause = Services.prefs.getBoolPref(PLATFORM_DATA_PREF) ? marker.causeName : GECKO_SYMBOL;
-      return {
-        [L10N.getStr("marker.field.causeName")]: cause
-      };
+    } else {
+      return L10N.getStr("marker.label.garbageCollection.incremental");
     }
   },
 
   GCFields: function (marker) {
     let fields = Object.create(null);
-    let cause = marker.causeName;
-    let label = L10N.getStr(`marker.gcreason.label.${cause}`) || cause;
 
-    fields[L10N.getStr("marker.field.causeName")] = label;
+    if ("causeName" in marker) {
+      let cause = marker.causeName;
+      let label = L10N.getStr(`marker.gcreason.label.${cause}`) || cause;
+      fields[L10N.getStr("marker.field.causeName")] = label;
+    }
 
     if ("nonincrementalReason" in marker) {
-      fields[L10N.getStr("marker.field.nonIncrementalCause")] = marker.nonincrementalReason;
+      let label = marker.nonincrementalReason;
+      fields[L10N.getStr("marker.field.nonIncrementalCause")] = label;
     }
 
     return fields;
   },
 
   MinorGCFields: function (marker) {
-    const cause = marker.causeName;
-    const label = L10N.getStr(`marker.gcreason.label.${cause}`) || cause;
+    let fields = Object.create(null);
+
+    if ("causeName" in marker) {
+      let cause = marker.causeName;
+      let label = L10N.getStr(`marker.gcreason.label.${cause}`) || cause;
+      fields[L10N.getStr("marker.field.causeName")] = label;
+    }
+
+    fields[L10N.getStr("marker.field.type")] = L10N.getStr("marker.nurseryCollection");
+
+    return fields;
+  },
+
+  CycleCollectionFields: function (marker) {
+    let label = marker.name.replace(/nsCycleCollector::/g, "");
     return {
-      [L10N.getStr("marker.field.type")]: L10N.getStr("marker.nurseryCollection"),
-      [L10N.getStr("marker.field.causeName")]: label,
+      [L10N.getStr("marker.field.type")]: label
     };
   },
 
-  DOMEventFields: function (marker) {
-    let fields = Object.create(null);
-    if ("type" in marker) {
-      fields[L10N.getStr("marker.field.DOMEventType")] = marker.type;
-    }
-    if ("eventPhase" in marker) {
-      let phase;
-      if (marker.eventPhase === Ci.nsIDOMEvent.AT_TARGET) {
-        phase = L10N.getStr("marker.value.DOMEventTargetPhase");
-      } else if (marker.eventPhase === Ci.nsIDOMEvent.CAPTURING_PHASE) {
-        phase = L10N.getStr("marker.value.DOMEventCapturingPhase");
-      } else if (marker.eventPhase === Ci.nsIDOMEvent.BUBBLING_PHASE) {
-        phase = L10N.getStr("marker.value.DOMEventBubblingPhase");
-      }
-      fields[L10N.getStr("marker.field.DOMEventPhase")] = phase;
-    }
-    return fields;
-  },
-
-  StylesFields: function (marker) {
-    if ("restyleHint" in marker) {
+  WorkerFields: function (marker) {
+    if ("workerOperation" in marker) {
+      let label = L10N.getStr(`marker.worker.${marker.workerOperation}`);
       return {
-        [L10N.getStr("marker.field.restyleHint")]: marker.restyleHint.replace(/eRestyle_/g, "")
+        [L10N.getStr("marker.field.type")]: label
       };
     }
   },
 
-  CycleCollectionFields: function (marker) {
-    return {
-      [L10N.getStr("marker.field.type")]: marker.name.replace(/nsCycleCollector::/g, "")
-    };
+  MessagePortFields: function (marker) {
+    if ("messagePortOperation" in marker) {
+      let label = L10N.getStr(`marker.messagePort.${marker.messagePortOperation}`);
+      return {
+        [L10N.getStr("marker.field.type")]: label
+      };
+    }
   },
 
-  WorkerFields: function(marker) {
-    return {
-      [L10N.getStr("marker.field.type")]:
-        L10N.getStr(`marker.worker.${marker.workerOperation}`)
-    };
-  },
+  /* Group 2 - User Controlled */
 
-  MessagePortFields: function(marker) {
-    return {
-      [L10N.getStr("marker.field.type")]:
-        L10N.getStr(`marker.messagePort.${marker.messagePortOperation}`)
-    };
-  }
+  ConsoleTimeFields: [{
+    label: L10N.getStr("marker.field.consoleTimerName"),
+    property: "causeName",
+  }],
+
+  TimeStampFields: [{
+    label: L10N.getStr("marker.field.label"),
+    property: "causeName",
+  }]
 };
 
-exports.Formatters = Formatters;
+/**
+ * Takes a main label (e.g. "Timestamp") and a property name (e.g. "causeName"),
+ * and returns a string that represents that property value for a marker if it
+ * exists (e.g. "Timestamp (rendering)"), or just the main label if it does not.
+ *
+ * @param string mainLabel
+ * @param string propName
+ */
+exports.Formatters.labelForProperty = function (mainLabel, propName) {
+  return (marker={}) => marker[propName] ? `${mainLabel} (${marker[propName]})` : mainLabel;
+};
--- a/devtools/client/performance/modules/markers.js
+++ b/devtools/client/performance/modules/markers.js
@@ -1,67 +1,65 @@
 /* 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 { L10N } = require("devtools/client/performance/modules/global");
-const { Formatters } = require("devtools/client/performance/modules/logic/marker-formatters");
+const { Formatters, labelForProperty } = require("devtools/client/performance/modules/marker-formatters");
 
 /**
  * A simple schema for mapping markers to the timeline UI. The keys correspond
  * to marker names, while the values are objects with the following format:
  *
  * - group: The row index in the overview graph; multiple markers
  *          can be added on the same row. @see <overview.js/buildGraphImage>
  * - label: The label used in the waterfall to identify the marker. Can be a
  *          string or just a function that accepts the marker and returns a
- *          string, if you want to use a dynamic property for the main label.
+ *          string (if you want to use a dynamic property for the main label).
  *          If you use a function for a label, it *must* handle the case where
- *          no marker is provided for a main label to describe all markers of
- *          this type.
+ *          no marker is provided, to get a generic label used to describe
+ *          all markers of this type.
  * - colorName: The label of the DevTools color used for this marker. If
  *              adding a new color, be sure to check that there's an entry
- *              for `.marker-details-bullet.{COLORNAME}` for the equivilent
- *              entry in ./devtools/client/themes/performance.inc.css
+ *              for `.marker-color-graphs-{COLORNAME}` for the equivilent
+ *              entry in "./devtools/client/themes/performance.css"
  *              https://developer.mozilla.org/en-US/docs/Tools/DevToolsColors
  * - collapsible: Whether or not this marker can contain other markers it
- *                eclipses, and becomes collapsible to reveal its nestable children.
- *                Defaults to true.
+ *                eclipses, and becomes collapsible to reveal its nestable
+ *                children. Defaults to true.
  * - nestable: Whether or not this marker can be nested inside an eclipsing
  *             collapsible marker. Defaults to true.
- * - fields: An optional array of marker properties you wish to display in the
- *           marker details view. For example, a field in the array such as
- *           { property: "aCauseName", label: "Cause" } would render a string
- *           like `Cause: ${marker.aCauseName}` in the marker details view.
- *           Each `field` item may take the following properties:
+ * - fields: An optional array of fields you wish to display in the marker
+ *           details view. For example, a field in the array such as
+ *           { label: "Cause", property: "causeName" } would render a string
+ *           like `Cause: ${marker.causeName}` in the marker details view.
+ *           Each field may take the following properties:
+ *           - label: The name of the property that should be displayed.
  *           - property: The property that must exist on the marker to render,
  *                       and the value of the property will be displayed.
- *           - label: The name of the property that should be displayed.
- *           - formatter: If a formatter is provided, instead of directly using
- *                        the `property` property on the marker, the marker is
- *                        passed into the formatter function to determine the
- *                        displayed value.
- *            Can also be a function that returns an object. Each key in the object
- *            will be rendered as a field, with its value rendering as the value.
+ *           Alternatively, this also be a function that returns an object.
+ *           Each key in that object will be rendered as one field's label,
+ *           with its value rendering as one field's value.
  *
  * Whenever this is changed, browser_timeline_waterfall-styles.js *must* be
  * updated as well.
  */
 const TIMELINE_BLUEPRINT = {
-  /* Default definition used for markers that occur but
-   * are not defined here. Should ultimately be defined, but this gives
-   * us room to work on the front end separately from the platform. */
+  /* Default definition used for markers that occur but are not defined here.
+   * Should ultimately be defined, but this gives us room to work on the
+   * front end separately from the platform. */
   "UNKNOWN": {
     group: 2,
     colorName: "graphs-grey",
     label: Formatters.UnknownLabel,
   },
 
   /* Group 0 - Reflow and Rendering pipeline */
+
   "Styles": {
     group: 0,
     colorName: "graphs-purple",
     label: L10N.getStr("marker.label.styles"),
     fields: Formatters.StylesFields,
   },
   "Reflow": {
     group: 0,
@@ -80,16 +78,17 @@ const TIMELINE_BLUEPRINT = {
   },
   "CompositeForwardTransaction": {
     group: 0,
     colorName: "graphs-bluegrey",
     label: L10N.getStr("marker.label.compositeForwardTransaction"),
   },
 
   /* Group 1 - JS */
+
   "DOMEvent": {
     group: 1,
     colorName: "graphs-yellow",
     label: L10N.getStr("marker.label.domevent"),
     fields: Formatters.DOMEventFields,
   },
   "document::DOMContentLoaded": {
     group: 1,
@@ -150,43 +149,28 @@ const TIMELINE_BLUEPRINT = {
   "MessagePort": {
     group: 1,
     colorName: "graphs-orange",
     label: L10N.getStr("marker.label.messagePort"),
     fields: Formatters.MessagePortFields
   },
 
   /* Group 2 - User Controlled */
+
   "ConsoleTime": {
     group: 2,
     colorName: "graphs-blue",
-    label: sublabelForProperty(L10N.getStr("marker.label.consoleTime"), "causeName"),
-    fields: [{
-      property: "causeName",
-      label: L10N.getStr("marker.field.consoleTimerName")
-    }],
+    label: Formatters.labelForProperty(L10N.getStr("marker.label.consoleTime"), "causeName"),
+    fields: Formatters.ConsoleTimeFields,
     nestable: false,
     collapsible: false,
   },
   "TimeStamp": {
     group: 2,
     colorName: "graphs-blue",
-    label: sublabelForProperty(L10N.getStr("marker.label.timestamp"), "causeName"),
-    fields: [{
-      property: "causeName",
-      label: "Label:"
-    }],
+    label: Formatters.labelForProperty(L10N.getStr("marker.label.timestamp"), "causeName"),
+    fields: Formatters.TimeStampFields,
     collapsible: false,
   },
 };
 
-/**
- * Takes a main label (like "Timestamp") and a property,
- * and returns a marker that will print out the property
- * value for a marker if it exists ("Timestamp (rendering)"),
- * or just the main label if it does not.
- */
-function sublabelForProperty (mainLabel, prop) {
-  return (marker={}) => marker[prop] ? `${mainLabel} (${marker[prop]})` : mainLabel;
-}
-
 // Exported symbols.
 exports.TIMELINE_BLUEPRINT = TIMELINE_BLUEPRINT;
--- a/devtools/client/performance/modules/moz.build
+++ b/devtools/client/performance/modules/moz.build
@@ -8,10 +8,11 @@ DIRS += [
     'widgets',
 ]
 
 DevToolsModules(
     'categories.js',
     'constants.js',
     'global.js',
     'io.js',
+    'marker-formatters.js',
     'markers.js',
 )