982557 - updating Fennec to a version with Synthetic APKs triggers app updates that hang on launch

Status: VERIFIED FIXED

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

Description

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

Updating Fennec to a version that supports Synthetic APKs and then checking for updates causes Fennec to tell you that your apps have updates and then prompt you to install them.  If you confirm the updates, it installs synthetic APKs.  But after you launch them, they hang on the splash screen and never run.

Steps to Reproduce:

1. install a version of Fennec without Synthetic APKs;
2. install an app;
3. update Fennec to a version with Synthetic APKs;
3. go to about:apps and "Check for Updates";
4. confirm download and then installation of the update;
5. launch the app via its icon on the All Apps screen.

Expected Results: the app launches and runs.

Actual Results: the app launches, but it doesn't run and hangs at the splash screen.

Comment 1

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

Something like this might fix the problem (and implement a minimally sufficient migration flow as a side-effect): on firstrun, check if there's an old appKey.  If so, migrate it to a new appKey, migrate iconKey, create originKey, and launch the webapp in a way that makes it aware that it needs to update the registry record to add the apkPackageName property.

Comment 2

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

Attachment: patch v1: migrates prefs, sets apkPackageName

There are two things we need to do:

1. Migrate old SharedPreferences to their new names.  There are two prefs, one that stores the origin (which used to be called "app") and another that stores the background color for the splash screen (which used to be called "icon").

(The origin pref is the important one; without it, WebappImpl.onCreate thinks the app is in the process of being installed into the DOMApplicationRegistry and stalls waiting for that installation to complete.)

2. Set apkPackageName to the APK package name in the DOMApplicationRegistry record.  Without this, the EventListener.getApkVersions doesn't get the APK's version, so checking for updates (manually or automatically) always finds an update for the app.

Comment 3

[Mark Finkle (:mfinkle) (use needinfo?)]

Comment on attachment 8393862 [details] [diff] [review]
patch v1: migrates prefs, sets apkPackageName

>diff --git a/mobile/android/base/webapp/Allocator.java b/mobile/android/base/webapp/Allocator.java
 
>+    private static final String PREFIX_OLD_APP = "app";
>+    private static final String PREFIX_OLD_ICON = "icon";

Add a comment about these being used for migration. Maybe we could plan to remove this code in the future.

>+    public void maybeMigrateOldPrefs(int index) {

>+        // Remove the old prefs so we don't migrate them the next time around.
>+        mPrefs.edit().remove(oldAppKey(index)).remove(oldIconKey(index)).apply();

editor.apply() is not available on Froyo (Android 2.2) which Fennec still supports (ugh). I know a bug was filed to fix this in general, but I don't remember if it was already fixed or if we decided to not block on it.

Comment 4

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

Attachment: patch v2: adds comment about prefixes being used for migration

(In reply to Mark Finkle (:mfinkle) from comment #3)

> >+    private static final String PREFIX_OLD_APP = "app";
> >+    private static final String PREFIX_OLD_ICON = "icon";
> 
> Add a comment about these being used for migration. Maybe we could plan to
> remove this code in the future.

Ok, I added this comment:

    // These prefixes are for prefs used by the old shortcut-based runtime.
    // We define them here so maybeMigrateOldPrefs can migrate them to their
    // new equivalents if this app was originally installed as a shortcut.
    // Maybe we can remove this code in the future!

https://github.com/mykmelez/gecko-dev/commit/cef9769ac2fcf541dcbbab4fee78ca0aea233877


> >+    public void maybeMigrateOldPrefs(int index) {
> 
> >+        // Remove the old prefs so we don't migrate them the next time around.
> >+        mPrefs.edit().remove(oldAppKey(index)).remove(oldIconKey(index)).apply();
> 
> editor.apply() is not available on Froyo (Android 2.2) which Fennec still
> supports (ugh). I know a bug was filed to fix this in general, but I don't
> remember if it was already fixed or if we decided to not block on it.

It's bug 973728, which we decided not to block on, but which I just assigned to myself so I can dig into it.  Note that this change doesn't make the problem worse, because maybeMigrateOldPrefs calls putOrigin and updateColor before it removes the old prefs, and both of those methods also call editor.apply.  So we might as well fix this instance when we fix those.


Here's the patch with the comments added.  This is the version I'll commit.

Comment 5

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

https://hg.mozilla.org/integration/mozilla-inbound/rev/2f5fff1478a8

Comment 6

[Phil Ringnalda (:philor)]

https://hg.mozilla.org/mozilla-central/rev/2f5fff1478a8

Comment 7

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

Comment on attachment 8395163 [details] [diff] [review]
patch v2: adds comment about prefixes being used for migration

[Approval Request Comment]
Bug caused by (feature/regressing bug #): 
  Bug 934760.

User impact if declined: 
  Fennec will offer users updates to their apps, and the updates will download
  and install, but they'll hang on launch.

Testing completed (on m-c, etc.): 
  I tested this locally, and it landed on Central four days ago.

Risk to taking this patch (and alternatives if risky): 
  The patch is straightforward and low-risk.

String or IDL/UUID changes made by this patch:
  None.

Comment 8

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/a2c41778abf3
https://hg.mozilla.org/releases/mozilla-beta/rev/df34df65cbd0

Comment 9

[Cristina Madaras, QA [:CristinaM]]

This is what happens after updating Nightly from version 28 to Nightly 31.0a1 (2014-04-02) and Beta from 28.0b10 to 29.0b4:
-the app is updated but can be launched from Home Screen or from about:apps only after the browser is restarted.
Can we consider this bug fixed and log a new one for the issue mentioned above?

Comment 10

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

(In reply to cristina.madaras from comment #9)
> This is what happens after updating Nightly from version 28 to Nightly
> 31.0a1 (2014-04-02) and Beta from 28.0b10 to 29.0b4:
> -the app is updated but can be launched from Home Screen or from about:apps
> only after the browser is restarted.
> Can we consider this bug fixed and log a new one for the issue mentioned
> above?

Yes, please do file a new bug!

Comment 11

[Cristina Madaras, QA [:CristinaM]]

I verified again this bug and is reproducible with other scenario, so I logged this new bug - Bug 997644.

Comment 12

[Cristina Madaras, QA [:CristinaM]]

Depends on Bug 997644.

Comment 13

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

(In reply to cristina.madaras from comment #12)
> Depends on Bug 997644.

Erm, do you mean that this bug blocks bug 997644?  This bug is resolved fixed, while that bug is still open, so this bug can't depend on that one (unless it isn't really fixed).

Comment 14

[Cristina Madaras, QA [:CristinaM]]

(In reply to Myk Melez [:myk] [@mykmelez] from comment #13)

> Erm, do you mean that this bug blocks bug 997644?  This bug is resolved
> fixed, while that bug is still open, so this bug can't depend on that one
> (unless it isn't really fixed).

This can't be verified because of Bug 997644.

Comment 15

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

(In reply to cristina.madaras from comment #14)
> (In reply to Myk Melez [:myk] [@mykmelez] from comment #13)
> 
> > Erm, do you mean that this bug blocks bug 997644?  This bug is resolved
> > fixed, while that bug is still open, so this bug can't depend on that one
> > (unless it isn't really fixed).
> 
> This can't be verified because of Bug 997644.

Aha, thanks for clarifying!  But is that really correct?  The Steps to Reproduce in comment 0 don't say to launch the app from Firefox's about:apps page; rather, they say to launch it from Android's All Apps screen.  So it should be possible to verify this bug by launching the app from All Apps, even though bug 997644 makes it impossible to launch the app from about:apps.

Comment 16

[Cristina Madaras, QA [:CristinaM]]

Ok, because this is not reproducible with steps from comment 0, I think we can consider this fixed.

Comment 17

[Cristina Madaras, QA [:CristinaM]]

I will mark this as verified fixed based on comment 15 & comment 16.