Bug 907289 - Trace client should always emit enteredFrame and exitedFrame events in sequence order. r=fitzgen
authorJake Bailey <rbailey@mozilla.com>
Tue, 20 Aug 2013 15:52:00 +0200
changeset 144928 b968cfd447a0a2a0dc17e26a0ae5e59880e0ec73
parent 144927 344094dcf795666eec4e5c9bb06d7d156aec011f
child 144929 f0ce14507a7808648905e04a924a7bd36073cfcf
push id25183
push useremorley@mozilla.com
push dateThu, 29 Aug 2013 14:02:53 +0000
treeherdermozilla-central@21b5af569ca2 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersfitzgen
bugs907289
milestone26.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 907289 - Trace client should always emit enteredFrame and exitedFrame events in sequence order. r=fitzgen
toolkit/devtools/client/dbg-client.jsm
toolkit/devtools/server/tests/unit/test_trace_client-01.js
toolkit/devtools/server/tests/unit/xpcshell.ini
--- a/toolkit/devtools/client/dbg-client.jsm
+++ b/toolkit/devtools/client/dbg-client.jsm
@@ -1670,30 +1670,30 @@ eventSource(ThreadClient.prototype);
  * @param aClient DebuggerClient
  *        The debugger client parent.
  * @param aActor string
  *        The actor ID for this thread.
  */
 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; },
 
   /**
    * Detach from the trace actor.
    */
   detach: DebuggerClient.requester({ type: "detach" },
                                    { telemetry: "TRACERDETACH" }),
@@ -1715,25 +1715,21 @@ TraceClient.prototype = {
     name: args(1),
     trace: args(0)
   }, {
     after: function(aResponse) {
       if (aResponse.error) {
         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;
     },
     telemetry: "STARTTRACE"
   }),
 
   /**
    * End a trace. If a name is provided, stop the named
@@ -1749,36 +1745,42 @@ TraceClient.prototype = {
     type: "stopTrace",
     name: args(0)
   }, {
     after: function(aResponse) {
       if (aResponse.error) {
         return aResponse;
       }
 
-      this._traces[aResponse.name].active = false;
-      this._activeTraces--;
+      this._activeTraces.delete(aResponse.name);
 
       return aResponse;
     },
     telemetry: "STOPTRACE"
   }),
 
   /**
-   * 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++;
+    }
   }
 };
 
 eventSource(TraceClient.prototype);
 
 /**
  * Grip clients are used to retrieve information about the relevant object.
  *
new file mode 100644
--- /dev/null
+++ b/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 });
+}
--- a/toolkit/devtools/server/tests/unit/xpcshell.ini
+++ b/toolkit/devtools/server/tests/unit/xpcshell.ini
@@ -161,8 +161,9 @@ reason = bug 820380
 [test_unsafeDereference.js]
 [test_add_actors.js]
 [test_trace_actor-01.js]
 [test_trace_actor-02.js]
 [test_trace_actor-03.js]
 [test_trace_actor-04.js]
 [test_trace_actor-05.js]
 [test_trace_actor-06.js]
+[test_trace_client-01.js]