Bug 1258661 - Make the `fields` property in the markers blueprint consistent, r=jsantell
authorVictor Porof <vporof@mozilla.com>
Mon, 28 Mar 2016 13:50:02 +0200
changeset 290532 a0f366bd78a48278208304775a1166396a3daad4
parent 290531 46fb0301d5ca4ca1f76087e7a182ff05811c4c88
child 290533 6a10bbd8a7124f235e3084920b9df6deaeb87ce5
child 290705 24b5fb250c49007ee5a239b57e9cecd790cd1ebf
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
bugs1258661
milestone48.0a1
Bug 1258661 - Make the `fields` property in the markers blueprint consistent, r=jsantell
devtools/client/performance/modules/marker-blueprint-utils.js
devtools/client/performance/modules/marker-formatters.js
devtools/client/performance/modules/markers.js
--- a/devtools/client/performance/modules/marker-blueprint-utils.js
+++ b/devtools/client/performance/modules/marker-blueprint-utils.js
@@ -47,30 +47,32 @@ exports.MarkerBlueprintUtils = {
   /**
    * 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;
+    let dynamic = typeof blueprint.label === "function";
+    let label = dynamic ? 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;
+    let dynamic = typeof blueprint.label === "function";
+    let generic = dynamic ? 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 {
@@ -86,27 +88,16 @@ exports.MarkerBlueprintUtils = {
    * 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] }));
-    }
+    let dynamic = typeof blueprint.fields === "function";
+    let fields = dynamic ? blueprint.fields(marker) : blueprint.fields;
 
-    // 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;
-    }, []);
+    return Object.entries(fields || {})
+      .filter(([_, value]) => dynamic ? true : value in marker)
+      .map(([label, value]) => ({ label, value: dynamic ? value : marker[value] }));
   },
 };
--- a/devtools/client/performance/modules/marker-formatters.js
+++ b/devtools/client/performance/modules/marker-formatters.js
@@ -167,25 +167,23 @@ exports.Formatters = {
       return {
         [L10N.getStr("marker.field.type")]: label
       };
     }
   },
 
   /* Group 2 - User Controlled */
 
-  ConsoleTimeFields: [{
-    label: L10N.getStr("marker.field.consoleTimerName"),
-    property: "causeName",
-  }],
+  ConsoleTimeFields: {
+    [L10N.getStr("marker.field.consoleTimerName")]: "causeName"
+  },
 
-  TimeStampFields: [{
-    label: L10N.getStr("marker.field.label"),
-    property: "causeName",
-  }]
+  TimeStampFields: {
+    [L10N.getStr("marker.field.label")]: "causeName"
+  }
 };
 
 /**
  * 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
--- a/devtools/client/performance/modules/markers.js
+++ b/devtools/client/performance/modules/markers.js
@@ -13,40 +13,32 @@ const { Formatters, labelForProperty } =
  * - 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).
  *          If you use a function for a label, it *must* handle the case where
  *          no marker is provided, to get a generic label used to describe
  *          all markers of this type.
+ * - fields: The fields used in the marker details view to display more
+ *           information about a currently selected marker. Can either be an
+ *           object of fields, or simply a function that accepts the marker and
+ *           returns such an object (if you want to use properties dynamically).
+ *           For example, a field in the object such as { "Cause": "causeName" }
+ *           would render something like `Cause: ${marker.causeName}` in the UI.
  * - 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-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.
  * - nestable: Whether or not this marker can be nested inside an eclipsing
  *             collapsible marker. Defaults to true.
- * - 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.
- *           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. */
   "UNKNOWN": {
     group: 2,
     colorName: "graphs-grey",