1045701 - [B2G][OTA][1.3 -> 2.0] Default Smart Collections not showing the proper icon after OTA

Status: RESOLVED FIXED

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

Description

[Josh Schmitt [Joshs]]

Attachment: Smart.png

Description:
After OTA from 1.3 to 2.0 the default Smart Collections show the default rocket ship not the correct icons.

Pre-requisites:
Make gaia on top of base image for 1.3 Flame setup
Push Smoketest Profile to DUT
Using B2G-Flast-Tool run these two lines:

./change_channel.sh -v nightly-b2g32

./change_ota_url.sh~ -u https://aus4.mozilla.org/update/3/B2G/28.0/20140711000201/flame/en-US/nightly-b2g32/Boot2Gecko%202.0.0.0-prerelease/default/default/update.xml?force=1


Repro Steps:
1) OTA from 1.3 to 2.0
2) Proceed through the FTU tutorial

Actual:
After OTA the Smart Collections on the Homescreen are not displayed correctly.

Expected:
Smart Collections show the correct icon on the Homescreen.

Environmental Variables:
Device: Flame 2.0
Build ID: 20140729000201
Gaia: b11775fcbfe076a3fc560c2041f5b2fe1b345009
Gecko: 86b56e101512
Version: 32.0 (2.0)
Firmware Version: v122
User Agent: Mozilla/5.0 (Mobile; rv:32.0) Gecko/32.0 Firefox/32.0

Notes:
Repro frequency: 100%
See attached: screenshot, logcat

Comment 1

[Josh Schmitt [Joshs]]

Attachment: log.txt

Comment 2

[Josh Schmitt [Joshs]]

Attachment: log2.txt

Adding two logcats, logcat 1 is during the download/install, logcat 2 is after the OTA.

Comment 3

[Peter Bylenga [:PBylenga]]

[Blocking Requested - why for this release]:
Nominating for 2.0 is a smoketest blocker.

This is different than bug 1035272, in that bug the Entertainment icon was not a default collection in 2.X and did not have a checkmark when imported from 1.X. In this bug, all four 1.X default collections are affected regardless if they are default collections in 2.X or not.

Comment 4

[Kevin Grandon :kgrandon]

I am currently unable to reproduce this from a vanilla 1.3 upgrade, icons maintain their background.

What is the "Smoketest Profile", and where can I download it? Have we tried this from a vanilla profile? I just want to rule out that there's nothing special in the profile that could be causing oddities.

Comment 5

[Kevin Grandon :kgrandon]

Also if there is some documentation that you can point me to for the smoketest profile that would be good. Do we have steps that the profile undertook from a clean profile to the current version as well?

Comment 6

[Peter Bylenga [:PBylenga]]

Hi Kevin,

Profile can be found in qa-testcase-data repo:
https://github.com/mozilla/qa-testcase-data/tree/gh-pages/OTAProfiles

I built the profile according to the this checklist.
https://etherpad.mozilla.org/4HYRU5Vfww

The profile is basically /data/local/, /data/b2g/, /data/misc/, and /storage/sdcard0 pushed to the flame (along with the channel/url change needed for branch migration).  There are some areas that need to be setup manually which are highlighted in the checklist.

Comment 7

[Kevin Grandon :kgrandon]

I am currently looking into the issues with the smoketest profile to determine exactly what's going on.

qawanted - is it possible to verify this bug without the special smoketest profile? When I tested without any special profile push the icons appeared for me in a 1.3 -> 2.0 OTA. My steps were basically flash 1.3, then update the OTA server to 2.0. Things mostly worked, but I had to trigger the settingsDB version dump.

I'm looking into the issues with the smoketest profile, but it's important to know if this works with a normal profile as well. Do you have some way of testing this without the special profile push?

Comment 8

[Peter Bylenga [:PBylenga]]

I'll take the qawanted on this one.  I can test it manually.

Comment 9

[Peter Bylenga [:PBylenga]]

I was able to manually reproduce this issue with the default 4 smart collection icons.

STRs:
1. Base the phone to OEM build
2. make production with latest 1.3 gaia
3. Change URL to reflect nightly-b2g32
4. Change Channel to nightly-b2g32
5. Turn on WiFi and Check for Updates
6. Download and Install update
7. After update observe Homescreen

Actual: the 4 default smart collection icons are rocketships

Environmental Variables:
Device: Flame 2.0
Build ID: 20140731000246
Gaia: f6702e2aab77ac144955b3cbc042483b1c5eefa7
Gecko: b87b3b927d3a
Version: 32.0 (2.0)
Firmware Version: v122
User Agent: Mozilla/5.0 (Mobile; rv:32.0) Gecko/32.0 Firefox/32.0

Comment 10

[Gregor Wagner [:gwagner]]

Naoki, can you help Kevin with reproducing this bug when you are in the office?

Comment 11

[Kevin Grandon :kgrandon]

I could not get the change_ota_url script working in comment 0, but I guess manually setting the pref should also work. I'm going to try with making the following changes:

user_pref("app.update.url.override", "https://aus4.mozilla.org/update/3/B2G/28.0/20140711000201/flame/en-US/nightly-b2g32/Boot2Gecko%202.0.0.0-prerelease/default/default/update.xml?force=1");

Comment 12

[Kevin Grandon :kgrandon]

I am finally able to reproduce this. The logs in this bug are not very helpful, but I was able to capture some useful logs after enabling the logging setting. It appears that there are a few small problems.

1 - Build system is wrong. It's populating icons as icon_90.png inside of the application.zip instead of icon.png.

2 - Icons are generated as a URL to the app:// schema even though we don't have the 'showbiz' collection.

3 - cName and categoryId are both undefined.

Here is the useful logs:

E/GeckoConsole( 4147): Content JS DEBUG at app://homescreen.gaiamobile.org/js/migrator.js:94 in Migrator.prototype.iteratePage/</<: Migrated to datastore {"url":"app://homescreen.gaiamobile.org/collections/social/manifest.collection","name":"Social","icon":"app://collection.gaiamobile.org/collections/social/icon.png","iconable":false,"useAsyncPanZoom":false,"type":"collection","pinned":[["app://communications.gaiamobile.org/manifest.webapp","dialer"],["app://sms.gaiamobile.org/manifest.webapp"],["app://communications.gaiamobile.org/manifest.webapp","contacts"],["app://email.gaiamobile.org/manifest.webapp"]],"id":289}
E/GeckoConsole( 4147): Content JS DEBUG at app://homescreen.gaiamobile.org/js/migrator.js:94 in Migrator.prototype.iteratePage/</<: Migrated to datastore {"url":"app://homescreen.gaiamobile.org/collections/music/manifest.collection","name":"Music","icon":"app://collection.gaiamobile.org/collections/music/icon.png","iconable":false,"useAsyncPanZoom":false,"type":"collection","pinned":[["app://fm.gaiamobile.org/manifest.webapp"],["app://music.gaiamobile.org/manifest.webapp"],["app://video.gaiamobile.org/manifest.webapp"]],"id":142}
E/GeckoConsole( 4147): Content JS DEBUG at app://homescreen.gaiamobile.org/js/migrator.js:94 in Migrator.prototype.iteratePage/</<: Migrated to datastore {"url":"app://homescreen.gaiamobile.org/collections/games/manifest.collection","name":"Games","icon":"app://collection.gaiamobile.org/collections/games/icon.png","iconable":false,"useAsyncPanZoom":false,"type":"collection","pinned":[],"id":207}
E/GeckoConsole( 4147): Content JS DEBUG at app://homescreen.gaiamobile.org/js/migrator.js:94 in Migrator.prototype.iteratePage/</<: Migrated to datastore {"url":"app://homescreen.gaiamobile.org/collections/showbiz/manifest.collection","name":"Showbiz","icon":"app://collection.gaiamobile.org/collections/showbiz/icon.png","iconable":false,"useAsyncPanZoom":false,"type":"collection","pinned":[],"id":376}
E/GeckoConsole( 4094): Content JS LOG at app://verticalhome.gaiamobile.org/gaia_build_defer_index.js:352 in GridView.prototype.add: Error, duplicate identifier:  http://mozilla.org GridView.prototype.add@app://verticalhome.gaiamobile.org/gaia_build_defer_index.js:352:53
E/GeckoConsole( 4094): proto.add@app://verticalhome.gaiamobile.org/gaia_build_defer_index.js:372:784
E/GeckoConsole( 4094): _eachResult@app://verticalhome.gaiamobile.org/gaia_build_defer_index.js:330:247
E/GeckoConsole( 4094): _all@app://verticalhome.gaiamobile.org/gaia_build_defer_index.js:330:202
E/GeckoConsole( 4094): ItemStore.prototype.all@app://verticalhome.gaiamobile.org/gaia_build_defer_index.js:319:1
E/GeckoConsole( 4094): ItemStore.prototype.notifyReady@app://verticalhome.gaiamobile.org/gaia_build_defer_index.js:326:286
E/GeckoConsole( 4094): finish@app://verticalhome.gaiamobile.org/gaia_build_defer_index.js:325:1
E/GeckoConsole( 4094): newTxn/txn.oncomplete@app://verticalhome.gaiamobile.org/gaia_build_defer_index.js:316:188
E/GeckoConsole( 4094): Content JS ERROR at app://verticalhome.gaiamobile.org/gaia_build_defer_index.js:392 in GridItem.prototype.doRenderIcon/<: Error fetching icon [Exception... "File error: Not found"  nsresult: "0x80520012 (NS_ERROR_FILE_NOT_FOUND)"  location: "JS frame :: app://verticalhome.gaiamobile.org/gaia_build_defer_index.js :: fetchBlob/< :: line 374"  data: no]
E/GeckoConsole( 4094): Content JS ERROR at app://verticalhome.gaiamobile.org/gaia_build_defer_index.js:392 in GridItem.prototype.doRenderIcon/<: Error fetching icon [Exception... "File error: Not found"  nsresult: "0x80520012 (NS_ERROR_FILE_NOT_FOUND)"  location: "JS frame :: app://verticalhome.gaiamobile.org/gaia_build_defer_index.js :: fetchBlob/< :: line 374"  data: no]
E/GeckoConsole( 4094): Content JS ERROR at app://verticalhome.gaiamobile.org/gaia_build_defer_index.js:392 in GridItem.prototype.doRenderIcon/<: Error fetching icon [Exception... "File error: Not found"  nsresult: "0x80520012 (NS_ERROR_FILE_NOT_FOUND)"  location: "JS frame :: app://verticalhome.gaiamobile.org/gaia_build_defer_index.js :: fetchBlob/< :: line 374"  data: no]
E/GeckoConsole( 4094): Content JS ERROR at app://verticalhome.gaiamobile.org/gaia_build_defer_index.js:392 in GridItem.prototype.doRenderIcon/<: Error fetching icon [Exception... "File error: Not found"  nsresult: "0x80520012 (NS_ERROR_FILE_NOT_FOUND)"  location: "JS frame :: app://verticalhome.gaiamobile.org/gaia_build_defer_index.js :: fetchBlob/< :: line 374"  data: no]

E/GeckoConsole( 4148): [JavaScript Error: "collection is null" {file: "app://collection.gaiamobile.org/gaia_build_defer_view.js" line: 450}]
E/GeckoConsole( 4148): [JavaScript Error: "NS_ERROR_XPC_JAVASCRIPT_ERROR_WITH_DETAILS: [JavaScript Error: "collection is null" {file: "app://collection.gaiamobile.org/gaia_build_defer_view.js" line: 450}]'[JavaScript Error: "collection is null" {file: "app://collection.gaiamobile.org/gaia_build_defer_view.js" line: 450}]' when calling method: [nsIDOMSystemMessageCallback::handleMessage]" {file: "null" line: 0}]
E/GeckoConsole( 4148): [JavaScript Error: "TypeError: collection is null" {file: "app://collection.gaiamobile.org/gaia_build_defer_view.js" line: 445}]
E/GeckoConsole( 4148): [JavaScript Error: "TypeError: collection is null" {file: "app://collection.gaiamobile.org/gaia_build_defer_view.js" line: 445}]

Comment 13

[Kevin Grandon :kgrandon]

I don't really want to be the one to fix this as I have a lot of perf bugs to investigate, but will assign to myself for now. Anyone feel free to steal this if you want ;-)

Comment 14

[Kevin Grandon :kgrandon]

Attachment: Collection App from OTA

Here is the collection app that is received from the OTA update. You can see that the build script must be wrong as we are copying over the icons wth a size suffix, instead of just icon.png.

Comment 15

[Naoki Hirata :nhirata (please use needinfo instead of cc)]

kgrandon, you will have to flash the OEM 1.3 build and then flash our 1.3 gaia on top for the user_pref to work.  Also to note we do have a repo to use for OTA testing:

https://github.com/mozilla/qa-testcase-data/tree/gh-pages/OTAProfiles

If you flash that profile on top of our 1.3 gaia, it should set all the preferences as well and have the test data you are looking for.

Comment 16

[Kevin Grandon :kgrandon]

Attachment: Pull Request - Part 1 - Properly name default collection icons

There will likely be a few patches to solve this one. The first one is the collection build script not properly naming the icons. Yuren - could you review this one?

Comment 17

[Kevin Grandon :kgrandon]

Next step is that we are not properly migrating all collections. 

It appears that the 'data' object is coming back null from asyncStorage. This means that categoryId and cName are both null after a migration. We are looking up a key[1] in migrator.js, and nothing is returned. This is unfortunately where my knowledge of the old homescreen starts to lack. Ni? on Ran/Amir/Cristian - could you guys help me track down and understand why this asyncStorage item is not returning with any data?

[1] Async storage key: 'evme-collectionsettings_app://homescreen.gaiamobile.org/collections/social/manifest.collection'

Comment 18

[Amir Nissim (:amirn)]

A.
I'm gonna take a wild guess here:
The Collection data doesn't exist because it has not been created. The setupCollectionStorage task runs on first E.me load in the old homescreen:
https://github.com/mozilla-b2g/gaia/blob/master/apps/homescreen/everything.me/js/Core.js#L114

If E.me was not loaded before the OTA upgrade, asyncStorage will not contain any information.
(E.me load is triggered by (1) E.me searchbar (2) opening a collection and (3) dragging an app onto collection)

B.
> [1] Async storage key: 'evme-collectionsettings_app://homescreen.gaiamobile.org/collections/social/manifest.collection'

This doesn't look like a valid collection id. Can you please link to relevant migrator.js code?

C.
As a reference, see bug 1040618, but not sure this scenario was tested.

D.
Is everything working after the icon patch is applied? if so, I think in we can skip the migration and simply ship the 2.0 default Collections.

Comment 19

[Kevin Grandon :kgrandon]

It appears that this may only be a problem with migrating the default collections, as the ID for collections is the manifestURL until population I believe: https://github.com/mozilla-b2g/gaia/blob/master/apps/homescreen/js/grid_components.js#L148

My guess is that we might have *some* default collections mixed with some custom collections, so the migration would need to work in both cases.

If for example you add a custom collection, but never open the default collection - are the default collections still "uninitialized"?

Comment 20

[Amir Nissim (:amirn)]

(In reply to Kevin Grandon :kgrandon from comment #19)
> If for example you add a custom collection, but never open the default
> collection - are the default collections still "uninitialized"?

Not sure if you mean user created custom collection, or custom pre-installed collections defined in the build config.

In the first case, E.me was loaded, so the init task must have run.
In the second case all pre-installed collections will be "uninitialized" until E.me loads for the first time.

Comment 21

[Kevin Grandon :kgrandon]

Ok, then I would tend to agree that if the user has never initialized the smart collections, that we should just use the default that are specified in the veritcal homescreen. Thanks Amir!

Comment 22

[Kevin Grandon :kgrandon]

This does have the funny side-effect that if they never initialized them, that they would go from four collections to three, but I think that's ok in this case as most people that are migrating would've used these features.

Comment 23

[Kevin Grandon :kgrandon]

Attachment: Pull Request - Part 2 - Do not migrate un-initialized smart collections

I think simply adding an early return should work in this case. What should happen is that we should start to use the default smart collections, instead of trying to migrate un-initialized smart collections.

Comment 24

[Kevin Grandon :kgrandon]

Comment on attachment 8467187 [details] [review]
Pull Request - Part 2 - Do not migrate un-initialized smart collections

Cristian or Amir - could you guys take a look and let me know if you think this is a reasonable approach? Thanks!

Comment 25

[Amir Nissim (:amirn)]

Comment on attachment 8467187 [details] [review]
Pull Request - Part 2 - Do not migrate un-initialized smart collections

LGTM. Thanks Kevin

Comment 26

[Yuren [:yurenju] (AFK ~ Oct. 12) (volunteer mode)]

CMIIW, collection/homescreen app should find the best matched icon for collection, so I think it seems not necessary to rename that icon file. kgrandon, is my understanding right?

Comment 27

[Kevin Grandon :kgrandon]

(In reply to Yuren [:yurenju] from comment #26)
> CMIIW, collection/homescreen app should find the best matched icon for
> collection, so I think it seems not necessary to rename that icon file.
> kgrandon, is my understanding right?

Maybe that's how it *should* work, but we currently only package a single one of these at a time in the build profile - so it needs to be named icon.png, and not have the sizing info. I think it would be more clear if we renamed the icons to follow the primary style of @1.5x, @2x, etc, but that's a much larger change than I want to do in 2.0.

Comment 28

[Kevin Grandon :kgrandon]

Comment on attachment 8467187 [details] [review]
Pull Request - Part 2 - Do not migrate un-initialized smart collections

Thanks for the review.

Comment 29

[Kevin Grandon :kgrandon]

Landing part 2 first: 

Master: https://github.com/mozilla-b2g/gaia/commit/e2d1ed0f0be087a36ff4ad76a303de805d71591c
v2.0: https://github.com/mozilla-b2g/gaia/commit/327e0e70c97c12b0c28478266bc88eaf6882b4d1

Comment 30

[Yuren [:yurenju] (AFK ~ Oct. 12) (volunteer mode)]

Comment on attachment 8466494 [details] [review]
Pull Request - Part 1 - Properly name default collection icons

okay that makes sense and r=yurenju if lint error for build/test/integration/collection_app.test.js is fixed.

Comment 31

[Kevin Grandon :kgrandon]

Part 1 landed, it's orange but this is because the recent nfc backout.

Master: https://github.com/mozilla-b2g/gaia/commit/256cb6bff259ba4552b0c3d4f0b4d4e72452843c
v2.0: https://github.com/mozilla-b2g/gaia/commit/5b767aa036a9a491376db9975a0b3cf273ecf930

Comment 32

[Kevin Grandon :kgrandon]

Attachment: Pull Request - Part 3 - Start collection upgrade for upgrade or non-upgrade paths

Hey folks - flagging a few people for a review because I think Francisco is out, and who knows if someone is out on PTO. This is just a small change to change the location of the collection app setup trigger. Anyone have time for a quick review? Thanks!

Comment 33

[Kevin Grandon :kgrandon]

To expand on the previous comment, patch #2 makes it so that we do not migrate the default collections if they have not been initialized by the homescreen. In order to actually have collections, we need to signal to the application to migrate them.

Comment 34

[Fernando Campo (:fcampo)]

Comment on attachment 8469037 [details] [review]
Pull Request - Part 3 - Start collection upgrade for upgrade or non-upgrade paths

Indeed Francisco is on PTO, and Borja is quite busy, so I'm taking it.

small patch, code looks ok, so good to go :)

Comment 35

[Kevin Grandon :kgrandon]

(In reply to Fernando Campo (:fcampo) from comment #34)
> Comment on attachment 8469037 [details] [review]
> Pull Request - Part 3 - Start collection upgrade for upgrade or non-upgrade
> paths
> 
> Indeed Francisco is on PTO, and Borja is quite busy, so I'm taking it.
> 
> small patch, code looks ok, so good to go :)

Thank you for the review!

Master: https://github.com/mozilla-b2g/gaia/commit/be82d92c6372bdfe24d9a1f810b829bc560e8d61
v2.0: https://github.com/mozilla-b2g/gaia/commit/3a5931c60b8734d96902d22019a087c59b73190b

I think we are mostly done here now, I just want to spend some time to verify that everything works with the latest OTA.

Comment 36

[Kevin Grandon :kgrandon]

The root issue of this bug should be solved now, but there is a pretty nasty upgrade bug that I ran into while investigating. The main bug I noticed is that the homescreen is blank after an OTA update. I'm not sure if this is due to the way I'm trying to simulate OTAs or if this is a real issue.

Comment 37

[Roland Kunkel [:RolandK]]

I am still seeing this issue on the latest 2.0 Flame build:

Device: Flame 2.0 319MB
BuildID: 20140812000205
Gaia: 1144cdc3a544f81c9bf71598aba1cb67d6c95a29
Gecko: 6495a7bd61ed
Version: 32.0 (2.0)
Firmware: v123
User Agent: Mozilla/5.0 (Mobile; rv:32.0) Gecko/32.0 Firefox/32.0

Some of the smart collections appear as the rocket ship and not the correct icons. I am not seeing a blank screen after OTA.

Comment 38

[Peter Bylenga [:PBylenga]]

Hi Kevin,

we are still able to reproduce this issue after the patch has been merged.

Comment 39

[Kevin Grandon :kgrandon]

Thank you Peter. After our chat on IRC it appears that we are seeing bug 1035272, which was closed. I've re-opened that bug for now while we investigate if it's an issue a user can hit or not. I would like to fix that one regardless, we just have to decide whether or not it should block or not.