Introduce intent manager.

Introduce intent manager.

The intent manager is an Android specific service that allows privileged
android mojo application to listen to external intents.

R=ppi@chromium.org

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

[qsr, 2015-04-07 14:01:20]

qsr@chromium.org changed reviewers:
+ alhaad@chromium.org, ppi@chromium.org

[qsr, 2015-04-07 14:02:50]

alhaad@chromium.org: This should allow you to use intent in the mojo world. See
the example in Location for geo-fencing. Please not that this only works with
non active intent. If the intent contains a Binder, that won't work.

ppi@chromium.org: For review, please do not look at anything in
services/location as this is just an example that will be removed before
pushing.

[ppi, 2015-04-07 15:42:14]

https://codereview.chromium.org/1061313003/diff/1/mojo/public/java/applicatio...
File
mojo/public/java/application/src/org/chromium/mojo/application/ApplicationHelper.java
(right):

https://codereview.chromium.org/1061313003/diff/1/mojo/public/java/applicatio...
mojo/public/java/application/src/org/chromium/mojo/application/ApplicationHelper.java:18:
public class ApplicationHelper {
just a thought - would it be fitting to call it more precisely "ShellHelper",
helping to get apps out of shell?

https://codereview.chromium.org/1061313003/diff/1/mojo/public/java/applicatio...
mojo/public/java/application/src/org/chromium/mojo/application/ApplicationHelper.java:31:
Pair<ServiceProvider.Proxy, InterfaceRequest<ServiceProvider>> ir =
how about calling this |providerRequest|?

https://codereview.chromium.org/1061313003/diff/1/mojo/public/java/applicatio...
mojo/public/java/application/src/org/chromium/mojo/application/ApplicationHelper.java:33:
try (ServiceProvider.Proxy spp = ir.first) {
how about calling this |provider|?

https://codereview.chromium.org/1061313003/diff/1/mojo/public/java/applicatio...
mojo/public/java/application/src/org/chromium/mojo/application/ApplicationHelper.java:35:
Pair<P, InterfaceRequest<I>> fir = manager.getInterfaceRequest(core);
how about calling this |serviceRequest|?

https://codereview.chromium.org/1061313003/diff/1/services/android/intent.mojom
File services/android/intent.mojom (right):

https://codereview.chromium.org/1061313003/diff/1/services/android/intent.moj...
services/android/intent.mojom:8: interface IntentReceiver {
Could you add comments for these interfaces?

https://codereview.chromium.org/1061313003/diff/1/services/android/intent.moj...
services/android/intent.mojom:13: GetIntent(IntentReceiver receiver) =>
(array<uint8> intent);
In particular, would that be a good place to document that it will not work for
intents with Binders?

https://codereview.chromium.org/1061313003/diff/1/services/android/intent.moj...
services/android/intent.mojom:13: GetIntent(IntentReceiver receiver) =>
(array<uint8> intent);
I feel there's too many different meaning of "intent" here. We call GetIntent
and we both get an intent back *and* we are passing a receiver for intents?

I wonder if we should clarify this by splitting into methods:
GetIntent() => (array<uint8> intent)
ReceiveIntents(array<uint8> intent, IntentReceiver intent);

wdyt?

https://codereview.chromium.org/1061313003/diff/1/shell/android/apk/src/org/c...
File shell/android/apk/src/org/chromium/mojo/shell/IntentReceiverRegistry.java
(right):

https://codereview.chromium.org/1061313003/diff/1/shell/android/apk/src/org/c...
shell/android/apk/src/org/chromium/mojo/shell/IntentReceiverRegistry.java:20: *
Java class for services/android/intent.mojom. Create intent for privileged mojo
applications and
The first sentence feels ambiguous. Please indicate if this is implementation of
an interface defined in intent.mojom. If not, please clarify how exactly (e.g.
through what intermediaries) this relates to intent.mojom.

https://codereview.chromium.org/1061313003/diff/1/shell/android/apk/src/org/c...
shell/android/apk/src/org/chromium/mojo/shell/IntentReceiverRegistry.java:36:
private final Map<String, Long> mReceiversByUUID = new HashMap<>();
Should this be mReceiversByUuid to match the style below?

https://codereview.chromium.org/1061313003/diff/1/shell/android/intent_manage...
File shell/android/intent_manager_factory.cc (right):

https://codereview.chromium.org/1061313003/diff/1/shell/android/intent_manage...
shell/android/intent_manager_factory.cc:7: #include "base/logging.h"
is this used here?

https://codereview.chromium.org/1061313003/diff/1/shell/android/intent_manage...
shell/android/intent_manager_factory.cc:8: #include
"mojo/public/cpp/bindings/strong_binding.h"
is this used here?

https://codereview.chromium.org/1061313003/diff/1/shell/android/intent_manage...
File shell/android/intent_manager_impl.cc (right):

https://codereview.chromium.org/1061313003/diff/1/shell/android/intent_manage...
shell/android/intent_manager_impl.cc:26: :
intent_receiver_(intent_receiver.Pass()) {
should we call Java_IntentReceiverRegistry_registerReceiver here instead of
outside the class?

https://codereview.chromium.org/1061313003/diff/1/shell/android/intent_manage...
File shell/android/intent_manager_impl.h (right):

https://codereview.chromium.org/1061313003/diff/1/shell/android/intent_manage...
shell/android/intent_manager_impl.h:15: class IntentManagerImpl : public
intent::IntentManager {
Why this has to be implemented in C++ (ie. why not Java?)

https://codereview.chromium.org/1061313003/diff/1/shell/android/intent_manage...
shell/android/intent_manager_impl.h:20: void GetIntent(intent::IntentReceiverPtr
receiver,
could you indicate the interface we're overriding in a comment?

[qsr, 2015-04-08 10:04:14]

https://codereview.chromium.org/1061313003/diff/1/mojo/public/java/applicatio...
File
mojo/public/java/application/src/org/chromium/mojo/application/ApplicationHelper.java
(right):

https://codereview.chromium.org/1061313003/diff/1/mojo/public/java/applicatio...
mojo/public/java/application/src/org/chromium/mojo/application/ApplicationHelper.java:18:
public class ApplicationHelper {
On 2015/04/07 15:42:13, ppi wrote:
> just a thought - would it be fitting to call it more precisely "ShellHelper",
> helping to get apps out of shell?

Done.

https://codereview.chromium.org/1061313003/diff/1/mojo/public/java/applicatio...
mojo/public/java/application/src/org/chromium/mojo/application/ApplicationHelper.java:31:
Pair<ServiceProvider.Proxy, InterfaceRequest<ServiceProvider>> ir =
On 2015/04/07 15:42:13, ppi wrote:
> how about calling this |providerRequest|?

Done.

https://codereview.chromium.org/1061313003/diff/1/mojo/public/java/applicatio...
mojo/public/java/application/src/org/chromium/mojo/application/ApplicationHelper.java:33:
try (ServiceProvider.Proxy spp = ir.first) {
On 2015/04/07 15:42:13, ppi wrote:
> how about calling this |provider|?

Done.

https://codereview.chromium.org/1061313003/diff/1/mojo/public/java/applicatio...
mojo/public/java/application/src/org/chromium/mojo/application/ApplicationHelper.java:35:
Pair<P, InterfaceRequest<I>> fir = manager.getInterfaceRequest(core);
On 2015/04/07 15:42:13, ppi wrote:
> how about calling this |serviceRequest|?

Done.

https://codereview.chromium.org/1061313003/diff/1/services/android/intent.mojom
File services/android/intent.mojom (right):

https://codereview.chromium.org/1061313003/diff/1/services/android/intent.moj...
services/android/intent.mojom:8: interface IntentReceiver {
On 2015/04/07 15:42:14, ppi wrote:
> Could you add comments for these interfaces?

Done.

https://codereview.chromium.org/1061313003/diff/1/services/android/intent.moj...
services/android/intent.mojom:13: GetIntent(IntentReceiver receiver) =>
(array<uint8> intent);
On 2015/04/07 15:42:13, ppi wrote:
> In particular, would that be a good place to document that it will not work
for
> intents with Binders?

Done.

https://codereview.chromium.org/1061313003/diff/1/services/android/intent.moj...
services/android/intent.mojom:13: GetIntent(IntentReceiver receiver) =>
(array<uint8> intent);
On 2015/04/07 15:42:14, ppi wrote:
> I feel there's too many different meaning of "intent" here. We call GetIntent
> and we both get an intent back *and* we are passing a receiver for intents?
> 
> I wonder if we should clarify this by splitting into methods:
> GetIntent() => (array<uint8> intent)
> ReceiveIntents(array<uint8> intent, IntentReceiver intent);
> 
> wdyt?

 As discussed, the intent received is the one that will trigger the passed
received. And passing intent back and forth is really inneficient (they use 400
bytes or something). Changed the name and add comment to make it easier to
understand.

https://codereview.chromium.org/1061313003/diff/1/shell/android/apk/src/org/c...
File shell/android/apk/src/org/chromium/mojo/shell/IntentReceiverRegistry.java
(right):

https://codereview.chromium.org/1061313003/diff/1/shell/android/apk/src/org/c...
shell/android/apk/src/org/chromium/mojo/shell/IntentReceiverRegistry.java:20: *
Java class for services/android/intent.mojom. Create intent for privileged mojo
applications and
On 2015/04/07 15:42:14, ppi wrote:
> The first sentence feels ambiguous. Please indicate if this is implementation
of
> an interface defined in intent.mojom. If not, please clarify how exactly (e.g.
> through what intermediaries) this relates to intent.mojom.

Done.

https://codereview.chromium.org/1061313003/diff/1/shell/android/apk/src/org/c...
shell/android/apk/src/org/chromium/mojo/shell/IntentReceiverRegistry.java:36:
private final Map<String, Long> mReceiversByUUID = new HashMap<>();
On 2015/04/07 15:42:14, ppi wrote:
> Should this be mReceiversByUuid to match the style below?

Done.

https://codereview.chromium.org/1061313003/diff/1/shell/android/intent_manage...
File shell/android/intent_manager_factory.cc (right):

https://codereview.chromium.org/1061313003/diff/1/shell/android/intent_manage...
shell/android/intent_manager_factory.cc:7: #include "base/logging.h"
Removed

https://codereview.chromium.org/1061313003/diff/1/shell/android/intent_manage...
shell/android/intent_manager_factory.cc:8: #include
"mojo/public/cpp/bindings/strong_binding.h"
Removed

https://codereview.chromium.org/1061313003/diff/1/shell/android/intent_manage...
File shell/android/intent_manager_impl.cc (right):

https://codereview.chromium.org/1061313003/diff/1/shell/android/intent_manage...
shell/android/intent_manager_impl.cc:26: :
intent_receiver_(intent_receiver.Pass()) {
On 2015/04/07 15:42:14, ppi wrote:
> should we call Java_IntentReceiverRegistry_registerReceiver here instead of
> outside the class?

 It is kind of hard, as registerReceiver is returning an intent, and a
constructor cannot return anything. I would need to keep the intent as an
internal member with a pass method, which is kind of artificial.

https://codereview.chromium.org/1061313003/diff/1/shell/android/intent_manage...
File shell/android/intent_manager_impl.h (right):

https://codereview.chromium.org/1061313003/diff/1/shell/android/intent_manage...
shell/android/intent_manager_impl.h:15: class IntentManagerImpl : public
intent::IntentManager {
On 2015/04/07 15:42:14, ppi wrote:
> Why this has to be implemented in C++ (ie. why not Java?)

Because we have currently no way to register and use mojo service in java in the
shell itself. As this is currently the only usage, I won't setup the full needed
infrastructure. As suggested, I opened a bug, and we can go back to this when
needed.

https://codereview.chromium.org/1061313003/diff/1/shell/android/intent_manage...
shell/android/intent_manager_impl.h:20: void GetIntent(intent::IntentReceiverPtr
receiver,
On 2015/04/07 15:42:14, ppi wrote:
> could you indicate the interface we're overriding in a comment?

Done.

[ppi, 2015-04-08 10:42:49]

https://codereview.chromium.org/1061313003/diff/20001/services/android/intent...
File services/android/intent.mojom (right):

https://codereview.chromium.org/1061313003/diff/20001/services/android/intent...
services/android/intent.mojom:8: // Service to interract with the android's
intents.
s/interract/interact/

https://codereview.chromium.org/1061313003/diff/20001/services/android/intent...
services/android/intent.mojom:8: // Service to interract with the android's
intents.
s/the android's intents/android intents/

https://codereview.chromium.org/1061313003/diff/20001/services/android/intent...
services/android/intent.mojom:9: interface IntentManager {
wdyt about calling this IntentReceiverManager and the entire thing
intent_receiver.mojom? "intent" seems broad given what we support here.

https://codereview.chromium.org/1061313003/diff/20001/services/android/intent...
services/android/intent.mojom:10: // This methods takes an |IntentReceiver| and
return a serialized intent.
s/methods/method/

https://codereview.chromium.org/1061313003/diff/20001/services/android/intent...
services/android/intent.mojom:10: // This methods takes an |IntentReceiver| and
return a serialized intent.
s/return/returns/

https://codereview.chromium.org/1061313003/diff/20001/services/android/intent...
services/android/intent.mojom:16: // received intent is active (contains either
a Binder, or a file
drop the comma after Binder

https://codereview.chromium.org/1061313003/diff/20001/shell/android/apk/src/o...
File shell/android/apk/src/org/chromium/mojo/shell/DelegatingIntentService.java
(right):

https://codereview.chromium.org/1061313003/diff/20001/shell/android/apk/src/o...
shell/android/apk/src/org/chromium/mojo/shell/DelegatingIntentService.java:13:
public class DelegatingIntentService extends IntentService {
Wdyt about calling this IntentReceiverService, to make it clear what is it for?

[qsr, 2015-04-08 11:03:23]

https://codereview.chromium.org/1061313003/diff/20001/services/android/intent...
File services/android/intent.mojom (right):

https://codereview.chromium.org/1061313003/diff/20001/services/android/intent...
services/android/intent.mojom:8: // Service to interract with the android's
intents.
On 2015/04/08 10:42:49, ppi wrote:
> s/the android's intents/android intents/

Done.

https://codereview.chromium.org/1061313003/diff/20001/services/android/intent...
services/android/intent.mojom:8: // Service to interract with the android's
intents.
On 2015/04/08 10:42:49, ppi wrote:
> s/interract/interact/

Done.

https://codereview.chromium.org/1061313003/diff/20001/services/android/intent...
services/android/intent.mojom:9: interface IntentManager {
On 2015/04/08 10:42:49, ppi wrote:
> wdyt about calling this IntentReceiverManager and the entire thing
> intent_receiver.mojom? "intent" seems broad given what we support here.

Done.

https://codereview.chromium.org/1061313003/diff/20001/services/android/intent...
services/android/intent.mojom:10: // This methods takes an |IntentReceiver| and
return a serialized intent.
On 2015/04/08 10:42:49, ppi wrote:
> s/return/returns/

Done.

https://codereview.chromium.org/1061313003/diff/20001/services/android/intent...
services/android/intent.mojom:10: // This methods takes an |IntentReceiver| and
return a serialized intent.
On 2015/04/08 10:42:49, ppi wrote:
> s/methods/method/

Done.

https://codereview.chromium.org/1061313003/diff/20001/services/android/intent...
services/android/intent.mojom:16: // received intent is active (contains either
a Binder, or a file
On 2015/04/08 10:42:49, ppi wrote:
> drop the comma after Binder

Done.

https://codereview.chromium.org/1061313003/diff/20001/shell/android/apk/src/o...
File shell/android/apk/src/org/chromium/mojo/shell/DelegatingIntentService.java
(right):

https://codereview.chromium.org/1061313003/diff/20001/shell/android/apk/src/o...
shell/android/apk/src/org/chromium/mojo/shell/DelegatingIntentService.java:13:
public class DelegatingIntentService extends IntentService {
On 2015/04/08 10:42:49, ppi wrote:
> Wdyt about calling this IntentReceiverService, to make it clear what is it
for?

Done.

[ppi, 2015-04-08 11:10:10]

https://codereview.chromium.org/1061313003/diff/40001/shell/android/android_h...
File shell/android/android_handler.h (right):

https://codereview.chromium.org/1061313003/diff/40001/shell/android/android_h...
shell/android/android_handler.h:39: IntentReceiverManagerFactory
intent_manager_factory_;
intent_receiver_manager_factory_?

https://codereview.chromium.org/1061313003/diff/40001/shell/android/apk/src/o...
File shell/android/apk/src/org/chromium/mojo/shell/IntentReceiverRegistry.java
(right):

https://codereview.chromium.org/1061313003/diff/40001/shell/android/apk/src/o...
shell/android/apk/src/org/chromium/mojo/shell/IntentReceiverRegistry.java:21: *
privileged mojo applications and route received intent back to the mojo
application. The
s/route/routes/

https://codereview.chromium.org/1061313003/diff/40001/shell/android/apk/src/o...
shell/android/apk/src/org/chromium/mojo/shell/IntentReceiverRegistry.java:21: *
privileged mojo applications and route received intent back to the mojo
application. The
s/received intent/received intents/

https://codereview.chromium.org/1061313003/diff/40001/shell/android/apk/src/o...
File shell/android/apk/src/org/chromium/mojo/shell/IntentReceiverService.java
(right):

https://codereview.chromium.org/1061313003/diff/40001/shell/android/apk/src/o...
shell/android/apk/src/org/chromium/mojo/shell/IntentReceiverService.java:11: *
Intent Service that will forward received intents to the {@link
IntentReceiverRegistry}.
maybe s/Intent Service/IntentService/ ?

https://codereview.chromium.org/1061313003/diff/40001/shell/android/intent_ma...
File shell/android/intent_manager_factory.h (right):

https://codereview.chromium.org/1061313003/diff/40001/shell/android/intent_ma...
shell/android/intent_manager_factory.h:14: class IntentReceiverManagerFactory
could you also rename the file to match the class name?

https://codereview.chromium.org/1061313003/diff/40001/shell/android/intent_ma...
File shell/android/intent_manager_impl.h (right):

https://codereview.chromium.org/1061313003/diff/40001/shell/android/intent_ma...
shell/android/intent_manager_impl.h:15: class IntentReceiverManagerImpl
could you also rename the file to match the class name?

[qsr, 2015-04-08 11:16:19]

https://codereview.chromium.org/1061313003/diff/40001/shell/android/android_h...
File shell/android/android_handler.h (right):

https://codereview.chromium.org/1061313003/diff/40001/shell/android/android_h...
shell/android/android_handler.h:39: IntentReceiverManagerFactory
intent_manager_factory_;
On 2015/04/08 11:10:10, ppi wrote:
> intent_receiver_manager_factory_?

Done.

https://codereview.chromium.org/1061313003/diff/40001/shell/android/apk/src/o...
File shell/android/apk/src/org/chromium/mojo/shell/IntentReceiverRegistry.java
(right):

https://codereview.chromium.org/1061313003/diff/40001/shell/android/apk/src/o...
shell/android/apk/src/org/chromium/mojo/shell/IntentReceiverRegistry.java:21: *
privileged mojo applications and route received intent back to the mojo
application. The
On 2015/04/08 11:10:10, ppi wrote:
> s/received intent/received intents/

Done.

https://codereview.chromium.org/1061313003/diff/40001/shell/android/apk/src/o...
shell/android/apk/src/org/chromium/mojo/shell/IntentReceiverRegistry.java:21: *
privileged mojo applications and route received intent back to the mojo
application. The
On 2015/04/08 11:10:10, ppi wrote:
> s/route/routes/

Done.

https://codereview.chromium.org/1061313003/diff/40001/shell/android/apk/src/o...
File shell/android/apk/src/org/chromium/mojo/shell/IntentReceiverService.java
(right):

https://codereview.chromium.org/1061313003/diff/40001/shell/android/apk/src/o...
shell/android/apk/src/org/chromium/mojo/shell/IntentReceiverService.java:11: *
Intent Service that will forward received intents to the {@link
IntentReceiverRegistry}.
On 2015/04/08 11:10:10, ppi wrote:
> maybe s/Intent Service/IntentService/ ?

Done.

https://codereview.chromium.org/1061313003/diff/40001/shell/android/intent_ma...
File shell/android/intent_manager_factory.h (right):

https://codereview.chromium.org/1061313003/diff/40001/shell/android/intent_ma...
shell/android/intent_manager_factory.h:14: class IntentReceiverManagerFactory
On 2015/04/08 11:10:10, ppi wrote:
> could you also rename the file to match the class name?

Done.

https://codereview.chromium.org/1061313003/diff/40001/shell/android/intent_ma...
File shell/android/intent_manager_impl.h (right):

https://codereview.chromium.org/1061313003/diff/40001/shell/android/intent_ma...
shell/android/intent_manager_impl.h:15: class IntentReceiverManagerImpl
On 2015/04/08 11:10:10, ppi wrote:
> could you also rename the file to match the class name?

Done.

[ppi, 2015-04-08 11:24:11]

lgtm

[qsr, 2015-04-08 11:47:56]

Committed patchset #4 (id:60001) manually as
1a8ef408019cc4e08d14d5f6001c44534ad841dd (presubmit successful).