Add interface versioning. Methods queryVersion and requireVersion.

mojo/public/js/bindings.js

Index: mojo/public/js/bindings.js
diff --git a/mojo/public/js/bindings.js b/mojo/public/js/bindings.js
index 06075b513f1867f9fc8b4e612738b64a4ef0fda1..5e449ec6fa93179de8f113a89fadd97b75eddc88 100644
--- a/mojo/public/js/bindings.js
+++ b/mojo/public/js/bindings.js
@@ -6,7 +6,8 @@ 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) {
 
   // ---------------------------------------------------------------------------
 
@@ -31,6 +32,7 @@ define("mojo/public/js/bindings", [
     // |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);
@@ -110,13 +112,31 @@ define("mojo/public/js/bindings", [
     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);
+  };
 
   // ---------------------------------------------------------------------------
 
@@ -163,7 +183,7 @@ define("mojo/public/js/bindings", [
       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]);
   };