Dart Bindings: ApplicationConnection

Dart Bindings: ApplicationConnection

Replaced the Dart ServiceProvider class with a version
of ApplicationConnection that can both provide services
and connect to (request) them. ApplicationConnection
objects are returned by the Application ConnectToApplication()
method and they're passed to the Application AcceptConnection()
method.

R=zra@google.com

Committed: https://chromium.googlesource.com/external/mojo/+/1d2aa96f7db059aeb7ddc4243be68f7b34c333b5

[hansmuller1, 2015-02-18 21:00:48]

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

[hansmuller1, 2015-02-18 21:07:51]

hansmuller@google.com changed reviewers:
+ hansmuller@google.com

[hansmuller1, 2015-02-18 21:07:52]

PTAL

[zra, 2015-02-18 21:45:33]

Are the long lines coming from dartfmt?

Do you have a sense for what functionality is not well-tested, yet?

This change will also break the Dart apps in modular.

https://codereview.chromium.org/934253003/diff/1/mojo/public/dart/src/applica...
File mojo/public/dart/src/application.dart (right):

https://codereview.chromium.org/934253003/diff/1/mojo/public/dart/src/applica...
mojo/public/dart/src/application.dart:42: // - Optionally override
acceptConnection() if services are to be provided.
It looks like this comment is out of date now. Should we keep both this and the
comment above acceptConnection? Maybe just one of them is enough.  I kind of
like this comment up here as guidance for folks who are implementing an app, but
I don't have my heart set on it.

https://codereview.chromium.org/934253003/diff/1/mojo/public/dart/src/applica...
mojo/public/dart/src/application.dart:118: void acceptConnection(String
requestorUrl, ApplicationConnection connection) {}
This curly brace looks out of place.

https://codereview.chromium.org/934253003/diff/1/mojo/public/dart/src/applica...
File mojo/public/dart/src/application_connection.dart (right):

https://codereview.chromium.org/934253003/diff/1/mojo/public/dart/src/applica...
mojo/public/dart/src/application_connection.dart:20: void
connectToService(String interfaceName, core.MojoMessagePipeEndpoint pipe) {
Long line

https://codereview.chromium.org/934253003/diff/1/mojo/public/dart/src/applica...
mojo/public/dart/src/application_connection.dart:72: assert(!proxy.isBound &&
(_remoteServiceProvider != null) && _remoteServiceProvider.isBound);
Long line.

[hansmuller1, 2015-02-18 22:07:24]

> Are the long lines coming from dartfmt?

No, I just used it for the first time. As I understand it, git cl format will
DTRT soon.

> Do you have a sense for what functionality is not well-tested, yet?

The fallback service provider feature isn't being tested. Beyond that I'm not
sure. It looks like all of the JS app tests were ported to Dart and I think they
cover all the basic service sharing scenarios.

> This change will also break the Dart apps in modular.

I will roll and fix.

https://codereview.chromium.org/934253003/diff/1/mojo/public/dart/src/applica...
File mojo/public/dart/src/application.dart (right):

https://codereview.chromium.org/934253003/diff/1/mojo/public/dart/src/applica...
mojo/public/dart/src/application.dart:42: // - Optionally override
acceptConnection() if services are to be provided.
On 2015/02/18 21:45:33, zra wrote:
> It looks like this comment is out of date now. Should we keep both this and
the
> comment above acceptConnection? Maybe just one of them is enough.  I kind of
> like this comment up here as guidance for folks who are implementing an app,
but
> I don't have my heart set on it.

I revised the comment and removed the part about "using" an Application.  I'm
not entirely sure what that paragraph was getting at; I thought that the
ApplicationConnection class comment covered the same area.

https://codereview.chromium.org/934253003/diff/1/mojo/public/dart/src/applica...
mojo/public/dart/src/application.dart:118: void acceptConnection(String
requestorUrl, ApplicationConnection connection) {}
On 2015/02/18 21:45:33, zra wrote:
> This curly brace looks out of place.

Another long line. Fixed.

https://codereview.chromium.org/934253003/diff/1/mojo/public/dart/src/applica...
File mojo/public/dart/src/application_connection.dart (right):

https://codereview.chromium.org/934253003/diff/1/mojo/public/dart/src/applica...
mojo/public/dart/src/application_connection.dart:20: void
connectToService(String interfaceName, core.MojoMessagePipeEndpoint pipe) {
On 2015/02/18 21:45:33, zra wrote:
> Long line

Fixed

https://codereview.chromium.org/934253003/diff/1/mojo/public/dart/src/applica...
mojo/public/dart/src/application_connection.dart:72: assert(!proxy.isBound &&
(_remoteServiceProvider != null) && _remoteServiceProvider.isBound);
On 2015/02/18 21:45:33, zra wrote:
> Long line.

Sorry about that, I fixed it while you where reviewing the code.

[zra, 2015-02-18 22:13:20]

lgtm

[hansmuller, 2015-02-18 22:18:03]

Committed patchset #3 (id:40001) manually as
1d2aa96f7db059aeb7ddc4243be68f7b34c333b5 (presubmit successful).