From dea1b85c7eabab88c174cb67dab1749c130ef574 Mon Sep 17 00:00:00 2001 From: Victor Porof Date: Tue, 22 Mar 2016 10:17:21 +0100 Subject: [PATCH] Bug 1258416 - Cleanup marker.js and marker-formatters.js by adding missing l10n strings, removing dead code and updating comments, r=jsantell --HG-- rename : devtools/client/performance/modules/logic/marker-formatters.js => devtools/client/performance/modules/marker-formatters.js --- .../client/locales/en-US/markers.properties | 49 +++-- .../performance/modules/logic/moz.build | 1 - .../modules/{logic => }/marker-formatters.js | 194 +++++++++++------- .../client/performance/modules/markers.js | 70 +++---- devtools/client/performance/modules/moz.build | 1 + 5 files changed, 170 insertions(+), 145 deletions(-) rename devtools/client/performance/modules/{logic => }/marker-formatters.js (52%) diff --git a/devtools/client/locales/en-US/markers.properties b/devtools/client/locales/en-US/markers.properties index bdf354a233a9..196febfa77f0 100644 --- a/devtools/client/locales/en-US/markers.properties +++ b/devtools/client/locales/en-US/markers.properties @@ -45,12 +45,6 @@ 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. @@ -58,31 +52,42 @@ marker.fieldFormat=%1$S (%2$S) marker.field.start=Start: marker.field.end=End: marker.field.duration=Duration: -# 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: +# 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: + # 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 diff --git a/devtools/client/performance/modules/logic/moz.build b/devtools/client/performance/modules/logic/moz.build index 3d97de2657c0..ee3535acaf7f 100644 --- a/devtools/client/performance/modules/logic/moz.build +++ b/devtools/client/performance/modules/logic/moz.build @@ -6,7 +6,6 @@ DevToolsModules( 'frame-utils.js', 'jit.js', - 'marker-formatters.js', 'marker-utils.js', 'telemetry.js', 'tree-model.js', diff --git a/devtools/client/performance/modules/logic/marker-formatters.js b/devtools/client/performance/modules/marker-formatters.js similarity index 52% rename from devtools/client/performance/modules/logic/marker-formatters.js rename to devtools/client/performance/modules/marker-formatters.js index 15e7c09beffe..87975e230d71 100644 --- a/devtools/client/performance/modules/logic/marker-formatters.js +++ b/devtools/client/performance/modules/marker-formatters.js @@ -8,9 +8,7 @@ * 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)"; @@ -38,15 +36,71 @@ const JS_MARKER_MAP = { /** * 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"); @@ -55,106 +109,88 @@ const Formatters = { // 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; - return { - [L10N.getStr("marker.field.type")]: L10N.getStr("marker.nurseryCollection"), - [L10N.getStr("marker.field.causeName")]: 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; + + 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; }, - StylesFields: function (marker) { - if ("restyleHint" in marker) { + CycleCollectionFields: function (marker) { + let label = marker.name.replace(/nsCycleCollector::/g, ""); + return { + [L10N.getStr("marker.field.type")]: label + }; + }, + + 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; +}; diff --git a/devtools/client/performance/modules/markers.js b/devtools/client/performance/modules/markers.js index 4757801549c9..c7ccd06d1466 100644 --- a/devtools/client/performance/modules/markers.js +++ b/devtools/client/performance/modules/markers.js @@ -4,7 +4,7 @@ "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 @@ -14,42 +14,39 @@ const { Formatters } = require("devtools/client/performance/modules/logic/marker * can be added on the same row. @see * - 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", @@ -57,6 +54,7 @@ const TIMELINE_BLUEPRINT = { }, /* Group 0 - Reflow and Rendering pipeline */ + "Styles": { group: 0, colorName: "graphs-purple", @@ -85,6 +83,7 @@ const TIMELINE_BLUEPRINT = { }, /* Group 1 - JS */ + "DOMEvent": { group: 1, colorName: "graphs-yellow", @@ -155,38 +154,23 @@ const TIMELINE_BLUEPRINT = { }, /* 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; diff --git a/devtools/client/performance/modules/moz.build b/devtools/client/performance/modules/moz.build index b92be87693e9..bf3add2cd514 100644 --- a/devtools/client/performance/modules/moz.build +++ b/devtools/client/performance/modules/moz.build @@ -13,5 +13,6 @@ DevToolsModules( 'constants.js', 'global.js', 'io.js', + 'marker-formatters.js', 'markers.js', )