mojo: Use ContentHandlers to bundle multiple Applications into a module.

mojo: Use ContentHandlers to bundle multiple Applications into a module.

The previous way of implementing core_services worked by having a
ServiceProvider as the external interface. This fell down because the
boundary between Applications in mojo is meaningful and is used in the
ViewManager and the WindowManager to restrict access to some services.

The (possibly misnamed) ContentHandler interface, on the other hand,
looks like a possible avenue of attack. ContentHandler's one method
starts new Applications. So mojo:core_services should be a
ContentHandler instead of a ServiceProvider and will spawn an
Application for each internal service.

This currently hard codes the mapping in context.cc, but eventually, we
should do something more intelligent for the packaging. This patch also
doesn't handle spawning new applications on new threads as we don't have
multiple applications yet.

BUG=477435
Committed: https://crrev.com/2cf387a31df4df41581982d28d964eb9e1607496
Cr-Commit-Position: refs/heads/master@{#327947}

[Elliot Glaysher, 2015-04-30 21:40:57]

erg@chromium.org changed reviewers:
+ ben@chromium.org, jam@chromium.org, sky@chromium.org

[Elliot Glaysher, 2015-04-30 21:40:57]

This is the most basic implementation of a ContentHandler based service bundle.

It does not deal with things like threads yet.

[sky, 2015-04-30 21:59:08]

https://codereview.chromium.org/1115363002/diff/1/components/core_services/co...
File components/core_services/core_services_application_delegate.cc (right):

https://codereview.chromium.org/1115363002/diff/1/components/core_services/co...
components/core_services/core_services_application_delegate.cc:36: new
mojo::ApplicationImpl(new clipboard::ClipboardApplicationDelegate,
I suspect this leaks. Can you add a TODO about tracking that down.

https://codereview.chromium.org/1115363002/diff/1/mojo/shell/application_mana...
File mojo/shell/application_manager.cc (right):

https://codereview.chromium.org/1115363002/diff/1/mojo/shell/application_mana...
mojo/shell/application_manager.cc:320:
LoadWithContentHandler(redirect_iter->second,
Doing it like you have here means multiple calls to ConnectToApplication() for
the same url (eg mojo:clipboard) will create multiple applications. For the
cases we care about, we don't want that. We still want only one mojo:clipboard.

[Elliot Glaysher, 2015-05-01 17:00:04]

ptal

https://codereview.chromium.org/1115363002/diff/1/components/core_services/co...
File components/core_services/core_services_application_delegate.cc (right):

https://codereview.chromium.org/1115363002/diff/1/components/core_services/co...
components/core_services/core_services_application_delegate.cc:36: new
mojo::ApplicationImpl(new clipboard::ClipboardApplicationDelegate,
On 2015/04/30 21:59:08, sky wrote:
> I suspect this leaks. Can you add a TODO about tracking that down.

Switched to smart ptrs.

https://codereview.chromium.org/1115363002/diff/1/mojo/shell/application_mana...
File mojo/shell/application_manager.cc (right):

https://codereview.chromium.org/1115363002/diff/1/mojo/shell/application_mana...
mojo/shell/application_manager.cc:320:
LoadWithContentHandler(redirect_iter->second,
On 2015/04/30 21:59:08, sky wrote:
> Doing it like you have here means multiple calls to ConnectToApplication() for
> the same url (eg mojo:clipboard) will create multiple applications. For the
> cases we care about, we don't want that. We still want only one
mojo:clipboard.

I don't believe this is the case. Above, RegisterShell creates a ShellImpl for
the url and we are just filling in the InterfaceRequest<Application>.

[Ben Goodger (Google), 2015-05-01 17:10:33]

https://codereview.chromium.org/1115363002/diff/20001/components/core_service...
File components/core_services/core_services_application_delegate.h (right):

https://codereview.chromium.org/1115363002/diff/20001/components/core_service...
components/core_services/core_services_application_delegate.h:17: class
BundledApplication;
???

https://codereview.chromium.org/1115363002/diff/20001/components/core_service...
components/core_services/core_services_application_delegate.h:46:
scoped_ptr<mojo::ApplicationImpl> clipboard_application_;
when you get this to spawn threads for applications, you will probably want to
stash this on some per-thread state object so this gets created/torn down on the
right thread & so that this code doesn't have to know about all of the apps.

https://codereview.chromium.org/1115363002/diff/20001/mojo/shell/application_...
File mojo/shell/application_manager.h (right):

https://codereview.chromium.org/1115363002/diff/20001/mojo/shell/application_...
mojo/shell/application_manager.h:95: void
RegisterApplicationPackageRedirect(const GURL& alias,
RegisterApplicationPackageAlias

https://codereview.chromium.org/1115363002/diff/20001/mojo/shell/application_...
mojo/shell/application_manager.h:137: using ApplicationPackagedRedirect =
std::map<GURL, GURL>;
ApplicationPackageAliasMap

[Elliot Glaysher, 2015-05-01 17:28:44]

https://codereview.chromium.org/1115363002/diff/20001/components/core_service...
File components/core_services/core_services_application_delegate.h (right):

https://codereview.chromium.org/1115363002/diff/20001/components/core_service...
components/core_services/core_services_application_delegate.h:17: class
BundledApplication;
On 2015/05/01 17:10:33, Ben Goodger (Google) wrote:
> ???

Removed stray mark.

https://codereview.chromium.org/1115363002/diff/20001/mojo/shell/application_...
File mojo/shell/application_manager.h (right):

https://codereview.chromium.org/1115363002/diff/20001/mojo/shell/application_...
mojo/shell/application_manager.h:95: void
RegisterApplicationPackageRedirect(const GURL& alias,
On 2015/05/01 17:10:33, Ben Goodger (Google) wrote:
> RegisterApplicationPackageAlias

Changed all 'redirect's to 'alias' here and in the implementation.

[Ben Goodger (Google), 2015-05-01 18:15:14]

lgtm

[Elliot Glaysher, 2015-05-01 18:16:58]

The CQ bit was checked by erg@chromium.org

[commit-bot: I haz the power, 2015-05-01 18:18:15]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1115363002/40001

[commit-bot: I haz the power, 2015-05-01 18:26:02]

Committed patchset #3 (id:40001)

[commit-bot: I haz the power, 2015-05-01 18:27:51]

Patchset 3 (id:??) landed as
https://crrev.com/2cf387a31df4df41581982d28d964eb9e1607496
Cr-Commit-Position: refs/heads/master@{#327947}