1066492 - [dolphin][FTU]interruption of power supply before choosing language in ftu when reset phone,buttons in dockbar and collection is disappear

Status: RESOLVED WONTFIX

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

Description

[ben.song]

Immediately interruption of power supply before choosing language in ftu when reset phone,buttons in dockbar and collection is disappear,the background is exist.

Comment 1

[ben.song]

Base on this problem,we find save data of homescreen into indexedDB has been interruption when interruption of power supply before changing language in ftu.So we could get db files of homescreen in /data/local/storage/persistent/1022+f+app+++homescreen.gaiamobile.org/idb has been lost or changed through execution instruction adb shell.

Comment 2

[James Zhang (Spreadtrum)]

Ben, please attach your patch here.

Comment 3

[Jan Jongboom [:janjongboom] (inactive)]

qawanted on non-Dolphin devices.

Comment 4

[ben.song]

In state.js of homescreen module we find function st_init would get data from indexedDB,and at the start,it would check the db if empty，if true,it would invoke function loadInitialState to save new data into indexedDB;if not,it would get data Immediately.
But in the process of interruption of power supply,homescreen began to render and save it's data into indexed,so the save of data would incomplete with a certain probability.

Comment 5

[ben.song]

Attachment: homescreen.patch

Comment 6

[ben.song]

[Blocking Requested - why for this release]:

Comment 7

[ben.song]

When this problem occurs we find document 1694510209hnoemeersc and file 1694510209hnoemeersc.sqlite-journal been lost, file 1694510209hnoemeersc.sqlite and 3350367306aesgyanrcoSt.sqlite been changed in /data/local/storage/persistent/1022+f+app+++homescreen.gaiamobile.org/idb.

Comment 8

[James Zhang (Spreadtrum)]

(In reply to Jan Jongboom [:janjongboom] (Telenor) from comment #3)
> qawanted on non-Dolphin devices.

You can delete 1694510209hnoemeersc and 1694510209hnoemeersc.sqlite-journal in /data/local/storage/persistent/1022+f+app+++homescreen.gaiamobile.org/idb on flame to reproduce this issue.

Comment 9

[Jan Jongboom [:janjongboom] (inactive)]

Comment on attachment 8488539 [details] [diff] [review]
homescreen.patch

Redirecting to Vivien for review.

Comment 10

[yifan [:yifan][:yliao]]

(In reply to ben.song from comment #4)
> In state.js of homescreen module we find function st_init would get data
> from indexedDB,and at the start,it would check the db if empty，if true,it
> would invoke function loadInitialState to save new data into indexedDB;if
> not,it would get data Immediately.
> But in the process of interruption of power supply,homescreen began to
> render and save it's data into indexed,so the save of data would incomplete
> with a certain probability.

Thank you Ben for the explanation and the fix. There is a flag 'emptyDB' which is used for checking if the IndexedDB of name 'homescreen' exists. The steps for the first time homescreen is launching are as follows:

1. open IndexedDB (emptyDB is false)
2. onupgradeneeded event (if event.oldversion is 0, set emptyDB to true and create an objectStore named 'grid')
3. onsuccess event (if emptyDB is true, loadInitialState)

If power is off between step 2 and 3, when power is on the next time 'loadInitialState' might not be running since in the onupgradeneeded event the event.oldversion > 0 and the emptyDB flag will never be true. Same applies to verticalhome.

Comment 11

[yifan [:yifan][:yliao]]

Hi Ben,

Instead of putting the checking flag in mozSettings, could you please try moving it from mozSettings to somewhere like localStorage? Since it's only applicable in homescreen app, we think it might not be mandatory to be saved in mozSettings.

Comment 12

[ben.song]

(In reply to Yi-Fan Liao [:yifan][:yliao] from comment #11)
> Hi Ben,
> 
> Instead of putting the checking flag in mozSettings, could you please try
> moving it from mozSettings to somewhere like localStorage? Since it's only
> applicable in homescreen app, we think it might not be mandatory to be saved
> in mozSettings.

Hi Yi-Fan,

   But if homescreen be killed or phone be restart ,whether this localstorage would be clean ?

Comment 13

[yifan [:yifan][:yliao]]

It is persistent so the value you saved will be retained for the homescreen app.

Comment 14

[ben.song]

(In reply to Yi-Fan Liao [:yifan][:yliao] from comment #13)
> It is persistent so the value you saved will be retained for the homescreen
> app.
Hi Yi-Fan,

Do you mean window.localStorage?Thanks.

Comment 15

[yifan [:yifan][:yliao]]

Yes, you might have seen this on MDN: https://developer.mozilla.org/en/docs/Web/Guide/API/DOM/Storage#localStorage .

Comment 16

[ben.song]

Attachment: new.patch

Hi Yi-Fan,

   I have updated this patch with what you said.Please review it.Thanks.

Comment 17

[yifan [:yifan][:yliao]]

Attachment: pull request

Thank you Ben for the fix! Here's the PR to be reviewed for v1.4.

* Add a homescreen IndexedDB check flag in localStorage to prevent grid initialization failure due to unexpected power off.

Comment 18

[Jason.Liu]

Dear Rachelle,
Plz help to push this patch review ,Thanks.

Comment 19

[Cristian Rodriguez (:crdlc)]

Comment on attachment 8489846 [details] [review]
pull request

I think that you have to use the asyncStorage because localStorage is synchronous and slower. What do you think? Thanks

Comment 20

[ben.song]

(In reply to Cristian Rodriguez (:crdlc) from comment #19)
> Comment on attachment 8489846 [details] [review]
> pull request
> 
> I think that you have to use the asyncStorage because localStorage is
> synchronous and slower. What do you think? Thanks

Hi  Cristian,

   OK,Does it work in this bug,because I have't used this object of asyncStorage.Thank.

Comment 21

[Cristian Rodriguez (:crdlc)]

Yes, the library is available 

https://github.com/begeeben/gaia/blob/1066492_interruption_of_power/apps/homescreen/index.html#L18

The difference is that this is asynchronous

https://github.com/begeeben/gaia/blob/1066492_interruption_of_power/shared/js/async_storage.js

Comment 22

[Wayne Chang [:wchang]]

I see review is active now. I don't see this as a blocker for 1.4 given the circumstances and the chances of this happening.

We should make sure the solution is given time to be refined instead of rushing it into a time pressured release.

Comment 23

[ben.song]

(In reply to Cristian Rodriguez (:crdlc) from comment #21)
> Yes, the library is available 
> 
> https://github.com/begeeben/gaia/blob/1066492_interruption_of_power/apps/
> homescreen/index.html#L18
> 
> The difference is that this is asynchronous
> 
> https://github.com/begeeben/gaia/blob/1066492_interruption_of_power/shared/
> js/async_storage.js

Hi Cristian,

  I would updated new.patch as your said.Thanks

Comment 24

[ben.song]

Attachment: update.patch

Hi Cristian,

   This is the updated patch,can you help me to review it? Thanks.

Comment 25

[yifan [:yifan][:yliao]]

Comment on attachment 8489846 [details] [review]
pull request

Thanks for pointing it out! And thank you Ben for the help. The change from localStorage to asyncStorage is updated in the PR.

Comment 26

[Cristian Rodriguez (:crdlc)]

Comment on attachment 8490499 [details] [diff] [review]
update.patch

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

https://github.com/begeeben/gaia/blob/1066492_interruption_of_power/shared/js/async_storage.js#L25

getItem sends the value as parameter in a callback

Comment 27

[Cristian Rodriguez (:crdlc)]

Comment on attachment 8489846 [details] [review]
pull request

LGTM, I left a comment on github

Comment 28

[yifan [:yifan][:yliao]]

Thank you! Sorry for the mistake, fixed in the PR.

Comment 29

[Rachelle Yang [:ryang][[e-mail]]]

Patch ready,clear ni.