959244 - APKs installed from anywhere except the marketplace fail to run.

Status: VERIFIED FIXED

(Firefox for Android Graveyard :: Web Apps (PWAs), defect)

Description

[James Hugman [:jhugman] [@jhugman]]

Steps to reproduce:

1. Launch fennec. 
2. Browse to the firefox marketplace
3. Install any app. e.g. Chess.
4. Launch the app.
5. Observe that it runs. 
6. Uninstall fennec. 
7. Install fennec. At this point, the APK is installed, but the webapp that is contained in the APK isn't. 
8. Launch the APK.

Expected: the splash screen shows, the web app installs, and then runs.
Observed: the splash screen shows, and then shows about:blank.

Comment 1

[James Hugman [:jhugman] [@jhugman]]

Attachment: Manifest URL and name not being passed back to Java.

Comment 2

[Myk Melez [:myk] [@mykmelez]]

Comment on attachment 8361168 [details] [diff] [review]
Manifest URL and name not being passed back to Java.

Review of attachment 8361168 [details] [diff] [review]:
-----------------------------------------------------------------

This seems alright, except that it adds some information to the message while at the same time it makes WebAppImpl stop relying on the message for that information.  Shouldn't we do either one or the other but not both?

After all, I don't see any other message handlers that might want that information, nor does WebAppImpl fall back on the information in the message if it can't get it from the ApkResources object.  If it did, because sometimes ApkResources doesn't have that info, then it would make sense to still send it in the message.

As it stands, I would only do one of these things.  But tell me if I'm mistaken!

Comment 3

[Myk Melez [:myk] [@mykmelez]]

Comment on attachment 8361168 [details] [diff] [review]
Manifest URL and name not being passed back to Java.

Feedback+ from me (modulo earlier remarks), so requesting review from wesj.

Comment 4

[Wesley Johnston (:wesj)]

Comment on attachment 8361168 [details] [diff] [review]
Manifest URL and name not being passed back to Java.

Review of attachment 8361168 [details] [diff] [review]:
-----------------------------------------------------------------

Like myk said, one or the other, but not both fixes.

::: mobile/android/base/webapp/WebAppImpl.java
@@ -293,5 @@
>          if (event.equals("WebApps:PostInstall")) {
>              String origin = message.optString("origin");
> -            String manifestUrl = message.optString("manifestURL");
> -            String name = message.optString("name", "WebApp");
> -            launchWebApp(origin, manifestUrl, name);

I think I'd lean towards leaving this in maybe.... and not using mApkResources. I'm not sure which (if either) is more reliable at this point. Is it possible that the Gecko install may have redirected the manifestURL or name?

Comment 5

[James Hugman [:jhugman] [@jhugman]]

The manifest url in mApkResources is the ultimate source of truth.

The manifest itself is only downloaded on the server, and is bundled with the APK.

I will remove the manifest URL from the Post:Install message.

Comment 6

[James Hugman [:jhugman] [@jhugman]]

Attachment: Second patch that addresses nits from previous r+ reviews.

Comment 7

[Myk Melez [:myk] [@mykmelez]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/771eb936595f

Comment 8

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/771eb936595f

Comment 9

[Myk Melez [:myk] [@mykmelez]]

This was actually fixed by jhugman, although I accidentally credited myself in the commit log.  My bad!