Add handler and mojo interface to accept android intent filters.

Add handler and mojo interface to accept android intent filters.

This adds the interface to recieve intent filters from android. Follow
up CL will store these and implement matching logic.

BUG=619137
Committed: https://crrev.com/a69ff524cf24d96b99f1b0185c40a0202e3838e0
Cr-Commit-Position: refs/heads/master@{#402203}

[zentaro, 2016-06-16 22:42:37]

Description was changed from

==========
Add handler and mojo interface to accept android intent filters.

This adds the interface to recieve intent filters from android. Follow
up CL will store these and implement matching logic.

BUG=619137
==========

to

==========
Add handler and mojo interface to accept android intent filters.

This adds the interface to recieve intent filters from android. Follow
up CL will store these and implement matching logic.

BUG=619137
==========

[zentaro, 2016-06-16 22:42:38]

zentaro@chromium.org changed reviewers:
+ yusukes@chromium.org

[zentaro, 2016-06-16 22:42:55]

On 2016/06/16 22:42:38, zentaro wrote:
> mailto:zentaro@chromium.org changed reviewers:
> + mailto:yusukes@chromium.org

PTAL

[Luis Héctor Chávez, 2016-06-16 22:45:19]

lhchavez@chromium.org changed reviewers:
+ lhchavez@chromium.org

[Luis Héctor Chávez, 2016-06-16 22:45:20]

drive-by

https://codereview.chromium.org/2078683002/diff/20001/components/arc/common/i...
File components/arc/common/intent_helper.mojom (right):

https://codereview.chromium.org/2078683002/diff/20001/components/arc/common/i...
components/arc/common/intent_helper.mojom:17: enum PatternType {
Is this expected to be fixed forever? Otherwise, add an [Extensible] attribute
to avoid generating overzealous range validation code that will break the
connection if you ever add a new entry.

https://codereview.chromium.org/2078683002/diff/20001/components/arc/intent_h...
File components/arc/intent_helper/arc_intent_helper_bridge.cc (right):

https://codereview.chromium.org/2078683002/diff/20001/components/arc/intent_h...
components/arc/intent_helper/arc_intent_helper_bridge.cc:100: // TODO(29248657):
Store the filters and implement matching.
In the past, we've required having an implementation for .mojom changes for the
security team's sake.

[Yusuke Sato, 2016-06-16 23:42:18]

initial comments. Please also run git cl try.

https://codereview.chromium.org/2078683002/diff/20001/components/arc/intent_h...
File components/arc/intent_helper/arc_intent_helper_bridge.cc (right):

https://codereview.chromium.org/2078683002/diff/20001/components/arc/intent_h...
components/arc/intent_helper/arc_intent_helper_bridge.cc:100: // TODO(29248657):
Store the filters and implement matching.
On 2016/06/16 22:45:20, Luis Héctor Chávez wrote:
> In the past, we've required having an implementation for .mojom changes for
the
> security team's sake.

What we could do I think is to remember |filters| in a LocalActivityFilter
object probably? I think you could instantiate the object in the same way as
arc::ActivityIconLoader. See components/arc/arc_service_manager.* for more
details.

https://codereview.chromium.org/2078683002/diff/20001/components/arc/intent_h...
File components/arc/intent_helper/arc_intent_helper_bridge.h (right):

https://codereview.chromium.org/2078683002/diff/20001/components/arc/intent_h...
components/arc/intent_helper/arc_intent_helper_bridge.h:44: void
OnOpenDownloads() override;
Does this compile? Line 50 seems to have the same method already.

https://codereview.chromium.org/2078683002/diff/20001/components/arc/intent_h...
components/arc/intent_helper/arc_intent_helper_bridge.h:45: void
OnIntentFiltersUpdated(
Move down to line 50?

[zentaro, 2016-06-17 21:42:25]

PTAAL

https://codereview.chromium.org/2078683002/diff/20001/components/arc/common/i...
File components/arc/common/intent_helper.mojom (right):

https://codereview.chromium.org/2078683002/diff/20001/components/arc/common/i...
components/arc/common/intent_helper.mojom:17: enum PatternType {
On 2016/06/16 22:45:20, Luis Héctor Chávez wrote:
> Is this expected to be fixed forever? Otherwise, add an [Extensible] attribute
> to avoid generating overzealous range validation code that will break the
> connection if you ever add a new entry.

Done.

https://codereview.chromium.org/2078683002/diff/20001/components/arc/intent_h...
File components/arc/intent_helper/arc_intent_helper_bridge.cc (right):

https://codereview.chromium.org/2078683002/diff/20001/components/arc/intent_h...
components/arc/intent_helper/arc_intent_helper_bridge.cc:100: // TODO(29248657):
Store the filters and implement matching.
On 2016/06/16 23:42:18, Yusuke Sato wrote:
> On 2016/06/16 22:45:20, Luis Héctor Chávez wrote:
> > In the past, we've required having an implementation for .mojom changes for
> the
> > security team's sake.
> 
> What we could do I think is to remember |filters| in a LocalActivityFilter
> object probably? I think you could instantiate the object in the same way as
> arc::ActivityIconLoader. See components/arc/arc_service_manager.* for more
> details.

Done.

https://codereview.chromium.org/2078683002/diff/20001/components/arc/intent_h...
File components/arc/intent_helper/arc_intent_helper_bridge.h (right):

https://codereview.chromium.org/2078683002/diff/20001/components/arc/intent_h...
components/arc/intent_helper/arc_intent_helper_bridge.h:44: void
OnOpenDownloads() override;
On 2016/06/16 23:42:18, Yusuke Sato wrote:
> Does this compile? Line 50 seems to have the same method already.

I merged incorrectly before upload. Fixed now.

https://codereview.chromium.org/2078683002/diff/20001/components/arc/intent_h...
components/arc/intent_helper/arc_intent_helper_bridge.h:45: void
OnIntentFiltersUpdated(
On 2016/06/16 23:42:18, Yusuke Sato wrote:
> Move down to line 50?

As above.

[Yusuke Sato, 2016-06-17 21:51:15]

lg, but the chromium revision you have seems too old (and because of that the
bots are red.) could you rebase your CL?

https://codereview.chromium.org/2078683002/diff/80001/components/arc/arc_serv...
File components/arc/arc_service_manager.cc (right):

https://codereview.chromium.org/2078683002/diff/80001/components/arc/arc_serv...
components/arc/arc_service_manager.cc:63: activity_resolver_ = new
LocalActivityResolver;
This code is fine, but it conflicts with a recent CL from nya@:
https://codereview.chromium.org/2061183002/

The CL moved ArcIntentHelperBridge initialization to
	chrome/browser/chromeos/arc/arc_service_launcher.cc. Please sync your local
checkout and follow the new way.

https://codereview.chromium.org/2078683002/diff/80001/components/arc/intent_h...
File components/arc/intent_helper/local_activity_resolver.h (right):

https://codereview.chromium.org/2078683002/diff/80001/components/arc/intent_h...
components/arc/intent_helper/local_activity_resolver.h:22: void
UpdateIntentFilters(mojo::Array<mojom::IntentFilterPtr>& intent_filters);
Is '&' necessary?

https://codereview.chromium.org/2078683002/diff/80001/components/arc/intent_h...
components/arc/intent_helper/local_activity_resolver.h:28:
mojo::Array<mojom::IntentFilterPtr> intent_filters_;
Mind adding a code comment to explain what this is?

[zentaro, 2016-06-17 23:00:41]

On 2016/06/17 21:51:15, Yusuke Sato wrote:
> lg, but the chromium revision you have seems too old (and because of that the
> bots are red.) could you rebase your CL?
> 
>
https://codereview.chromium.org/2078683002/diff/80001/components/arc/arc_serv...
> File components/arc/arc_service_manager.cc (right):
> 
>
https://codereview.chromium.org/2078683002/diff/80001/components/arc/arc_serv...
> components/arc/arc_service_manager.cc:63: activity_resolver_ = new
> LocalActivityResolver;
> This code is fine, but it conflicts with a recent CL from nya@:
> https://codereview.chromium.org/2061183002/
> 
> The CL moved ArcIntentHelperBridge initialization to
> 	chrome/browser/chromeos/arc/arc_service_launcher.cc. Please sync your local
> checkout and follow the new way.
> 
>
https://codereview.chromium.org/2078683002/diff/80001/components/arc/intent_h...
> File components/arc/intent_helper/local_activity_resolver.h (right):
> 
>
https://codereview.chromium.org/2078683002/diff/80001/components/arc/intent_h...
> components/arc/intent_helper/local_activity_resolver.h:22: void
> UpdateIntentFilters(mojo::Array<mojom::IntentFilterPtr>& intent_filters);
> Is '&' necessary?
> 
>
https://codereview.chromium.org/2078683002/diff/80001/components/arc/intent_h...
> components/arc/intent_helper/local_activity_resolver.h:28:
> mojo::Array<mojom::IntentFilterPtr> intent_filters_;
> Mind adding a code comment to explain what this is?

Rebased and updated. PTAAL.

[Yusuke Sato, 2016-06-17 23:14:33]

Thanks for the fix. LGTM with nits. For the mojom change, please add either
dcheng@c or rickyz@c as a (security) reviewer.

https://codereview.chromium.org/2078683002/diff/140001/components/arc/intent_...
File components/arc/intent_helper/arc_intent_helper_bridge.cc (right):

https://codereview.chromium.org/2078683002/diff/140001/components/arc/intent_...
components/arc/intent_helper/arc_intent_helper_bridge.cc:124:
activity_resolver_->UpdateIntentFilters(filters);
How about changing this to

 activity_resolver_->UpdateIntentFilters(std::move(filters));

and dropping & from the function signature in local_activity_resolver.h?

 void UpdateIntentFilters(mojo::Array<mojom::IntentFilterPtr> intent_filters);

According to the coding guide, "within function parameter lists all references
must be const":
https://google.github.io/styleguide/cppguide.html#Reference_Arguments

https://codereview.chromium.org/2078683002/diff/140001/components/arc/intent_...
File components/arc/intent_helper/arc_intent_helper_bridge.h (right):

https://codereview.chromium.org/2078683002/diff/140001/components/arc/intent_...
components/arc/intent_helper/arc_intent_helper_bridge.h:30: class
LocalActivityResolver;
nit: sort?

[zentaro, 2016-06-17 23:37:17]

On 2016/06/17 23:14:33, Yusuke Sato wrote:
> Thanks for the fix. LGTM with nits. For the mojom change, please add either
> dcheng@c or rickyz@c as a (security) reviewer.
> 
>
https://codereview.chromium.org/2078683002/diff/140001/components/arc/intent_...
> File components/arc/intent_helper/arc_intent_helper_bridge.cc (right):
> 
>
https://codereview.chromium.org/2078683002/diff/140001/components/arc/intent_...
> components/arc/intent_helper/arc_intent_helper_bridge.cc:124:
> activity_resolver_->UpdateIntentFilters(filters);
> How about changing this to
> 
>  activity_resolver_->UpdateIntentFilters(std::move(filters));
> 
> and dropping & from the function signature in local_activity_resolver.h?
> 
>  void UpdateIntentFilters(mojo::Array<mojom::IntentFilterPtr> intent_filters);
> 
> According to the coding guide, "within function parameter lists all references
> must be const":
> https://google.github.io/styleguide/cppguide.html#Reference_Arguments
> 
>
https://codereview.chromium.org/2078683002/diff/140001/components/arc/intent_...
> File components/arc/intent_helper/arc_intent_helper_bridge.h (right):
> 
>
https://codereview.chromium.org/2078683002/diff/140001/components/arc/intent_...
> components/arc/intent_helper/arc_intent_helper_bridge.h:30: class
> LocalActivityResolver;
> nit: sort?

Done.

[zentaro, 2016-06-17 23:37:35]

https://codereview.chromium.org/2078683002/diff/80001/components/arc/arc_serv...
File components/arc/arc_service_manager.cc (right):

https://codereview.chromium.org/2078683002/diff/80001/components/arc/arc_serv...
components/arc/arc_service_manager.cc:63: activity_resolver_ = new
LocalActivityResolver;
On 2016/06/17 21:51:15, Yusuke Sato wrote:
> This code is fine, but it conflicts with a recent CL from nya@:
> https://codereview.chromium.org/2061183002/
> 
> The CL moved ArcIntentHelperBridge initialization to
> 	chrome/browser/chromeos/arc/arc_service_launcher.cc. Please sync your local
> checkout and follow the new way.

Done.

https://codereview.chromium.org/2078683002/diff/80001/components/arc/intent_h...
File components/arc/intent_helper/local_activity_resolver.h (right):

https://codereview.chromium.org/2078683002/diff/80001/components/arc/intent_h...
components/arc/intent_helper/local_activity_resolver.h:22: void
UpdateIntentFilters(mojo::Array<mojom::IntentFilterPtr>& intent_filters);
On 2016/06/17 21:51:15, Yusuke Sato wrote:
> Is '&' necessary?

Yes - otherwise is will try to copy which isn't allowed on Array.

https://codereview.chromium.org/2078683002/diff/80001/components/arc/intent_h...
components/arc/intent_helper/local_activity_resolver.h:28:
mojo::Array<mojom::IntentFilterPtr> intent_filters_;
On 2016/06/17 21:51:15, Yusuke Sato wrote:
> Mind adding a code comment to explain what this is?

Done.

https://codereview.chromium.org/2078683002/diff/140001/components/arc/intent_...
File components/arc/intent_helper/arc_intent_helper_bridge.cc (right):

https://codereview.chromium.org/2078683002/diff/140001/components/arc/intent_...
components/arc/intent_helper/arc_intent_helper_bridge.cc:124:
activity_resolver_->UpdateIntentFilters(filters);
On 2016/06/17 23:14:32, Yusuke Sato wrote:
> How about changing this to
> 
>  activity_resolver_->UpdateIntentFilters(std::move(filters));
> 
> and dropping & from the function signature in local_activity_resolver.h?
> 
>  void UpdateIntentFilters(mojo::Array<mojom::IntentFilterPtr> intent_filters);
> 
> According to the coding guide, "within function parameter lists all references
> must be const":
> https://google.github.io/styleguide/cppguide.html#Reference_Arguments

Done.

https://codereview.chromium.org/2078683002/diff/140001/components/arc/intent_...
File components/arc/intent_helper/arc_intent_helper_bridge.h (right):

https://codereview.chromium.org/2078683002/diff/140001/components/arc/intent_...
components/arc/intent_helper/arc_intent_helper_bridge.h:30: class
LocalActivityResolver;
On 2016/06/17 23:14:32, Yusuke Sato wrote:
> nit: sort?

Done.

[zentaro, 2016-06-17 23:38:09]

zentaro@chromium.org changed reviewers:
+ rickyz@chromium.org

[zentaro, 2016-06-17 23:38:33]

On 2016/06/17 23:37:35, zentaro wrote:
>
https://codereview.chromium.org/2078683002/diff/80001/components/arc/arc_serv...
> File components/arc/arc_service_manager.cc (right):
> 
>
https://codereview.chromium.org/2078683002/diff/80001/components/arc/arc_serv...
> components/arc/arc_service_manager.cc:63: activity_resolver_ = new
> LocalActivityResolver;
> On 2016/06/17 21:51:15, Yusuke Sato wrote:
> > This code is fine, but it conflicts with a recent CL from nya@:
> > https://codereview.chromium.org/2061183002/
> > 
> > The CL moved ArcIntentHelperBridge initialization to
> > 	chrome/browser/chromeos/arc/arc_service_launcher.cc. Please sync your local
> > checkout and follow the new way.
> 
> Done.
> 
>
https://codereview.chromium.org/2078683002/diff/80001/components/arc/intent_h...
> File components/arc/intent_helper/local_activity_resolver.h (right):
> 
>
https://codereview.chromium.org/2078683002/diff/80001/components/arc/intent_h...
> components/arc/intent_helper/local_activity_resolver.h:22: void
> UpdateIntentFilters(mojo::Array<mojom::IntentFilterPtr>& intent_filters);
> On 2016/06/17 21:51:15, Yusuke Sato wrote:
> > Is '&' necessary?
> 
> Yes - otherwise is will try to copy which isn't allowed on Array.
> 
>
https://codereview.chromium.org/2078683002/diff/80001/components/arc/intent_h...
> components/arc/intent_helper/local_activity_resolver.h:28:
> mojo::Array<mojom::IntentFilterPtr> intent_filters_;
> On 2016/06/17 21:51:15, Yusuke Sato wrote:
> > Mind adding a code comment to explain what this is?
> 
> Done.
> 
>
https://codereview.chromium.org/2078683002/diff/140001/components/arc/intent_...
> File components/arc/intent_helper/arc_intent_helper_bridge.cc (right):
> 
>
https://codereview.chromium.org/2078683002/diff/140001/components/arc/intent_...
> components/arc/intent_helper/arc_intent_helper_bridge.cc:124:
> activity_resolver_->UpdateIntentFilters(filters);
> On 2016/06/17 23:14:32, Yusuke Sato wrote:
> > How about changing this to
> > 
> >  activity_resolver_->UpdateIntentFilters(std::move(filters));
> > 
> > and dropping & from the function signature in local_activity_resolver.h?
> > 
> >  void UpdateIntentFilters(mojo::Array<mojom::IntentFilterPtr>
intent_filters);
> > 
> > According to the coding guide, "within function parameter lists all
references
> > must be const":
> > https://google.github.io/styleguide/cppguide.html#Reference_Arguments
> 
> Done.
> 
>
https://codereview.chromium.org/2078683002/diff/140001/components/arc/intent_...
> File components/arc/intent_helper/arc_intent_helper_bridge.h (right):
> 
>
https://codereview.chromium.org/2078683002/diff/140001/components/arc/intent_...
> components/arc/intent_helper/arc_intent_helper_bridge.h:30: class
> LocalActivityResolver;
> On 2016/06/17 23:14:32, Yusuke Sato wrote:
> > nit: sort?
> 
> Done.

rizkyz@ PTAL at the mojo change.

Thanks!

[zentaro, 2016-06-20 17:34:28]

zentaro@chromium.org changed reviewers:
+ dcheng@chromium.org

[dcheng, 2016-06-20 18:42:42]

How will the intent filters to be used? It's hard to provide a security review
with this CL. For example, I can't tell if the various array in the IntentFilter
struct are all intended to be the same length or not, nor what the different
fields mean.

[zentaro, 2016-06-20 18:46:29]

On 2016/06/20 18:42:42, dcheng wrote:
> How will the intent filters to be used? It's hard to provide a security review
> with this CL. For example, I can't tell if the various array in the
IntentFilter
> struct are all intended to be the same length or not, nor what the different
> fields mean.

The struct is a subset of
https://developer.android.com/reference/android/content/IntentFilter.html

The fields don't have to have the same number of items. The array of filters
will later be searched to determine if a given URL matches any of the filters.

[dcheng, 2016-06-20 19:03:07]

On 2016/06/20 18:46:29, zentaro wrote:
> On 2016/06/20 18:42:42, dcheng wrote:
> > How will the intent filters to be used? It's hard to provide a security
review
> > with this CL. For example, I can't tell if the various array in the
> IntentFilter
> > struct are all intended to be the same length or not, nor what the different
> > fields mean.
> 
> The struct is a subset of
> https://developer.android.com/reference/android/content/IntentFilter.html
> 
> The fields don't have to have the same number of items. The array of filters
> will later be searched to determine if a given URL matches any of the filters.

It would still be better to land this as part of the CL that implements
ShouldChromeHandleUrl().

[dcheng, 2016-06-22 14:10:57]

Can we just add the fields as needed? It's fine that this is still in
development, but if we could incrementally add fields, that would make the
security review more straightforward (and we could just let the OWNERS system do
the heavy lifting of deciding when things need to go through mojom review).

[zentaro, 2016-06-22 17:25:04]

On 2016/06/22 14:10:57, dcheng wrote:
> Can we just add the fields as needed? It's fine that this is still in
> development, but if we could incrementally add fields, that would make the
> security review more straightforward (and we could just let the OWNERS system
do
> the heavy lifting of deciding when things need to go through mojom review).

OK - I removed the fields that are not currently being used.

[Yusuke Sato, 2016-06-22 19:45:52]

lgtm with nits:

https://codereview.chromium.org/2078683002/diff/240001/components/arc/intent_...
File components/arc/intent_helper/local_activity_resolver.cc (right):

https://codereview.chromium.org/2078683002/diff/240001/components/arc/intent_...
components/arc/intent_helper/local_activity_resolver.cc:11:
LocalActivityResolver::LocalActivityResolver() {}
= default;

https://codereview.chromium.org/2078683002/diff/240001/components/arc/intent_...
components/arc/intent_helper/local_activity_resolver.cc:13:
LocalActivityResolver::~LocalActivityResolver() {}
same

https://codereview.chromium.org/2078683002/diff/240001/components/arc/intent_...
components/arc/intent_helper/local_activity_resolver.cc:49: if (action ==
"android.intent.action.VIEW") {
nit: can you define a constant?

 namespace {

 constexpr char kXXX[] = "android.intent.action.VIEW";

 }  // namespace

https://codereview.chromium.org/2078683002/diff/240001/components/arc/intent_...
components/arc/intent_helper/local_activity_resolver.cc:60: if (category ==
"android.intent.category.BROWSABLE") {
same

https://codereview.chromium.org/2078683002/diff/240001/components/arc/intent_...
File components/arc/intent_helper/local_activity_resolver.h (right):

https://codereview.chromium.org/2078683002/diff/240001/components/arc/intent_...
components/arc/intent_helper/local_activity_resolver.h:26:
~LocalActivityResolver();
override; ?

https://codereview.chromium.org/2078683002/diff/240001/components/arc/intent_...
components/arc/intent_helper/local_activity_resolver.h:36:
DISALLOW_COPY_AND_ASSIGN(LocalActivityResolver);
nit: a space between line 35 and 36.

[zentaro, 2016-06-22 20:16:14]

https://codereview.chromium.org/2078683002/diff/240001/components/arc/intent_...
File components/arc/intent_helper/local_activity_resolver.cc (right):

https://codereview.chromium.org/2078683002/diff/240001/components/arc/intent_...
components/arc/intent_helper/local_activity_resolver.cc:11:
LocalActivityResolver::LocalActivityResolver() {}
On 2016/06/22 19:45:51, Yusuke Sato wrote:
> = default;

I had it in the .h but then it complained about inline constructor in complex
class. I didn't realize you could also do it in the .cc file.

https://codereview.chromium.org/2078683002/diff/240001/components/arc/intent_...
components/arc/intent_helper/local_activity_resolver.cc:13:
LocalActivityResolver::~LocalActivityResolver() {}
On 2016/06/22 19:45:52, Yusuke Sato wrote:
> same

Done.

https://codereview.chromium.org/2078683002/diff/240001/components/arc/intent_...
components/arc/intent_helper/local_activity_resolver.cc:49: if (action ==
"android.intent.action.VIEW") {
On 2016/06/22 19:45:52, Yusuke Sato wrote:
> nit: can you define a constant?
> 
>  namespace {
> 
>  constexpr char kXXX[] = "android.intent.action.VIEW";
> 
>  }  // namespace

Done.

https://codereview.chromium.org/2078683002/diff/240001/components/arc/intent_...
components/arc/intent_helper/local_activity_resolver.cc:60: if (category ==
"android.intent.category.BROWSABLE") {
On 2016/06/22 19:45:52, Yusuke Sato wrote:
> same

Done.

https://codereview.chromium.org/2078683002/diff/240001/components/arc/intent_...
File components/arc/intent_helper/local_activity_resolver.h (right):

https://codereview.chromium.org/2078683002/diff/240001/components/arc/intent_...
components/arc/intent_helper/local_activity_resolver.h:26:
~LocalActivityResolver();
On 2016/06/22 19:45:52, Yusuke Sato wrote:
> override; ?

Can't put it here. It's not virtual in the base class. See also
activity_icon_loader.h for comparable example.

https://codereview.chromium.org/2078683002/diff/240001/components/arc/intent_...
components/arc/intent_helper/local_activity_resolver.h:36:
DISALLOW_COPY_AND_ASSIGN(LocalActivityResolver);
On 2016/06/22 19:45:52, Yusuke Sato wrote:
> nit: a space between line 35 and 36.

Done.

[Yusuke Sato, 2016-06-22 20:38:02]

https://codereview.chromium.org/2078683002/diff/240001/components/arc/intent_...
File components/arc/intent_helper/local_activity_resolver.h (right):

https://codereview.chromium.org/2078683002/diff/240001/components/arc/intent_...
components/arc/intent_helper/local_activity_resolver.h:26:
~LocalActivityResolver();
On 2016/06/22 20:16:14, zentaro wrote:
> On 2016/06/22 19:45:52, Yusuke Sato wrote:
> > override; ?
> 
> Can't put it here. It's not virtual in the base class. See also
> activity_icon_loader.h for comparable example.

my bad, sorry. you're right.

[dcheng, 2016-06-24 22:32:36]

lgtm

https://codereview.chromium.org/2078683002/diff/240001/components/arc/common/...
File components/arc/common/intent_helper.mojom (right):

https://codereview.chromium.org/2078683002/diff/240001/components/arc/common/...
components/arc/common/intent_helper.mojom:33: };
Looks like lines 18-33 aren't used yet either.

https://codereview.chromium.org/2078683002/diff/280001/components/arc/intent_...
File components/arc/intent_helper/local_activity_resolver.cc (right):

https://codereview.chromium.org/2078683002/diff/280001/components/arc/intent_...
components/arc/intent_helper/local_activity_resolver.cc:78: if (scheme == "http"
|| scheme == "https") {
Use kHttpScheme and kHttpsScheme at least. I don't really like this, but I don't
have a better idea without knowing how this code is going to evolve.

[zentaro, 2016-06-27 16:15:38]

https://codereview.chromium.org/2078683002/diff/240001/components/arc/common/...
File components/arc/common/intent_helper.mojom (right):

https://codereview.chromium.org/2078683002/diff/240001/components/arc/common/...
components/arc/common/intent_helper.mojom:33: };
On 2016/06/24 22:32:35, dcheng wrote:
> Looks like lines 18-33 aren't used yet either.

Done.

https://codereview.chromium.org/2078683002/diff/280001/components/arc/intent_...
File components/arc/intent_helper/local_activity_resolver.cc (right):

https://codereview.chromium.org/2078683002/diff/280001/components/arc/intent_...
components/arc/intent_helper/local_activity_resolver.cc:78: if (scheme == "http"
|| scheme == "https") {
On 2016/06/24 22:32:35, dcheng wrote:
> Use kHttpScheme and kHttpsScheme at least. I don't really like this, but I
don't
> have a better idea without knowing how this code is going to evolve.

Done.

[zentaro, 2016-06-27 16:54:05]

The CQ bit was checked by zentaro@chromium.org

[zentaro, 2016-06-27 16:54:05]

The patchset sent to the CQ was uploaded after l-g-t-m from
yusukes@chromium.org, dcheng@chromium.org
Link to the patchset: https://codereview.chromium.org/2078683002/#ps300001
(title: "Add code accidentally removed.")

[commit-bot: I haz the power, 2016-06-27 16:54:37]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-06-27 16:59:43]

Description was changed from

==========
Add handler and mojo interface to accept android intent filters.

This adds the interface to recieve intent filters from android. Follow
up CL will store these and implement matching logic.

BUG=619137
==========

to

==========
Add handler and mojo interface to accept android intent filters.

This adds the interface to recieve intent filters from android. Follow
up CL will store these and implement matching logic.

BUG=619137
==========

[commit-bot: I haz the power, 2016-06-27 16:59:44]

Committed patchset #16 (id:300001)

[commit-bot: I haz the power, 2016-06-27 17:02:32]

Description was changed from

==========
Add handler and mojo interface to accept android intent filters.

This adds the interface to recieve intent filters from android. Follow
up CL will store these and implement matching logic.

BUG=619137
==========

to

==========
Add handler and mojo interface to accept android intent filters.

This adds the interface to recieve intent filters from android. Follow
up CL will store these and implement matching logic.

BUG=619137

Committed: https://crrev.com/a69ff524cf24d96b99f1b0185c40a0202e3838e0
Cr-Commit-Position: refs/heads/master@{#402203}
==========

[commit-bot: I haz the power, 2016-06-27 17:02:33]

Patchset 16 (id:??) landed as
https://crrev.com/a69ff524cf24d96b99f1b0185c40a0202e3838e0
Cr-Commit-Position: refs/heads/master@{#402203}