Dart: Futures -> Callbacks.

mojo/dart/packages/mojo/lib/src/proxy.dart

Index: mojo/dart/packages/mojo/lib/src/proxy.dart
diff --git a/mojo/dart/packages/mojo/lib/src/proxy.dart b/mojo/dart/packages/mojo/lib/src/proxy.dart
index c0576b5ae25812da3be8a3d49a02bf3d7da17a83..4870a5ebd93d6b24adf93fe638ce7381ec0b0b74 100644
--- a/mojo/dart/packages/mojo/lib/src/proxy.dart
+++ b/mojo/dart/packages/mojo/lib/src/proxy.dart
@@ -69,7 +69,7 @@ abstract class ServiceConnector {
 
 abstract class ProxyMessageHandler extends core.MojoEventHandler
                                    implements MojoInterfaceControl {
-  HashMap<int, Completer> _completerMap = new HashMap<int, Completer>();
+  HashMap<int, Function> _callbackMap = new HashMap<int, Function>();
   Completer _errorCompleter = new Completer();
   Set<Completer> _errorCompleters;
   int _nextId = 0;
@@ -131,9 +131,8 @@ abstract class ProxyMessageHandler extends core.MojoEventHandler
 
   @override
   Future close({bool immediate: false}) {
-    // Drop the completers for outstanding calls. The Futures will never
-    // complete.
-    _completerMap.clear();
+    // Drop the callbacks for outstanding calls. They will never be called.
+    _callbackMap.clear();
 
     // Signal to any pending calls that the ProxyMessageHandler is closed.
     if (_pendingCount > 0) {
@@ -160,11 +159,11 @@ abstract class ProxyMessageHandler extends core.MojoEventHandler
     }
   }
 
-  Future sendMessageWithRequestId(Struct message, int name, int id, int flags) {
-    var completer = new Completer();
+  void sendMessageWithRequestId(
+      Struct message, int name, int id, int flags, Function callback) {
     if (!isBound) {
-      proxyError("The ProxyMessageHandler is closed.");
-      return completer.future;
+      proxyError("The Proxy is closed.");
+      return;
     }
     if (!isOpen) {
       beginHandlingEvents();
@@ -179,16 +178,15 @@ abstract class ProxyMessageHandler extends core.MojoEventHandler
         serviceMessage.handles);
 
     if (endpoint.status == core.MojoResult.kOk) {
-      _completerMap[id] = completer;
+      _callbackMap[id] = callback;
       _pendingCount++;
     } else {
       proxyError("Write to message pipe endpoint failed: ${endpoint}");
     }
-    return completer.future;
   }
 
   // Need a getter for this for access in subclasses.
-  HashMap<int, Completer> get completerMap => _completerMap;
+  HashMap<int, Function> get callbackMap => _callbackMap;
 
   @override
   String toString() {
@@ -198,15 +196,19 @@ abstract class ProxyMessageHandler extends core.MojoEventHandler
 
   /// Queries the max version that the remote side supports.
   /// Updates [version].
-  Future<int> queryVersion() async {
+  Future<int> queryVersion() {
+    Completer<int> completer = new Completer<int>();
     var params = new icm.RunMessageParams();
     params.reserved0 = 16;
     params.reserved1 = 0;
     params.queryVersion = new icm.QueryVersion();
-    var response = await sendMessageWithRequestId(
-        params, icm.kRunMessageId, -1, MessageHeader.kMessageExpectsResponse);
-    _version = response.queryVersionResult.version;
-    return _version;
+    sendMessageWithRequestId(
+        params, icm.kRunMessageId,  -1, MessageHeader.kMessageExpectsResponse,
+        (r0, r1, queryResult) {
+      _version = queryResult.version;
+      completer.complete(_version);
+    });
+    return completer.future;
   }
 
   /// If the remote side doesn't support the [requiredVersion], it will close
@@ -287,22 +289,111 @@ abstract class ProxyMessageHandler extends core.MojoEventHandler
           "${message.header.type}");
       return;
     }
-
     var response = icm.RunResponseMessageParams.deserialize(message.payload);
     if (!message.header.hasRequestId) {
       proxyError("Expected a message with a valid request Id.");
       return;
     }
-    Completer c = completerMap[message.header.requestId];
-    if (c == null) {
+    Function callback = callbackMap[message.header.requestId];
+    if (callback == null) {
       proxyError("Message had unknown request Id: ${message.header.requestId}");
       return;
     }
-    completerMap.remove(message.header.requestId);
-    if (c.isCompleted) {
-      proxyError("Control message response completer already completed");
-      return;
-    }
-    c.complete(response);
+    callbackMap.remove(message.header.requestId);
+    callback(
+        response.reserved0, response.reserved1, response.queryVersionResult);
+    return;
+  }
+}
+
+// A class that acts like a function, but which completes a completer with the
+// the result of the function rather than returning the result. E.g.:
+//
+// Completer c = new Completer();
+// var completerator = new Completerator._(c, f);
+// completerator(a, b);
+// await c.future;
+//
+// This completes the future c with the result of passing a and b to f.
+//
+// More usefully for Mojo, e.g.:
+// await _Completerator.completerate(
+//     proxy.method, argList, MethodResponseParams#init);
+class _Completerator implements Function {
+  final Completer _c;
+  final Function _toComplete;
+
+  _Completerator._(this._c, this._toComplete);
+
+  static Future completerate(Function f, List args, Function ctor) {
+    Completer c = new Completer();
+    var newArgs = new List.from(args);
+    newArgs.add(new _Completerator._(c, ctor));

[Cutch, 2016/06/07 17:16:54]

What's the performance hit for using a _Completerator?

[zra, 2016/06/07 22:11:50]

Compared to the callback-based API it is the cost of redirecting twice through
noSuchMethod. So there's the recommended, fast callback API, and the slower,
not-recommended Future API.

+    Function.apply(f, newArgs);
+    return c.future;
   }
+
+  // Work-around to avoid checked-mode only having grudging support for
+  // Function implemented with noSuchMethod. See:
+  // https://github.com/dart-lang/sdk/issues/26528
+  dynamic call([

[Cutch, 2016/06/07 17:16:54]

whoa.

[zra, 2016/06/07 22:11:50]

Acknowledged.

+      dynamic a1, dynamic a2, dynamic a3, dynamic a4, dynamic a5,
+      dynamic a6, dynamic a7, dynamic a8, dynamic a9, dynamic a10,
+      dynamic a11, dynamic a12, dynamic a13, dynamic a14, dynamic a15,
+      dynamic a16, dynamic a17, dynamic a18, dynamic a19, dynamic a20]);
+
+  @override
+  dynamic noSuchMethod(Invocation invocation) =>
+      (invocation.memberName == #call)
+      ? _c.complete(Function.apply(_toComplete, invocation.positionalArguments))
+      : super.noSuchMethod(invocation);
+}
+
+/// Base class for Proxy class Futurizing wrappers. It turns callback-based
+/// methods on the Proxy into Future based methods in derived classes. E.g.:
+///
+/// class FuturizedHostResolverProxy extends FuturizedProxy<HostResolverProxy> {
+///   Map<Symbol, Function> _mojoMethods;
+///
+///   FuturizedHostResolverProxy(HostResolverProxy proxy) : super(proxy) {
+///     _mojoMethods = <Symbol, Function>{

[Cutch, 2016/06/07 17:16:54]

Can we get the code generator to generate FuturizedFooProxy for all mojo
interfaces? Hand coding this seems error prone (especially when the interface of
the mojom changes).

[zra, 2016/06/07 22:11:50]

I think the guidance will be that the callback-based API is what folks should
use, and the FuturizedProxy will be around for convenience only---maybe to use
only during porting to the new API. I'll add some warnings.

Since it's not the recommended way to use the bindings, I don't want to make it
/too/ attractive by generating code for it.

+///       #getHostAddresses: proxy.getHostAddresses,
+///     };
+///   }
+///   Map<Symbol, Function> get mojoMethods => _mojoMethods;
+///
+///   FuturizedHostResolverProxy.unbound() :
+///       this(new HostResolverProxy.unbound());
+///
+///   static final Map<Symbol, Function> _mojoResponses = {
+///     #getHostAddresses: new HostResolverGetHostAddressesResponseParams#init,
+///   };
+///   Map<Symbol, Function> get mojoResponses => _mojoResponses;
+/// }
+///
+/// Then:
+///
+/// HostResolveProxy proxy = ...
+/// var futurizedProxy = new FuturizedHostResolverProxy(proxy);
+/// var response = await futurizedProxy.getHostAddresses(host, family);
+/// // etc.
+abstract class FuturizedProxy<T extends Proxy> {
+  final T _proxy;
+  Map<Symbol, Function> get mojoMethods;
+  Map<Symbol, Function> get mojoResponses;
+
+  FuturizedProxy(T this._proxy);
+
+  T get proxy => _proxy;
+  Future responseOrError(Future f) => _proxy.responseOrError(f);
+  Future close({immediate: false}) => _proxy.close(immediate: immediate);
+
+  @override
+  dynamic noSuchMethod(Invocation invocation) =>
+      mojoMethods.containsKey(invocation.memberName)
+      ? _Completerator.completerate(
+            mojoMethods[invocation.memberName],
+            invocation.positionalArguments,
+            mojoResponses[invocation.memberName])
+      : super.noSuchMethod(invocation);
 }