Refactor getInstalledRelatedApps code and add manifest logic and tests.

Refactor getInstalledRelatedApps code and add manifest logic and tests.

This changes the architecture of getInstalledRelatedApps for "onion
soup". Under the previous (unfinished) architecture,
InstalledAppController (Blink) would call
WebInstalledAppClient::getInstalledRelatedApps (content/renderer) which
was planned to get the related apps from the manifest, then call a Mojo
service in the browser to filter the apps.

This CL renames WebInstalledAppClient to WebRelatedAppsFetcher, and now
its job is *only* to get the related apps from the manifest (not talk to
the browser) and return it to InstalledAppController.
InstalledAppController will (in a future CL) get the browser to filter
the apps.

This CL also implements the concrete RelatedAppsFetcher in
content/renderer which actually gets the related apps from the manifest,
and adds layout tests for the API.

Manifest code based on CL by dhnishi:
https://codereview.chromium.org/1756793004/

BUG=587623
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_site_isolation

Review-Url: https://codereview.chromium.org/2488573002
Cr-Commit-Position: refs/heads/master@{#450868}
Committed: https://chromium.googlesource.com/chromium/src/+/be2c7115703a6610b25128809eb388ac28d07285

[Matt Giuca, 2016-11-08 07:35:38]

Description was changed from

==========
WIP for navigator.getInstalledRelatedApps (Chrome-side).

Originally written by dhnishi:
https://codereview.chromium.org/1756793004/
"Chrome-side patch for IsAppInstalled."

Rebased and maintained by benwells:
https://codereview.chromium.org/2461303002/

BUG=587623
==========

to

==========
WIP for navigator.getInstalledRelatedApps (Chrome-side).

Originally written by dhnishi:
https://codereview.chromium.org/1756793004/
"Chrome-side patch for IsAppInstalled."

Rebased and maintained by benwells:
https://codereview.chromium.org/2461303002/

DO NOT SUBMIT

BUG=587623
==========

[Matt Giuca, 2017-01-06 01:46:35]

Description was changed from

==========
WIP for navigator.getInstalledRelatedApps (Chrome-side).

Originally written by dhnishi:
https://codereview.chromium.org/1756793004/
"Chrome-side patch for IsAppInstalled."

Rebased and maintained by benwells:
https://codereview.chromium.org/2461303002/

DO NOT SUBMIT

BUG=587623
==========

to

==========
WIP for navigator.getInstalledRelatedApps (Chrome-side).

Originally written by dhnishi:
https://codereview.chromium.org/1756793004/
"Chrome-side patch for IsAppInstalled."

Rebased and maintained by benwells:
https://codereview.chromium.org/2461303002/

DO NOT SUBMIT

BUG=587623
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_site_isolation
==========

[Matt Giuca, 2017-01-10 02:59:33]

The CQ bit was checked by mgiuca@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-01-10 03:00:18]

Dry run: 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, 2017-01-10 03:05:27]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-01-10 03:05:28]

Dry run: Try jobs failed on following builders:
  android_arm64_dbg_recipe on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_arm6...)
  android_clang_dbg_recipe on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_clan...)
  android_compile_dbg on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_comp...)
  android_cronet on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_cron...)
  cast_shell_android on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/cast_shell_a...)
  linux_android_rel_ng on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/linux_androi...)

[Matt Giuca, 2017-02-01 23:28:07]

Patchset #27 (id:520001) has been deleted

[Matt Giuca, 2017-02-01 23:28:14]

Patchset #26 (id:500001) has been deleted

[Matt Giuca, 2017-02-02 07:09:27]

Description was changed from

==========
WIP for navigator.getInstalledRelatedApps (Chrome-side).

Originally written by dhnishi:
https://codereview.chromium.org/1756793004/
"Chrome-side patch for IsAppInstalled."

Rebased and maintained by benwells:
https://codereview.chromium.org/2461303002/

DO NOT SUBMIT

BUG=587623
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_site_isolation
==========

to

==========
Refactor getInstalledRelatedApps code and add manifest logic and tests.

This changes the architecture of getInstalledRelatedApps for "onion
soup". Under the previous (unfinished) architecture,
InstalledAppController (Blink) would call
WebInstalledAppClient::getInstalledRelatedApps (content/renderer) which
was planned to get the related apps from the manifest, then call a Mojo
service in the browser to filter the apps.

This CL renames WebInstalledAppClient to WebRelatedAppsFetcher, and now
its job is *only* to get the related apps from the manifest (not talk to
the browser) and return it to InstalledAppController.
InstalledAppController will (in a future CL) get the browser to filter
the apps.

This CL also implements the concrete RelatedAppsFetcher in
content/renderer which actually gets the related apps from the manifest,
and adds layout tests for the API.

Manifest code based on CL by dhnishi:
https://codereview.chromium.org/1756793004/

BUG=587623
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_site_isolation
==========

[Matt Giuca, 2017-02-02 07:13:06]

Description was changed from

==========
Refactor getInstalledRelatedApps code and add manifest logic and tests.

This changes the architecture of getInstalledRelatedApps for "onion
soup". Under the previous (unfinished) architecture,
InstalledAppController (Blink) would call
WebInstalledAppClient::getInstalledRelatedApps (content/renderer) which
was planned to get the related apps from the manifest, then call a Mojo
service in the browser to filter the apps.

This CL renames WebInstalledAppClient to WebRelatedAppsFetcher, and now
its job is *only* to get the related apps from the manifest (not talk to
the browser) and return it to InstalledAppController.
InstalledAppController will (in a future CL) get the browser to filter
the apps.

This CL also implements the concrete RelatedAppsFetcher in
content/renderer which actually gets the related apps from the manifest,
and adds layout tests for the API.

Manifest code based on CL by dhnishi:
https://codereview.chromium.org/1756793004/

BUG=587623
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_site_isolation
==========

to

==========
Refactor getInstalledRelatedApps code and add manifest logic and tests.

This changes the architecture of getInstalledRelatedApps for "onion
soup". Under the previous (unfinished) architecture,
InstalledAppController (Blink) would call
WebInstalledAppClient::getInstalledRelatedApps (content/renderer) which
was planned to get the related apps from the manifest, then call a Mojo
service in the browser to filter the apps.

This CL renames WebInstalledAppClient to WebRelatedAppsFetcher, and now
its job is *only* to get the related apps from the manifest (not talk to
the browser) and return it to InstalledAppController.
InstalledAppController will (in a future CL) get the browser to filter
the apps.

This CL also implements the concrete RelatedAppsFetcher in
content/renderer which actually gets the related apps from the manifest,
and adds layout tests for the API.

Manifest code based on CL by dhnishi:
https://codereview.chromium.org/1756793004/

BUG=587623
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_site_isolation
==========

[Matt Giuca, 2017-02-02 07:21:14]

mgiuca@chromium.org changed reviewers:
+ esprehn@chromium.org, haraken@chromium.org

[Matt Giuca, 2017-02-02 07:21:16]

esprehn: Please review changes in content/renderer.
haraken: Please review changes in third_party/WebKit.

Note: I raised this on platform-architecture-dev two weeks ago. This CL is the
refactor from the old architecture:
https://docs.google.com/drawings/d/12LEt5OjIl_wD_8Ixp64twX6caWDRCccmtq3xYajVV...
to my proposed architecture which you (haraken) said looks OK:
https://docs.google.com/drawings/d/1D9zDpCXe3GWVYO2XS4w0mIDY21CHbGhjQVOOJtcWW...

(This only implements the "Renderer" box in that diagram; browser side is coming
in https://codereview.chromium.org/2671683002)

Thanks!

Matt

[esprehn, 2017-02-02 07:34:32]

I see routing IDs here which is scary, I thought we didn't use those anymore?

The plan mentioned where the browser filters will let us remove this
content/renderer code? Definitely we want to be working to get rid of all of
content/renderer eventually. :)

[Matt Giuca, 2017-02-02 23:35:42]

On 2017/02/02 07:34:32, esprehn wrote:
> I see routing IDs here which is scary, I thought we didn't use those anymore?

Sorry, I didn't really think that through (I inherited this code from dhnishi's
original CL):
https://codereview.chromium.org/1756793004/diff/1/content/renderer/installeda...
which is about a year old so maybe things have changed in the mean time.

What would you recommend for RelatedAppsFetcher to access the ManifestManager?
Can it just take a raw pointer to a ManifestManager (which is owned by the
RenderFrameImpl) directly?

I'm trying to understand the ownership/lifetime stuff here. RenderFrameImpl has
a *raw* pointer to ManifestManager and doesn't delete it; ManifestManager is a
RenderFrameObserver and deletes itself when it sees the OnDestruct signal, which
is in turn called by ~RenderFrameImpl. This seems to be a rather roundabout
alternative to just having a unique_ptr to a ManifestManager. Is there a reason
to do it using a RenderFrameObserver rather than a unique_ptr? And if so, should
RelatedAppsFetcher be doing the same thing (right now, RelatedAppsFetcher is
just owned by RenderFrameImpl via a unique_ptr)? (All I can think of is that we
need to delete these things early in the RenderFrameImpl destructor, rather than
after all the cleanup like unregistering the render frame and removing its
route.)

Destruction order aside, it looks like RelatedAppsFetcher and ManifestManager
will have the same lifetime so it should be safe for RelatedAppsFetcher to
directly have a raw pointer to ManifestManager. (Done.)

> The plan mentioned where the browser filters will let us remove this
> content/renderer code? Definitely we want to be working to get rid of all of
> content/renderer eventually. :)

Not sure what you mean by "browser filters". The eventual plan is to move
ManifestManager into Blink (and thus all the code this CL puts into
content/renderer can move to Blink too). That sounds like a bit of a long
project since it requires Manifest to be accessible to Blink and that probably
means mojoifying Manifest and making a type map for all of its clients. sammc@
is working on it. In the mean time, I made sure *only* the manifest-getting code
is in content/renderer, and all the Mojo-talking code is in Blink.

[Matt Giuca, 2017-02-02 23:36:09]

The CQ bit was checked by mgiuca@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-02-02 23:37:02]

Dry run: 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, 2017-02-03 01:31:22]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-02-03 01:31:23]

Dry run: This issue passed the CQ dry run.

[haraken, 2017-02-03 08:57:37]

On 2017/02/02 23:35:42, Matt Giuca wrote:
> On 2017/02/02 07:34:32, esprehn wrote:
> > I see routing IDs here which is scary, I thought we didn't use those
anymore?

I agree. PS 35 removed the rounting ID, which looks good to me.

> 
> Sorry, I didn't really think that through (I inherited this code from
dhnishi's
> original CL):
>
https://codereview.chromium.org/1756793004/diff/1/content/renderer/installeda...
> which is about a year old so maybe things have changed in the mean time.
> 
> What would you recommend for RelatedAppsFetcher to access the ManifestManager?
> Can it just take a raw pointer to a ManifestManager (which is owned by the
> RenderFrameImpl) directly?
> 
> I'm trying to understand the ownership/lifetime stuff here. RenderFrameImpl
has
> a *raw* pointer to ManifestManager and doesn't delete it; ManifestManager is a
> RenderFrameObserver and deletes itself when it sees the OnDestruct signal,
which
> is in turn called by ~RenderFrameImpl. This seems to be a rather roundabout
> alternative to just having a unique_ptr to a ManifestManager. Is there a
reason
> to do it using a RenderFrameObserver rather than a unique_ptr? And if so,
should
> RelatedAppsFetcher be doing the same thing (right now, RelatedAppsFetcher is
> just owned by RenderFrameImpl via a unique_ptr)? (All I can think of is that
we
> need to delete these things early in the RenderFrameImpl destructor, rather
than
> after all the cleanup like unregistering the render frame and removing its
> route.)

As far as I scan the code, I don't see any reason we cannot use a unique_ptr. I
think RenderFrameImpl should own unique_ptr<ManifestManager>.

> Destruction order aside, it looks like RelatedAppsFetcher and ManifestManager
> will have the same lifetime so it should be safe for RelatedAppsFetcher to
> directly have a raw pointer to ManifestManager. (Done.)
> 
> > The plan mentioned where the browser filters will let us remove this
> > content/renderer code? Definitely we want to be working to get rid of all of
> > content/renderer eventually. :)
> 
> Not sure what you mean by "browser filters". The eventual plan is to move
> ManifestManager into Blink (and thus all the code this CL puts into
> content/renderer can move to Blink too). That sounds like a bit of a long
> project since it requires Manifest to be accessible to Blink and that probably
> means mojoifying Manifest and making a type map for all of its clients. sammc@
> is working on it. In the mean time, I made sure *only* the manifest-getting
code
> is in content/renderer, and all the Mojo-talking code is in Blink.

I think this CL is a good step forward to move more Manifest-related things from
content/renderer/ to Blink.

[Matt Giuca, 2017-02-06 06:45:52]

So, does this need further review or is it good?

(Haraken used "looks good to me" but not the magic acronym... lol.)

[haraken, 2017-02-06 07:44:24]

On 2017/02/06 06:45:52, Matt Giuca wrote:
> So, does this need further review or is it good?
> 
> (Haraken used "looks good to me" but not the magic acronym... lol.)

LGTM if Elliott is happy :)

[Matt Giuca, 2017-02-07 23:57:10]

On 2017/02/06 07:44:24, haraken wrote:
> On 2017/02/06 06:45:52, Matt Giuca wrote:
> > So, does this need further review or is it good?
> > 
> > (Haraken used "looks good to me" but not the magic acronym... lol.)
> 
> LGTM if Elliott is happy :)

Elliott, do you have time to review content/renderer?

[Matt Giuca, 2017-02-07 23:57:18]

The CQ bit was checked by mgiuca@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-02-07 23:58:06]

Dry run: 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, 2017-02-08 00:46:54]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-02-08 00:46:55]

Dry run: Try jobs failed on following builders:
  linux_android_rel_ng on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/linux_androi...)

[esprehn, 2017-02-08 04:40:34]

https://codereview.chromium.org/2488573002/diff/740001/third_party/WebKit/Sou...
File third_party/WebKit/Source/modules/installedapp/InstalledAppController.cpp
(right):

https://codereview.chromium.org/2488573002/diff/740001/third_party/WebKit/Sou...
third_party/WebKit/Source/modules/installedapp/InstalledAppController.cpp:46:
void CallbackFilter::onSuccess(
lets just make this virtual and call the super method or override it. No need to
have callbacks and binding.

[Matt Giuca, 2017-02-08 08:27:11]

https://codereview.chromium.org/2488573002/diff/740001/third_party/WebKit/Sou...
File third_party/WebKit/Source/modules/installedapp/InstalledAppController.cpp
(right):

https://codereview.chromium.org/2488573002/diff/740001/third_party/WebKit/Sou...
third_party/WebKit/Source/modules/installedapp/InstalledAppController.cpp:46:
void CallbackFilter::onSuccess(
On 2017/02/08 04:40:33, esprehn wrote:
> lets just make this virtual and call the super method or override it. No need
to
> have callbacks and binding.

Sorry, I don't understand what you're asking for here. This method is virtual,
and there is no subclass to CallbackFilter.

This is essentially a way of being able to give an AppInstalledCallbacks object
to the content/renderer code, that will 1. do the filter, then 2. call the
callback supplied from NavigatorInstalledApp. This class used to have the
filtering function in it, but then it had to have heaps of state variables too
so I refactored it by moving the state into InstalledAppController and just
having it take a bound filter function (so essentially this class is just a
stateless/logic-less composer now).

Long term, this won't be needed because the code in content/renderer will come
into Blink. But shorter term, I plan to fold InstalledAppController into
NavigatorInstalledApp and then I think this will be significantly simplified as
well. But I tried to do it and ran into a huge rats nest and backed out for the
time being.

So is this OK? (If not, please state concretely what you want me to change.)

[Matt Giuca, 2017-02-13 06:34:50]

https://codereview.chromium.org/2488573002/diff/740001/third_party/WebKit/Sou...
File third_party/WebKit/Source/modules/installedapp/InstalledAppController.cpp
(right):

https://codereview.chromium.org/2488573002/diff/740001/third_party/WebKit/Sou...
third_party/WebKit/Source/modules/installedapp/InstalledAppController.cpp:46:
void CallbackFilter::onSuccess(
On 2017/02/08 08:27:11, Matt Giuca wrote:
> On 2017/02/08 04:40:33, esprehn wrote:
> > lets just make this virtual and call the super method or override it. No
need
> to
> > have callbacks and binding.
> 
> Sorry, I don't understand what you're asking for here. This method is virtual,
> and there is no subclass to CallbackFilter.
> 
> This is essentially a way of being able to give an AppInstalledCallbacks
object
> to the content/renderer code, that will 1. do the filter, then 2. call the
> callback supplied from NavigatorInstalledApp. This class used to have the
> filtering function in it, but then it had to have heaps of state variables too
> so I refactored it by moving the state into InstalledAppController and just
> having it take a bound filter function (so essentially this class is just a
> stateless/logic-less composer now).
> 
> Long term, this won't be needed because the code in content/renderer will come
> into Blink. But shorter term, I plan to fold InstalledAppController into
> NavigatorInstalledApp and then I think this will be significantly simplified
as
> well. But I tried to do it and ran into a huge rats nest and backed out for
the
> time being.
> 
> So is this OK? (If not, please state concretely what you want me to change.)

sammc@ helped me to remove some of the abstraction here (removing the bind and
filter function, and just directly calling filterByInstalledApps). Is this more
what you wanted?

[esprehn, 2017-02-15 01:56:50]

lgtm, the NullableString -> WebString conversions are very sketchy since you
ignore the null bit. Also you wrote DO NOT SUBMIT in there... please fix that
before landing.

https://codereview.chromium.org/2488573002/diff/800001/content/renderer/insta...
File content/renderer/installedapp/related_apps_fetcher.cc (right):

https://codereview.chromium.org/2488573002/diff/800001/content/renderer/insta...
content/renderer/installedapp/related_apps_fetcher.cc:41:
blink::WebString::fromUTF16(relatedApplication.platform.string());
This doesn't preserve the nullability, WebString can be null, but you're making
a null platform into an empty string, is that right?

https://codereview.chromium.org/2488573002/diff/800001/content/renderer/insta...
content/renderer/installedapp/related_apps_fetcher.cc:43:
blink::WebString::fromUTF16(relatedApplication.id.string());
ditto

https://codereview.chromium.org/2488573002/diff/800001/third_party/WebKit/Sou...
File third_party/WebKit/Source/modules/installedapp/InstalledAppController.cpp
(right):

https://codereview.chromium.org/2488573002/diff/800001/third_party/WebKit/Sou...
third_party/WebKit/Source/modules/installedapp/InstalledAppController.cpp:21: //
TODO(mgiuca): Inline this. DO NOT SUBMIT.
???

https://codereview.chromium.org/2488573002/diff/800001/third_party/WebKit/Sou...
third_party/WebKit/Source/modules/installedapp/InstalledAppController.cpp:66: //
https://crbug.com/687846
Do we have a test for this behavior?

[Matt Giuca, 2017-02-15 07:12:53]

Patchset #41 (id:840001) has been deleted

[Matt Giuca, 2017-02-15 07:55:35]

https://codereview.chromium.org/2488573002/diff/800001/content/renderer/insta...
File content/renderer/installedapp/related_apps_fetcher.cc (right):

https://codereview.chromium.org/2488573002/diff/800001/content/renderer/insta...
content/renderer/installedapp/related_apps_fetcher.cc:41:
blink::WebString::fromUTF16(relatedApplication.platform.string());
On 2017/02/15 01:56:50, esprehn wrote:
> This doesn't preserve the nullability, WebString can be null, but you're
making
> a null platform into an empty string, is that right?

Done. (WebString::fromUTF16 has an overload that takes a NullableString16, so
just use that.)

Note: This can't be tested just yet (because the code currently drops this data
on the floor) -- the next CL will pass it to Mojo and it will be tested there (I
have already verified that the tests in the follow-up CL catch this).

https://codereview.chromium.org/2488573002/diff/800001/content/renderer/insta...
content/renderer/installedapp/related_apps_fetcher.cc:43:
blink::WebString::fromUTF16(relatedApplication.id.string());
On 2017/02/15 01:56:50, esprehn wrote:
> ditto

Done.

https://codereview.chromium.org/2488573002/diff/800001/content/renderer/insta...
content/renderer/installedapp/related_apps_fetcher.cc:45:
blink::WebString::fromUTF8(relatedApplication.url.spec());
url doesn't have a concept of null, but I felt it was right to translate
emptiness into null (rather than the empty string).

https://codereview.chromium.org/2488573002/diff/800001/third_party/WebKit/Sou...
File third_party/WebKit/Source/modules/installedapp/InstalledAppController.cpp
(right):

https://codereview.chromium.org/2488573002/diff/800001/third_party/WebKit/Sou...
third_party/WebKit/Source/modules/installedapp/InstalledAppController.cpp:21: //
TODO(mgiuca): Inline this. DO NOT SUBMIT.
On 2017/02/15 01:56:50, esprehn wrote:
> ???

I wrote this as a comment to myself to do before landing the CL. (Basically to
inline all the methods into the class declaration since I don't think they
warrant being outside.)

I write DO NOT SUBMIT so the presubmit will pick up on it if I forget to do it
before landing (you don't need to ask me to act on a DO NOT SUBMIT because I
will be forced to).

https://codereview.chromium.org/2488573002/diff/800001/third_party/WebKit/Sou...
third_party/WebKit/Source/modules/installedapp/InstalledAppController.cpp:66: //
https://crbug.com/687846
On 2017/02/15 01:56:50, esprehn wrote:
> Do we have a test for this behavior?

Hmmmmmm..

This is a "not supposed to happen" case (but it can't be a DCHECK because
theoretically contextDestroyed could be called). I'm not familiar enough with
the lifetime of these objects to know exactly how to trigger it in a test.

I think this will require some major changes to the testing infrastructure (I
had a long chat with sammc@; I don't understand this fully but his suggestion
was to have a layout test with an iframe, triggering a navigation on the iframe
while still holding onto the context in the outer frame, then call
getInstalledRelatedApps on the outer frame).

I think this gets a lot simpler in my plan to combine InstalledAppController
with NavigatorInstalledApp -- I think then this case will actually disappear.

Given the difficulty of testing this, plus the fact that this is technically a
problem with the existing code (I'm just moving this from Line 49, not
introducing it), plus the fact that this isn't launched yet, plus the fact that
this issue might go away when I refactor it, I'm going to save testing this edge
case for later on when this code is simpler.

[Matt Giuca, 2017-02-15 07:55:45]

The CQ bit was checked by mgiuca@chromium.org

[Matt Giuca, 2017-02-15 07:55:45]

The patchset sent to the CQ was uploaded after l-g-t-m from
haraken@chromium.org, esprehn@chromium.org
Link to the patchset: https://codereview.chromium.org/2488573002/#ps880001
(title: "Use SecurityOrigin, not WebSecurityOrigin.")

[commit-bot: I haz the power, 2017-02-15 07:55:54]

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, 2017-02-15 11:01:30]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-02-15 11:01:32]

Try jobs failed on following builders:
  linux_site_isolation on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_site_isol...)

[Matt Giuca, 2017-02-15 23:04:20]

The CQ bit was checked by mgiuca@chromium.org

[commit-bot: I haz the power, 2017-02-15 23:05:12]

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, 2017-02-16 04:05:12]

CQ is committing da patch.

Bot data: {"patchset_id": 880001, "attempt_start_ts": 1487199860405090,
"parent_rev": "3dd2a5021d4d2640bef95c0c79fc01d2727e1f29", "commit_rev":
"be2c7115703a6610b25128809eb388ac28d07285"}

[commit-bot: I haz the power, 2017-02-16 04:09:56]

CQ is committing da patch.

Bot data: {"patchset_id": 880001, "attempt_start_ts": 1487199860405090,
"parent_rev": "3dd2a5021d4d2640bef95c0c79fc01d2727e1f29", "commit_rev":
"be2c7115703a6610b25128809eb388ac28d07285"}

[commit-bot: I haz the power, 2017-02-16 04:10:27]

Description was changed from

==========
Refactor getInstalledRelatedApps code and add manifest logic and tests.

This changes the architecture of getInstalledRelatedApps for "onion
soup". Under the previous (unfinished) architecture,
InstalledAppController (Blink) would call
WebInstalledAppClient::getInstalledRelatedApps (content/renderer) which
was planned to get the related apps from the manifest, then call a Mojo
service in the browser to filter the apps.

This CL renames WebInstalledAppClient to WebRelatedAppsFetcher, and now
its job is *only* to get the related apps from the manifest (not talk to
the browser) and return it to InstalledAppController.
InstalledAppController will (in a future CL) get the browser to filter
the apps.

This CL also implements the concrete RelatedAppsFetcher in
content/renderer which actually gets the related apps from the manifest,
and adds layout tests for the API.

Manifest code based on CL by dhnishi:
https://codereview.chromium.org/1756793004/

BUG=587623
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_site_isolation
==========

to

==========
Refactor getInstalledRelatedApps code and add manifest logic and tests.

This changes the architecture of getInstalledRelatedApps for "onion
soup". Under the previous (unfinished) architecture,
InstalledAppController (Blink) would call
WebInstalledAppClient::getInstalledRelatedApps (content/renderer) which
was planned to get the related apps from the manifest, then call a Mojo
service in the browser to filter the apps.

This CL renames WebInstalledAppClient to WebRelatedAppsFetcher, and now
its job is *only* to get the related apps from the manifest (not talk to
the browser) and return it to InstalledAppController.
InstalledAppController will (in a future CL) get the browser to filter
the apps.

This CL also implements the concrete RelatedAppsFetcher in
content/renderer which actually gets the related apps from the manifest,
and adds layout tests for the API.

Manifest code based on CL by dhnishi:
https://codereview.chromium.org/1756793004/

BUG=587623
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_site_isolation

Review-Url: https://codereview.chromium.org/2488573002
Cr-Commit-Position: refs/heads/master@{#450868}
Committed:
https://chromium.googlesource.com/chromium/src/+/be2c7115703a6610b25128809eb3...
==========

[commit-bot: I haz the power, 2017-02-16 04:10:29]

Committed patchset #42 (id:880001) as
https://chromium.googlesource.com/chromium/src/+/be2c7115703a6610b25128809eb3...