Bug 1673897 - [devtools] Throw if a spec method defines an array return value r=ochameau

Depends on D96940

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

devtools/shared/protocol/Response.js

@@ -16,6 +16,10 @@ var { types } = require("devtools/shared/protocol/types");
  */
 var Response = function(template = {}) {
   this.template = template;
+  if (this.template instanceof RetVal && this.template.isArrayType()) {
+    throw Error("Arrays should be wrapped in objects");
+  }
+
   const placeholders = findPlaceholders(template, RetVal);
   if (placeholders.length > 1) {
     throw Error("More than one RetVal specified in response");
@@ -83,6 +87,7 @@ exports.Response = Response;
  *    The return value should be marshalled as this type.
  */
 var RetVal = function(type) {
+  this._type = type;
   // Prevent force loading all RetVal types by accessing it only when needed
   loader.lazyGetter(this, "type", function() {
     return types.getType(type);
@@ -97,6 +102,12 @@ RetVal.prototype = {
   read: function(v, ctx) {
     return this.type.read(v, ctx);
   },
+
+  isArrayType: function() {
+    // `_type` should always be a string, but a few incorrect RetVal calls
+    // pass `0`. See Bug 1677703.
+    return typeof this._type === "string" && this._type.startsWith("array:");
+  },
 };
 
 // Outside of protocol.js, RetVal is called as factory method, without the new keyword.

devtools/shared/protocol/tests/xpcshell/test_protocol_invalid_response.js

@@ -0,0 +1,52 @@
+/* Any copyright is dedicated to the Public Domain.
+   http://creativecommons.org/publicdomain/zero/1.0/ */
+
+"use strict";
+
+const protocol = require("devtools/shared/protocol");
+const { RetVal } = protocol;
+
+// Test invalid response specs throw when generating the Actor specification.
+
+// Test top level array response
+add_task(async function() {
+  Assert.throws(() => {
+    protocol.generateActorSpec({
+      typeName: "invalidArrayResponse",
+      methods: {
+        invalidMethod: {
+          response: RetVal("array:string"),
+        },
+      },
+    });
+  }, /Arrays should be wrapped in objects/);
+
+  protocol.generateActorSpec({
+    typeName: "validArrayResponse",
+    methods: {
+      validMethod: {
+        response: {
+          someArray: RetVal("array:string"),
+        },
+      },
+    },
+  });
+  ok(true, "Arrays wrapped in object are valid response packets");
+});
+
+// Test response with several placeholders
+add_task(async function() {
+  Assert.throws(() => {
+    protocol.generateActorSpec({
+      typeName: "tooManyPlaceholdersResponse",
+      methods: {
+        invalidMethod: {
+          response: {
+            prop1: RetVal("json"),
+            prop2: RetVal("json"),
+          },
+        },
+      },
+    });
+  }, /More than one RetVal specified in response/);
+});

devtools/shared/protocol/tests/xpcshell/xpcshell.ini

@@ -9,6 +9,7 @@ support-files =
 [test_protocol_async.js]
 [test_protocol_children.js]
 [test_protocol_index.js]
+[test_protocol_invalid_response.js]
 [test_protocol_longstring.js]
 [test_protocol_simple.js]
 [test_protocol_stack.js]