Bug 907289 - Trace client should always emit enteredFrame and exitedFrame events in sequence order. r=fitzgen

toolkit/devtools/client/dbg-client.jsm

@@ -1675,20 +1675,20 @@ eventSource(ThreadClient.prototype);
 function TraceClient(aClient, aActor) {
   this._client = aClient;
   this._actor = aActor;
-  this._traces = Object.create(null);
-  this._activeTraces = 0;
+  this._activeTraces = new Set();
+  this._waitingPackets = new Map();
+  this._expectedPacket = 0;
 
-  this._client.addListener(UnsolicitedNotifications.enteredFrame,
-                           this.onEnteredFrame.bind(this));
-  this._client.addListener(UnsolicitedNotifications.exitedFrame,
-                           this.onExitedFrame.bind(this));
+  this.onPacket = this.onPacket.bind(this);
+  this._client.addListener(UnsolicitedNotifications.enteredFrame, this.onPacket);
+  this._client.addListener(UnsolicitedNotifications.exitedFrame, this.onPacket);
 
   this.request = this._client.request;
 }
 
 TraceClient.prototype = {
   get actor()   { return this._actor; },
-  get tracing() { return this._activeTraces > 0; },
+  get tracing() { return this._activeTraces.size > 0; },
 
   get _transport() { return this._client._transport; },
 
@@ -1720,15 +1720,11 @@ TraceClient.prototype = {
         return aResponse;
       }
 
-      let name = aResponse.name;
-
-      if (!this._traces[name] || !this._traces[name].active) {
-        this._activeTraces++;
+      if (!this.tracing) {
+        this._waitingPackets.clear();
+        this._expectedPacket = 0;
       }
-
-      this._traces[name] = {
-        active: true
-      };
+      this._activeTraces.add(aResponse.name);
 
       return aResponse;
     },
@@ -1754,8 +1750,7 @@ TraceClient.prototype = {
         return aResponse;
       }
 
-      this._traces[aResponse.name].active = false;
-      this._activeTraces--;
+      this._activeTraces.delete(aResponse.name);
 
       return aResponse;
     },
@@ -1763,17 +1758,24 @@ TraceClient.prototype = {
   }),
 
   /**
-   * Called when the trace actor notifies that a frame has been entered.
+   * Called when the trace actor notifies that a frame has been
+   * entered or exited.
+   *
+   * @param aEvent string
+   *        The type of the unsolicited packet (enteredFrame|exitedFrame).
+   *
+   * @param aPacket object
+   *        Packet received over the RDP from the trace actor.
    */
-  onEnteredFrame: function JSTC_onEnteredFrame(aEvent, aResponse) {
-    this.notify("enteredFrame", aResponse);
-  },
+  onPacket: function JSTC_onPacket(aEvent, aPacket) {
+    this._waitingPackets.set(aPacket.sequence, aPacket);
 
-  /**
-   * Called when the trace actor notifies that a frame has been exited.
-   */
-  onExitedFrame: function JSTC_onExitedFrame(aEvent, aResponse) {
-    this.notify("exitedFrame", aResponse);
+    while (this._waitingPackets.has(this._expectedPacket)) {
+      let packet = this._waitingPackets.get(this._expectedPacket);
+      this._waitingPackets.delete(this._expectedPacket);
+      this.notify(packet.type, packet);
+      this._expectedPacket++;
+    }
   }
 };
 

toolkit/devtools/server/tests/unit/test_trace_client-01.js

@@ -0,0 +1,101 @@
+/* Any copyright is dedicated to the Public Domain.
+ http://creativecommons.org/publicdomain/zero/1.0/ */
+
+/**
+ * Tests that TraceClient emits enteredFrame and exitedFrame events in
+ * order when receiving packets out of order.
+ */
+
+let {defer, resolve} = devtools.require("sdk/core/promise");
+
+var gDebuggee;
+var gClient;
+var gTraceClient;
+
+function run_test()
+{
+  initTestTracerServer();
+  gDebuggee = addTestGlobal("test-tracer-actor");
+  gClient = new DebuggerClient(DebuggerServer.connectPipe());
+  gClient.connect(function() {
+    attachTestTab(gClient, "test-tracer-actor", function(aResponse, aTabClient) {
+      gClient.attachTracer(aResponse.traceActor, function(aResponse, aTraceClient) {
+        gTraceClient = aTraceClient;
+        test_packet_order();
+      });
+    });
+  });
+  do_test_pending();
+}
+
+function test_packet_order()
+{
+  let sequence = 0;
+
+  function check_packet(aEvent, aPacket) {
+    do_check_eq(aPacket.sequence, sequence,
+                'packet should have sequence number ' + sequence);
+    sequence++;
+  }
+
+  gTraceClient.addListener("enteredFrame", check_packet);
+  gTraceClient.addListener("exitedFrame", check_packet);
+
+  start_trace()
+    .then(mock_packets)
+    .then(start_trace)
+    .then(mock_packets.bind(null, 14))
+    .then(stop_trace)
+    .then(stop_trace)
+    .then(function() {
+      // All traces were stopped, so the sequence number resets
+      sequence = 0;
+      return resolve();
+    })
+    .then(start_trace)
+    .then(mock_packets)
+    .then(stop_trace)
+    .then(function() {
+      finishClient(gClient);
+    });
+}
+
+function start_trace()
+{
+  let deferred = defer();
+  gTraceClient.startTrace([], null, function() { deferred.resolve(); });
+  return deferred.promise;
+}
+
+function stop_trace()
+{
+  let deferred = defer();
+  gTraceClient.stopTrace(null, function() { deferred.resolve(); });
+  return deferred.promise;
+}
+
+function mock_packets(s = 0)
+{
+  gTraceClient.onPacket("", { type: "exitedFrame",  sequence: s + 5 });
+  gTraceClient.onPacket("", { type: "enteredFrame", sequence: s + 3 });
+  gTraceClient.onPacket("", { type: "exitedFrame",  sequence: s + 2 });
+  gTraceClient.onPacket("", { type: "enteredFrame", sequence: s + 4 });
+  gTraceClient.onPacket("", { type: "enteredFrame", sequence: s + 1 });
+
+  gTraceClient.onPacket("", { type: "enteredFrame", sequence: s + 7 });
+  gTraceClient.onPacket("", { type: "enteredFrame", sequence: s + 8 });
+
+  // Triggers 0-5
+  gTraceClient.onPacket("", { type: "enteredFrame", sequence: s + 0 });
+
+  gTraceClient.onPacket("", { type: "exitedFrame",  sequence: s + 9 });
+  gTraceClient.onPacket("", { type: "exitedFrame",  sequence: s + 10 });
+
+  // Triggers 6-10
+  gTraceClient.onPacket("", { type: "exitedFrame",  sequence: s + 6 });
+
+  // Each following packet is expected; event is fired immediately
+  gTraceClient.onPacket("", { type: "enteredFrame", sequence: s + 11 });
+  gTraceClient.onPacket("", { type: "exitedFrame",  sequence: s + 12 });
+  gTraceClient.onPacket("", { type: "exitedFrame",  sequence: s + 13 });
+}

toolkit/devtools/server/tests/unit/xpcshell.ini

@@ -166,3 +166,4 @@ reason = bug 820380
 [test_trace_actor-04.js]
 [test_trace_actor-05.js]
 [test_trace_actor-06.js]
+[test_trace_client-01.js]