Bug 1453846 - Remove DevTools RDP events. r=Honza draft
authorJ. Ryan Stinnett <jryans@gmail.com>
Fri, 13 Apr 2018 12:57:48 -0500
changeset 783142 67fc015e15161e83b76977b439f0db37d4180693
parent 782895 6276ec7ebbf33e3484997b189f20fc1511534187
push id106623
push userbmo:jryans@gmail.com
push dateMon, 16 Apr 2018 17:52:21 +0000
reviewersHonza
bugs1453846, 1126274
milestone61.0a1
Bug 1453846 - Remove DevTools RDP events. r=Honza A few years ago in bug 1126274, we added RDP events to the DevTools transport so that the add-on RDP Inspector could display packets as they flow by. The add-on no longer works as it is not a WebExtension. Maybe someday we'll revisit this, but for now this removes some dead code. MozReview-Commit-ID: AvgQhYWwBUA
devtools/client/framework/test/browser.ini
devtools/client/framework/test/browser_toolbox_transport_events.js
devtools/shared/client/debugger-client.js
devtools/shared/transport/tests/unit/test_transport_events.js
devtools/shared/transport/tests/unit/xpcshell.ini
devtools/shared/transport/transport.js
--- a/devtools/client/framework/test/browser.ini
+++ b/devtools/client/framework/test/browser.ini
@@ -109,17 +109,16 @@ skip-if = e10s # Bug 1069044 - destroyIn
 [browser_toolbox_textbox_context_menu.js]
 [browser_toolbox_theme.js]
 [browser_toolbox_theme_registration.js]
 [browser_toolbox_toggle.js]
 [browser_toolbox_tool_ready.js]
 [browser_toolbox_tool_remote_reopen.js]
 [browser_toolbox_toolbar_overflow.js]
 [browser_toolbox_tools_per_toolbox_registration.js]
-[browser_toolbox_transport_events.js]
 [browser_toolbox_view_source_01.js]
 [browser_toolbox_view_source_02.js]
 [browser_toolbox_view_source_03.js]
 [browser_toolbox_view_source_04.js]
 [browser_toolbox_window_reload_target.js]
 [browser_toolbox_window_shortcuts.js]
 skip-if = os == "mac" && os_version == "10.8" || os == "win" && os_version == "5.1" # Bug 851129 - Re-enable browser_toolbox_window_shortcuts.js test after leaks are fixed
 [browser_toolbox_window_title_changes.js]
deleted file mode 100644
--- a/devtools/client/framework/test/browser_toolbox_transport_events.js
+++ /dev/null
@@ -1,112 +0,0 @@
-/* -*- indent-tabs-mode: nil; js-indent-level: 2 -*- */
-/* vim: set ft=javascript ts=2 et sw=2 tw=80: */
-/* Any copyright is dedicated to the Public Domain.
- * http://creativecommons.org/publicdomain/zero/1.0/ */
-
-"use strict";
-
-const { on, off } = require("devtools/shared/event-emitter");
-const { DebuggerClient } = require("devtools/shared/client/debugger-client");
-
-function test() {
-  gDevTools.on("toolbox-created", onToolboxCreated);
-  on(DebuggerClient, "connect", onDebuggerClientConnect);
-
-  addTab("about:blank").then(function() {
-    let target = TargetFactory.forTab(gBrowser.selectedTab);
-    gDevTools.showToolbox(target, "webconsole").then(testResults);
-  });
-}
-
-function testResults(toolbox) {
-  testPackets(sent1, received1);
-  testPackets(sent2, received2);
-
-  cleanUp(toolbox);
-}
-
-function cleanUp(toolbox) {
-  gDevTools.off("toolbox-created", onToolboxCreated);
-  off(DebuggerClient, "connect", onDebuggerClientConnect);
-
-  toolbox.destroy().then(function() {
-    // TODO: fixme.
-    // eslint-disable-next-line mozilla/no-arbitrary-setTimeout
-    setTimeout(() => {
-      gBrowser.removeCurrentTab();
-      executeSoon(function() {
-        finish();
-      });
-    }, 1000);
-  });
-}
-
-function testPackets(sent, received) {
-  ok(sent.length > 0, "There must be at least one sent packet");
-  ok(received.length > 0, "There must be at leaset one received packet");
-
-  if (!sent.length || received.length) {
-    return;
-  }
-
-  let sentPacket = sent[0];
-  let receivedPacket = received[0];
-
-  is(receivedPacket.from, "root",
-    "The first received packet is from the root");
-  is(receivedPacket.applicationType, "browser",
-    "The first received packet has browser type");
-  is(sentPacket.type, "listTabs",
-    "The first sent packet is for list of tabs");
-}
-
-// Listen to the transport object that is associated with the
-// default Toolbox debugger client
-var sent1 = [];
-var received1 = [];
-
-function send1(packet) {
-  sent1.push(packet);
-}
-
-function onPacket1(packet) {
-  received1.push(packet);
-}
-
-function onToolboxCreated(toolbox) {
-  toolbox.target.makeRemote();
-  let client = toolbox.target.client;
-  let transport = client._transport;
-
-  transport.on("send", send1);
-  transport.on("packet", onPacket1);
-
-  client.addOneTimeListener("closed", () => {
-    transport.off("send", send1);
-    transport.off("packet", onPacket1);
-  });
-}
-
-// Listen to all debugger client object protocols.
-var sent2 = [];
-var received2 = [];
-
-function send2(packet) {
-  sent2.push(packet);
-}
-
-function onPacket2(packet) {
-  received2.push(packet);
-}
-
-function onDebuggerClientConnect(client) {
-  let transport = client._transport;
-
-  transport.on("send", send2);
-  transport.on("packet", onPacket2);
-
-  client.addOneTimeListener("closed", () => {
-    transport.off("send", send2);
-    transport.off("packet", onPacket2);
-  });
-}
--- a/devtools/shared/client/debugger-client.js
+++ b/devtools/shared/client/debugger-client.js
@@ -169,21 +169,16 @@ DebuggerClient.prototype = {
    * @return Promise
    *         Resolves once connected with an array whose first element
    *         is the application type, by default "browser", and the second
    *         element is the traits object (help figure out the features
    *         and behaviors of the server we connect to. See RootActor).
    */
   connect: function(onConnected) {
     let deferred = promise.defer();
-    this.emit("connect");
-
-    // Also emit the event on the |DebuggerClient| object (not on the instance),
-    // so it's possible to track all instances.
-    EventEmitter.emit(DebuggerClient, "connect", this);
 
     this.addOneTimeListener("connected", (name, applicationType, traits) => {
       this.traits = traits;
       if (onConnected) {
         onConnected(applicationType, traits);
       }
       deferred.resolve([applicationType, traits]);
     });
deleted file mode 100644
--- a/devtools/shared/transport/tests/unit/test_transport_events.js
+++ /dev/null
@@ -1,70 +0,0 @@
-/* Any copyright is dedicated to the Public Domain.
-   http://creativecommons.org/publicdomain/zero/1.0/ */
-
-"use strict";
-
-function run_test() {
-  initTestDebuggerServer();
-
-  add_task(async function() {
-    await test_transport_events("socket", socket_transport);
-    await test_transport_events("local", local_transport);
-    DebuggerServer.destroy();
-  });
-
-  run_next_test();
-}
-
-async function test_transport_events(name, transportFactory) {
-  info(`Started testing of transport: ${name}`);
-
-  Assert.equal(Object.keys(DebuggerServer._connections).length, 0);
-
-  let transport = await transportFactory();
-
-  // Transport expects the hooks to be not null
-  transport.hooks = {
-    onPacket: () => {},
-    onClosed: () => {},
-  };
-
-  let rootReceived = transport.once("packet", packet => {
-    info(`Packet event: ${JSON.stringify(packet)}`);
-    Assert.equal(packet.from, "root");
-  });
-
-  transport.ready();
-  await rootReceived;
-
-  let echoSent = transport.once("send", packet => {
-    info(`Send event: ${JSON.stringify(packet)}`);
-    Assert.equal(packet.to, "root");
-    Assert.equal(packet.type, "echo");
-  });
-
-  let echoReceived = transport.once("packet", packet => {
-    info(`Packet event: ${JSON.stringify(packet)}`);
-    Assert.equal(packet.from, "root");
-    Assert.equal(packet.type, "echo");
-  });
-
-  transport.send({ to: "root", type: "echo" });
-  await echoSent;
-  await echoReceived;
-
-  let clientClosed = transport.once("close", () => {
-    info(`Close event`);
-  });
-
-  let serverClosed = DebuggerServer.once("connectionchange", type => {
-    info(`Server closed`);
-    Assert.equal(type, "closed");
-  });
-
-  transport.close();
-
-  await clientClosed;
-  await serverClosed;
-
-  info(`Finished testing of transport: ${name}`);
-}
--- a/devtools/shared/transport/tests/unit/xpcshell.ini
+++ b/devtools/shared/transport/tests/unit/xpcshell.ini
@@ -12,9 +12,8 @@ support-files =
 [test_client_server_bulk.js]
 [test_dbgsocket.js]
 [test_dbgsocket_connection_drop.js]
 [test_delimited_read.js]
 [test_no_bulk.js]
 [test_packet.js]
 [test_queue.js]
 [test_transport_bulk.js]
-[test_transport_events.js]
--- a/devtools/shared/transport/transport.js
+++ b/devtools/shared/transport/transport.js
@@ -25,17 +25,16 @@
   const DevToolsUtils = require("devtools/shared/DevToolsUtils");
   const { dumpn, dumpv } = DevToolsUtils;
   const flags = require("devtools/shared/flags");
   const StreamUtils = require("devtools/shared/transport/stream-utils");
   const { Packet, JSONPacket, BulkPacket } =
   require("devtools/shared/transport/packets");
   const promise = require("promise");
   const defer = require("devtools/shared/defer");
-  const EventEmitter = require("devtools/shared/event-emitter");
 
   DevToolsUtils.defineLazyGetter(this, "Pipe", () => {
     return CC("@mozilla.org/pipe;1", "nsIPipe", "init");
   });
 
   DevToolsUtils.defineLazyGetter(this, "ScriptableInputStream", () => {
     return CC("@mozilla.org/scriptableinputstream;1",
             "nsIScriptableInputStream", "init");
@@ -96,18 +95,16 @@
    * - onClosed(reason) - called when the connection is closed. |reason| is
    *   an optional nsresult or object, typically passed when the transport is
    *   closed due to some error in a underlying stream.
    *
    * See ./packets.js and the Remote Debugging Protocol specification for more
    * details on the format of these packets.
    */
   function DebuggerTransport(input, output) {
-    EventEmitter.decorate(this);
-
     this._input = input;
     this._scriptableInput = new ScriptableInputStream(input);
     this._output = output;
 
     // The current incoming (possibly partial) header, which will determine which
     // type of Packet |_incoming| below will become.
     this._incomingHeader = "";
     // The current incoming Packet object
@@ -129,18 +126,16 @@
      * Transmit an object as a JSON packet.
      *
      * This method returns immediately, without waiting for the entire
      * packet to be transmitted, registering event handlers as needed to
      * transmit the entire packet. Packets are transmitted in the order
      * they are passed to this method.
      */
     send: function(object) {
-      this.emit("send", object);
-
       let packet = new JSONPacket(this);
       packet.object = object;
       this._outgoing.push(packet);
       this._flushOutgoing();
     },
 
     /**
      * Transmit streaming data via a bulk packet.
@@ -179,34 +174,30 @@
      *                     The stream to copy from.
      *             @return Promise
      *                     The promise is resolved when copying completes or
      *                     rejected if any (unexpected) errors occur.
      *                     This object also emits "progress" events for each chunk
      *                     that is copied.  See stream-utils.js.
      */
     startBulkSend: function(header) {
-      this.emit("startbulksend", header);
-
       let packet = new BulkPacket(this);
       packet.header = header;
       this._outgoing.push(packet);
       this._flushOutgoing();
       return packet.streamReadyForWriting;
     },
 
     /**
      * Close the transport.
      * @param reason nsresult / object (optional)
      *        The status code or error message that corresponds to the reason for
      *        closing the transport (likely because a stream closed or failed).
      */
     close: function(reason) {
-      this.emit("close", reason);
-
       this.active = false;
       this._input.close();
       this._scriptableInput.close();
       this._output.close();
       this._destroyIncoming();
       this._destroyAllOutgoing();
       if (this.hooks) {
         this.hooks.onClosed(reason);
@@ -473,33 +464,31 @@
     /**
      * Handler triggered by an incoming JSONPacket completing it's |read| method.
      * Delivers the packet to this.hooks.onPacket.
      */
     _onJSONObjectReady: function(object) {
       DevToolsUtils.executeSoon(DevToolsUtils.makeInfallible(() => {
       // Ensure the transport is still alive by the time this runs.
         if (this.active) {
-          this.emit("packet", object);
           this.hooks.onPacket(object);
         }
       }, "DebuggerTransport instance's this.hooks.onPacket"));
     },
 
     /**
      * Handler triggered by an incoming BulkPacket entering the |read| phase for
      * the stream portion of the packet.  Delivers info about the incoming
      * streaming data to this.hooks.onBulkPacket.  See the main comment on the
      * transport at the top of this file for more details.
      */
     _onBulkReadReady: function(...args) {
       DevToolsUtils.executeSoon(DevToolsUtils.makeInfallible(() => {
       // Ensure the transport is still alive by the time this runs.
         if (this.active) {
-          this.emit("bulkpacket", ...args);
           this.hooks.onBulkPacket(...args);
         }
       }, "DebuggerTransport instance's this.hooks.onBulkPacket"));
     },
 
     /**
      * Remove all handlers and references related to the current incoming packet,
      * either because it is now complete or because the transport is closing.
@@ -523,35 +512,31 @@
    * connection it merely calls the packet dispatcher of the other side.
    *
    * @param other LocalDebuggerTransport
    *        The other endpoint for this debugger connection.
    *
    * @see DebuggerTransport
    */
   function LocalDebuggerTransport(other) {
-    EventEmitter.decorate(this);
-
     this.other = other;
     this.hooks = null;
 
     // A packet number, shared between this and this.other. This isn't used by the
     // protocol at all, but it makes the packet traces a lot easier to follow.
     this._serial = this.other ? this.other._serial : { count: 0 };
     this.close = this.close.bind(this);
   }
 
   LocalDebuggerTransport.prototype = {
     /**
      * Transmit a message by directly calling the onPacket handler of the other
      * endpoint.
      */
     send: function(packet) {
-      this.emit("send", packet);
-
       let serial = this._serial.count++;
       if (flags.wantLogging) {
         // Check 'from' first, as 'echo' packets have both.
         if (packet.from) {
           dumpn("Packet " + serial + " sent from " + uneval(packet.from));
         } else if (packet.to) {
           dumpn("Packet " + serial + " sent to " + uneval(packet.to));
         }
@@ -560,35 +545,32 @@
       let other = this.other;
       if (other) {
         DevToolsUtils.executeSoon(DevToolsUtils.makeInfallible(() => {
           // Avoid the cost of JSON.stringify() when logging is disabled.
           if (flags.wantLogging) {
             dumpn("Received packet " + serial + ": " + JSON.stringify(packet, null, 2));
           }
           if (other.hooks) {
-            other.emit("packet", packet);
             other.hooks.onPacket(packet);
           }
         }, "LocalDebuggerTransport instance's this.other.hooks.onPacket"));
       }
     },
 
     /**
      * Send a streaming bulk packet directly to the onBulkPacket handler of the
      * other endpoint.
      *
      * This case is much simpler than the full DebuggerTransport, since there is
      * no primary stream we have to worry about managing while we hand it off to
      * others temporarily.  Instead, we can just make a single use pipe and be
      * done with it.
      */
     startBulkSend: function({actor, type, length}) {
-      this.emit("startbulksend", {actor, type, length});
-
       let serial = this._serial.count++;
 
       dumpn("Sent bulk packet " + serial + " for actor " + actor);
       if (!this.other) {
         let error = new Error("startBulkSend: other side of transport missing");
         return promise.reject(error);
       }
 
@@ -611,17 +593,16 @@
             StreamUtils.copyStream(pipe.inputStream, output, length);
             deferred.resolve(copying);
             return copying;
           },
           stream: pipe.inputStream,
           done: deferred
         };
 
-        this.other.emit("bulkpacket", packet);
         this.other.hooks.onBulkPacket(packet);
 
         // Await the result of reading from the stream
         deferred.promise.then(() => pipe.inputStream.close(), this.close);
       }, "LocalDebuggerTransport instance's this.other.hooks.onBulkPacket"));
 
       // Sender
       let sendDeferred = defer();
@@ -648,18 +629,16 @@
 
       return sendDeferred.promise;
     },
 
     /**
      * Close the transport.
      */
     close: function() {
-      this.emit("close");
-
       if (this.other) {
         // Remove the reference to the other endpoint before calling close(), to
         // avoid infinite recursion.
         let other = this.other;
         this.other = null;
         other.close();
       }
       if (this.hooks) {
@@ -707,18 +686,16 @@
    *
    * |prefix| is a string included in the message names, to distinguish
    * multiple servers running in the same child process.
    *
    * This transport exchanges messages named 'debug:<prefix>:packet', where
    * <prefix> is |prefix|, whose data is the protocol packet.
    */
   function ChildDebuggerTransport(mm, prefix) {
-    EventEmitter.decorate(this);
-
     this._mm = mm;
     this._messageName = "debug:" + prefix + ":packet";
   }
 
   /*
    * To avoid confusion, we use 'message' to mean something that
    * nsIMessageSender conveys, and 'packet' to mean a remote debugging
    * protocol packet.
@@ -746,22 +723,20 @@
     },
 
     ready: function() {
       this._addListener();
     },
 
     close: function() {
       this._removeListener();
-      this.emit("close");
       this.hooks.onClosed();
     },
 
     receiveMessage: function({data}) {
-      this.emit("packet", data);
       this.hooks.onPacket(data);
     },
 
     /**
      * Helper method to ensure a given `object` can be sent across message manager
      * without being serialized to JSON.
      * See https://searchfox.org/mozilla-central/rev/6bfadf95b4a6aaa8bb3b2a166d6c3545983e179a/dom/base/nsFrameMessageManager.cpp#458-469
      */
@@ -784,17 +759,16 @@
           }
           return [key];
         }
       }
       return [];
     },
 
     send: function(packet) {
-      this.emit("send", packet);
       if (flags.testing && !this._canBeSerialized(packet)) {
         let attributes = this.pathToUnserializable(packet);
         let msg = "Following packet can't be serialized: " + JSON.stringify(packet);
         msg += "\nBecause of attributes: " + attributes.join(", ") + "\n";
         msg += "Did you pass a function or an XPCOM object in it?";
         throw new Error(msg);
       }
       try {