Remove Client relationship between mojo.Shell/mojo.Application

Remove Client relationship between mojo.Shell/mojo.Application

Instead of mojo.Shell and mojo.Application being clients of each other
they are now separate interfaces. An implementation of mojo.Application
receives a handle to the shell in its Initialize call, as described in
this doc:

https://docs.google.com/document/d/1xjt_TPjTu0elix8fNdBgWmnjJdJAtqSr1XDS_C-Ct8E/edit?usp=sharing

An analogous change is made to the content handler definition.

In C++, this is handled by the mojo::ApplicationImpl class which stores
shell handle in its implementation of Initialize. Thus the only change
for most C++ applications is the meaning of the handle in MojoMain is
different, although mains that use the ApplicationRunners do the same
thing with it.  Connecting to other apps is largely the same although
instead of grabbing the ApplicationImpl's client() to talk to the shell
code must now use the ApplicationImpl::shell() getter.

In JavaScript, the initialization sequence is a bit different although
this is hidden mostly in the Application class which calls initialize()
on its subclass with the same set of parameters. Connecting to another
application looks different, especially for sky code using the shell
proxy handle exposed via internals. Hans has some ideas about how to
make this a bit nicer.

Python apps similarly need to change their startup sequence a bit. I
didn't find a common library to take care of this dance, although it's
possible I just missed it.

Other languages probably a bit of reworking - I fixed everything here
that is covered by mojob.py test but some might be missing.

This patch also uses typed handles (i.e. InterfacePtr<> or
InterfaceRequest<> or their type aliases) wherever possible instead of
ScopedMessagePipeHandle for clarity.

R=davemoore@chromium.org

Committed: https://chromium.googlesource.com/external/mojo/+/e5ae9e408fb4f1b888a93502af65c6a9615a0ad1

[jamesr, 2015-01-26 23:08:01]

jamesr@chromium.org changed reviewers:
+ abarth@chromium.org, davemoore@chromium.org, hansmuller@chromium.org,
qsr@chromium.org, zra@google.com

[jamesr, 2015-01-26 23:08:02]

Dave - please review the C++ and //mojo/public/interfaces changes

Hans - please review the JS changes and TODOs I gave you

Zach (FYI only) - please take a look at the Dart changes. This compiles, but it
probably doesn't work correctly :).  I'd be happy to work on fixing things
asynchronously to this patch (since this patch is already kind of a beast)

qsr - Please take a look at the Python changes.  I'm pretty sure the example I
modified works (and it's covered by apptests) but the changes might not be
sufficient and I noticed some stuff that's not wired up.

Also, are there java things somewhere I should be testing?

[jamesr, 2015-01-26 23:19:49]

https://codereview.chromium.org/868463008/diff/20001/mojo/services/public/js/...
File mojo/services/public/js/application.js (right):

https://codereview.chromium.org/868463008/diff/20001/mojo/services/public/js/...
mojo/services/public/js/application.js:14: serviceProvider, shell) {
hah! line wrapping this (to make the 80col presubmit happy) breaks the sky amd
parser. hilarious...

[zra, 2015-01-26 23:27:01]

On 2015/01/26 23:08:02, jamesr wrote:
> Zach (FYI only) - please take a look at the Dart changes. This compiles, but
it
> probably doesn't work correctly :).  I'd be happy to work on fixing things
> asynchronously to this patch (since this patch is already kind of a beast)

The fix for Dart should be small and similar in nature to the JS fix. I'll patch
this
CL in and see if it's as easy as I think it is.

[jamesr, 2015-01-26 23:36:40]

OK, found the android stuff.  The android build now compiles and the tests pass
with 'mojob.py test --android'.

Zach - that sounds great.  Let me or Hans know if you hit any issues (Hans is
likely to be more useful, but I can try).

[DaveMoore, 2015-01-27 00:41:29]

C++ code LGTM

https://codereview.chromium.org/868463008/diff/40001/mojo/public/cpp/applicat...
File mojo/public/cpp/application/application_impl.h (right):

https://codereview.chromium.org/868463008/diff/40001/mojo/public/cpp/applicat...
mojo/public/cpp/application/application_impl.h:83: // Initialize.
Nit: This seems very specific to our test infrastructure. Are there other
reasons an app would do this? Perhaps more doc.

https://codereview.chromium.org/868463008/diff/40001/mojo/public/cpp/applicat...
mojo/public/cpp/application/application_impl.h:108: bool initialized_;
Nit: Can't initialized_ go away now?

[jamesr, 2015-01-27 00:59:04]

https://codereview.chromium.org/868463008/diff/40001/mojo/public/cpp/applicat...
File mojo/public/cpp/application/application_impl.h (right):

https://codereview.chromium.org/868463008/diff/40001/mojo/public/cpp/applicat...
mojo/public/cpp/application/application_impl.h:83: // Initialize.
On 2015/01/27 00:41:29, DaveMoore wrote:
> Nit: This seems very specific to our test infrastructure. Are there other
> reasons an app would do this? Perhaps more doc.

You could do this if you wanted to bind to another thread, for instance (since
our bindings associate you with the current thread's message loop).  I can try
to add some more documentation.

https://codereview.chromium.org/868463008/diff/40001/mojo/public/cpp/applicat...
mojo/public/cpp/application/application_impl.h:108: bool initialized_;
On 2015/01/27 00:41:29, DaveMoore wrote:
> Nit: Can't initialized_ go away now?

Ah right, yes.  It's redundant with shell_ now.

[jamesr, 2015-01-27 01:50:25]

Hans - gonna go ahead and land now to minimize churn on other patches.  You've
seen most of this, so hopefully it's OK-ish.

[jamesr, 2015-01-27 01:53:23]

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

[zra, 2015-01-27 15:44:24]

On 2015/01/27 01:53:23, jamesr wrote:
> Committed patchset #3 (id:40001) manually as
> e5ae9e408fb4f1b888a93502af65c6a9615a0ad1 (presubmit successful).

Fixing Dart stuff here: https://codereview.chromium.org/878883002/

[qsr, 2015-01-28 13:35:33]

On 2015/01/26 23:08:02, jamesr wrote:
> Dave - please review the C++ and //mojo/public/interfaces changes
> 
> Hans - please review the JS changes and TODOs I gave you
> 
> Zach (FYI only) - please take a look at the Dart changes. This compiles, but
it
> probably doesn't work correctly :).  I'd be happy to work on fixing things
> asynchronously to this patch (since this patch is already kind of a beast)
> 
> qsr - Please take a look at the Python changes.  I'm pretty sure the example I
> modified works (and it's covered by apptests) but the changes might not be
> sufficient and I noticed some stuff that's not wired up.

 Not sure to understand what you mean by not wiring up, the app in question
doesn't care about exposed_services, so it let it dies which will close the
handle.

> Also, are there java things somewhere I should be testing?

 No, there is no content handler in pure java for now.