Bug 1378862 - remove usage of sdk heritage in devtools protocol.js;r=zer0 draft
authorJulian Descottes <jdescottes@mozilla.com>
Mon, 21 Aug 2017 20:21:48 +0200
changeset 652452 9e5d09ab497b9b2ad102b45d33328f109f63cbeb
parent 652451 a876bd10e7ffb57525f4986a2dad04697d79a917
child 728093 18d46662fb49fdb036a544e77adbf8f8760c9ee3
push id76061
push userjdescottes@mozilla.com
push dateThu, 24 Aug 2017 21:13:10 +0000
reviewerszer0
bugs1378862
milestone57.0a1
Bug 1378862 - remove usage of sdk heritage in devtools protocol.js;r=zer0 MozReview-Commit-ID: 8hyEroQwnNL
devtools/server/tests/unit/test_protocol_children.js
devtools/shared/protocol.js
--- a/devtools/server/tests/unit/test_protocol_children.js
+++ b/devtools/server/tests/unit/test_protocol_children.js
@@ -309,17 +309,17 @@ var RootFront = protocol.FrontClassWithS
     this.actorID = "root";
     protocol.Front.prototype.initialize.call(this, client);
     // Root actor owns itself.
     this.manage(this);
   },
 
   getTemporaryChild: protocol.custom(function (id) {
     if (!this._temporaryHolder) {
-      this._temporaryHolder = protocol.Front(this.conn);
+      this._temporaryHolder = new protocol.Front(this.conn);
       this._temporaryHolder.actorID = this.actorID + "_temp";
       this._temporaryHolder = this.manage(this._temporaryHolder);
     }
     return this._getTemporaryChild(id);
   }, {
     impl: "_getTemporaryChild"
   }),
 
--- a/devtools/shared/protocol.js
+++ b/devtools/shared/protocol.js
@@ -1,17 +1,17 @@
 /* This Source Code Form is subject to the terms of the Mozilla Public
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 "use strict";
 
 var promise = require("promise");
 var defer = require("devtools/shared/defer");
-var {Class} = require("sdk/core/heritage");
+const { extend } = require("devtools/shared/extend");
 var EventEmitter = require("devtools/shared/event-emitter");
 var {getStack, callFunctionWithAsyncStack} = require("devtools/shared/platform/stack");
 var {settleAll} = require("devtools/shared/DevToolsUtils");
 
 /**
  * Types: named marshallers/demarshallers.
  *
  * Types provide a 'write' function that takes a js representation and
@@ -439,38 +439,42 @@ types.JSON = types.addType("json");
  * Placeholder for simple arguments.
  *
  * @param number index
  *    The argument index to place at this position.
  * @param type type
  *    The argument should be marshalled as this type.
  * @constructor
  */
-var Arg = Class({
-  initialize: function (index, type) {
-    this.index = index;
-    this.type = types.getType(type);
-  },
+var Arg = function (index, type) {
+  this.index = index;
+  this.type = types.getType(type);
+};
 
+Arg.prototype = {
   write: function (arg, ctx) {
     return this.type.write(arg, ctx);
   },
 
   read: function (v, ctx, outArgs) {
     outArgs[this.index] = this.type.read(v, ctx);
   },
 
   describe: function () {
     return {
       _arg: this.index,
       type: this.type.name,
     };
   }
-});
-exports.Arg = Arg;
+};
+
+// Outside of protocol.js, Arg is called as factory method, without the new keyword.
+exports.Arg = function (index, type) {
+  return new Arg(index, type);
+};
 
 /**
  * Placeholder for an options argument value that should be hoisted
  * into the packet.
  *
  * If provided in a method specification:
  *
  *   { optionArg: Option(1)}
@@ -479,22 +483,21 @@ exports.Arg = Arg;
  * value's place.
  *
  * @param number index
  *    The argument index of the options value.
  * @param type type
  *    The argument should be marshalled as this type.
  * @constructor
  */
-var Option = Class({
-  extends: Arg,
-  initialize: function (index, type) {
-    Arg.prototype.initialize.call(this, index, type);
-  },
+var Option = function (index, type) {
+  Arg.call(this, index, type);
+};
 
+Option.prototype = extend(Arg.prototype, {
   write: function (arg, ctx, name) {
     // Ignore if arg is undefined or null; allow other falsy values
     if (arg == undefined || arg[name] == undefined) {
       return undefined;
     }
     let v = arg[name];
     return this.type.write(v, ctx);
   },
@@ -511,45 +514,51 @@ var Option = Class({
   describe: function () {
     return {
       _option: this.index,
       type: this.type.name,
     };
   }
 });
 
-exports.Option = Option;
+// Outside of protocol.js, Option is called as factory method, without the new keyword.
+exports.Option = function (index, type) {
+  return new Option(index, type);
+};
 
 /**
  * Placeholder for return values in a response template.
  *
  * @param type type
  *    The return value should be marshalled as this type.
  */
-var RetVal = Class({
-  initialize: function (type) {
-    this.type = types.getType(type);
-  },
+var RetVal = function (type) {
+  this.type = types.getType(type);
+};
 
+RetVal.prototype = {
   write: function (v, ctx) {
     return this.type.write(v, ctx);
   },
 
   read: function (v, ctx) {
     return this.type.read(v, ctx);
   },
 
   describe: function () {
     return {
       _retval: this.type.name
     };
   }
-});
+};
 
-exports.RetVal = RetVal;
+// Outside of protocol.js, RetVal is called as factory method, without the new keyword.
+exports.RetVal = function (type) {
+  return new RetVal(type);
+};
 
 /* Template handling functions */
 
 /**
  * Get the value at a given path, or undefined if not found.
  */
 function getPath(obj, path) {
   for (let name of path) {
@@ -595,23 +604,23 @@ function describeTemplate(template) {
 
 /**
  * Manages a request template.
  *
  * @param object template
  *    The request template.
  * @construcor
  */
-var Request = Class({
-  initialize: function (template = {}) {
-    this.type = template.type;
-    this.template = template;
-    this.args = findPlaceholders(template, Arg);
-  },
+var Request = function (template = {}) {
+  this.type = template.type;
+  this.template = template;
+  this.args = findPlaceholders(template, Arg);
+};
 
+Request.prototype = {
   /**
    * Write a request.
    *
    * @param array fnArgs
    *    The function arguments to place in the request.
    * @param object ctx
    *    The object making the request.
    * @returns a request packet.
@@ -645,39 +654,39 @@ var Request = Class({
       arg.read(getPath(packet, path), ctx, fnArgs, name);
     }
     return fnArgs;
   },
 
   describe: function () {
     return describeTemplate(this.template);
   }
-});
+};
 
 /**
  * Manages a response template.
  *
  * @param object template
  *    The response template.
  * @construcor
  */
-var Response = Class({
-  initialize: function (template = {}) {
-    this.template = template;
-    let placeholders = findPlaceholders(template, RetVal);
-    if (placeholders.length > 1) {
-      throw Error("More than one RetVal specified in response");
-    }
-    let placeholder = placeholders.shift();
-    if (placeholder) {
-      this.retVal = placeholder.placeholder;
-      this.path = placeholder.path;
-    }
-  },
+var Response = function (template = {}) {
+  this.template = template;
+  let placeholders = findPlaceholders(template, RetVal);
+  if (placeholders.length > 1) {
+    throw Error("More than one RetVal specified in response");
+  }
+  let placeholder = placeholders.shift();
+  if (placeholder) {
+    this.retVal = placeholder.placeholder;
+    this.path = placeholder.path;
+  }
+};
 
+Response.prototype = {
   /**
    * Write a response for the given return value.
    *
    * @param val ret
    *    The return value.
    * @param object ctx
    *    The object writing the response.
    */
@@ -704,45 +713,39 @@ var Response = Class({
     }
     let v = getPath(packet, this.path);
     return this.retVal.read(v, ctx);
   },
 
   describe: function () {
     return describeTemplate(this.template);
   }
-});
+};
 
 /**
  * Actor and Front implementations
  */
 
 /**
  * A protocol object that can manage the lifetime of other protocol
- * objects.
+ * objects. Pools are used on both sides of the connection to help coordinate lifetimes.
+ *
+ * @param optional conn
+ *   Either a DebuggerServerConnection or a DebuggerClient.  Must have
+ *   addActorPool, removeActorPool, and poolFor.
+ *   conn can be null if the subclass provides a conn property.
+ * @constructor
  */
-var Pool = Class({
-  extends: EventEmitter,
+var Pool = function (conn) {
+  if (conn) {
+    this.conn = conn;
+  }
+};
 
-  /**
-   * Pools are used on both sides of the connection to help coordinate
-   * lifetimes.
-   *
-   * @param optional conn
-   *   Either a DebuggerServerConnection or a DebuggerClient.  Must have
-   *   addActorPool, removeActorPool, and poolFor.
-   *   conn can be null if the subclass provides a conn property.
-   * @constructor
-   */
-  initialize: function (conn) {
-    if (conn) {
-      this.conn = conn;
-    }
-  },
-
+Pool.prototype = extend(EventEmitter.prototype, {
   /**
    * Return the parent pool for this client.
    */
   parent: function () {
     return this.conn.poolFor(this.actorID);
   },
 
   /**
@@ -860,46 +863,44 @@ var Pool = Class({
   cleanup: function () {
     this.destroy();
   }
 });
 exports.Pool = Pool;
 
 /**
  * An actor in the actor tree.
+ *
+ * @param optional conn
+ *   Either a DebuggerServerConnection or a DebuggerClient.  Must have
+ *   addActorPool, removeActorPool, and poolFor.
+ *   conn can be null if the subclass provides a conn property.
+ * @constructor
  */
-var Actor = Class({
-  extends: Pool,
+var Actor = function (conn) {
+  Pool.call(this, conn);
 
+  // Forward events to the connection.
+  if (this._actorSpec && this._actorSpec.events) {
+    for (let key of this._actorSpec.events.keys()) {
+      let name = key;
+      let sendEvent = this._sendEvent.bind(this, name);
+      this.on(name, (...args) => {
+        sendEvent.apply(null, args);
+      });
+    }
+  }
+};
+
+Actor.prototype = extend(Pool.prototype, {
   // Will contain the actor's ID
   actorID: null,
 
-  /**
-   * Initialize an actor.
-   *
-   * @param optional conn
-   *   Either a DebuggerServerConnection or a DebuggerClient.  Must have
-   *   addActorPool, removeActorPool, and poolFor.
-   *   conn can be null if the subclass provides a conn property.
-   * @constructor
-   */
-  initialize: function (conn) {
-    Pool.prototype.initialize.call(this, conn);
-
-    // Forward events to the connection.
-    if (this._actorSpec && this._actorSpec.events) {
-      for (let key of this._actorSpec.events.keys()) {
-        let name = key;
-        let sendEvent = this._sendEvent.bind(this, name);
-        this.on(name, (...args) => {
-          sendEvent.apply(null, args);
-        });
-      }
-    }
-  },
+  // Existing Actors extending this class expect initialize to contain constructor logic.
+  initialize: Actor,
 
   toString: function () {
     return "[Actor " + this.typeName + "/" + this.actorID + "]";
   },
 
   _sendEvent: function (name, ...args) {
     if (!this._actorSpec.events.has(name)) {
       // It's ok to emit events that don't go over the wire.
@@ -1000,50 +1001,50 @@ var generateActorSpec = function (actorD
         actorSpec[name] = types.addDictType(actorDesc.typeName + "__" + name, desc.value);
       }
     }
 
     if (desc.value._methodSpec) {
       let methodSpec = desc.value._methodSpec;
       let spec = {};
       spec.name = methodSpec.name || name;
-      spec.request = Request(Object.assign({type: spec.name},
+      spec.request = new Request(Object.assign({type: spec.name},
                                           methodSpec.request || undefined));
-      spec.response = Response(methodSpec.response || undefined);
+      spec.response = new Response(methodSpec.response || undefined);
       spec.release = methodSpec.release;
       spec.oneway = methodSpec.oneway;
 
       actorSpec.methods.push(spec);
     }
   }
 
   // Find additional method specifications
   if (actorDesc.methods) {
     for (let name in actorDesc.methods) {
       let methodSpec = actorDesc.methods[name];
       let spec = {};
 
       spec.name = methodSpec.name || name;
-      spec.request = Request(Object.assign({type: spec.name},
+      spec.request = new Request(Object.assign({type: spec.name},
                                           methodSpec.request || undefined));
-      spec.response = Response(methodSpec.response || undefined);
+      spec.response = new Response(methodSpec.response || undefined);
       spec.release = methodSpec.release;
       spec.oneway = methodSpec.oneway;
 
       actorSpec.methods.push(spec);
     }
   }
 
   // Find event specifications
   if (actorDesc.events) {
     actorSpec.events = new Map();
     for (let name in actorDesc.events) {
       let eventRequest = actorDesc.events[name];
       Object.freeze(eventRequest);
-      actorSpec.events.set(name, Request(Object.assign({type: name}, eventRequest)));
+      actorSpec.events.set(name, new Request(Object.assign({type: name}, eventRequest)));
     }
   }
 
   if (!registeredTypes.has(actorSpec.typeName)) {
     types.addActorType(actorSpec.typeName);
   }
   registeredTypes.get(actorSpec.typeName).actorSpec = actorSpec;
 
@@ -1151,56 +1152,59 @@ exports.ActorClass = function (actorProt
  *    The actor prototype. Should have method definitions, can have event
  *    definitions.
  */
 var ActorClassWithSpec = function (actorSpec, actorProto) {
   if (!actorSpec.typeName) {
     throw Error("Actor specification must have a typeName member.");
   }
 
-  actorProto.extends = Actor;
-  let cls = Class(generateRequestHandlers(actorSpec, actorProto));
+  // Existing Actors are relying on the initialize instead of constructor methods.
+  let cls = function () {
+    let instance = Object.create(cls.prototype);
+    instance.initialize.apply(instance, arguments);
+    return instance;
+  };
+  cls.prototype = extend(Actor.prototype, generateRequestHandlers(actorSpec, actorProto));
 
   return cls;
 };
 exports.ActorClassWithSpec = ActorClassWithSpec;
 
 /**
  * Base class for client-side actor fronts.
+ *
+ * @param optional conn
+ *   Either a DebuggerServerConnection or a DebuggerClient.  Must have
+ *   addActorPool, removeActorPool, and poolFor.
+ *   conn can be null if the subclass provides a conn property.
+ * @param optional form
+ *   The json form provided by the server.
+ * @constructor
  */
-var Front = Class({
-  extends: Pool,
+var Front = function (conn = null, form = null, detail = null, context = null) {
+  Pool.call(this, conn);
+  this._requests = [];
 
+  // protocol.js no longer uses this data in the constructor, only external
+  // uses do.  External usage of manually-constructed fronts will be
+  // drastically reduced if we convert the root and tab actors to
+  // protocol.js, in which case this can probably go away.
+  if (form) {
+    this.actorID = form.actor;
+    form = types.getType(this.typeName).formType(detail).read(form, this, detail);
+    this.form(form, detail, context);
+  }
+};
+
+Front.prototype = extend(Pool.prototype, {
   actorID: null,
 
-  /**
-   * The base class for client-side actor fronts.
-   *
-   * @param optional conn
-   *   Either a DebuggerServerConnection or a DebuggerClient.  Must have
-   *   addActorPool, removeActorPool, and poolFor.
-   *   conn can be null if the subclass provides a conn property.
-   * @param optional form
-   *   The json form provided by the server.
-   * @constructor
-   */
-  initialize: function (conn = null, form = null, detail = null, context = null) {
-    Pool.prototype.initialize.call(this, conn);
-    this._requests = [];
-
-    // protocol.js no longer uses this data in the constructor, only external
-    // uses do.  External usage of manually-constructed fronts will be
-    // drastically reduced if we convert the root and tab actors to
-    // protocol.js, in which case this can probably go away.
-    if (form) {
-      this.actorID = form.actor;
-      form = types.getType(this.typeName).formType(detail).read(form, this, detail);
-      this.form(form, detail, context);
-    }
-  },
+  // Existing Fronts extending this class expect initialize to contain constructor logic.
+  initialize: Front,
 
   destroy: function () {
     // Reject all outstanding requests, they won't make sense after
     // the front is destroyed.
     while (this._requests && this._requests.length > 0) {
       let { deferred, to, type, stack } = this._requests.shift();
       let msg = "Connection closed, pending request to " + to +
                 ", type " + type + " failed" +
@@ -1338,16 +1342,17 @@ var Front = Class({
    *
    * @return Promise
    *         Resolved when all requests have settled.
    */
   waitForRequestsToSettle() {
     return settleAll(this._requests.map(({ deferred }) => deferred.promise));
   },
 });
+
 exports.Front = Front;
 
 /**
  * A method tagged with preEvent will be called after recieving a packet
  * for that event, and before the front emits the event.
  */
 exports.preEvent = function (eventName, fn) {
   fn._preEvent = eventName;
@@ -1503,18 +1508,23 @@ exports.FrontClass = function (actorType
  *
  * @param object actorSpec
  *    The actor specification you're creating a front for.
  * @param object proto
  *    The object prototype.  Must have a 'typeName' property,
  *    should have method definitions, can have event definitions.
  */
 var FrontClassWithSpec = function (actorSpec, frontProto) {
-  frontProto.extends = Front;
-  let cls = Class(generateRequestMethods(actorSpec, frontProto));
+  // Existing Fronts are relying on the initialize instead of constructor methods.
+  let cls = function () {
+    let instance = Object.create(cls.prototype);
+    instance.initialize.apply(instance, arguments);
+    return instance;
+  };
+  cls.prototype = extend(Front.prototype, generateRequestMethods(actorSpec, frontProto));
 
   if (!registeredTypes.has(actorSpec.typeName)) {
     types.addActorType(actorSpec.typeName);
   }
   registeredTypes.get(actorSpec.typeName).frontClass = cls;
 
   return cls;
 };