802683 - mozApp.download() packaged app issues

Status: RESOLVED FIXED

(Core Graveyard :: DOM: Apps, defect, P1)

Description

[Etienne Segonzac (:etienne)]

I have a packaged app with |downloadAvailable| true, and calling |.download()| isn't doing much.

Leads:
- We send a |Webapps:Download| message while the ppmm listens for |Webapps::Download|
- |startDownload| is doing a |this.getAppByManifestURL(manifestURL)| while the parameter name is |aManifestURL|

But fixing those isn't enough...

PS: The gaia branch where the basic app update workflow is implemented is here https://github.com/etiennesegonzac/gaia/tree/app-updates

Comment 1

[Etienne Segonzac (:etienne)]

Attachment: Content used for the fake app

Just uncompress at the http server root, I'm pointer fakeapp.com at 127.0.0.1.

The installation is done by opening the browser app, going to fakeapp.com and clicking Install.

The manifest in the zip is different from the one at the root, so the app always has a download available.

Comment 2

[Andrew Overholt [:overholt]]

Fabrice says he'll take this and that it's a blocker.

Comment 3

[[:fabrice] Fabrice Desré]

Attachment: wip

Etienne, can you try this patch? Here's what I got with it:

XXX FIXME : Got a mozContentEvent: webapps-install-granted
-*-*- Webapps.jsm : {"name":"Fake app","installOrigin":"http://fakeapp.com","origin":"http://fakeapp.com","receipts":[],"installTime":1350522676422,"manifestURL":"http://fakeapp.com/manifest.webapp","appStatus":1,"removable":true,"localId":1001,"progress":0,"installState":"pending","downloadAvailable":true,"downloading":true,"readyToApplyDownload":false,"downloadSize":0,"lastUpdateCheck":1350522676422,"etag":"\"3006f0-150-4cc42a828da00\"","basePath":"/home/fabrice/dev/gaia/profile/webapps"}
-*-*- Webapps.jsm : About to download http://fakeapp.com/application.zip
-*-*- Webapps.jsm : onProgress: 5601/5601
[Child 10598] WARNING: NS_ENSURE_TRUE(IsChromeProcess()) failed: file /home/fabrice/dev/inbound/content/base/src/nsFrameMessageManager.cpp, line 687
-*-*- Webapps.jsm : About to fire Webapps:PackageEvent
JavaScript error: app://homescreen.gaiamobile.org/js/grid.js, line 644: Applications.getManifest(...) is undefined

Everything looked good in the webapps.json registry, and indeed when relaunching b2g I had the new app listed and loadable.

Comment 4

[Etienne Segonzac (:etienne)]

Attachment: Wip with fixed typo?

I can't find anywhere where we listen to |Webapps::Download| and the switch case refers to |Webapps:Download|.

So sending a |Webapps:Download|.

Comment 5

[Etienne Segonzac (:etienne)]

With this patch I get a:

-*-*- Webapps.jsm : startDownload: No update manifest found at /home/segonzac/Desktop/gaia/profile/webapps/{4ac2a55a-129b-4622-a79f-76f5c0b4dcdc}/update.webapp http://fakeapp.com/manifest.webapp

But it's unclear to me where the update.webapp should come from.

Some more details to clarify:
- No issue at app install
- even if the manifest in the zip is the same that outside, |downloadAvailable| is always true after the first |checkForUpdate()|
- calling |download()| at this point will trigger the "no update manifest error"

Comment 6

[[:fabrice] Fabrice Desré]

Attachment: wip v2

Another wip for you to try, with also a bunch of new debug statements.

Also, you need to change your test site to use mozApp.installPackage() instead of install() !

Here's a small gaia fix to let this work now that the first manifest is app.updateManifest and not app.manifest for packaged apps:

diff --git a/apps/system/js/permission_manager.js b/apps/system/js/permission_manager.js
index 1ee6ed1..07f8617 100644
--- a/apps/system/js/permission_manager.js
+++ b/apps/system/js/permission_manager.js
@@ -77,8 +77,9 @@ var PermissionManager = (function() {
       return;
     }
 
-    var name = app.manifest.name;
-    var locales = app.manifest.locales;
+    var manifest = app.manifest ? app.manifest : app.updateManifest;
+    var name = manifest.name;
+    var locales = manifest.locales;
     var lang = navigator.language;
     if (locales && locales[lang] && locales[lang].name)
       name = locales[lang].name;

Comment 7

[Etienne Segonzac (:etienne)]

Attachment: WIP v3

Some events names fixes, a pinch of app retrieval change, a zest of typos corrections.

And...
IT'S UPDATING \o/

Comment 8

[Etienne Segonzac (:etienne)]

Downloading and updating still work fine, but new issue (trying to group them here).

Every time checkForUpdate is called, downloadAvailable is set to true, even if the app just got updated and the manifest are identical.

Can this just be an apache config error causing etag issues?

Comment 9

[[:fabrice] Fabrice Desré]

Attachment: wip 4

Etienne, we were not setting the If-None-Match header properly, so we were seeing a new package each time. Can you test again?

Comment 10

[Etienne Segonzac (:etienne)]

Some progress...

* After the initial installation I won't get any downloadAvailable until I change the manifest on the server.

* _But_, after an update I get into the same state than before where checkForUpdate always put |downloadAvailable| to true
* The last version of your patch still calls |this.ApplyDownload| instead of |this.applyDownload|

Comment 11

[Etienne Segonzac (:etienne)]

Attachment: wip v5

Persisting the etag properly on update.

Comment 12

[[:fabrice] Fabrice Desré]

Attachment: patch

Very similar to v5 - I removed a duplicate |let id =...| in startDownload()

Comment 13

[Vivien Nicolas (:vingtetun) (:21) - (NOT reading bugmails, needinfo? please)]

Comment on attachment 674765 [details] [diff] [review]
patch

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

r+ with the debug method turned off by default.

::: dom/apps/src/Webapps.jsm
@@ +19,5 @@
>  Cu.import("resource://gre/modules/PermissionsInstaller.jsm");
>  Cu.import("resource://gre/modules/OfflineCacheInstaller.jsm");
>  
>  function debug(aMsg) {
> +  dump("-*-*- Webapps.jsm : " + aMsg + "\n");

You forgot to comment that back. :)

@@ +848,5 @@
>        aMm.sendAsyncMessage("Webapps:CheckForUpdate:Return:KO", aData);
>        return;
>      }
>  
> +    let installOrigin = app.installOrigin;

Is is used?

Comment 14

[[:fabrice] Fabrice Desré]

https://hg.mozilla.org/integration/mozilla-inbound/rev/3e4d63da1bce

Comment 15

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/3e4d63da1bce

Comment 16

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/767e1f67423a