Mojo JS Bindings: Simplify sharing services for content-provided JS applications

mojo/apps/js/mojo.js

Index: mojo/apps/js/mojo.js
diff --git a/mojo/apps/js/mojo.js b/mojo/apps/js/mojo.js
index 23bacffa5290bf65f29576b6c5cdec8aa0563fc3..4970f29a1d8eeec790829e8920dfa6e3e82be3e9 100644
--- a/mojo/apps/js/mojo.js
+++ b/mojo/apps/js/mojo.js
@@ -3,24 +3,162 @@
 // found in the LICENSE file.
 
 define("mojo/apps/js/mojo", [
+  "mojo/public/interfaces/application/service_provider.mojom",
   "mojo/public/js/bindings/connection",
-  "mojo/apps/js/bridge",
-], function(connection, mojo) {
+  "mojo/public/js/bindings/core",
+  "mojo/apps/js/bridge", "console"

[Aaron Boodman, 2014/10/24 22:17:19]

Put each import on own line for readability.

[hansmuller, 2014/10/27 22:43:22]

Sorry, the console import shouldn't have been there; that was a leftover from
debugging.

+], function(service, connection, core, bridge, console) {
 
-  function connectToService(url, service, client) {
-    var serviceHandle = mojo.connectToService(url, service.name);
+  function Shell() {
+    this.applications_ = new Map();
+  }
+
+  Shell.prototype.connectToApplication = function(url) {
+    var application = this.applications_.get(url);
+    if (application)
+      return application;

[Aaron Boodman, 2014/10/24 22:17:19]

hm. This is different than the C++ model where every call to connect() yields a
new SP and a new pipe. Have you thought through the consequences of that?

[hansmuller, 2014/10/27 22:43:22]

I don't understand the implications in terms of existing applications; are there
(C++) applications that depend on getting a new SP for each connectToApplication
call?

Having the application at particular URL be a singleton from the client's
perspective does make intuitive sense to me.  We're planning to make connecting
to the same url,name service act that way, so wouldn't it be inconsistent to
treat applications differently?

+    application = new ServiceProvider(bridge.connectToApplication(url));
+    this.applications_.set(url, application);
+    return application;
+  };
+
+  Shell.prototype.connectToService = function (url, service, client) {
+    return this.connectToApplication(url).connectToService(service, client);
+  };
+
+  Shell.prototype.close = function() {
+    shell().applications_.forEach(function(application, url) {
+      application.close();
+    });
+    shell().applications_.clear();
+  };
+
+  var shell_ = null;

[Aaron Boodman, 2014/10/24 22:17:19]

I have not seen the convention of trailing underscore for locals. Have we
adopted it? I am only used to seeing that on fields of an object which are
pseudo-private.

[hansmuller, 2014/10/27 22:43:22]

I've replaced the underscores with a Value suffix.

+
+  function shell() {
+    if (!shell_)
+      shell_ = new Shell();
+    return shell_;
+  }
+
+  var requestor_ = null;
+
+  function requestor() {
+    if (!requestor_) {
+      var handle = bridge.requestorMessagePipeHandle();
+      requestor_ = handle && new ServiceProvider(handle);
+    }
+    return requestor_;
+  }
+
+  function connectToServiceImpl(serviceName, serviceHandle) {
+    var provider = this.providers_.get(serviceName);
+    if (!provider) {
+      this.pendingRequests_.set(serviceName, serviceHandle);
+      return;
+    }
+
+    var serviceConnection = new connection.Connection(
+      serviceHandle,
+      provider.service.delegatingStubClass,
+      provider.service.client && provider.service.client.proxyClass);
+
+    serviceConnection.local.connection$ = serviceConnection;
+    serviceConnection.local.delegate$ =
+      new provider.factory(serviceConnection.remote);
+
+    provider.connections.push(serviceConnection);
+  }
+
+  function ServiceProvider(messagePipeHandle) {
+    // TODO(hansmuller): if messagePipeHandle is null, throw an exception.
+    this.initFields();
+    this.handle_ = messagePipeHandle;
+    this.connection_ =  new connection.Connection(
+      this.handle_,
+      service.ServiceProvider.client.delegatingStubClass,
+      service.ServiceProvider.proxyClass);
+    this.connection_.local.delegate$ = {
+      connectToService: connectToServiceImpl.bind(this)
+    };
+  }
+
+  ServiceProvider.prototype.initFields = function() {
+    this.connections_ = new Map();
+    this.providers_ = new Map();
+    this.pendingRequests_ = new Map();
+    this.handle_ = null;
+    this.connection_ = null;
+  };
+
+  ServiceProvider.prototype.provideService = function(service, factory) {
+    // TODO(hansmuller): if !factory, remove provider and close its connections.
+    var provider = {
+      service: service,
+      factory: factory,
+      connections: [],
+    };
+    this.providers_.set(service.name, provider);
+
+    if (this.pendingRequests_.has(service.name)) {
+      connectToServiceImpl(service.name, pendingRequests_.get(service.name));
+      pendingRequests_.delete(service.name);
+    }
+
+    return this;
+  };
+
+  ServiceProvider.prototype.connectToService = function(service, client) {
+    // TODO(hansmuler): if service.name isn't defined, throw an exception
+    var serviceConnection = this.connections_.get(service.name);
+    if (serviceConnection)
+      return serviceConnection.remote;
+
+    var pipe = core.createMessagePipe();
+    this.connection_.remote.connectToService(service.name, pipe.handle1);
     var clientClass = client && service.client.delegatingStubClass;
     var serviceConnection =
-      new connection.Connection(serviceHandle, clientClass, service.proxyClass);
+      new connection.Connection(pipe.handle0, clientClass, service.proxyClass);
     if (serviceConnection.local)
       serviceConnection.local.delegate$ = client;
-    serviceConnection.remote.connection$ = serviceConnection;
+
+    this.connections_.set(service.name, serviceConnection);
     return serviceConnection.remote;
+  };
+
+  ServiceProvider.prototype.close = function() {
+    try {
+      // Outgoing connections
+      this.connections_.forEach(function(connection, serviceName) {
+        connection.close();
+      });
+      // Incoming connections
+      this.providers_.forEach(function(provider, serviceName) {
+        provider.connections.forEach(function(connection) {
+          connection.close();
+        });
+      });
+      this.connection_.close();
+    } finally {
+      this.initFields();

[Aaron Boodman, 2014/10/24 22:17:19]

Almost seems like instead all the fields should become null so that it is an
error to try to use this object after close().

[hansmuller, 2014/10/27 22:43:22]

Good point; the objective here was just to drop all of the internal references,
not to enable re-use.

+      shell().applications_.forEach(function(application, url) {
+        if (application === this)
+          shell().applications_.delete(url);
+      }, this);
+    }
+  };
+
+  function quit() {
+    if (requestor_)
+      requestor().close();
+    if (shell_)
+      shell().close();
+    bridge.quit();
   }
 
   var exports = {};
-  exports.connectToService = connectToService;
-  exports.quit = mojo.quit;
+  exports.requestor = requestor;
+  exports.shell = shell;
+  exports.quit = quit;
   return exports;
 });
-