I can take this.

We need an end-to-end test (even just a trivial one) to verify that the Mojo implementation is working.

Troy, did you ever find the culprit CL for this?

CL to add end-to-end test: https://chromium-review.googlesource.com/c/597608 (fails).

I can't figure out what the problem is though. From Troy's email:

> From what I gathered, we're not calling registerInterfaces on the ChromeRenderFrameHostInterfaceRegistrar,
> and so our InstalledAppProvider is never hit. I couldn't seem to find any changes that would have caused
> this to break, would you have any idea what could have happened here Matt?

ChromeRenderFrameHostInterfaceRegistrar is registered up in registerMojoInterfaces in ChromeInterfaceRegistrar.java. I've verified that the registration code is hit. However, you're right that registerInterfaces is never being called; if I put a crash in there, it doesn't crash (the E2E test just times out because getInstalledRelatedApps' promise never returns).

It's supposed to be called here:
https://cs.chromium.org/chromium/src/content/public/android/java/src/org/chromium/content_public/browser/InterfaceRegistrar.java?l=88

I'll have to dig deeper into why this isn't being called (unless someone else is looking at this already?). A bisect may be in order. There's nothing obviously wrong. Troy: Can you give me an approximate date of the last time you know this was working?

I'll give a proper repro steps:

1. Go to https://killer-marmot.appspot.com/web

Expected behaviour:
Under "getInstalledRelatedApps" it should say "Installed related apps:" <nothing>. (Or it may list an app, if you have the correct app installed.)

Actual behaviour:
Under "getInstalledRelatedApps", there is nothing.

This code in chrome_content_browser_client.cc (ChromeContentBrowserClient::InitFrameInterfaces) IS being hit:

  frame_interfaces_parameterized_->AddInterface(base::Bind(
      &ForwardToJavaWebContentsRegistry<blink::mojom::InstalledAppProvider>));

But ForwardToJavaFrameRegistry is never being called from that binding. I think getInstalledRelatedApps is the only API that uses ForwardToJavaFrameRegistry.

All this code was changed in r480972 (ben@). I'll have a look at that CL soon.

Confirmed the regression was caused by r480972.

Ah lol, the bug can be spotted by carefully reading my comment #3:

  frame_interfaces_parameterized_->AddInterface(base::Bind(
      &ForwardToJavaWebContentsRegistry<blink::mojom::InstalledAppProvider>));

It's supposed to be ForwardToJavaFrameRegistry. I'll fix in the same CL (https://chromium-review.googlesource.com/c/597608).

I wonder why this isn't a type error (since InstalledAppProvider takes a RenderFrameHost).

OK makes sense: it's not a type error because it's simply the case that the WebContents has no InstalledAppProvider service attached to it. When the request is forwarded to the WebContents, it simply fails (as if nothing had been registered at all).

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/ee4aefb654913f995a8bad2bc4f95ab341573251

commit ee4aefb654913f995a8bad2bc4f95ab341573251
Author: Matt Giuca <mgiuca@chromium.org>
Date: Fri Aug 04 03:11:55 2017

Fix getInstalledRelatedApps on Android.

Mojo bindings were incorrectly registered since r480972, resulting in
calls to Navigator.getInstalledRelatedApps never resolving or rejecting.
This correctly registers with a RenderFrameHost, not WebContents.

Adds an end-to-end test for getInstalledRelatedApps on Android, which
would've caught this breakage. (The feature is already covered by Blink
Layout Tests and Android Java unit tests, but there was nothing that
ensures the whole system is working.)

Bug:  750348 
Change-Id: Ife895f26c42c6344906ea842ffb48f73d881b236
Reviewed-on: https://chromium-review.googlesource.com/597608
Commit-Queue: Matt Giuca <mgiuca@chromium.org>
Reviewed-by: Ken Rockot <rockot@chromium.org>
Cr-Commit-Position: refs/heads/master@{#491924}
[modify] https://crrev.com/ee4aefb654913f995a8bad2bc4f95ab341573251/chrome/browser/chrome_content_browser_client.cc

Fixed (tests incoming in https://crrev.com/c/601132).

Requesting a merge to M61. Rationale: A feature was inadvertently removed; this puts it back. Single-line fix.

This bug requires manual review: M61 has already been promoted to the beta branch, so this requires manual review
Please contact the milestone owner if you have questions.
Owners: amineer@(Android), cmasso@(iOS), ketakid@(ChromeOS), govind@(Desktop)

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot

Approved for M61 branch 3163.

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/fade9f5ecde6d6902d1c578778f636b97918d911

commit fade9f5ecde6d6902d1c578778f636b97918d911
Author: Matt Giuca <mgiuca@chromium.org>
Date: Tue Aug 08 01:41:04 2017

Fix getInstalledRelatedApps on Android.

Mojo bindings were incorrectly registered since r480972, resulting in
calls to Navigator.getInstalledRelatedApps never resolving or rejecting.
This correctly registers with a RenderFrameHost, not WebContents.

Adds an end-to-end test for getInstalledRelatedApps on Android, which
would've caught this breakage. (The feature is already covered by Blink
Layout Tests and Android Java unit tests, but there was nothing that
ensures the whole system is working.)

TBR=mgiuca@chromium.org

(cherry picked from commit ee4aefb654913f995a8bad2bc4f95ab341573251)

Bug:  750348 
Change-Id: Ife895f26c42c6344906ea842ffb48f73d881b236
Reviewed-on: https://chromium-review.googlesource.com/597608
Commit-Queue: Matt Giuca <mgiuca@chromium.org>
Reviewed-by: Ken Rockot <rockot@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#491924}
Reviewed-on: https://chromium-review.googlesource.com/604969
Reviewed-by: Matt Giuca <mgiuca@chromium.org>
Cr-Commit-Position: refs/branch-heads/3163@{#376}
Cr-Branched-From: ff259bab28b35d242e10186cd63af7ed404fae0d-refs/heads/master@{#488528}
[modify] https://crrev.com/fade9f5ecde6d6902d1c578778f636b97918d911/chrome/browser/chrome_content_browser_client.cc

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/ce25855bed32388a7c94ef99a9fa35884fce0efa

commit ce25855bed32388a7c94ef99a9fa35884fce0efa
Author: Matt Giuca <mgiuca@chromium.org>
Date: Thu Aug 10 08:05:57 2017

Added end-to-end test for getInstalledRelatedApps on Android.

Regression test for  https://crbug.com/750348 . (The feature is already
covered by Blink Layout Tests and Android Java unit tests, but there was
nothing that ensures the whole system is working.)

Bug:  750348 
Change-Id: I93d9ac66658a486e7329cbed98af5c2072204f84
Reviewed-on: https://chromium-review.googlesource.com/601132
Reviewed-by: David Trainor <dtrainor@chromium.org>
Commit-Queue: Matt Giuca <mgiuca@chromium.org>
Cr-Commit-Position: refs/heads/master@{#493329}
[modify] https://crrev.com/ce25855bed32388a7c94ef99a9fa35884fce0efa/chrome/android/BUILD.gn
[modify] https://crrev.com/ce25855bed32388a7c94ef99a9fa35884fce0efa/chrome/android/java_sources.gni
[add] https://crrev.com/ce25855bed32388a7c94ef99a9fa35884fce0efa/chrome/android/javatests/src/org/chromium/chrome/browser/InstalledAppTest.java
[add] https://crrev.com/ce25855bed32388a7c94ef99a9fa35884fce0efa/content/test/data/android/installedapp.html