Bug 1016526 - The framerate actor returns incorrect data, r=benwa

toolkit/devtools/server/actors/framerate.js

@@ -52,44 +52,20 @@ let FramerateActor = exports.FramerateActor = protocol.ActorClass({
   }),
 
   /**
-   * Stops monitoring framerate, returning the recorded values at an
-   * interval defined by the specified resolution, in milliseconds.
+   * Stops monitoring framerate, returning the recorded values.
    */
-  stopRecording: method(function(resolution = 100) {
+  stopRecording: method(function() {
     if (!this._recording) {
-      return {};
+      return [];
     }
     this._recording = false;
 
-    let timeline = {};
-    let ticks = this._ticks;
-    let totalTicks = ticks.length;
-
     // We don't need to store the ticks array for future use, release it.
+    let ticks = this._ticks;
     this._ticks = null;
-
-    // If the refresh driver didn't get a chance to tick before the
-    // recording was stopped, assume framerate was 0.
-    if (totalTicks == 0) {
-      timeline[resolution] = 0;
-      return timeline;
-    }
-
-    let pivotTick = 0;
-    let lastTick = ticks[totalTicks - 1];
-
-    for (let bucketTime = resolution; bucketTime < lastTick; bucketTime += resolution) {
-      let frameCount = 0;
-      while (ticks[pivotTick++] < bucketTime) frameCount++;
-
-      let framerate = 1000 / (resolution / frameCount);
-      timeline[bucketTime] = framerate;
-    }
-
-    return timeline;
+    return ticks;
   }, {
-    request: { resolution: Arg(0, "nullable:number") },
-    response: { timeline: RetVal("json") }
+    response: { timeline: RetVal("array:number") }
   }),
 
   /**
@@ -115,5 +91,52 @@ let FramerateFront = exports.FramerateFront = protocol.FrontClass(FramerateActor
   initialize: function(client, { framerateActor }) {
     protocol.Front.prototype.initialize.call(this, client, { actor: framerateActor });
     this.manage(this);
+  },
+
+  /**
+   * Plots the frames per second on a timeline.
+   *
+   * @param array ticks
+   *        The raw data received from the framerate actor, which represents
+   *        the elapsed time on each refresh driver tick.
+   * @param number interval
+   *        The maximum amount of time to wait between calculations.
+   * @return array
+   *         A collection of { delta, value } objects representing the
+   *         framerate value at every delta time.
+   */
+  plotFPS: function(ticks, interval = 100) {
+    let timeline = [];
+    let totalTicks = ticks.length;
+
+    // If the refresh driver didn't get a chance to tick before the
+    // recording was stopped, assume framerate was 0.
+    if (totalTicks == 0) {
+      timeline.push({ delta: 0, value: 0 });
+      timeline.push({ delta: interval, value: 0 });
+      return timeline;
+    }
+
+    let frameCount = 0;
+    let prevTime = ticks[0];
+
+    for (let i = 1; i < totalTicks; i++) {
+      let currTime = ticks[i];
+      frameCount++;
+
+      let elapsedTime = currTime - prevTime;
+      if (elapsedTime < interval) {
+        continue;
+      }
+
+      let framerate = 1000 / (elapsedTime / frameCount);
+      timeline.push({ delta: prevTime, value: framerate });
+      timeline.push({ delta: currTime, value: framerate });
+
+      frameCount = 0;
+      prevTime = currTime;
+    }
+
+    return timeline;
   }
 });

toolkit/devtools/server/tests/mochitest/test_framerate_01.html

@@ -46,8 +46,8 @@ window.onload = function() {
       window.setTimeout(() => {
         front.startRecording().then(() => {
           window.setTimeout(() => {
-            front.stopRecording().then(timeline => {
-              onRecordingStopped(timeline);
+            front.stopRecording().then(rawData => {
+              onRecordingStopped(front, rawData);
             });
           }, 1000);
         });
@@ -55,24 +55,42 @@ window.onload = function() {
     });
   });
 
-  function onRecordingStopped(timeline) {
-    ok(timeline, "There should be a recording available.");
+  function onRecordingStopped(front, rawData) {
+    ok(rawData, "There should be a recording available.");
 
-    var ticks = Object.keys(timeline);
-    var values = ticks.map(e => timeline[e]);
+    var timeline = front.plotFPS(rawData);
+    ok(timeline.length >= 2,
+      "There should be at least one measurement available, with two entries.");
 
-    ok(ticks.length >= 1,
-      "There should be at least one measurement available.");
-    is(ticks[0], 100,
-      "The first measurement should be performed when exactly 100ms passed.");
+    var prevTimeStart = timeline[0].delta;
 
-    for (var tick of ticks) {
-      info("Testing tick: " + tick);
-      is(tick % 100, 0, "All ticks should be divisible by the resolution.");
+    for (var i = 0; i < timeline.length; i += 2) {
+      var currTimeStart = timeline[i].delta;
+      var currTimeEnd = timeline[i + 1].delta;
+      info("Testing delta: " + currTimeStart + " vs. " + currTimeEnd);
+
+      ok(currTimeStart < currTimeEnd,
+        "The start and end time deltas should be consecutive.");
+      is(currTimeStart, prevTimeStart,
+        "There should be two time deltas for each framerate value.");
+
+      prevTimeStart = currTimeEnd;
     }
-    for (var value of values) {
-      info("Testing value: " + value);
-      is(typeof value, "number", "All values should be integers.");
+
+    var prevFramerateValue = -1;
+
+    for (var i = 0; i < timeline.length; i += 2) {
+      var currFramerateStart = timeline[i].value;
+      var currFramerateEnd = timeline[i + 1].value;
+      info("Testing framerate: " + currFramerateStart);
+
+      is(currFramerateStart, currFramerateEnd,
+        "The start and end framerate values should be equal.");
+      isnot(currFramerateStart, prevFramerateValue,
+        "There should be different framerate values for each bucket.");
+
+      is(typeof currFramerateStart, "number", "All values should be numbers.");
+      prevFramerateValue = currFramerateStart;
     }
 
     client.close(() => {

toolkit/devtools/server/tests/mochitest/test_framerate_02.html

@@ -43,9 +43,25 @@ window.onload = function() {
       var form = aResponse.tabs[aResponse.selected];
       var front = FramerateFront(client, form);
 
-      front.stopRecording().then(timeline => {
-        ok(timeline, "There should be a recording available.");
-        is(Object.keys(timeline).length, 0, "...but it should be empty.");
+      front.stopRecording().then(rawData => {
+        ok(rawData, "There should be a recording available.");
+        is(rawData.length, 0, "...but it should be empty.");
+
+        var timeline = front.plotFPS(rawData);
+        is(timeline.length, 2,
+          "There should be one measurement plotted, with two entries.");
+
+        info("The framerate should be assumed to be 0 if the recording is empty.");
+
+        is(timeline[0].delta, 0,
+          "The first time delta should be 0.");
+        is(timeline[0].value, 0,
+          "The first framerate value should be 0.");
+
+        is(timeline[1].delta, 100,
+          "The last time delta should be 100 (the default interval value).");
+        is(timeline[1].value, 0,
+          "The last framerate value should be 0.");
 
         client.close(() => {
           DebuggerServer.destroy();