Mojo JS Bindings: restore the ServiceProvider class, add an application test

Mojo JS Bindings: restore the ServiceProvider class, add an application test

Removed the mojo.js module and created a new one that just contains the ServiceProvider class.

Renamed the mojo bridge module and restored its quit method.

BUG=
R=aa@chromium.org

Committed: https://chromium.googlesource.com/external/mojo/+/3208471b175a9964b7b560d6090aa4100de401d3

[hansmuller, 2014-12-01 20:10:46]

hansmuller@chromium.org changed reviewers:
+ aa@chromium.org

[hansmuller, 2014-12-01 20:13:45]

Patchset #1 (id:1) has been deleted

[hansmuller, 2014-12-01 20:15:14]

Patchset #1 (id:20001) has been deleted

[hansmuller, 2014-12-01 20:18:04]

Patchset #1 (id:40001) has been deleted

[hansmuller, 2014-12-01 20:20:44]

This CL is incomplete because, as far as I can tell, the JS content handler
keeps the test application from exiting. Suggestions about how to make a
graceful exit would be appreciated.

[hansmuller, 2014-12-01 22:44:41]

This version works correctly with the test harness. The JS echo service is
explicitly quit after each test.

PTAL

[Aaron Boodman, 2014-12-02 01:13:22]

lgtm

https://codereview.chromium.org/767323002/diff/100001/mojo/services/public/js...
File mojo/services/public/js/service-provider.js (right):

https://codereview.chromium.org/767323002/diff/100001/mojo/services/public/js...
mojo/services/public/js/service-provider.js:5:
define("mojo/services/public/js/service-provider", [
We typically use underscores, not dashes, in filenames. Is there a reason to use
dash?

https://codereview.chromium.org/767323002/diff/100001/mojo/services/public/js...
mojo/services/public/js/service-provider.js:9: ], function(spModule,
connectionModule, coreModule,console) {
Missing space before 'console'.

https://codereview.chromium.org/767323002/diff/100001/services/js/js_apptests.cc
File services/js/js_apptests.cc (right):

https://codereview.chromium.org/767323002/diff/100001/services/js/js_apptests...
services/js/js_apptests.cc:57: echo_service_->EchoString("quit", callback);
So is the issue that ApplicationTestBase doesn't exit until all apps shut down
or something? Perhaps that should be fixed. Now that I see this, it seems
unfortunate that every test will have to ensure that everything it calls quits.

[hansmuller, 2014-12-02 15:55:01]

https://codereview.chromium.org/767323002/diff/100001/mojo/services/public/js...
File mojo/services/public/js/service-provider.js (right):

https://codereview.chromium.org/767323002/diff/100001/mojo/services/public/js...
mojo/services/public/js/service-provider.js:5:
define("mojo/services/public/js/service-provider", [
On 2014/12/02 01:13:22, Aaron Boodman wrote:
> We typically use underscores, not dashes, in filenames. Is there a reason to
use
> dash? 

JavaScript (and Sky) files typically use dashes. I'll change this post-facto if
you think Mojo filename consistency outweighs JS filename convention.

https://codereview.chromium.org/767323002/diff/100001/mojo/services/public/js...
mojo/services/public/js/service-provider.js:9: ], function(spModule,
connectionModule, coreModule,console) {
On 2014/12/02 01:13:22, Aaron Boodman wrote:
> Missing space before 'console'.

Oops, sorry about that. The console import was just there for debugging.

https://codereview.chromium.org/767323002/diff/100001/services/js/js_apptests.cc
File services/js/js_apptests.cc (right):

https://codereview.chromium.org/767323002/diff/100001/services/js/js_apptests...
services/js/js_apptests.cc:57: echo_service_->EchoString("quit", callback);
On 2014/12/02 01:13:22, Aaron Boodman wrote:
> So is the issue that ApplicationTestBase doesn't exit until all apps shut down
> or something? Perhaps that should be fixed. Now that I see this, it seems
> unfortunate that every test will have to ensure that everything it calls
quits.

I agree. And when tests/apps generate more complicated application graphs it's
going to be difficult to get the whole graph to gracefully shut down.

In this case, as I understand it, the JS content handler waits for all of the
threads it has launched to exit. A JS app will not exit until its message loop
is explicitly quit (I assume that's at least true of Mojo apps that start a
base::MessageLoop). I'm not sure how the explicit quit can be avoided. 


Maybe apps should automatically shutdown when their shell handle closes?
Assuming that there's some way to close the shell for all apps.

[hansmuller, 2014-12-02 15:55:52]

Committed patchset #4 (id:120001) manually as
3208471b175a9964b7b560d6090aa4100de401d3 (presubmit successful).