1018854 - Start the migration after upgrading

Status: VERIFIED FIXED

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

Comment 1

[Sam Foster [:sfoster] (he/him)]

What will happen here? Is this just data migration, or does this include the changes necessary to message the user?

Comment 2

[Cristian Rodriguez (:crdlc)]

It is just to land data migration behind scene

Comment 3

[Cristian Rodriguez (:crdlc)]

Basically to start the process implemented in bug 1018852.

Comment 4

[Cristian Rodriguez (:crdlc)]

Hi mates,

   I am wondering how do it because I am bit lost :)... I have already implemented the mechanism to propagate collections and bookmarks to datastores in bug 1018852. The idea is to send a message to old homescreen to start the migration via IAC.

My questions are:

   From where can I send this message to old homescreen?
   How can I know what we are upgrading from 1.x to 2.0?

Thanks guys

Comment 5

[Sam Foster [:sfoster] (he/him)]

Hi Cristian, 
You can see the work-in-progress on the upgrade/FTU stuff here: https://github.com/sfoster/gaia/tree/bug-939174-upgrade-tutorial (on bug 939174). Basically, the FTU launcher will launch it if it sees that the device.previous_os and device.os are different (and a tutorial config exists). You can look through that branch to see what's going on, but I'm not sure if that really answers your question? You could make the same checks in an 'ftudone' event handler maybe?

Comment 6

[Kevin Grandon :kgrandon]

Sounds like this depends on bug 939174 landing, or taking a small piece of it. I think we should use the same solution if possible. Thanks for the info in Sam.

Comment 7

[Cristian Rodriguez (:crdlc)]

I think that I could wait for that bug IMHO

Comment 8

[Sam Foster [:sfoster] (he/him)]

oh, sec that wont work currently, as FtuLauncher clears the device.previous_os before firing ftudone. We could clear that setting later maybe - it just needs to be wiped before the FTU next runs. I'm open to suggestions

Comment 9

[Cristian Rodriguez (:crdlc)]

It makes sense although I don't have all the context (to be honest nothing) because I have never touched anything piece of code in the FTU. Basically we want to send a message via IAC to old homescreen from FTU app to migrate data after upgrading from 1.x to 2.0. Maybe it could be implemented by someone with more knowledge thought or perhaps after landing that bug I can see how do it

Comment 10

[Kyle Machulis [:qdot] [:kmachulis] (INACTIVE)]

Could you could check the version of the IDB in the old homescreen app, to see if it correlates to a version in the 1.x series?

Comment 11

[Cristian Rodriguez (:crdlc)]

Yes, I can get the revisionId for bookmarks datastore for instance in a private IDB in old homescreen. If it does not exists we have to migrate, otherwise, we can receive the IAC message called 'migrate' and nothing to do if the old homescreen has a revisionId. This is like to have a flag to say, we have migrated or not. But it is useful only when we don't know from the FTU if we are upgrading or not. So we could send the message always that the FTU is started without exception and the old homescreen will do migrate or not depending on the previous explained. Did you mean this Kyle?

Comment 12

[Carmen Jimenez Cabezas]

As I spoke with Cristian, I'm taking this

Comment 13

[Carmen Jimenez Cabezas]

Attachment: Proposed patch v1

Comment 14

[Cristian Rodriguez (:crdlc)]

Comment on attachment 8438721 [details] [review]
Proposed patch v1

R+ from home side, please ask to somebody related to FTU, Fernando, could you review or address it properly? Thanks a lot

Comment 15

[Cristian Rodriguez (:crdlc)]

Comment on attachment 8438721 [details] [review]
Proposed patch v1

Hi Fernando, do you have any chance to take a look to this today? We will appreciate it a lot mate

Comment 16

[Francisco Jordano [:arcturus] [:francisco]]

Hi,

just one comment about the use of IAC in the FTU.

Not saying that is not the solution here, but we should be careful with the use of this tecnique. Right now we are launching as well a IAC for notifying contacts of the FB token, could this to call clash?

Also, the homescreen should be already launched, but what's the impact of the new process, in the case of contacts is 6.9 MB, just for handing a value that will be stored on indexeddb.

Could you use firewatch [1] to measure the impact of this?

Take into account that tarako could get 2.0 update and too many IAC calls together could end up in a mess if OOMKiller joins the party.

Again, not saying that is not the solution, but want a good meassure of the impact.

Thanks.

[1] https://www.npmjs.org/package/firewatch

Comment 17

[Fernando Campo (:fcampo)]

Comment on attachment 8438721 [details] [review]
Proposed patch v1

Codewise, looks good. 

Only concern to address in github comment, and please check what Francisco said before about memory and possible clashes. Maybe we can post here the firewatch results to check that doesn't clash?

Comment 18

[Fernando Campo (:fcampo)]

Comment on attachment 8438721 [details] [review]
Proposed patch v1

ooops, just realized that no unit tests. really needed before merging, sorry.

Comment 19

[Carmen Jimenez Cabezas]

Comment on attachment 8438721 [details] [review]
Proposed patch v1

added unit test

Comment 20

[Fernando Campo (:fcampo)]

Comment on attachment 8438721 [details] [review]
Proposed patch v1

Looks good now, thanks for the unit test.

Cristian already r+ed before for the homescreen part, so good to merge when travis green.

Comment 21

[Cristian Rodriguez (:crdlc)]

Merged in master:

https://github.com/mcjimenez/gaia/commit/0d3291fa28ddd8be8e43762f93bed7f27c233aeb

Comment 22

[Carmen Jimenez Cabezas]

Requesting blocking 2.0 because migrating previously user collections is essential to avoid users' data loss

Comment 23

[Carmen Jimenez Cabezas]

Sorry for the mistake, this should be approval only, not blocking

Comment 24

[Carmen Jimenez Cabezas]

Comment on attachment 8438721 [details] [review]
Proposed patch v1

NOTE: Please see https://wiki.mozilla.org/Release_Management/B2G_Landing to better understand the B2G approval process and landings.

[Approval Request Comment]
[Bug caused by] (feature/regressing bug #): Vertical homescreen
[User impact] if declined: If a user is updating from 1.3/1.4 to 2.0, he will lose any collection he might have defined, since they won't be migrated to the new homescreen
[Testing completed]: On the device. The patch includes unit tests also.
[Risk to taking this patch] (and alternatives if risky): Low
[String changes made]: None

NOTE: Please see https://wiki.mozilla.org/Release_Management/B2G_Landing to better understand the B2G approval process and landings.

Comment 25

[bhavana bajaj [:bajaj]]

Comment on attachment 8438721 [details] [review]
Proposed patch v1

Johan can you please help verify this on 2.0 once it lands.

Comment 26

[Ryan VanderMeulen [:RyanVM]]

v2.0: https://github.com/mozilla-b2g/gaia/commit/8a6791411fdca9407fb600b6292e109d8f3286d9

Comment 27

[Johan Lorenzo [:jlorenzo]]

Verified as per comment https://bugzilla.mozilla.org/show_bug.cgi?id=1017069#c20

Comment 28

[Shally Li（leave from Mozilla）]

Per Comment 25&27 ,clear "verifyme" keywords.

Comment 29

[Shally Li（leave from Mozilla）]

Per Comment 25 & Comment 27,if someone feels convenient,please verify on v2.1.