mash/chrome: Update manifest for exe:chrome.

mash/chrome: Update manifest for exe:chrome.

Make sure exe:chrome declares 'app' class. Fix //services/shell
to erase just the '*' from the requested interfaces if target
requested shell:explicit_class, instead of overriding the other
explicit interface requests.

BUG=594600
Committed: https://crrev.com/cd59809a77388530bdc617ec0ed3bdf3919f35da
Cr-Commit-Position: refs/heads/master@{#390980}

[sadrul, 2016-04-28 06:20:56]

sadrul@chromium.org changed reviewers:
+ ben@chromium.org

[sadrul, 2016-04-28 06:20:56]

I have no idea if this is the right thing to do or not (is there some readme on
how manifests work?). But this does fix the issue of exe:chrome not being able
to connect to mojo:mus in browser_tests (the connections are still refused when
just running 'chrome --mash', even with this patch ... so perhaps something else
needs to change for that?)

[Ben Goodger (Google), 2016-04-28 14:06:30]

lgtm

[sadrul, 2016-04-29 17:37:54]

I updated the CL to explicitly list the requested interfaces from mojo:mus. I
had to make a change in Shell so that having a '*' in the requested interfaces
does not erase all the other explicitly listed interfaces. (is there a test I
could add for this?) PTAL

[Ben Goodger (Google), 2016-04-29 17:39:56]

https://codereview.chromium.org/1930813002/diff/20001/chrome/app/mojo/chrome_...
File chrome/app/mojo/chrome_manifest.json (right):

https://codereview.chromium.org/1930813002/diff/20001/chrome/app/mojo/chrome_...
chrome/app/mojo/chrome_manifest.json:9: "mus::mojom::WindowTreeFactory",
why was requesting "app" insufficient? That you have to list these explicitly
seems like a bug.

[sadrul, 2016-04-29 17:43:28]

https://codereview.chromium.org/1930813002/diff/20001/chrome/app/mojo/chrome_...
File chrome/app/mojo/chrome_manifest.json (right):

https://codereview.chromium.org/1930813002/diff/20001/chrome/app/mojo/chrome_...
chrome/app/mojo/chrome_manifest.json:9: "mus::mojom::WindowTreeFactory",
On 2016/04/29 17:39:56, Ben Goodger (Google) wrote:
> why was requesting "app" insufficient? That you have to list these explicitly
> seems like a bug.

This seems to be because mus manifest lists shell:explicit_class as required
(although the requirement is for mojo:shell ... not sure if it's WAI that this
affects exe:chrome too)

[Ben Goodger (Google), 2016-04-29 17:45:22]

On 2016/04/29 17:43:28, sadrul wrote:
>
https://codereview.chromium.org/1930813002/diff/20001/chrome/app/mojo/chrome_...
> File chrome/app/mojo/chrome_manifest.json (right):
> 
>
https://codereview.chromium.org/1930813002/diff/20001/chrome/app/mojo/chrome_...
> chrome/app/mojo/chrome_manifest.json:9: "mus::mojom::WindowTreeFactory",
> On 2016/04/29 17:39:56, Ben Goodger (Google) wrote:
> > why was requesting "app" insufficient? That you have to list these
explicitly
> > seems like a bug.
> 
> This seems to be because mus manifest lists shell:explicit_class as required
> (although the requirement is for mojo:shell ... not sure if it's WAI that this
> affects exe:chrome too)

The intent (and my code may be buggy) is that when a platform service requests
"shell:explicit_class" from the shell, that means that no client can connect to
*any* interface from it by requesting a wildcard. Instead, clients must specify
interfaces individually like you did here OR pick a class provided by that
platform service. In this case, mus provides this group of interfaces in the
"app" class, so this should work?

[sadrul, 2016-04-29 17:54:05]

On 2016/04/29 17:45:22, Ben Goodger (Google) wrote:
> On 2016/04/29 17:43:28, sadrul wrote:
> >
>
https://codereview.chromium.org/1930813002/diff/20001/chrome/app/mojo/chrome_...
> > File chrome/app/mojo/chrome_manifest.json (right):
> > 
> >
>
https://codereview.chromium.org/1930813002/diff/20001/chrome/app/mojo/chrome_...
> > chrome/app/mojo/chrome_manifest.json:9: "mus::mojom::WindowTreeFactory",
> > On 2016/04/29 17:39:56, Ben Goodger (Google) wrote:
> > > why was requesting "app" insufficient? That you have to list these
> explicitly
> > > seems like a bug.
> > 
> > This seems to be because mus manifest lists shell:explicit_class as required
> > (although the requirement is for mojo:shell ... not sure if it's WAI that
this
> > affects exe:chrome too)
> 
> The intent (and my code may be buggy) is that when a platform service requests
> "shell:explicit_class" from the shell, that means that no client can connect
to
> *any* interface from it by requesting a wildcard. Instead, clients must
specify
> interfaces individually like you did here OR pick a class provided by that
> platform service. In this case, mus provides this group of interfaces in the
> "app" class, so this should work?

Ah yep, you are right. As long as we don't erase everything, and just remove
'*', chrome doesn't actually need to list the explicit interfaces since it has
the 'app' class. Removed the explicit interface listing from chrome's manifest.

[Ben Goodger (Google), 2016-04-29 17:56:23]

lgtm

[sadrul, 2016-04-29 18:36:23]

Description was changed from

==========
mash/chrome: Update manifest for exe:chrome.

Without this, when running browser_tests, attempts to connect to mojo:mus from
exe:chrome (for mojom::Gpu etc.) get rejected.

BUG=594600
==========

to

==========
mash/chrome: Update manifest for exe:chrome.

Make sure exe:chrome declares 'app' class. Fix //services/shell
to erase just the '*' from the requested interfaces if target
requested shell:explicit_class, instead of overriding the other
explicit interface requests.

BUG=594600
==========

[sadrul, 2016-05-02 16:32:45]

The CQ bit was checked by sadrul@chromium.org

[sadrul, 2016-05-02 16:32:46]

The patchset sent to the CQ was uploaded after l-g-t-m from ben@chromium.org
Link to the patchset: https://codereview.chromium.org/1930813002/#ps60001
(title: "tot.merge")

[commit-bot: I haz the power, 2016-05-02 16:33:02]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1930813002/60001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1930813002/60001

[commit-bot: I haz the power, 2016-05-02 17:25:07]

Description was changed from

==========
mash/chrome: Update manifest for exe:chrome.

Make sure exe:chrome declares 'app' class. Fix //services/shell
to erase just the '*' from the requested interfaces if target
requested shell:explicit_class, instead of overriding the other
explicit interface requests.

BUG=594600
==========

to

==========
mash/chrome: Update manifest for exe:chrome.

Make sure exe:chrome declares 'app' class. Fix //services/shell
to erase just the '*' from the requested interfaces if target
requested shell:explicit_class, instead of overriding the other
explicit interface requests.

BUG=594600
==========

[commit-bot: I haz the power, 2016-05-02 17:25:09]

Committed patchset #4 (id:60001)

[commit-bot: I haz the power, 2016-05-02 17:25:53]

Description was changed from

==========
mash/chrome: Update manifest for exe:chrome.

Make sure exe:chrome declares 'app' class. Fix //services/shell
to erase just the '*' from the requested interfaces if target
requested shell:explicit_class, instead of overriding the other
explicit interface requests.

BUG=594600
==========

to

==========
mash/chrome: Update manifest for exe:chrome.

Make sure exe:chrome declares 'app' class. Fix //services/shell
to erase just the '*' from the requested interfaces if target
requested shell:explicit_class, instead of overriding the other
explicit interface requests.

BUG=594600

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

[commit-bot: I haz the power, 2016-05-02 17:25:54]

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