Bug 1162662 - Map JS markers to human readable keys, and hide if platform related via (Gecko). r=vp, a=sledru
authorJordan Santell <jsantell@mozilla.com>
Tue, 19 May 2015 23:02:28 -0700
changeset 274864 efff9db98e86851faa7a480d6107293294a77ada
parent 274863 701cee0f9ec1641c3c78c924e1132057485e7144
child 274865 7bc05d558a2de9d4ec171111d335623ac24a2c61
push id863
push userraliiev@mozilla.com
push dateMon, 03 Aug 2015 13:22:43 +0000
treeherdermozilla-release@f6321b14228d [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersvp, sledru
bugs1162662
milestone40.0a2
Bug 1162662 - Map JS markers to human readable keys, and hide if platform related via (Gecko). r=vp, a=sledru
browser/devtools/performance/test/browser_marker-utils.js
browser/devtools/shared/timeline/global.js
browser/devtools/shared/timeline/marker-utils.js
--- a/browser/devtools/performance/test/browser_marker-utils.js
+++ b/browser/devtools/performance/test/browser_marker-utils.js
@@ -4,39 +4,47 @@
 /**
  * Tests the marker utils methods.
  */
 
 function spawnTest () {
   let { TIMELINE_BLUEPRINT } = devtools.require("devtools/shared/timeline/global");
   let Utils = devtools.require("devtools/shared/timeline/marker-utils");
 
+  Services.prefs.setBoolPref(PLATFORM_DATA_PREF, false);
+
   is(Utils.getMarkerLabel({ name: "DOMEvent" }), "DOM Event",
     "getMarkerLabel() returns a simple label");
-  is(Utils.getMarkerLabel({ name: "Javascript", causeName: "js" }), "js",
+  is(Utils.getMarkerLabel({ name: "Javascript", causeName: "setTimeout handler" }), "setTimeout",
     "getMarkerLabel() returns a label defined via function");
 
   ok(Utils.getMarkerFields({ name: "Paint" }).length === 0,
     "getMarkerFields() returns an empty array when no fields defined");
 
   let fields = Utils.getMarkerFields({ name: "ConsoleTime", causeName: "snowstorm" });
   is(fields[0].label, "Timer Name:", "getMarkerFields() returns an array with proper label");
   is(fields[0].value, "snowstorm", "getMarkerFields() returns an array with proper value");
 
   fields = Utils.getMarkerFields({ name: "DOMEvent", type: "mouseclick" });
   is(fields.length, 1, "getMarkerFields() ignores fields that are not found on marker");
   is(fields[0].label, "Event Type:", "getMarkerFields() returns an array with proper label");
   is(fields[0].value, "mouseclick", "getMarkerFields() returns an array with proper value");
 
   fields = Utils.getMarkerFields({ name: "DOMEvent", eventPhase: Ci.nsIDOMEvent.AT_TARGET, type: "mouseclick" });
-  is(fields.length, 2, "getMarkerFields() returns multiple fields when they exist");
-  is(fields[0].label, "Event Type:", "getMarkerFields() returns an array with proper label (ordered)");
-  is(fields[0].value, "mouseclick", "getMarkerFields() returns an array with proper value (ordered)");
-  is(fields[1].label, "Phase:", "getMarkerFields() returns an array with proper label (ordered)");
-  is(fields[1].value, "Target", "getMarkerFields() uses the `formatter` function when available");
+  is(fields.length, 2, "getMarkerFields() returns multiple fields when using a fields function");
+  is(fields[0].label, "Event Type:", "getMarkerFields() correctly returns fields via function (1)");
+  is(fields[0].value, "mouseclick", "getMarkerFields() correctly returns fields via function (2)");
+  is(fields[1].label, "Phase:", "getMarkerFields() correctly returns fields via function (3)");
+  is(fields[1].value, "Target", "getMarkerFields() correctly returns fields via function (4)");
+
+  is(Utils.getMarkerFields({ name: "Javascript", causeName: "Some Platform Field" })[0].value, "(Gecko)",
+    "Correctly obfuscates JS markers when platform data is off.");
+  Services.prefs.setBoolPref(PLATFORM_DATA_PREF, true);
+  is(Utils.getMarkerFields({ name: "Javascript", causeName: "Some Platform Field" })[0].value, "Some Platform Field",
+    "Correctly deobfuscates JS markers when platform data is on.");
 
   is(Utils.getMarkerClassName("Javascript"), "Function Call",
     "getMarkerClassName() returns correct string when defined via function");
   is(Utils.getMarkerClassName("GarbageCollection"), "GC Event",
     "getMarkerClassName() returns correct string when defined via function");
   is(Utils.getMarkerClassName("Reflow"), "Layout",
     "getMarkerClassName() returns correct string when defined via string");
 
--- a/browser/devtools/shared/timeline/global.js
+++ b/browser/devtools/shared/timeline/global.js
@@ -1,24 +1,43 @@
 /* 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 {Cc, Ci, Cu, Cr} = require("chrome");
+loader.lazyRequireGetter(this, "ViewHelpers",
+  "resource:///modules/devtools/ViewHelpers.jsm", true);
+loader.lazyRequireGetter(this, "Services");
 
-Cu.import("resource:///modules/devtools/ViewHelpers.jsm");
+// String used to fill in platform data when it should be hidden.
+const GECKO_SYMBOL = "(Gecko)";
 
 /**
  * Localization convenience methods.
  */
 const STRINGS_URI = "chrome://browser/locale/devtools/timeline.properties";
 const L10N = new ViewHelpers.L10N(STRINGS_URI);
 
 /**
+ * Monitor "show-platform-data" pref.
+ */
+const prefs = new ViewHelpers.Prefs("devtools.performance.ui", {
+  showPlatformData: ["Bool", "show-platform-data"]
+});
+
+let SHOW_PLATFORM_DATA = Services.prefs.getBoolPref("devtools.performance.ui.show-platform-data");
+prefs.registerObserver();
+prefs.on("pref-changed", (_,  prefName, prefValue) => {
+  if (prefName === "showPlatformData") {
+    SHOW_PLATFORM_DATA = prefValue;
+  }
+});
+
+/**
  * 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 timeline 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.
@@ -33,19 +52,19 @@ const L10N = new ViewHelpers.L10N(STRING
  *             marker details view. For example, a field of
  *             { property: "aCauseName", label: "Cause" }
  *             would render a field like `Cause: ${marker.aCauseName}`.
  *             Each `field` item may take the following properties:
  *
  *             - 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 display 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.
  *
  * Whenever this is changed, browser_timeline_waterfall-styles.js *must* be
  * updated as well.
  */
 const TIMELINE_BLUEPRINT = {
   /* Group 0 - Reflow and Rendering pipeline */
   "Styles": {
     group: 0,
@@ -64,29 +83,23 @@ const TIMELINE_BLUEPRINT = {
     label: L10N.getStr("timeline.label.paint")
   },
 
   /* Group 1 - JS */
   "DOMEvent": {
     group: 1,
     colorName: "graphs-yellow",
     label: L10N.getStr("timeline.label.domevent"),
-    fields: [{
-      property: "type",
-      label: L10N.getStr("timeline.markerDetail.DOMEventType")
-    }, {
-      property: "eventPhase",
-      label: L10N.getStr("timeline.markerDetail.DOMEventPhase"),
-      formatter: getEventPhaseName
-    }]
+    fields: getDOMEventFields,
   },
   "Javascript": {
     group: 1,
     colorName: "graphs-yellow",
     label: getJSLabel,
+    fields: getJSFields,
   },
   "Parse HTML": {
     group: 1,
     colorName: "graphs-yellow",
     label: L10N.getStr("timeline.label.parseHTML")
   },
   "Parse XML": {
     group: 1,
@@ -123,41 +136,82 @@ const TIMELINE_BLUEPRINT = {
     }]
   },
 };
 
 /**
  * A series of formatters used by the blueprint.
  */
 
-function getEventPhaseName (marker) {
-  if (marker.eventPhase === Ci.nsIDOMEvent.AT_TARGET) {
-    return L10N.getStr("timeline.markerDetail.DOMEventTargetPhase");
-  } else if (marker.eventPhase === Ci.nsIDOMEvent.CAPTURING_PHASE) {
-    return L10N.getStr("timeline.markerDetail.DOMEventCapturingPhase");
-  } else if (marker.eventPhase === Ci.nsIDOMEvent.BUBBLING_PHASE) {
-    return L10N.getStr("timeline.markerDetail.DOMEventBubblingPhase");
-  }
-}
-
 function getGCLabel (marker={}) {
   let label = L10N.getStr("timeline.label.garbageCollection");
   // Only if a `nonincrementalReason` exists, do we want to label
   // this as a non incremental GC event.
   if ("nonincrementalReason" in marker) {
     label = `${label} (Non-incremental)`;
   }
   return label;
 }
 
+/**
+ * Mapping of JS marker causes to a friendlier form. Only
+ * markers that are considered "from content" should be labeled here.
+ */
+const JS_MARKER_MAP = {
+  "<script> element":          "Script Tag",
+  "setInterval handler":       "setInterval",
+  "setTimeout handler":        "setTimeout",
+  "FrameRequestCallback":      "requestAnimationFrame",
+  "promise callback":          "Promise Callback",
+  "promise initializer":       "Promise Init",
+  "Worker runnable":           "Worker",
+  "javascript: URI":           "JavaScript URI",
+  // As far as I know, the difference between these two
+  // event handler markers are differences in their webidl implementation.
+  "EventHandlerNonNull":       "Event Handler",
+  "EventListener.handleEvent": "Event Handler",
+};
+
 function getJSLabel (marker={}) {
+  let generic = L10N.getStr("timeline.label.javascript2");
   if ("causeName" in marker) {
-    return marker.causeName;
+    return JS_MARKER_MAP[marker.causeName] || generic;
+  }
+  return generic;
+}
+
+/**
+ * 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.
+ */
+function getJSFields (marker) {
+  if ("causeName" in marker && !JS_MARKER_MAP[marker.causeName]) {
+    return { Reason: (SHOW_PLATFORM_DATA ? marker.causeName : GECKO_SYMBOL) };
   }
-  return L10N.getStr("timeline.label.javascript2");
+}
+
+function getDOMEventFields (marker) {
+  let fields = Object.create(null);
+  if ("type" in marker) {
+    fields[L10N.getStr("timeline.markerDetail.DOMEventType")] = marker.type;
+  }
+  if ("eventPhase" in marker) {
+    let phase;
+    if (marker.eventPhase === Ci.nsIDOMEvent.AT_TARGET) {
+      phase = L10N.getStr("timeline.markerDetail.DOMEventTargetPhase");
+    } else if (marker.eventPhase === Ci.nsIDOMEvent.CAPTURING_PHASE) {
+      phase = L10N.getStr("timeline.markerDetail.DOMEventCapturingPhase");
+    } else if (marker.eventPhase === Ci.nsIDOMEvent.BUBBLING_PHASE) {
+      phase = L10N.getStr("timeline.markerDetail.DOMEventBubblingPhase");
+    }
+    fields[L10N.getStr("timeline.markerDetail.DOMEventPhase")] = phase;
+  }
+  return fields;
 }
 
 function getStylesFields (marker) {
   if ("restyleHint" in marker) {
     return { "Restyle Hint": marker.restyleHint.replace(/eRestyle_/g, "") };
   }
 }
 
--- a/browser/devtools/shared/timeline/marker-utils.js
+++ b/browser/devtools/shared/timeline/marker-utils.js
@@ -64,35 +64,34 @@ exports.getMarkerClassName = getMarkerCl
  * in the marker details view.
  *
  * @param {ProfileTimelineMarker} marker
  * @return {Array<object>}
  */
 function getMarkerFields (marker) {
   let blueprint = TIMELINE_BLUEPRINT[marker.name];
 
+  // If blueprint.fields is a function, use that
   if (typeof blueprint.fields === "function") {
     let fields = blueprint.fields(marker);
     // Add a ":" to the label since the localization files contain the ":"
     // if not present. This should be changed, ugh.
     return Object.keys(fields || []).map(label => {
+      // TODO revisit localization strings for markers bug 1163763
       let normalizedLabel = label.indexOf(":") !== -1 ? label : (label + ":");
       return { label: normalizedLabel, 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];
-      // If a formatter function defined, use it to get the
-      // value we actually want to display.
-      if (typeof field.formatter === "function") {
-        value = field.formatter(marker);
-      }
       fields.push({ label, value });
     }
     return fields;
   }, []);
 }
 exports.getMarkerFields = getMarkerFields;
 
 /**