823150 - Installing a signed packaged app, updating it on the server, and doing a manual sync - no update notification found for at least 45 minutes

Status: VERIFIED FIXED

(Firefox OS Graveyard :: General, defect)

Description

[Jason Smith [:jsmith]]

Build: B2G 18 12/19/2012
Device: Unagi

Steps:

1. Install the first version of the privileged app here - https://marketplace-dev.allizom.org/app/privileged-app-test
2. After install completes, switch the version on marketplace to be the 1.1 version instead of 1.0 version that contains app manifest & content changes
3. On the device, do a manual sync

Expected:

An update notification should fire indicating an update is available for this signed packaged privileged app.

Actual:

No update notification fires.

Comment 1

[Jason Smith [:jsmith]]

Attachment: Version 1.0 Packaged App Zip File

Comment 2

[Jason Smith [:jsmith]]

Attachment: Version 1.1 Packaged App Zip File

Comment 3

[Jason Smith [:jsmith]]

Attachment: Version 1.0 Mini-manifest

Comment 4

[Jason Smith [:jsmith]]

Attachment: Version 1.1 Mini-Manifest

Comment 5

[Jason Smith [:jsmith]]

Etienne - Any ideas?

Note - since this is a signed privileged app, we'll only be able to test through marketplace dev. If you need access to be a developer of that app, let me know.

Comment 6

[Jason Smith [:jsmith]]

A detailed summary of how to go about testing this on dev:

1. Submit the version 1.0 of this privileged app to marketplace-dev.allizom.org
2. Get a reviewer to approve the app and push it out publicly
3. Install the privileged app to your device
4. Upload a new version of the privileged app (v1.1 zip file attached) in the manage status & versions area
5. Get a reviewer to approve the app and push it out publicly
6. Go to settings --> device info --> check for updates (check now)

Expected - You should get an update notification fired.

Actual - No update notification fires.

Comment 7

[Jason Smith [:jsmith]]

Wait a second...I just got the update. But that took a while....

Comment 8

[Jason Smith [:jsmith]]

I'm going to dig into this one a bit more to try to get more information.

Comment 9

[Rob Hudson [:robhudson]]

I checked with Jason and this shouldn't be cached by anything out in front of our webheads.

I did a test of my own and verified with curl that things look good:

$ curl -v https://marketplace-dev.allizom.org/app/a1455d70-0082-4d48-bade-549a964bd811/manifest.webapp
> GET /app/a1455d70-0082-4d48-bade-549a964bd811/manifest.webapp HTTP/1.1
> User-Agent: curl/7.24.0 (x86_64-apple-darwin12.0) libcurl/7.24.0 OpenSSL/0.9.8r zlib/1.2.5
> Host: marketplace-dev.allizom.org
> Accept: */*
> 
< HTTP/1.1 200 OK
< Server: gunicorn/0.15.0
< Vary: X-Requested-With, Accept-Language, Cookie, X-Mobile, User-Agent
< X-Backend-Server: dev1.addons.phx1.mozilla.com
< Content-Type: application/x-web-app-manifest+json
< Strict-Transport-Security: max-age=2592000
< Date: Wed, 19 Dec 2012 21:40:45 GMT
< Transfer-Encoding: chunked
< ETag: "2d4d08e7ad605415182ed6d526fe07a3"
< X-Content-Security-Policy-Report-Only: policy-uri /services/csp/policy?build=d9d9
< Via: Moz-pp-zlb09
< Connection: keep-alive
< Set-Cookie: lang="en-US\054"; Path=/
< Set-Cookie: region=us; Path=/
< x-frame-options: DENY
< 
{"name": "Tookchande Cuowblelpen Ethsci", "icons": {"32": "/icons/32.png", "256": "/icons/256.png", "16": "/icons/16.png"}, "version": "1.0", "package_path": "https://marketplace-dev.allizom.org/downloads/file/181513/tookchande-cuowblelpen-eths-1.0.zip", "size": 96760, "release_notes": null, "locales": {"pt-br": {"name": "Ovtucout Lievingtant Thimartse", "description": "Futmastbubtiz ightlyterock trapourin necspeedne cyca ovtucout bagrar yleri io ightlyterock didhu itnowing hispo necspeedne aclivtournban."}, "es": {"name": "Ivafu Adealor Itnowing", "description": "Rincal trapourin pondwioxer hispo abertirym clastsordbenver ightlyterock aclivtournban bipostred hawshansperif telbatwhim headinphy ousmemy gemastac enellwa."}}, "developer": {"url": "http://mozillalabs.com", "name": "Mozilla Labs"}}


I tried again with the "If-None-Match" header to verify the 304 happens w/o any updates:

$ curl -v https://marketplace-dev.allizom.org/app/a1455d70-0082-4d48-bade-549a964bd811/manifest.webapp -H 'If-None-Match: "2d4d08e7ad605415182ed6d526fe07a3"'
> GET /app/a1455d70-0082-4d48-bade-549a964bd811/manifest.webapp HTTP/1.1
> User-Agent: curl/7.24.0 (x86_64-apple-darwin12.0) libcurl/7.24.0 OpenSSL/0.9.8r zlib/1.2.5
> Host: marketplace-dev.allizom.org
> Accept: */*
> If-None-Match: "2d4d08e7ad605415182ed6d526fe07a3"
> 
< HTTP/1.1 304 NOT MODIFIED
< Server: gunicorn/0.15.0
< Vary: X-Requested-With, Accept-Language, Cookie, X-Mobile, User-Agent
< X-Backend-Server: dev2.addons.phx1.mozilla.com
< Content-Type: text/html; charset=utf-8
< Strict-Transport-Security: max-age=2592000
< Date: Wed, 19 Dec 2012 21:43:13 GMT
< ETag: "2d4d08e7ad605415182ed6d526fe07a3"
< X-Content-Security-Policy-Report-Only: policy-uri /services/csp/policy?build=d9d9
< Via: Moz-pp-zlb09
< Connection: keep-alive
< Set-Cookie: lang="en-US\054"; Path=/
< Set-Cookie: region=us; Path=/
< x-frame-options: DENY
< Content-Length: 0


I then uploaded a new version and approved it as a reviewer and hit it again:

$ curl -v https://marketplace-dev.allizom.org/app/a1455d70-0082-4d48-bade-549a964bd811/manifest.webapp -H 'If-None-Match: "2d4d08e7ad605415182ed6d526fe07a3"'
> GET /app/a1455d70-0082-4d48-bade-549a964bd811/manifest.webapp HTTP/1.1
> User-Agent: curl/7.24.0 (x86_64-apple-darwin12.0) libcurl/7.24.0 OpenSSL/0.9.8r zlib/1.2.5
> Host: marketplace-dev.allizom.org
> Accept: */*
> If-None-Match: "2d4d08e7ad605415182ed6d526fe07a3"
> 
< HTTP/1.1 200 OK
< Server: gunicorn/0.15.0
< Vary: X-Requested-With, Accept-Language, Cookie, X-Mobile, User-Agent
< X-Backend-Server: dev1.addons.phx1.mozilla.com
< Content-Type: application/x-web-app-manifest+json
< Strict-Transport-Security: max-age=2592000
< Date: Wed, 19 Dec 2012 21:45:40 GMT
< Transfer-Encoding: chunked
< ETag: "444430df3223c5e2e7e1b9d0790d9fa1"
< X-Content-Security-Policy-Report-Only: policy-uri /services/csp/policy?build=d9d9
< Via: Moz-pp-zlb09
< Connection: keep-alive
< Set-Cookie: lang="en-US\054"; Path=/
< Set-Cookie: region=us; Path=/
< x-frame-options: DENY
< 
{"name": "Tookchande Cuowblelpen Ethsci", "icons": {"32": "/icons/32.png", "256": "/icons/256.png", "16": "/icons/16.png"}, "version": "1.1", "package_path": "https://marketplace-dev.allizom.org/downloads/file/181519/tookchande-cuowblelpen-eths-1.1.zip", "size": 96774, "release_notes": "This is version 1.1", "locales": {"pt-br": {"name": "Aclivtournban Ovtucout Headinphy", "description": "Bogiki yleri atsusheco rincal cuowblelpen entquarver omcunmane ererly slilstwhoomre ivafu adealor trapourin didhu ereaser plamapcasnem."}, "es": {"name": "Telbatwhim Entquarver Trapourin", "description": "Yleri pencout plamapcasnem moundetmak ousmemy pencout nerthocylfon yleri enellwa futmastbubtiz headinphy adealor crousdowgrist bipostred ousmemy."}}, "developer": {"url": "http://mozillalabs.com", "name": "Mozilla Labs"}}

It looks like the server is doing the right thing. Do we need to provide any other headers here, e.g., to explicitly tell the browser to not cache this client-side?

Comment 10

[Jason Smith [:jsmith]]

Just retested this with a brand new privileged app doing a similar repro steps. Same bug occurs. So this is definitely 100% reproducible.

Comment 11

[David Scravaglieri [:scravag]]

Not blocking but tracking. Workaround would be to delete the app then reload it from Marketplace.

Comment 12

[Etienne Segonzac (:etienne)]

(In reply to David Scravaglieri [:scravag] from comment #11)
> Not blocking but tracking. Workaround would be to delete the app then reload
> it from Marketplace.

Not blocking but marked as blocker?

Anyway I'd argue that the workaround is to wait 45 minutes :)

Comment 13

[Etienne Segonzac (:etienne)]

(In reply to Rob Hudson [:robhudson] from comment #9)
> It looks like the server is doing the right thing. Do we need to provide any
> other headers here, e.g., to explicitly tell the browser to not cache this
> client-side?

Thanks, very helpful!
And indeed, those look good.

Comment 14

[Tim Guan-tin Chien [:timdream] (please needinfo)]

How is this a Gaia::System bug?

Comment 15

[Etienne Segonzac (:etienne)]

(In reply to Jason Smith [:jsmith] from comment #5)
> Etienne - Any ideas?
> 
> Note - since this is a signed privileged app, we'll only be able to test
> through marketplace dev. If you need access to be a developer of that app,
> let me know.

Can you reproduce the bug with a non-certified app?
Can you uninstall the certified app in question?

Comment 16

[Fernando Jiménez Moreno [:ferjm]]

(In reply to David Scravaglieri [:scravag] from comment #11)
> Not blocking but tracking. Workaround would be to delete the app then reload
> it from Marketplace.

Did you mean to r- this bug?

Comment 17

[Jason Smith [:jsmith]]

(In reply to David Scravaglieri [:scravag] from comment #11)
> Not blocking but tracking. Workaround would be to delete the app then reload
> it from Marketplace.

That's not an acceptable work-around. That basically destroys the whole point of the app updates feature. So I completely disagree with you.

Comment 18

[Jason Smith [:jsmith]]

(In reply to Etienne Segonzac (:etienne) from comment #15)
> (In reply to Jason Smith [:jsmith] from comment #5)
> > Etienne - Any ideas?
> > 
> > Note - since this is a signed privileged app, we'll only be able to test
> > through marketplace dev. If you need access to be a developer of that app,
> > let me know.
> 
> Can you reproduce the bug with a non-certified app?
> Can you uninstall the certified app in question?

This bug actually is about a non-certified app. I was testing it with a privileged signed packaged app from marketplace.

Comment 19

[Jason Smith [:jsmith]]

(In reply to Tim Guan-tin Chien [:timdream] (MoCo-TPE) from comment #14)
> How is this a Gaia::System bug?

I originally thought it could be a front-end bug with the updater, but I tink you are right. Fabrice was mentioning there was some problem with how we handle the etags we are getting from marketplace. I wonder if that's what causing this bug.

Comment 20

[Andrew Overholt [:overholt]]

Fabrice is going to reproduce this.

Comment 21

[[:fabrice] Fabrice Desré]

I reproduced Jason's steps from comment #0, and the platform side correctly checked that we have an update and set the downloadAvailable property on the application, and the main update prompt correctly reports:

UpdatePrompt: appsUpdated: 1 apps to update

So I tend to think that something regressed in gaia.

Comment 22

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

Over to Etienne then.

Comment 23

[Caitlin Galimidi [:c8o]]

bonjour etienne! j'espere que vos vacances s'est bien passe!

could you please let us know the status on this when you get a moment?
when do you expect to land a fix?

Comment 24

[Etienne Segonzac (:etienne)]

I'll have a patch tomorrow but there is gaia and a gecko part so it might take a little time to land.

Comment 25

[Etienne Segonzac (:etienne)]

So, this was caused by 2 issues:

- On the gaia side, we weren't creating the AppUpdatable object properly for fresh app installations
- On the gecko side the ondowloadavailable callback was called only on the app instance where checkForUpdate() was called

Patches incoming...

Comment 26

[Etienne Segonzac (:etienne)]

Attachment: Gaia part

Comment 27

[Etienne Segonzac (:etienne)]

Attachment: Gecko part

Comment 28

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

Comment on attachment 696271 [details] [diff] [review]
Gecko part

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

r+ with a comment or with the use of the proper fix from Margaret's patch if it lands sooner.

Comment 29

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

Comment on attachment 696271 [details] [diff] [review]
Gecko part

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

Sorry I was trying to r+ the other part. I use the wrong window...

Comment 30

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

Comment on attachment 696270 [details] [diff] [review]
Gaia part

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

r+ with a comment or with the use of the proper fix from Margaret's patch if it lands sooner.

::: apps/system/js/updatable.js
@@ +16,5 @@
>    this._mgmt = navigator.mozApps.mgmt;
>    this.app = app;
>  
> +  var manifest = app.manifest ? app.manifest : app.updateManifest;
> +  this.name = manifest.name;

The name won't be localize correctly. This could be nice to add a 'Bug 824418' as a comment so it could be easily find when the bug will be resolved.

Comment 31

[Etienne Segonzac (:etienne)]

Gaia PR with comment addressed:
https://github.com/mozilla-b2g/gaia/pull/7228

Comment 32

[Ryan VanderMeulen [:RyanVM]]

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

Comment 33

[Graeme McCutcheon [:graememcc]]

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

Comment 34

[Jason Smith [:jsmith]]

Still need to land the Gaia part. Reopening.

Comment 35

[Etienne Segonzac (:etienne)]

https://github.com/mozilla-b2g/gaia/commit/9044cb46d4997adaa9adc576731ec75ee2e48759

Comment 36

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/5eceeeb7d157
https://hg.mozilla.org/releases/mozilla-b2g18/rev/1ea414302535

Comment 37

[Jason Smith [:jsmith]]

See bug 827047 for the followup bug. Looks like the gecko portion of this patch *might* be working, but the Gaia piece definitely isn't.

Tested on a 1/5/2013 build.

Comment 38

[Jason Smith [:jsmith]]

Marking as verified as I can confirm an update happened within the 30 sec timeframe Etienne identified with a different app update. The followup bug is a for a separate case which I'll test as well.