829610 - When updating a packaged app with different sizes in the old vs. new packaged app, we always show the old size in the app update confirmation prompt

Status: VERIFIED FIXED

(Firefox OS Graveyard :: Gaia::System, defect)

Description

[Jason Smith [:jsmith]]

Build: B2G 18 1/11/2013
Device: Unagi

Steps:

1. Install a packaged app with a small size
2. Update it on the server
3. Check for updates (manual or automatic)
4. Click the update notification

Expected:

The size of the packaged app shown should be the *new* size about to be downloaded.

Actual:

The size of the packaged app shown ends up being the *old* size about to be downloaded.

Additional Notes:

Talking with Josh, this is really bad. Why? We are entirely lieing to the user on how resources we are requesting to download. So for example:

- Old Packaged App - 17 KB
- New Packaged App - 20 MB
- Display in Update Prompt - 17 KB

We need to get this fixed for 1.0 release.

Comment 1

[Alexandre Poirot [:ochameau]]

Attachment: Pull request 7605

I'm wondering if we are beaten by the call to `delete aData.app.updateManifest;` right after sending the event dispatch.
It appears that `app.updateManifest` is null in gaia AppUpdatable.availableCallBack method.
The platform code is explicitely adding a `downloadSize` attribute on the app object, so that we can just fetch this attribute instead of manually looking into the update manifest. And this attribute isn't null.

http://mxr.mozilla.org/mozilla-central/source/dom/apps/src/Webapps.jsm#1149
1155       app.downloadAvailable = true;
1156       app.downloadSize = manifest.size;
1157       aData.event = "downloadavailable";
1158       aData.app = {
1159         downloadAvailable: true,
1160         downloadSize: manifest.size,
1161         updateManifest: aManifest
1162       }
1163       DOMApplicationRegistry._saveApps(function() {
1164         DOMApplicationRegistry.broadcastMessage("Webapps:CheckForUpdate:Return:OK",
1165                                                 aData);
1166         delete aData.app.updateManifest;

Comment 2

[Jason Smith [:jsmith]]

I wonder if the "null" piece described in comment 1 could potentially explain what's causing bug 830206. Could this patch fix bug 830206 by caveat?

Comment 3

[Alexandre Poirot [:ochameau]]

Oh! I didn't knew about bug 830206, but I wasn't seeing update size at all. I thought it was the issue behind 829610. So this patch is more about bug 830206 as I was reproducing the no size STR. But there is many chances it will fix both.

Comment 4

[Etienne Segonzac (:etienne)]

Comment on attachment 702067 [details]
Pull request 7605

the change looks good, but please update the updatable_test.js and the mock_app.js accordingly.

Comment 5

[Alexandre Poirot [:ochameau]]

Comment on attachment 702067 [details]
Pull request 7605

Updated the unit test.
And I tried multiple scenarios and it seems to fix both bugs (829610 and 830206)

Comment 6

[Etienne Segonzac (:etienne)]

Comment on attachment 702067 [details]
Pull request 7605

Thanks!

Comment 7

[Alexandre Poirot [:ochameau]]

Landed:
https://github.com/mozilla-b2g/gaia/commit/74dd6b1f85dcd1473b94c52f316824ad08223d2f

Comment 8

[Jason Smith [:jsmith]]

Verified on 1/16 build. lgtm

Comment 9

[Lukas Blakk [:lsblakk] use ?needinfo]

Landed on mozilla-b2g18/gaia master prior to the 1/25 branching to mozilla-b2g18_v1_0_0/v1.0.0, updating status-b2g-v1.0.0 to fixed.