Bug 1123825 - ThreadNode constructor params are a mess; use an options param instead, r=jsantell

browser/devtools/performance/views/details-call-tree.js

@@ -85,12 +85,17 @@ let CallTreeView = {
    * Called when the recording is stopped and prepares data to
    * populate the call tree.
    */
-  _prepareCallTree: function (profilerData, beginAt, endAt, options) {
+  _prepareCallTree: function (profilerData, startTime, endTime, options) {
     let threadSamples = profilerData.profile.threads[0].samples;
     let contentOnly = !Prefs.showPlatformData;
     let invertTree = PerformanceController.getPref("invert-call-tree");
 
-    let threadNode = new ThreadNode(threadSamples, contentOnly, beginAt, endAt, invertTree);
+    let threadNode = new ThreadNode(threadSamples,
+      { startTime, endTime, contentOnly, invertTree });
+
+    // If we have an empty profile (no samples), then don't invert the tree, as
+    // it would hide the root node and a completely blank call tree space can be
+    // mis-interpreted as an error.
     options.inverted = invertTree && threadNode.samples > 0;
 
     return threadNode;

browser/devtools/profiler/test/browser_profiler_tree-model-03.js

@@ -11,7 +11,7 @@ function test() {
 
   // Create a root node from a given samples array, filtering by time.
 
-  let root = new ThreadNode(gSamples, false, 11, 18);
+  let root = new ThreadNode(gSamples, { startTime: 11, endTime: 18 });
 
   // Test the root node.
 

browser/devtools/profiler/test/browser_profiler_tree-model-04.js

@@ -11,7 +11,7 @@ function test() {
 
   // Create a root node from a given samples array, filtering by time.
 
-  let root = new ThreadNode(gSamples, true, 11, 18);
+  let root = new ThreadNode(gSamples, { startTime: 11, endTime: 18, contentOnly: true });
 
   // Test the root node.
 

browser/devtools/profiler/test/browser_profiler_tree-model-05.js

@@ -45,7 +45,7 @@ let samples = [{
 function test() {
   let { ThreadNode } = devtools.require("devtools/profiler/tree-model");
 
-  let root = new ThreadNode(samples, undefined, undefined, undefined, true);
+  let root = new ThreadNode(samples, { invertTree: true });
 
   is(Object.keys(root.calls).length, 2,
      "Should get the 2 youngest frames, not the 1 oldest frame");

browser/devtools/profiler/ui-profile.js

@@ -452,20 +452,21 @@ let ProfileView = {
    *        The <panel> element in this <tabbox>.
    * @param object profilerData
    *        The data source for this tree.
-   * @param number beginAt
+   * @param number startTime
    *        The earliest time in the data source to start at (in milliseconds).
-   * @param number endAt
+   * @param number endTime
    *        The latest time in the data source to end at (in milliseconds).
    * @param object options
    *        Additional options supported by this operation.
    *        @see ProfileView._populatePanelWidgets
    */
-  _populateCallTree: function(panel, profilerData, beginAt, endAt, options = {}) {
+  _populateCallTree: function(panel, profilerData, startTime, endTime, options = {}) {
     let threadSamples = profilerData.profile.threads[0].samples;
     let contentOnly = !Prefs.showPlatformData;
     let invertChecked = this._invertTree.hasAttribute("checked");
-    let threadNode = new ThreadNode(threadSamples, contentOnly, beginAt, endAt,
-                                    invertChecked);
+    let threadNode = new ThreadNode(threadSamples,
+      { startTime, endTime, contentOnly, invertChecked });
+
     // If we have an empty profile (no samples), then don't invert the tree, as
     // it would hide the root node and a completely blank call tree space can be
     // mis-interpreted as an error.

browser/devtools/profiler/utils/tree-model.js

@@ -44,23 +44,21 @@ exports.FrameNode.isContent = isContent;
  *
  * @param object threadSamples
  *        The raw samples array received from the backend.
- * @param boolean contentOnly [optional]
- *        @see ThreadNode.prototype.insert
- * @param number beginAt [optional]
- *        @see ThreadNode.prototype.insert
- * @param number endAt [optional]
- *        @see ThreadNode.prototype.insert
- * @param boolean invert [optional]
- *        @see ThreadNode.prototype.insert
+ * @param object options
+ *        Additional supported options, @see ThreadNode.prototype.insert
+ *          - number startTime [optional]
+ *          - number endTime [optional]
+ *          - boolean contentOnly [optional]
+ *          - boolean invertTree [optional]
  */
-function ThreadNode(threadSamples, contentOnly, beginAt, endAt, invert) {
+function ThreadNode(threadSamples, options = {}) {
   this.samples = 0;
   this.duration = 0;
   this.calls = {};
   this._previousSampleTime = 0;
 
   for (let sample of threadSamples) {
-    this.insert(sample, contentOnly, beginAt, endAt, invert);
+    this.insert(sample, options);
   }
 }
 
@@ -72,19 +70,18 @@ ThreadNode.prototype = {
    *        The { frames, time } sample, containing an array of frames and
    *        the time the sample was taken. This sample is assumed to be older
    *        than the most recently inserted one.
-   * @param boolean contentOnly [optional]
-   *        Specifies if platform frames shouldn't be taken into consideration.
-   * @param number beginAt [optional]
-   *        The earliest sample to start at (in milliseconds).
-   * @param number endAt [optional]
-   *        The latest sample to end at (in milliseconds).
-   * @param boolean inverted [optional]
-   *        Specifies if the call tree should be inverted (youngest -> oldest
-   *        frames).
+   * @param object options [optional]
+   *        Additional supported options:
+   *          - number startTime: the earliest sample to start at (in milliseconds)
+   *          - number endTime: the latest sample to end at (in milliseconds)
+   *          - boolean contentOnly: if platform frames shouldn't be used
+   *          - boolean invertTree: if the call tree should be inverted
    */
-  insert: function(sample, contentOnly = false, beginAt = 0, endAt = Infinity, inverted = false) {
+  insert: function(sample, options = {}) {
+    let startTime = options.startTime || 0;
+    let endTime = options.endTime || Infinity;
     let sampleTime = sample.time;
-    if (!sampleTime || sampleTime < beginAt || sampleTime > endAt) {
+    if (!sampleTime || sampleTime < startTime || sampleTime > endTime) {
       return;
     }
 
@@ -92,17 +89,20 @@ ThreadNode.prototype = {
 
     // Filter out platform frames if only content-related function calls
     // should be taken into consideration.
-    if (contentOnly) {
+    if (options.contentOnly) {
       // The (root) node is not considered a content function, it'll be removed.
       sampleFrames = sampleFrames.filter(isContent);
     } else {
       // Remove the (root) node manually.
       sampleFrames = sampleFrames.slice(1);
     }
+    // If no frames remain after filtering, then this is a leaf node, no need
+    // to continue.
     if (!sampleFrames.length) {
       return;
     }
-    if (inverted) {
+    // Invert the tree after filtering, if preferred.
+    if (options.invertTree) {
       sampleFrames.reverse();
     }