Dart: Futures -> Callbacks.

Dart: Futures -> Callbacks.

As discussed in issue https://github.com/domokit/mojo/issues/777,
this change replaces the Future return types for Mojo interface
calls with callbacks.

fixes #777

R=johnmccutchan@google.com

Committed: https://chromium.googlesource.com/external/mojo/+/6efece8cde1702743c645eb6a1588d99578c0531

[zra, 2016-05-26 17:31:13]

Description was changed from

==========
Dart: Futures -> Callbacks.

Change templates, Proxy, Stub, no tests
==========

to

==========
Dart: Futures -> Callbacks.

As discussed in issue https://github.com/domokit/mojo/issues/777,
this change replaces the Future return types for Mojo interface
calls with callbacks.

fixes #777
==========

[P.Y.L., 2016-05-26 18:16:34]

https://codereview.chromium.org/2006093002/diff/60001/examples/dart/echo_clie...
File examples/dart/echo_client/lib/main.dart (right):

https://codereview.chromium.org/2006093002/diff/60001/examples/dart/echo_clie...
examples/dart/echo_client/lib/main.dart:30: c.compete(null);
complete

[zra, 2016-06-02 22:28:21]

zra@google.com changed reviewers:
+ johnmccutchan@google.com

[zra, 2016-06-02 22:28:24]

This is a breaking change that will require serious porting, but let's do the
code review, and then give plenty of lead time on the mailing list before
landing and pushing the Dart packages.

https://codereview.chromium.org/2006093002/diff/60001/examples/dart/echo_clie...
File examples/dart/echo_client/lib/main.dart (right):

https://codereview.chromium.org/2006093002/diff/60001/examples/dart/echo_clie...
examples/dart/echo_client/lib/main.dart:30: c.compete(null);
On 2016/05/26 18:16:34, pylaligand wrote:
> complete

Done.

[zra, 2016-06-07 14:53:19]

ping

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

Sorry for the delay! 

Initial set of comments below

https://codereview.chromium.org/2006093002/diff/90001/mojo/dart/embedder/io/m...
File mojo/dart/embedder/io/mojo_patch.dart (right):

https://codereview.chromium.org/2006093002/diff/90001/mojo/dart/embedder/io/m...
mojo/dart/embedder/io/mojo_patch.dart:85: _TcpBoundSocketProxy.unbound() :
this(new TcpBoundSocketProxy.unbound());
I haven't seen this syntax before.

": this" === ": _TcpBoundSocketProxy" ?

https://codereview.chromium.org/2006093002/diff/90001/mojo/dart/embedder/io/m...
mojo/dart/embedder/io/mojo_patch.dart:134: #read: proxy.read,
I worry about going through this abstraction for something like I/O. Seems like
the callback interface is better for performance. I'm fine with a TODO(zra):
Port to callback interface or something.

https://codereview.chromium.org/2006093002/diff/90001/mojo/dart/packages/mojo...
File mojo/dart/packages/mojo/lib/src/proxy.dart (right):

https://codereview.chromium.org/2006093002/diff/90001/mojo/dart/packages/mojo...
mojo/dart/packages/mojo/lib/src/proxy.dart:331: newArgs.add(new
_Completerator._(c, ctor));
What's the performance hit for using a _Completerator?

https://codereview.chromium.org/2006093002/diff/90001/mojo/dart/packages/mojo...
mojo/dart/packages/mojo/lib/src/proxy.dart:339: dynamic call([
whoa.

https://codereview.chromium.org/2006093002/diff/90001/mojo/dart/packages/mojo...
mojo/dart/packages/mojo/lib/src/proxy.dart:359: ///     _mojoMethods = <Symbol,
Function>{
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]

https://codereview.chromium.org/2006093002/diff/90001/mojo/dart/embedder/io/m...
File mojo/dart/embedder/io/mojo_patch.dart (right):

https://codereview.chromium.org/2006093002/diff/90001/mojo/dart/embedder/io/m...
mojo/dart/embedder/io/mojo_patch.dart:85: _TcpBoundSocketProxy.unbound() :
this(new TcpBoundSocketProxy.unbound());
On 2016/06/07 17:16:54, Cutch wrote:
> I haven't seen this syntax before.
> 
> ": this" === ": _TcpBoundSocketProxy" ?

Yah. You can redirect a constructor to another constructor.

https://codereview.chromium.org/2006093002/diff/90001/mojo/dart/embedder/io/m...
mojo/dart/embedder/io/mojo_patch.dart:134: #read: proxy.read,
On 2016/06/07 17:16:54, Cutch wrote:
> I worry about going through this abstraction for something like I/O. Seems
like
> the callback interface is better for performance. I'm fine with a TODO(zra):
> Port to callback interface or something.

It probably depends on the IO latency whether a few (dozen?) extra instructions
to redirect through noSuchMethod is significant.

Added TODO

https://codereview.chromium.org/2006093002/diff/90001/mojo/dart/packages/mojo...
File mojo/dart/packages/mojo/lib/src/proxy.dart (right):

https://codereview.chromium.org/2006093002/diff/90001/mojo/dart/packages/mojo...
mojo/dart/packages/mojo/lib/src/proxy.dart:331: newArgs.add(new
_Completerator._(c, ctor));
On 2016/06/07 17:16:54, Cutch wrote:
> What's the performance hit for using a _Completerator?

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.

https://codereview.chromium.org/2006093002/diff/90001/mojo/dart/packages/mojo...
mojo/dart/packages/mojo/lib/src/proxy.dart:339: dynamic call([
On 2016/06/07 17:16:54, Cutch wrote:
> whoa.

Acknowledged.

https://codereview.chromium.org/2006093002/diff/90001/mojo/dart/packages/mojo...
mojo/dart/packages/mojo/lib/src/proxy.dart:359: ///     _mojoMethods = <Symbol,
Function>{
On 2016/06/07 17:16:54, Cutch wrote:
> 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).

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.

[Cutch, 2016-06-07 22:23:53]

lgtm

[zra, 2016-06-09 17:02:08]

Description was changed from

==========
Dart: Futures -> Callbacks.

As discussed in issue https://github.com/domokit/mojo/issues/777,
this change replaces the Future return types for Mojo interface
calls with callbacks.

fixes #777
==========

to

==========
Dart: Futures -> Callbacks.

As discussed in issue https://github.com/domokit/mojo/issues/777,
this change replaces the Future return types for Mojo interface
calls with callbacks.

fixes #777

R=johnmccutchan@google.com

Committed:
https://chromium.googlesource.com/external/mojo/+/6efece8cde1702743c645eb6a15...
==========

[zra, 2016-06-09 17:02:10]

Committed patchset #13 (id:120001) manually as
6efece8cde1702743c645eb6a1588d99578c0531 (presubmit successful).