Add interface versioning. Methods queryVersion and requireVersion.

mojo/public/js/lib/control_message_handler.js

+// Copyright 2017 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+define("mojo/public/js/lib/control_message_handler", [
+  "mojo/public/interfaces/bindings/interface_control_messages.mojom",
+  "mojo/public/js/validator",

[yzshen1, 2017/02/11 01:11:29]

order the list alphabetically, please.

[wangjimmy, 2017/02/13 20:43:06]

Done.

+  "mojo/public/js/codec",
+], function(interfaceCtrlMsgs, validator, codec) {

[yzshen1, 2017/02/11 01:11:29]

interfaceCtrlMsgs -> controlMessages ?

[wangjimmy, 2017/02/13 20:43:06]

Done.

+
+  var Validator = validator.Validator;
+
+  function validateControlRequestWithResponse(message) {
+    var messageValidator = new Validator(message);
+    var error = messageValidator.validateMessageIsRequestExpectingResponse();
+    if (error !== validator.validationError.NONE) {
+      throw error;
+    }
+
+    if (!message.getName() &&

[yzshen1, 2017/02/11 01:11:29]

!message.getName() seems wrong, I guess you mean message.getName()?
Do we need this check at all?

[wangjimmy, 2017/02/13 20:43:07]

Done.

+        message.getName() != interfaceCtrlMsgs.kRunMessageId) {
+      throw new Error('Control message name is not kRunMessageId');

[yzshen1, 2017/02/11 01:11:29]

We should probably use double quotes here (and below) to be consistent.

[yzshen1, 2017/02/11 01:11:29]

We also need to validate the payload of the message. 

[wangjimmy, 2017/02/13 20:43:06]

Done.

[wangjimmy, 2017/02/13 20:43:06]

Done.

+    }
+  }
+
+  function validateControlRequestWithoutResponse(message) {
+    var messageValidator = new Validator(message);
+    var error = messageValidator.validateMessageIsRequestWithoutResponse();
+    if (error != validator.validationError.NONE) {
+      throw error;
+    }
+
+    if (!message.getName() &&

[yzshen1, 2017/02/11 01:11:29]

see comment above.

[wangjimmy, 2017/02/13 20:43:06]

Done.

+        message.getName() != interfaceCtrlMsgs.kRunOrClosePipeMessageId) {
+      throw new Error('Control message name is not kRunOrClosePipeMessageId');

[yzshen1, 2017/02/11 01:11:28]

We also need to validate the payload.

[wangjimmy, 2017/02/13 20:43:06]

Done.

+    }
+  }
+
+  function runOrClosePipe(message, interface_version) {
+    var reader = new codec.MessageReader(message);
+    var runOrClosePipeMsgParams = reader.decodeStruct(
+      interfaceCtrlMsgs.RunOrClosePipeMessageParams);
+    return interface_version >= runOrClosePipeMsgParams.input

[yzshen1, 2017/02/11 01:11:29]

Formatting:

return interface_version >=
    runOrClosePipeMsgParams.input.require_version.version;

[wangjimmy, 2017/02/13 20:43:06]

Done.

+                                                       .require_version
+                                                       .version;
+  }
+
+  function run(message, responder, interface_version) {
+    var reader = new codec.MessageReader(message);
+    var runMsgParams = reader.decodeStruct(interfaceCtrlMsgs.RunMessageParams);
+    var runOutput = null;
+
+    if (runMsgParams.input.query_version) {
+      runOutput = new interfaceCtrlMsgs.RunOutput();
+      runOutput.query_version_result = new
+        interfaceCtrlMsgs.QueryVersionResult({'version': interface_version});

[yzshen1, 2017/02/11 01:11:28]

4 spaces.

[wangjimmy, 2017/02/13 20:43:06]

Done.

+    }
+
+    var runResponseMsgParams = new interfaceCtrlMsgs.RunResponseMessageParams();
+    runResponseMsgParams.output = runOutput;
+
+    var messageName = interfaceCtrlMsgs.kRunMessageId;
+    var payloadSize = interfaceCtrlMsgs.RunResponseMessageParams.encodedSize;
+    var requestID = reader.requestID;
+    var builder = new codec.MessageWithRequestIDBuilder(messageName,
+      payloadSize, codec.kMessageIsResponse, requestID);

[yzshen1, 2017/02/11 01:11:29]

4 spaces

[wangjimmy, 2017/02/13 20:43:06]

Done.

+    builder.encodeStruct(interfaceCtrlMsgs.RunResponseMessageParams,
+                         runResponseMsgParams);
+    responder.accept(builder.finish());
+    return true;
+  }
+
+  function isControlMessage(message) {
+    return message.getName() == interfaceCtrlMsgs.kRunMessageId ||
+           message.getName() == interfaceCtrlMsgs.kRunOrClosePipeMessageId;
+  }
+
+  function ControlMessageHandler(responder, interface_version) {

[yzshen1, 2017/02/11 01:11:28]

Maybe we should remove responder from the constructor? Please also see my
comment on line 88.

[wangjimmy, 2017/02/13 20:43:07]

Done.

+    this.responder = responder;
+    this.interface_version = interface_version;
+  }
+
+  ControlMessageHandler.prototype.accept = function(message) {
+    validateControlRequestWithoutResponse(message);
+    return runOrClosePipe(message, this.interface_version);
+  };
+
+  ControlMessageHandler.prototype.acceptWithResponder = function(message) {

[yzshen1, 2017/02/11 01:11:28]

The calling code pass a responder to this method, please see router.js.

[wangjimmy, 2017/02/13 20:43:06]

Done.

+    validateControlRequestWithResponse(message);
+    return run(message, this.responder, this.interface_version);
+  };
+
+  var exports = {};
+  exports.ControlMessageHandler = ControlMessageHandler;
+  exports.isControlMessage = isControlMessage;
+
+  return exports;
+});