runat-1128988: Improve support for custom properties for commands

GCLI has allowed (by turning a blind eye) to commands and other items
that have additional properties. This is used by Firefox to allow
commands to be placed in the toolbox and toolbar buttons with custom
icons.

With remote GCLI it's important that these custom properties are
transferred along with the standard command metadata. This change allows
the 'specs' function to take an array of custom properties which should
be transferred.

Signed-off-by: Joe Walker <jwalker@mozilla.com>

Author: joewalker

lib/gcli/commands/commands.js

@@ -161,15 +161,19 @@ function Command(types, commandSpec) {
 
 /**
  * JSON serializer that avoids non-serializable data
+ * @param customProps Array of strings containing additional properties which,
+ * if specified in the command spec, will be included in the JSON. Normally we
+ * transfer only the properties required for GCLI to function.
  */
-Command.prototype.toJson = function() {
+Command.prototype.toJson = function(customProps) {
   var json = {
     item: 'command',
     name: this.name,
     params: this.params.map(function(param) { return param.toJson(); }),
     returnType: this.returnType,
     isParent: (this.exec == null)
   };
+
   if (this.description !==  l10n.lookup('canonDescNone')) {
     json.description = this.description;
   }
@@ -179,6 +183,15 @@ Command.prototype.toJson = function() {
   if (this.hidden != null) {
     json.hidden = this.hidden;
   }
+
+  if (Array.isArray(customProps)) {
+    customProps.forEach(function(prop) {
+      if (this[prop] != null) {
+        json[prop] = this[prop];
+      }
+    }.bind(this));
+  }
+
   return json;
 };
 
@@ -425,14 +438,17 @@ Commands.prototype.getAll = function() {
 /**
  * Get access to the stored commandMetaDatas (i.e. before they were made into
  * instances of Command/Parameters) so we can remote them.
+ * @param customProps Array of strings containing additional properties which,
+ * if specified in the command spec, will be included in the JSON. Normally we
+ * transfer only the properties required for GCLI to function.
  */
-Commands.prototype.getCommandSpecs = function() {
+Commands.prototype.getCommandSpecs = function(customProps) {
   var commandSpecs = [];
 
   Object.keys(this._commands).forEach(function(name) {
     var command = this._commands[name];
     if (!command.noRemote) {
-      commandSpecs.push(command.toJson());
+      commandSpecs.push(command.toJson(customProps));
     }
   }.bind(this));
 

lib/gcli/connectors/index.js

@@ -18,7 +18,7 @@
 
 var createSystem = require('../system').createSystem;
 var connectFront = require('../system').connectFront;
-var GcliFront = require('./connectors/remoted').GcliFront;
+var GcliFront = require('./remoted').GcliFront;
 
 // Patch-up IE9
 require('../util/legacy');

lib/gcli/connectors/remoted.js

@@ -78,11 +78,16 @@ Remoter.prototype.removeListener = function(action) {
 Remoter.prototype.exposed = {
   /**
    * Retrieve a list of the remotely executable commands
+   * @param customProps Array of strings containing additional properties which,
+   * if specified in the command spec, will be included in the JSON. Normally we
+   * transfer only the properties required for GCLI to function.
    */
-  specs: method(function() {
-    return this.requisition.system.commands.getCommandSpecs();
+  specs: method(function(customProps) {
+    return this.requisition.system.commands.getCommandSpecs(customProps);
   }, {
-    request: {},
+    request: {
+      customProps: Arg(0, "nullable:array:string")
+    },
     response: RetVal("json")
   }),
 

lib/gcli/system.js

@@ -17,6 +17,7 @@
 'use strict';
 
 var Promise = require('./util/promise').Promise;
+var util = require('./util/util');
 var Commands = require('./commands/commands').Commands;
 var Connectors = require('./connectors/connectors').Connectors;
 var Converters = require('./converters/converters').Converters;
@@ -242,34 +243,41 @@ exports.createSystem = function(options) {
 
 /**
  * Connect a local system with another at the other end of a connector
+ * @param system System to which we're adding commands
+ * @param front Front which allows access to the remote system from which we
+ * import commands
+ * @param customProps Array of strings specifying additional properties defined
+ * on remote commands that should be considered part of the metadata for the
+ * commands imported into the local system
  */
-exports.connectFront = function(system, front) {
+exports.connectFront = function(system, front, customProps) {
   front.on('commandsChanged', function(specs) {
-    syncItems(system, specs, front);
+    syncItems(system, front, customProps).then(null, util.errorHandler);
   });
 
-  return front.specs().then(function(specs) {
-    syncItems(system, specs, front);
-    return system;
-  });
+  return syncItems(system, front, customProps);
 };
 
 /**
  * Remove the items in this system that came from a previous sync action, and
- * re-add them
+ * re-add them. See connectFront() for explanation of properties
  */
-function syncItems(system, specs, front) {
-  // Go through all the commands removing any that are associated with the given
-  // front. The method of association is the hack in addLocalFunctions.
-  system.commands.getAll().forEach(function(command) {
-    if (command.front === front) {
-      system.commands.remove(command);
-    }
-  });
+function syncItems(system, front, customProps) {
+  return front.specs(customProps).then(function(specs) {
+    // Go through all the commands removing any that are associated with the
+    // given front. The method of association is the hack in addLocalFunctions.
+    system.commands.getAll().forEach(function(command) {
+      if (command.front === front) {
+        system.commands.remove(command);
+      }
+    });
 
-  var remoteItems = addLocalFunctions(specs, front);
-  system.addItems(remoteItems);
-}
+    var remoteItems = addLocalFunctions(specs, front);
+    system.addItems(remoteItems);
+
+    return system;
+  });
+};
 
 /**
  * Take the data from the 'specs' command (or the 'commandsChanged' event) and

lib/gcli/test/testCanon.js

@@ -195,6 +195,9 @@ exports.testAltCommands = function(options) {
       { name: 'num', type: 'number' },
       { name: 'opt', type: { name: 'selection', data: [ '1', '2', '3' ] } },
     ],
+    customProp1: 'localValue',
+    customProp2: true,
+    customProp3: 42,
     exec: function(args, context) {
       return context.commandName + ':' +
               args.str + ':' + args.num + ':' + args.opt;
@@ -211,6 +214,24 @@ exports.testAltCommands = function(options) {
             '],"isParent":false}]',
             'JSON.stringify(commandSpecs)');
 
+  var customProps = [ 'customProp1', 'customProp2', 'customProp3', ];
+  var commandSpecs2 = altCommands.getCommandSpecs(customProps);
+  assert.is(JSON.stringify(commandSpecs2),
+            '[{' +
+              '"item":"command",' +
+              '"name":"tss",' +
+              '"params":[' +
+                '{"name":"str","type":"string"},' +
+                '{"name":"num","type":"number"},' +
+                '{"name":"opt","type":{"name":"selection","data":["1","2","3"]}}' +
+              '],' +
+              '"isParent":false,' +
+              '"customProp1":"localValue",' +
+              '"customProp2":true,' +
+              '"customProp3":42' +
+            '}]',
+            'JSON.stringify(commandSpecs)');
+
   var remoter = function(args, context) {
     assert.is(context.commandName, 'tss', 'commandName is tss');