Closed Bug 823150 Opened 11 years ago Closed 11 years ago

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

Categories

(Firefox OS Graveyard :: General, defect)

ARM
Gonk (Firefox OS)
defect
Not set
normal

Tracking

(blocking-basecamp:+, firefox19 fixed, firefox20 fixed, b2g18 fixed)

VERIFIED FIXED
B2G C3 (12dec-1jan)
blocking-basecamp +
Tracking Status
firefox19 --- fixed
firefox20 --- fixed
b2g18 --- fixed

People

(Reporter: jsmith, Assigned: etienne)

References

Details

Attachments

(6 files)

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.
blocking-basecamp: --- → ?
Blocks: b2g-app-updates
No longer blocks: privileged-apps
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.
Flags: needinfo?(etienne)
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.
Wait a second...I just got the update. But that took a while....
Summary: Installing a signed packaged app, updating it on the server, and doing a manual sync - no update notification found → Installing a signed packaged app, updating it on the server, and doing a manual sync - no update notification found for at least 45 minutes
I'm going to dig into this one a bit more to try to get more information.
Keywords: qawanted
QA Contact: jsmith
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?
Just retested this with a brand new privileged app doing a similar repro steps. Same bug occurs. So this is definitely 100% reproducible.
Keywords: qawanted
Not blocking but tracking. Workaround would be to delete the app then reload it from Marketplace.
blocking-basecamp: ? → +
tracking-b2g18: --- → +
(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 :)
Flags: needinfo?(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.
How is this a Gaia::System bug?
Component: Gaia::System → General
(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?
(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?
blocking-basecamp: + → ?
(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.
tracking-b2g18: + → ---
(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.
(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.
Blocks: 823525
No longer blocks: 823525
Fabrice is going to reproduce this.
blocking-basecamp: ? → +
Target Milestone: --- → B2G C3 (12dec-1jan)
Assignee: nobody → fabrice
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.
Assignee: fabrice → nobody
Over to Etienne then.
Component: General → Gaia::System
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?
I'll have a patch tomorrow but there is gaia and a gecko part so it might take a little time to land.
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...
Attached patch Gaia partSplinter Review
Attachment #696270 - Flags: review?(21)
Attached patch Gecko partSplinter Review
Attachment #696271 - Flags: review?(21)
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.
Attachment #696271 - Flags: review?(21) → review+
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...
Attachment #696271 - Flags: review+ → review?(21)
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.
Attachment #696270 - Flags: review?(21) → review+
Gaia PR with comment addressed:
https://github.com/mozilla-b2g/gaia/pull/7228
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/3c67ead5818a
Status: NEW → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Still need to land the Gaia part. Reopening.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
https://github.com/mozilla-b2g/gaia/commit/9044cb46d4997adaa9adc576731ec75ee2e48759
Status: REOPENED → RESOLVED
Closed: 11 years ago11 years ago
Resolution: --- → FIXED
Keywords: verifyme
Depends on: 827047
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.
Keywords: verifyme
Whiteboard: [qa verification blocked]
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.
Status: RESOLVED → VERIFIED
Whiteboard: [qa verification blocked]
No longer blocks: market-packaged-apps
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: