Bug 1453846 - Remove DevTools RDP events. r=Honza
authorJ. Ryan Stinnett <jryans@gmail.com>
Fri, 13 Apr 2018 12:57:48 -0500
changeset 467459 4012129c0b4023a46e345b8f99e76b31dcd3514f
parent 467458 00a9881a5ceb8e1d345a39c4c425f5fd88b4aa76
child 467460 c89827f5b8eb5e07c4378a385e218aeb2569ca8e
push id9165
push userasasaki@mozilla.com
push dateThu, 26 Apr 2018 21:04:54 +0000
treeherdermozilla-beta@064c3804de2e [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersHonza
bugs1453846, 1126274
milestone61.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 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
@@ -110,17 +110,16 @@ skip-if = e10s # Bug 1069044 - destroyIn
 [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_toolbar_reorder_by_width.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 {