962858 - test failure(s) installing app with synthetic APKs enabled

Status: RESOLVED FIXED

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

Description

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

My change to enable synthetic APKs (bug 960811) landed <https://hg.mozilla.org/integration/mozilla-inbound/rev/f5c17b4a4199> and then bounced <https://hg.mozilla.org/integration/mozilla-inbound/rev/261584a2a470> today because of mochitest failures <https://tbpl.mozilla.org/?tree=Mozilla-Inbound&rev=f5c17b4a4199> like this one in dom/datastore/tests/test_app_install.html, which calls mozApps.install to install an app, triggering an APK download/install that fails presumably because the test machine doesn't have internet access:

> 01-22 15:02:49.573 E/GeckoConsole( 1863): * * WebappManager.jsm: downloadApk for http://test/tests/dom/datastore/tests/file_app.sjs?testToken=file_app_install.html
> 01-22 15:02:49.573 E/GeckoConsole( 1863): * * WebappManager.jsm: downloading APK from http://dapk.net/application.apk?manifestUrl=http%3A%2F%2Ftest%2Ftests%2Fdom%2Fdatastore%2Ftests%2Ffile_app.sjs%3FtestToken%3Dfile_app_install.html
> 01-22 15:02:49.593 E/GeckoConsole( 1863): * * WebappManager.jsm: downloading APK to /mnt/sdcard/Download/httptesttestsdomdatastoretestsfileappsjstestTokenfileappinstallhtml.apk
> 01-22 15:06:51.253 E/filemap ( 2118): mmap(0,0) failed: Invalid argument
> 01-22 15:06:51.253 W/zipro   ( 2118): Unable to map '/mnt/sdcard/Download/httptesttestsdomdatastoretestsfileappsjstestTokenfileappinstallhtml.apk': Invalid argument
> 01-22 15:06:51.263 W/PackageParser( 2118): Unable to read AndroidManifest.xml of /mnt/sdcard/Download/httptesttestsdomdatastoretestsfileappsjstestTokenfileappinstallhtml.apk
> 01-22 15:06:51.263 W/PackageParser( 2118): java.io.FileNotFoundException: AndroidManifest.xml
…
> 01-22 15:06:51.263 W/PackageInstaller( 2118): Parse error when parsing manifest. Discontinuing installation

- <https://tbpl.mozilla.org/php/getParsedLog.php?id=33420761&tree=Mozilla-Inbound>

We could build APKs for each such test and serve them from the test machines, but there are a growing number of tests that use apps, and building/maintaining APKs for them would be cumbersome and painful.  It might be feasible for Fennec-specific tests (although even then I'm skeptical), but it certainly isn't for tests generally, including cross-platform/project tests like the ones in dom/.

Nor is it feasible to disable these tests.  They need to run on Fennec so we can verify that Fennec supports the platform we claim it provides.

Over in bug 746848, Felipe implemented a browser.mozApps.installer.dry_run flag for the desktop runtime that causes WebappsInstaller to skip native installation of apps.  I wonder if we can do something similar here: "install" the webapp in the registry without natively installing it via an APK, and launch it within Fennec as needed, so tests that install and launch apps still work without requiring their native installation.

Comment 1

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

Attachment: patch v1: disables native installation during automated tests

Here's a patch that implements what I suggested in comment 0: it sets a pref that causes WebappManager to skip APK download/installation and proceed directly to OWA installation during automated testing.

Unlike the flag for the desktop runtime, this one is enabled by default, so all tests get it automatically.  I suspect we actually want the same behavior for the desktop runtime, but I'll suggest that in a separate bug against the desktop runtime.

With this patch, tests that were failing are succeeding locally.  Try push:

https://tbpl.mozilla.org/?tree=Try&rev=1731cd5c89ba

Comment 2

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

Comment on attachment 8364787 [details] [diff] [review]
patch v1: disables native installation during automated tests

This is fine to me.

I was thinking about the naming of the pref (pedantic, I know). We have two patterns:
"browser.webapps.apkFactoryUrl"
"dom.webapps.useCurrentProfile"

I don't know if we want to use a single style or not. The touched code does cross both "browser" and "dom". Food for future thought.

Comment 3

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

(In reply to Mark Finkle (:mfinkle) from comment #2)
> I was thinking about the naming of the pref (pedantic, I know).

I'm right there with you!


> We have two
> patterns:
> "browser.webapps.apkFactoryUrl"
> "dom.webapps.useCurrentProfile"
> 
> I don't know if we want to use a single style or not. The touched code does
> cross both "browser" and "dom". Food for future thought.

We also have webapps.update.interval, which is currently only used by B2G, although I'm currently using it in the updates patch.

I think it's reasonable to have two styles, especially if the preferences are being accessed exclusively in one component or another (as they are in this case), or at least the DOM code doesn't depend on browser.* preferences, since the runtime (i.e. browser) is upstack from it.

But I could be convinced otherwise!  I've also considered a runtime.webapps namespace for prefs like apkFactoryUrl, to clarify that the prefs in question are for the runtime.

Comment 4

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

Here's a full try run to make sure I didn't break anything else in the process of fixing those tests:

https://tbpl.mozilla.org/?tree=Try&rev=327cc714d889

Comment 5

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

https://hg.mozilla.org/integration/mozilla-inbound/rev/78f00eacf00a

Comment 6

[Phil Ringnalda (:philor)]

https://hg.mozilla.org/mozilla-central/rev/78f00eacf00a