Add associated interfaces & bindings.

mojo/public/js/router.js

Index: mojo/public/js/router.js
diff --git a/mojo/public/js/router.js b/mojo/public/js/router.js
index 89d9a2f66b596f120c296aabb6e141c20f4a93d1..b4bd85c98451f89deb2299fe833969189b6a6478 100644
--- a/mojo/public/js/router.js
+++ b/mojo/public/js/router.js
@@ -74,11 +74,49 @@ define("mojo/public/js/router", [
     this.setInterfaceIdNamespaceBit_ = setInterfaceIdNamespaceBit;
     this.controlMessageHandler_ = new PipeControlMessageHandler(this);
     this.controlMessageProxy_ = new PipeControlMessageProxy(this.connector_);
-    this.nextInterfaceIdValue = 1;
+    this.nextInterfaceIdValue_ = 1;
     this.encounteredError_ = false;
     this.endpoints_ = new Map();
   }
 
+  Router.prototype.associateInterface = function(handleToSend) {
+    if (!handleToSend.pendingAssociation()) {
+      return types.kInvalidInterfaceId;
+    }
+
+    var id = 0;
+    do {
+      if (this.nextInterfaceIdValue_ >= types.kInterfaceIdNamespaceMask) {
+        this.nextInterfaceIdValue_ = 1;
+      }
+      id = this.nextInterfaceIdValue_++;
+      if (this.setInterfaceIdNamespaceBit_) {
+        id += types.kInterfaceIdNamespaceMask;
+      }
+    } while (this.endpoints_.has(id));
+
+    var endpoint = new InterfaceEndpoint(this, id);
+    this.endpoints_.set(id, endpoint);
+    if (this.encounteredError_) {
+      this.updateEndpointStateMayRemove(endpoint,
+          EndpointStateUpdateType.PEER_ENDPOINT_CLOSED);
+    }
+    endpoint.handleCreated = true;
+
+    if (!handleToSend.notifyAssociation(id, this)) {
+      // The peer handle of |handleToSend|, which is supposed to join this
+      // associated group, has been closed.
+      if (endpoint) {

[yzshen1, 2017/04/19 21:11:34]

No need to check "if (endpoint)".

[wangjimmy, 2017/04/20 15:36:44]

Done.

+        this.updateEndpointStateMayRemove(endpoint,
+            EndpointStateUpdateType.ENDPOINT_CLOSED);
+      }
+      pipeControlMessageproxy.notifyPeerEndpointClosed(id,
+          handleToSend.disconnectReason());
+    }
+
+    return id;
+  };
+
   Router.prototype.attachEndpointClient = function(
       interfaceEndpointHandle, interfaceEndpointClient) {
     check(types.isValidInterfaceId(interfaceEndpointHandle.id()));
@@ -149,21 +187,27 @@ define("mojo/public/js/router", [
     var ok = false;
     if (err !== validator.validationError.NONE) {
       validator.reportValidationError(err);
-    } else if (controlMessageHandler.isPipeControlMessage(message)) {
-      ok = this.controlMessageHandler_.accept(message);
     } else {
-      var interfaceId = message.getInterfaceId();
-      var endpoint = this.endpoints_.get(interfaceId);
-      if (!endpoint || endpoint.closed) {
-        return true;
+      if (!message.deserializeAssociatedEndpointHandles(this)) {
+        return false;

[yzshen1, 2017/04/19 21:11:34]

Do we need to also run the logic of line 213 - 215 before returning?

[wangjimmy, 2017/04/20 15:36:44]

In the C++ code

bool Connector::ReadSingleMessage(MojoResult* read_result) {
  receiver_result =
        incoming_receiver_ && incoming_receiver_->Accept(&message);
  if (enforce_errors_from_incoming_receiver_ && !receiver_result) {
    HandleError(true, false);
    return false;
  }
  return true;
}

bool MultiplexRouter::Accept(Message* message) {
  if (!message->DeserializeAssociatedEndpointHandles(this))
    return false;

  bool processed = tasks_.empty() && ProcessIncomingMessage(message, 
    client_call_behavior, connector_.task_runner());

  // Always return true. If we see errors during message processing, we will
  // explicitly call Connector::RaiseError() to disconnect the message pipe.
  return true;
}

// Which calls MultiplexRouter::ProcessIncomingMessage(message,..)

// These are the only cases where RaiseErrorInNonTestingMode() 
// (Connector::RaiseError()) calls gets called.

bool MultiplexRouter::ProcessIncomingMessage(message,..) {
  if (PipeControlMessageHandler::IsPipeControlMessage(message)) {
    result = control_message_handler_.Accept(message);
  if (!result)
      RaiseErrorInNonTestingMode();
  return true;
  }
  
  result = client->HandleIncomingMessage(message);
  if (!result)
    RaiseErrorInNonTestingMode();
  return true;
}

Since RaiseErrorInNonTestingMode() isn't called when you do
      if (!message.deserializeAssociatedEndpointHandles(this)) {
        return false;
      }

      if (!endpoint.client) {
        // We need to wait until a client is attached in order to dispatch
        // further messages.
        return false;
      }

Correct me if I'm wrong. We shouldn't disconnect the message pipe by calling
close() because RaiseErrorInNonTestingMode() is not called in the above 2 cases.
But since HandleError() in bool Connector::ReadSingleMessage is called because
of the returned false value we need to call the connection error handler.

I will change to code to this
      if (!message.deserializeAssociatedEndpointHandles(this)) {
        this.onPipeConnectionError();
        return false;
      }

      if (!endpoint.client) {
        // We need to wait until a client is attached in order to dispatch
        // further messages.
        this.onPipeConnectionError();
        return false;
      }

[yzshen1, 2017/04/20 18:12:13]

Please note that these two "return false" means different things:
- The first is in MultiplexRouter::Accept(), returning false means "the message
is invalid", which will cause HandleError() in Connector::ReadSingleMessage().
"Returning false" here is equivalent to "returning true &&
RaiseErrorInNonTestingMode()".
- The second is in ProcessIncomingMessage(), "returning false" means "the
message is queued and not yet processed".


So your change of adding this.onPipeConnectionError() to these two cases are
incorrect.
Please feel free to chat with me if you have further questions. Thanks!

       }
 
-      if (!endpoint.client) {
-        // We need to wait until a client is attached in order to dispatch
-        // further messages.
-        return false;
+      if (controlMessageHandler.isPipeControlMessage(message)) {
+        ok = this.controlMessageHandler_.accept(message);
+      } else {
+        var interfaceId = message.getInterfaceId();
+        var endpoint = this.endpoints_.get(interfaceId);
+        if (!endpoint || endpoint.closed) {
+          return true;
+        }
+
+        if (!endpoint.client) {
+          // We need to wait until a client is attached in order to dispatch
+          // further messages.

[yzshen1, 2017/04/19 21:11:34]

In this case, you are dropping the message.

Instead, you will need to cache this message and ask the underlying connector to
pause receiving messages.

If later a client is attached to an endpoint, you need to check whether the
cached message is targetting that endpoint, if yes, post a task to process that
cached message, and ask the underlying connector to resume.

[yzshen1, 2017/04/20 18:12:13]

In case you didn't notice: this comment has not been addressed. But I am fine if
you would like to add a TODO and address it in a separate CL.

+          return false;
+        }
+        ok = endpoint.client.handleIncomingMessage_(message, messageValidator);
       }
-      ok = endpoint.client.handleIncomingMessage_(message);
     }
 
     if (!ok) {