Bug 1814013 - [devtools] Remove test-only echo request from the root actor. r=devtools-reviewers,devtools-backward-compat-reviewers,nchevobbe

This wasn't so trivial to revisit tests, but at least we stop shipping test-only
features in RDP. And the cryptic echo implement now only exists in tests.

Differential Revision: https://phabricator.services.mozilla.com/D168360

devtools/server/actors/root.js

@@ -105,23 +105,6 @@ class RootActor extends Actor {
   constructor(conn, parameters) {
     super(conn, rootSpec);
 
-    /**
-     * This `echo` request can't be easily specified via protocol.js types
-     * as it is a JSON value in the packet itself. Protocol.js only allows
-     * arbitrary json object in one property of the packet.
-     * In order to bypass protocol.js, declare the request method directly
-     * on the prototype/requestTypes, which is populated by Actor's constructor
-     *
-     * Note that this request is only used by tests.
-     */
-    this.requestTypes.echo = function(request) {
-      /*
-       * Request packets are frozen. Copy request, so that
-       * DevToolsServerConnection.onPacket can attach a 'from' property.
-       */
-      return Cu.cloneInto(request, {});
-    };
-
     this._parameters = parameters;
     this._onTabListChanged = this.onTabListChanged.bind(this);
     this._onAddonListChanged = this.onAddonListChanged.bind(this);

devtools/server/tests/xpcshell/test_forwardingprefix.js

@@ -68,55 +68,57 @@ function createMainConnection() {
  *
  * To avoid deep stacks, we call completed from the next tick.
  */
-function tryActors(reachables, completed) {
-  let count = 0;
-
-  let outerActor;
-  for (outerActor of [
+async function tryActors(reachables, completed) {
+  for (const actor of [
     "root",
     "prefix1/root",
     "prefix1/actor",
     "prefix2/root",
     "prefix2/actor",
   ]) {
-    /*
-     * Let each callback capture its own iteration's value; outerActor is
-     * local to the whole loop, not to a single iteration.
-     */
-    const actor = outerActor;
-
-    count++;
-
-    let promise;
-    // phone home
-    if (actor == "root") {
-      promise = gClient.mainRoot.echo({ value: "tango" });
-    } else {
-      promise = gClient.request({ to: actor, type: "echo", value: "tango" });
+    let response;
+    try {
+      if (actor.endsWith("root")) {
+        // Root actor doesn't expose any echo method,
+        // so fallback on getRoot which returns `{ from: "root" }`.
+        // For the top level root actor, we have to use its front.
+        if (actor == "root") {
+          response = await gClient.mainRoot.getRoot();
+        } else {
+          response = await gClient.request({ to: actor, type: "getRoot" });
+        }
+      } else {
+        response = await gClient.request({
+          to: actor,
+          type: "echo",
+          value: "tango",
+        });
+      }
+    } catch (e) {
+      response = e;
     }
-    const callback = response => {
-      if (reachables.has(actor)) {
+    if (reachables.has(actor)) {
+      if (actor.endsWith("root")) {
+        // RootActor's getRoot response is almost empty on xpcshell
+        Assert.deepEqual({ from: actor }, response);
+      } else {
         Assert.deepEqual(
           { from: actor, to: actor, type: "echo", value: "tango" },
           response
         );
-      } else {
-        Assert.deepEqual(
-          {
-            from: actor,
-            error: "noSuchActor",
-            message: "No such actor for ID: " + actor,
-          },
-          response
-        );
       }
-
-      if (--count == 0) {
-        executeSoon(completed, "tryActors callback " + completed.name);
-      }
-    };
-    promise.then(callback, callback);
+    } else {
+      Assert.deepEqual(
+        {
+          from: actor,
+          error: "noSuchActor",
+          message: "No such actor for ID: " + actor,
+        },
+        response
+      );
+    }
   }
+  executeSoon(completed, "tryActors callback " + completed.name);
 }
 
 /*

devtools/server/tests/xpcshell/test_requestTypes.js

@@ -17,9 +17,6 @@ add_task(async function() {
 
   const response = await client.mainRoot.requestTypes();
   const expectedRequestTypes = Object.keys(generateRequestTypes(rootSpec));
-  // As "echo" can't be implemented via protocol.js this is manually added from RootActor
-  // while bypassing the specification.
-  expectedRequestTypes.push("echo");
 
   Assert.ok(Array.isArray(response.requestTypes));
   Assert.equal(

devtools/shared/security/tests/chrome/test_websocket-transport.html

@@ -55,10 +55,9 @@ window.onload = function() {
 
     await client.connect();
 
-    // Send a message the server that will echo back
-    const message = "message";
-    const reply = await client.mainRoot.echo({ message });
-    is(reply.message, message, "Echo message matches");
+    // Send a message the server
+    const reply = await client.mainRoot.getRoot();
+    is(reply.from, "root", "Got expected response");
 
     client.off("closed", onUnexpectedClose);
     transport.close();

devtools/shared/specs/root.js

@@ -103,9 +103,6 @@ const rootSpecPrototype = {
       request: {},
       response: RetVal("json"),
     },
-
-    // Note that RootFront also implements 'echo' requests
-    // that can't be described via protocol.js specs.
   },
 
   events: {

devtools/shared/transport/tests/xpcshell/test_dbgsocket.js

@@ -2,6 +2,8 @@
    http://creativecommons.org/publicdomain/zero/1.0/ */
 "use strict";
 
+/* global structuredClone */
+
 var gPort;
 var gExtraListener;
 
@@ -16,6 +18,26 @@ function run_test() {
   run_next_test();
 }
 
+const { Actor } = require("resource://devtools/shared/protocol/Actor.js");
+class EchoTestActor extends Actor {
+  constructor(conn) {
+    super(conn);
+
+    this.typeName = "EchoTestActor";
+    this.requestTypes = {
+      echo: EchoTestActor.prototype.onEcho,
+    };
+  }
+
+  onEcho(request) {
+    /*
+     * Request packets are frozen. Copy request, so that
+     * DevToolsServerConnection.onPacket can attach a 'from' property.
+     */
+    return structuredClone(request);
+  }
+}
+
 async function test_socket_conn() {
   Assert.equal(DevToolsServer.listeningSockets, 0);
   const AuthenticatorType = DevToolsServer.Authenticators.get("PROMPT");
@@ -40,12 +62,23 @@ async function test_socket_conn() {
   Assert.ok(!DevToolsServer.hasConnection());
 
   info("Starting long and unicode tests at " + new Date().toTimeString());
-  const unicodeString = "(╯°□°）╯︵ ┻━┻";
+  // We can't use EventEmitter.once as this is the second argument we care about...
+  const onConnectionChange = new Promise(res => {
+    DevToolsServer.once("connectionchange", (type, conn) => res(conn));
+  });
+
   const transport = await DevToolsClient.socketConnect({
     host: "127.0.0.1",
     port: gPort,
   });
   Assert.ok(DevToolsServer.hasConnection());
+  info("Wait for server connection");
+  const conn = await onConnectionChange;
+
+  // Register a custom actor to do echo requests
+  const actor = new EchoTestActor(conn);
+  actor.actorID = "echo-actor";
+  conn.addActor(actor);
 
   // Assert that connection settings are available on transport object
   const settings = transport.connectionSettings;
@@ -53,6 +86,7 @@ async function test_socket_conn() {
   Assert.equal(settings.port, gPort);
 
   const onDebuggerConnectionClosed = DevToolsServer.once("connectionchange");
+  const unicodeString = "(╯°□°）╯︵ ┻━┻";
   await new Promise(resolve => {
     transport.hooks = {
       onPacket(packet) {
@@ -63,7 +97,7 @@ async function test_socket_conn() {
         // Verify that things work correctly when bigger than the output
         // transport buffers and when transporting unicode...
         transport.send({
-          to: "root",
+          to: "echo-actor",
           type: "echo",
           reallylong: really_long(),
           unicode: unicodeString,