Same bug as bug 860681 in it's concept. Upon doing a lot of partner build testing across multiple phones, QA managed to still reproduce bug 860681 with Inari and Ikura devices with partner builds with partner customizations. We also confirmed that this isn't a partner customization bug, as different phones revealed different icons lost. Here's three such scenarios we saw: - Phone #1 - Notes app lost it's icon - Phone #2 - Movistar Recommenda lost it's icon - Phone #3 - Tuenti & Movistar Recommenda lost it's icons We did generally notice that the rate and # of icons lost with the fix in bug 860681 was less overall. But we've definitely confirmed the bug is still happening with multiple phones with different apps losing icons in different cases.

Phone #1 - Build was from 4/26/2013 Phone #2 - Build was from 5/2/2013 Phone #3 - Build was from 4/26/2013

I can confirm that partner customization bugs were only triggering this bug, and this bug was really in the homescreen. Jason, can you absolutely confirm that those apps definitely had their good icons to begin with ?

Julien, you seem to be the right person to continue this one while Jason is answering your question.

(In reply to Julien Wajsberg [:julienw] from comment #2) > I can confirm that partner customization bugs were only triggering this bug, > and this bug was really in the homescreen. > > Jason, can you absolutely confirm that those apps definitely had their good > icons to begin with ? From what I'm seeing using partner builds directly, I believe these icons are valid. I'll try building my own custom builds though with make PRODUCTION=1 with the partner customizations: - https://github.com/telefonicaid/firefoxos-gaia-latam - https://github.com/telefonicaid/firefoxos-gaia-spain

I could but I won't likely have time before 13th of May as I'm in holiday most of next week + MMS work. Cristian, could you please investigate/reproduce/fix ;) while I'm busy ?

I can take a look during this week, thanks

ok, I assign to you then, thanks a lot ! If I find anything I'll keep you posted here, please do the same :)

My steps: 1) git checkout v1.0.1 2) git pull origin v1.0.1 3) cp -r firefoxos-gaia-spain/external-apps/* myGaiaV1.0.1/external-apps/ 4) cp browser, contacts, homescreens and settings jsons to myGaiaV1.0.1/dist/ 5) MOZILLA_OFFICIAL=1 GAIA_DISTRIBUTION_DIR=dist make production (from myGaiaV1.0.1) Result: I restarted three times the device and force updates but til now I have all icons on the grid. I can see this error in logs: E/GeckoConsole( 108): [JavaScript Error: "PermissionsTable.jsm: expandPermissions: Unknown Permission: orientation" {file: "resource://gre/modules/PermissionsTable.jsm" line: 300}] E/GeckoConsole( 108): [JavaScript Error: "PermissionsInstaller.jsm: 'orientation' is not a valid Webapps permission name." {file: "resource://gre/modules/PermissionsInstaller.jsm" line: 122}] E/GeckoConsole( 108): Offline cache manifest HTTP request failed, URL=http://everythingme.github.com/ffos-notes/notes.appcache Waiting for the lost icons ;) Are there any good steps to reproduce the bug? Thanks a lot!!

Mi Movistar app says [1] although doesn't revert icon [1] E/GeckoConsole( 1164): Content JS INFO at app://system.gaiamobile.org/js/updatable.js:102 in anonymous: downloadError event, error code is INVALID_SIGNATURE

The STR that was used to reproduce this bug requires flashing partner ZTE builds onto Ikura devices that are 4/26 or later. The direct STR that was used in house was: 1. Flash a 4/26 or 5/2 ZTE partner build 2. Enable Mozilla Guest wifi (aka weak wifi network) 3. Complete FTE and go to homescreen 4. Pan through icons on homescreen Result - Occasionally, you'll see one or two homescreen icons missing.

Btw, something that's probably worth noting that might help out here in analysis: Every single hosted app manifest in a partner customization is going try to update the app locally against the remote contents immediately during the FTE, given that the app manifest in the partner customization is different than the app manifest hosted remotely. The reasoning for this is the partner customizations are using data uris for the icons for hosted app manifests, which is different than the relative paths used for icons on remote servers. What could be potentially be failing here is that due to the weak network, we failed to retrieve the icon for the app, but the app manifest updated successfully, which means the path of the icon changed to something that doesn't exist.

Cristian, could this be that we try to update the app before displaying its icon the first time ? If fetching the icon for the updated app fails, and if we haven't displayed the previous working icon yet, we wouldn't have any rendered icon, and we would then display the default icon. Does that sound reasonable ? Jason, do you think you saw the good icons for these applications on these devices ? I'd bet you never saw them on the devices that display the default icons. If that's the reason, I don't think we can do much more than fix Bug 863337 (and that is, if this app was correctly customized).

Hey, same idea in the same moment :)

(In reply to Jason Smith [:jsmith] from comment #11) > Btw, something that's probably worth noting that might help out here in > analysis: > > Every single hosted app manifest in a partner customization is going try to > update the app locally against the remote contents immediately during the > FTE, given that the app manifest in the partner customization is different > than the app manifest hosted remotely. The reasoning for this is the partner > customizations are using data uris for the icons for hosted app manifests, > which is different than the relative paths used for icons on remote servers. Note that if the Etag are configured correctly, we wouldn't check the new manifest at all.

(In reply to Julien Wajsberg [:julienw] from comment #12) > Cristian, could this be that we try to update the app before displaying its > icon the first time ? If fetching the icon for the updated app fails, and if > we haven't displayed the previous working icon yet, we wouldn't have any > rendered icon, and we would then display the default icon. That sounds like what potentially be going on here. > > Does that sound reasonable ? Jason, do you think you saw the good icons for > these applications on these devices ? I'd bet you never saw them on the > devices that display the default icons. I never saw the good icons.

Ok, so there are multiple problems: * see Bug 860681 comment 28. I said I would file a new bug, I haven't yet. Jason, I'd appreciate if you could file it. * Bug 863337. Bug 863337 is probably fixable soon (I have a WIP patch there) but I think this won't resolve what you're seeing here. Doing a retry for getting an icon is (imho) much more work and I'd like to reconsider the tef+ status that this bug has.

(In reply to Julien Wajsberg [:julienw] from comment #16) > Ok, so there are multiple problems: > > * see Bug 860681 comment 28. I said I would file a new bug, I haven't yet. > Jason, I'd appreciate if you could file it. I'm not sure what I would file off of that comment? That comment seems to imply the "if we fail to retrieve the icon due to a bad network during an app update, then we get a default icon." Although that sounds a lot like what I'm seeing in this bug. > * Bug 863337. > > Bug 863337 is probably fixable soon (I have a WIP patch there) but I think > this won't resolve what you're seeing here. Doing a retry for getting an > icon is (imho) much more work and I'd like to reconsider the tef+ status > that this bug has. bug 863337 likely isn't going to solve this bug. I don't fully understand the renom however - I'd likely see our partner's certification processes claiming this is a problem (the next round of testing will likely see this bug). A simple fix that could happen on the partner customization side is to ensure that the hosted app manifest data matches what's on the remote server - that would prevent the weird case that happens here.

(In reply to Jason Smith [:jsmith] from comment #17) > (In reply to Julien Wajsberg [:julienw] from comment #16) > > * see Bug 860681 comment 28. I said I would file a new bug, I haven't yet. > > Jason, I'd appreciate if you could file it. > > I'm not sure what I would file off of that comment? That comment seems to > imply the "if we fail to retrieve the icon due to a bad network during an > app update, then we get a default icon." Although that sounds a lot like > what I'm seeing in this bug. There are so many cases to handle : * navigator.onLine === false * and: if the request fails, should we try again later ? Next reboot ? Next "check for update" cycle ? We need to answer these questions first. I can try to come up with a detailed answer if you like. We can use this bug for that though, maybe we don't need to file a new one. > I don't fully understand the renom however - I'd likely see our partner's > certification processes claiming this is a problem (the next round of > testing will likely see this bug). I renomed because I feel there is some non-trivial code to write and I don't feel the added value is worth the risk for tef+. > A simple fix that could happen on the partner customization side is to > ensure that the hosted app manifest data matches what's on the remote server > - that would prevent the weird case that happens here. Yep, if ETags are the same, then we likely won't see this bug. Or use data-url in the remote manifests.

Certainly, there's a lot of possibilities to handle here - but I think there's a design flaw here in how we update hosted apps without appcache that might be leading to this problem of why there's many cases to handle. With hosted apps without appcache, we try to update without a true fail fast strategy if a network connection fails, where as in the controlled app update workflow (hosted apps with appcache or packaged apps), we do have a fallback strategy when the network is lost. In other words, the controlled app update workflow follows the ACID properties correctly to rollback on failure, but the hosted app updates without appcache violate the ACID properties of an update, which is what is being seen here. The right way to fix this problem is to ensure that we have either have a clean update entirely or fail the update at any possible portion that fails, rollback the update, and try to update again after a reboot. That would ensure that we don't end up with an unclean app update that doesn't fully have the resources required.

Jason, the icon fetching has nothing to do with the app type; whether we update automatically an hosted app, or with the user consent for an hosted+appcache app or a packaged app, we'll likely have the network at that time, and if the network is bad at that moment, we'll have the exact same problem.

(In reply to Julien Wajsberg [:julienw] from comment #20) > Jason, the icon fetching has nothing to do with the app type; whether we > update automatically an hosted app, or with the user consent for an > hosted+appcache app or a packaged app, we'll likely have the network at that > time, and if the network is bad at that moment, we'll have the exact same > problem. I'm not sure if you are seeing my point I'm making here. What I'm discussing above is the fact that a hosted app w/appcache & a packaged app have a controlled app update flow that fails on network loss, where as regular hosted apps without appcache do not. So I would think there would be a far less likelihood of us hitting the icon retrieval problem with the controlled app update flow, given that it has fail fast methods in place when the network is lost.

nope, because the very thing that triggers an update for an hosted app is that... we found that we had an update. We found it be cause we actually downloaded the manifest and checked it changed. And therefore we already have that manifest when we perform the update. And therefore we had a network connection.

Hi all, Yesterday, I was out office and I could not read anything, sorry. I would like to implement a simple module that after fails it tries to get the icons again window.addEventListener("online", function(e) { // Try again pending operations }) Are you comfortable Julien?

Hi again, https://github.com/mozilla-b2g/gaia/pull/9591 I've implemented a library that loads icons from internet when the mobile phone is online. If there would be an error while fetching, it tries again the download when the device will be online. I would like to know your feedback Julien about this code. If we decide that it is good I will do unit tests. Thanks

I commented on the PR, this is clearly the way to go if we want this, but this is non trivial and will change how icons are loaded, hence my renomination to estimate the risk vs the value.

Well I am not the person to estimate it :) so waiting for a decision. I stop working here at this time. Thanks Julien for your comments!!

TL;DR Is the problem here that in case the manifest is downloaded properly but not the icon, the icon will be wrong until a new update is received?

(In reply to Daniel Coloma:dcoloma from comment #27) > TL;DR Is the problem here that in case the manifest is downloaded properly > but not the icon, the icon will be wrong until a new update is received? That sounds right to me. It seems to happen with partner customized builds a lot more, mainly because we have a lot of apps preinstalled by default.

Daniel: that's it. Except that in most cases we had at least once a good icon, and we keep it if we can't retrieve another icon for some reason. But in partner customized builds we sometimes have no good icon at all because we get the new manifest before loading the icon that was specified in the customized manifest. In that new manifest we have an icon that needs to be fetched from the network, and due to either Bug 863337 we may fetch it when we have no network, or due to a bad app metadata.json (that we hopefully all fixed already) we could fetch a bad URL. So... that's it.

Cristian, could you check the app http://everlong.org/mozilla/brokenhostedappcache/ ? There is a wrong icon (so I get a 404) but the "downloading" animation is never removed, yet the app is launchable. Maybe this is related to this...

please test with this patch https://github.com/mozilla-b2g/gaia/pull/9591

Cristian, could you please add a patch here and ask me for review ? so that I don't lose it :)

Go ahead with the first version, be not too hard jejeje Please if you are comfortable step by step, I mean, first af all we can review the code and when everything will be ok, we can enter into unit tests. But up to you :) It is only my suggestion my friend. Thanks a lot

The case explained in comment 30 is dealt here https://github.com/mozilla-b2g/gaia/pull/9591/files#L3R186 Thanks

okay, thanks Cristian ! Will review this again probably tomorrow.

Perfect, I've checked unit test right now and work fine after landing tons of tests during today

FTR we did a review already yesterday.

Comment on attachment 749247 [details] Patch v1 r=me thanks, this seems to work great

v1-train: 08074c417f9e095168f38a449dabe0e2325b7353 v1.0.1: e98024772590f97d9443acf1af3958b111ed9cb6

The builds I've tested so far have not shown the icons removed, so marking as verified. If I end up seeing this again, I'll let you know.

crossing fingers ;)