Add interface versioning. Methods queryVersion and requireVersion.

mojo/public/js/bindings.js

 // Copyright 2014 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/bindings", [
   "mojo/public/js/core",
   "mojo/public/js/interface_types",
   "mojo/public/js/router",
-], function(core, types, router) {
+  "mojo/public/js/lib/control_message_proxy",

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

alphabetically, please.

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

Done.

+], function(core, types, router, controlMessageProxy) {
 
   // ---------------------------------------------------------------------------
 
   function makeRequest(interfacePtr) {
     var pipe = core.createMessagePipe();
     interfacePtr.ptr.bind(new types.InterfacePtrInfo(pipe.handle0, 0));
     return new types.InterfaceRequest(pipe.handle1);
   }
 
   // ---------------------------------------------------------------------------
 
   // Operations used to setup/configure an interface pointer. Exposed as the
   // |ptr| field of generated interface pointer classes.
   // |ptrInfoOrHandle| could be omitted and passed into bind() later.
   function InterfacePtrController(interfaceType, ptrInfoOrHandle) {
     this.version = 0;
 
     this.interfaceType_ = interfaceType;
     this.router_ = null;
     this.proxy_ = null;
 
     // |router_| is lazily initialized. |handle_| is valid between bind() and
     // the initialization of |router_|.
     this.handle_ = null;
+    this.controlMessageProxy_ = new controlMessageProxy.ControlMessageProxy();

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

Please consider setting the value to null and moving creation of new object into
configureProxyIfNecessary_().

Because one common operation is to create an interface pointer and pass it over
a message pipe. In that case, creating the controlMessageProxy_ here is a little
wasteful. And you could also pass receiver as a constructor parameter instead of
directly accessing the member.

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

Done.

 
     if (ptrInfoOrHandle)
       this.bind(ptrInfoOrHandle);
   }
 
   InterfacePtrController.prototype.bind = function(ptrInfoOrHandle) {
     this.reset();
 
     if (ptrInfoOrHandle instanceof types.InterfacePtrInfo) {
       this.version = ptrInfoOrHandle.version;
@@ skipping 59 matching lines @@
     return this.router_.enableTestingMode();
   };
 
   InterfacePtrController.prototype.configureProxyIfNecessary_ = function() {
     if (!this.handle_)
       return;
 
     this.router_ = new router.Router(this.handle_);
     this.handle_ = null;
     this.router_ .setPayloadValidators([this.interfaceType_.validateResponse]);
+    this.controlMessageProxy_.receiver = this.router_;
 
     this.proxy_ = new this.interfaceType_.proxyClass(this.router_);
   };
 
-  // TODO(yzshen): Implement the following methods.
-  //   InterfacePtrController.prototype.queryVersion
-  //   InterfacePtrController.prototype.requireVersion
+  InterfacePtrController.prototype.queryVersion = function() {
+    function onQueryVersion(version) {
+      this.version = version;
+      return version;
+    }
+
+    this.configureProxyIfNecessary_();
+    return this.controlMessageProxy_.queryVersion().then(
+      onQueryVersion.bind(this));
+  };
+
+  InterfacePtrController.prototype.requireVersion = function(version) {
+    this.configureProxyIfNecessary_();
+
+    if (this.version >= version) {

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

Whether to have {} around single-line body is not enforced by the style guide.
But it is nice to be consistent with the surrounding code. See line 109 for
example.

Another thing to be noted is that if-else should have {} for both blocks or none
of them.

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

Done.

+      return;
+    }
+    this.version = version;
+    this.controlMessageProxy_.requireVersion(version);
+  };
 
   // ---------------------------------------------------------------------------
 
   // |request| could be omitted and passed into bind() later.
   //
   // Example:
   //
   //    // FooImpl implements mojom.Foo.
   //    function FooImpl() { ... }
   //    FooImpl.prototype.fooMethod1 = function() { ... }
@@ skipping 26 matching lines @@
 
   Binding.prototype.bind = function(requestOrHandle) {
     this.close();
 
     var handle = requestOrHandle instanceof types.InterfaceRequest ?
         requestOrHandle.handle : requestOrHandle;
     if (!core.isHandle(handle))
       return;
 
     this.stub_ = new this.interfaceType_.stubClass(this.impl_);
-    this.router_ = new router.Router(handle);
+    this.router_ = new router.Router(handle, this.interfaceType_.version);

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

|version| should be named kVersion since it is a constant.

The corresponding field in C++ doesn't follow the style guide. It should really
be kVersion in C++, but it is named Version_ for historical reasons.

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

Done.

     this.router_.setIncomingReceiver(this.stub_);
     this.router_ .setPayloadValidators([this.interfaceType_.validateRequest]);
   };
 
   Binding.prototype.close = function() {
     if (!this.isBound())
       return;
 
     this.router_.close();
     this.router_ = null;
@@ skipping 77 matching lines @@
   var exports = {};
   exports.InterfacePtrInfo = types.InterfacePtrInfo;
   exports.InterfaceRequest = types.InterfaceRequest;
   exports.makeRequest = makeRequest;
   exports.InterfacePtrController = InterfacePtrController;
   exports.Binding = Binding;
   exports.BindingSet = BindingSet;
 
   return exports;
 });