Closed Bug 829522 Opened 7 years ago Closed 7 years ago

Check for updates never finds app update notifications until after you restart the phone due to hosted+appcache app check for updates failure

Categories

(Core Graveyard :: DOM: Apps, defect)

ARM
Gonk (Firefox OS)
defect
Not set

Tracking

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

VERIFIED FIXED
B2G C4 (2jan on)
blocking-basecamp +
Tracking Status
firefox19 --- wontfix
firefox20 --- wontfix
firefox21 --- fixed
b2g18 --- fixed

People

(Reporter: julienw, Assigned: fabrice)

References

Details

Attachments

(1 file, 2 obsolete files)

STR:
* have a recent enough gecko+gaia so that bug 828939 is fixed
* settings > device information > check now
* wait

expected:
* "checking for updates" should hide after a moment 

actual:
* it's not

By adding logs in the settings app, I verified that we never get the "app.updateStatus" change as we should.

However, if I reboot the phone, I get the update notification for HERE maps (and Firefox Marketplace BTW) which means that app.downloadAvailable is correctly true.

I verified that removing HERE maps fixes this bug. I suspect this is because of the special "preloaded hosted+appcache app" case.
Yikes. Dug into this with Julien - apparently the maps app doing a failure in check for updates ends up causing a domino effect and causing a failure to check for updates for *all* apps until the phone is restarted and you check for updates again.

Blocker!
Summary: check for updates never returns if we have HERE maps → Check for updates never finds app update notifications until after you restart the phone due to maps app check for updates failure
blocking-basecamp: ? → +
Giving this to baku, but do talk to Fabrice and Julien to figure out who should do what here.
Assignee: nobody → amarchesini
I've confirmed Julien's findings.

If I try to check for updates with the maps app as being one of the apps were hunting for updates, i won't find any app updates until after a phone restart.

If I try to check for updates without the maps app as being one of the apps were hunting for updates, I will find app updates within 30 seconds without a phone restart.
Can it be that your .userconfig doesn't contain VARIANT=user or VARIANT=userdebug ?
Component: DOM: Apps → Gaia
Product: Core → Boot2Gecko
Target Milestone: --- → B2G C4 (2jan on)
Version: Trunk → unspecified
Component: Gaia → DOM: Apps
Product: Boot2Gecko → Core
Attached patch patch (obsolete) — Splinter Review
Attachment #701155 - Flags: review?(fabrice)
Attachment #701155 - Flags: feedback?(jsmith)
jst said in theory the patch looked good, but he wanted fabrice to confirm it. I'm going to test the patch shortly.
Comment on attachment 701155 [details] [diff] [review]
patch

sorry I don't follow the logic of this patch at all.

gecko.updateStatus is for system updates.
(In reply to Andrea Marchesini (:baku) from comment #4)
> Can it be that your .userconfig doesn't contain VARIANT=user or
> VARIANT=userdebug ?

no everything works great as soon as we remove the Maps app.
Comment on attachment 701155 [details] [diff] [review]
patch

Testing this on the phone with the build with this patch, this didn't work for me. I believe Fabrice indicated that the root cause of this bug might actually be on the Gaia side.
Attachment #701155 - Flags: feedback?(jsmith) → feedback-
(In reply to Julien Wajsberg [:julienw] from comment #8)
> (In reply to Andrea Marchesini (:baku) from comment #4)
> > Can it be that your .userconfig doesn't contain VARIANT=user or
> > VARIANT=userdebug ?
> 
> no everything works great as soon as we remove the Maps app.

Right. I showed this Andrea directly and he confirmed the issue I was seeing with a fresh flashed build with the old maps app preloading appcache.

Julien - Any ideas on how this could be a potentially be a Gaia issue?
Well, I'm quite sure this is _not_ a Gaia issue, as we don't get the apps.updateStatus change at all. (we get the gecko.updateStatus change though).

I'll work on creating an hosted+appcache app on Monday to try and reproduce this with something else than the Maps app (that is now packaged).
stealing this for today, I'll do some test to try and see what's going on.
Assignee: amarchesini → felash
Now it doesn't do that with Maps anymore, I think that's because it was converted to a Packaged app.

However I see this with the Slithering app from the marketplace. The Slithering app is a hosted+appcache app.
Summary: Check for updates never finds app update notifications until after you restart the phone due to maps app check for updates failure → Check for updates never finds app update notifications until after you restart the phone due to hosted+appcache app check for updates failure
Comment on attachment 701155 [details] [diff] [review]
patch

Pretty sure this isn't what we'll be doing here per discussion with Fabrice, so obsoleting this patch.
Attachment #701155 - Attachment is obsolete: true
Attachment #701155 - Flags: review?(fabrice)
Attached patch debug patch (obsolete) — Splinter Review
with this patch, I saw that |updateObserver.observe| is never called back by |updateSvc.checkForUpdate|.

Handing this over to fabrice now.
Assignee: felash → fabrice
Blocks: 829853
Attached patch patchSplinter Review
So, we were doing a couple of things wrong:
- trying to load an appcache with the url of the app manifest.
- not setting downloadAvailable properly (using the topic name instead of the event name).
Attachment #701859 - Attachment is obsolete: true
Attachment #701991 - Flags: review?(ferjmoreno)
Comment on attachment 701991 [details] [diff] [review]
patch

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

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

Undo!
I will let you decide which debug statements you want to keep in the remaining code.
Attachment #701991 - Flags: review?(ferjmoreno) → review+
Keywords: verifyme
QA Contact: jsmith
Fabrice, why didn't I see |updateObserver.observe| being called ? If this is because the AppCache URL was bad then this might be Bug 830044, right ?
Verified on 1/17.
Status: RESOLVED → VERIFIED
Keywords: verifyme
Duplicate of this bug: 829799
Product: Core → Core Graveyard
You need to log in before you can comment on or make changes to this bug.