861462 - preload contacts will be removed after factory reset

Status: VERIFIED FIXED

(Firefox OS Graveyard :: General, defect, P1)

Description

[Firefox_Mozilla]

User Agent: Mozilla/5.0 (Windows NT 6.1) AppleWebKit/537.31 (KHTML, like Gecko) Chrome/26.0.1410.64 Safari/537.31

Steps to reproduce:

1）make a build with preload contacts
2) load images to the device
3) start up the device
4) make sure the preload contacts are existing
5) do factory reset by "setting->Device information->more infomation->Reset Phone"



Actual results:

After factory reset, the preload contacts will be removed


Expected results:

The preload contacts should be existing after factory reset.

Comment 1

[Firefox_Mozilla]

Since the database of preload contacts is stored in the user data partition, which will be formated during factory reset, so the preload contacts will lost.

Comment 2

[David Palomino [:dpalomino]]

Hi, 

This issue will be blocking for the certification. marking p1/tef? and adding dep with meta bug. 

Thks!
David

Comment 3

[Joe Cheng [:jcheng] (please needinfo)]

yuren, mind taking a look at this? thanks

Comment 4

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

I can repoduce it, investigating...

Comment 5

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

root cause is factory reset will earse all data in /data/local, and indexedDB for contact store there.

there are two options to fix it:
1. store to another place and copy to /data/local after recoveried (should be implemented in B2G/recovery/recovery.c)
2. use same way as browser machanism to do customization.

I don't like those two options but I have no other ideas for that.

Tim, do you have any idea?

Comment 6

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

also needinfo? to this feature's author. Albert, do you have any idea?

Comment 7

[Tim Guan-tin Chien [:timdream] (please needinfo)]

I would recommend a Gecko fix, similar to the behavior of Settings DB; in which,

1) A set of defaults will be stored in /system/b2g/defaults/contacts.json, which will survive the recovery.
2) No IndexedDB should be built for the Gaia build process
3) Gecko will build the new contacts DB with default contacts if it finds no databases exists in the profile.

(not sure is will be out of scope of tef+ though, but it shouldn't be.)

Comment 8

[[:fabrice] Fabrice Desré]

Tim is right, this is how to do it. Cc-ing the contact people...

Comment 9

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

Agree Tim's comment.

We have contact.json on gaia-distribution-sample
https://github.com/yurenju/gaia-distribution-sample/blob/master/contacts.json

we may use this format and move build/contacts.js to gecko and create preload contacts when first time running gecko with preload contacts.

here are something we need to do:
1. [gaia] if we have contacts.json in GAIA_DISTRIBUTION_DIR or GAIA_DIR/distribution/, copy it to /system/b2g/
2. [gecko] if /system/b2g/contacts exists on first time run, create preload contacts -- we can move GAIA_DIR/build/contacts.js to gecko.

deassign and waiting next triage to assign to gecko's guys.

Comment 10

[Gregor Wagner [:gwagner]]

We should just copy the behavior from the settingsDB.

Comment 11

[Gregor Wagner [:gwagner]]

Do we have a sample JSON file I can use for testing?

Comment 12

[Tim Guan-tin Chien [:timdream] (please needinfo)]

Thanks Gregor for taking the bug.

(In reply to Gregor Wagner [:gwagner] from comment #11)
> Do we have a sample JSON file I can use for testing?

I think a file is available in the zip here:
https://wiki.mozilla.org/B2G/MarketCustomizations#Customization_Overview
Or you could ask Yuren to give you one directly.

Note that we will still need a Gaia build system fix. Gregor, do you intend to fix that altogether in this bug, or we should clone a bug for Gaia and have someone else to fix that?

Comment 13

[Tim Guan-tin Chien [:timdream] (please needinfo)]

https://github.com/telefonicaid/firefoxos-gaia-spain/blob/master/contacts.json

Ah, there is one right here.

Comment 14

[Tim Guan-tin Chien [:timdream] (please needinfo)]

https://github.com/mozilla-b2g/gaia/blob/master/build/contacts.js

The code to fill the database from JSON file can be moved from here in Gaia.

Comment 15

[Albert [:albert]]

I'm agree, Tim's proposal is the best approach,  like preloaded APNs.

Comment 16

[Gregor Wagner [:gwagner]]

Attachment: patch

Comment 17

[Ben Turner (not reading bugmail, use the needinfo flag!)]

Comment on attachment 738097 [details] [diff] [review]
patch

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

r=me!

::: dom/contacts/fallback/ContactDB.jsm
@@ +309,5 @@
> +
> +    // Add default contacts
> +    if (aOldVersion == 0) {
> +      Cu.import("resource://gre/modules/FileUtils.jsm");
> +      Cu.import("resource://gre/modules/NetUtil.jsm");

Let's put this on a temporary object just to be safe.

@@ +313,5 @@
> +      Cu.import("resource://gre/modules/NetUtil.jsm");
> +      // Loading resource://app/defaults/contacts.json doesn't work because
> +      // contacts.json is not in the omnijar.
> +      // So we look for the app dir instead and go from here...
> +      let contactsFile = FileUtils.getFile("DefRt", ["contacts.json"], false);

One of these days this JSON-loading code should be factored out into a helper somewhere so that settings can use it too. I don't think we should care now but let's get a bug on file for it.

@@ +338,5 @@
> +      } catch(e) {
> +        if (DEBUG) debug("Error parsing " + contactsFile.path + " : " + e);
> +        return;
> +      }
> +      stream.close();

Let's move this right after the ConvertToUnicode call.

@@ +341,5 @@
> +      }
> +      stream.close();
> +      let idService = Cc["@mozilla.org/uuid-generator;1"].getService(Ci.nsIUUIDGenerator);
> +      if (!objectStore) {
> +        objectStore = aTransaction.objectStore(STORE_NAME);

Relying on the upgrade loop to always leave the correct objectStore set seems fragile. Let's always set here rather than using whatever is left over.

@@ +346,5 @@
> +      }
> +      for (let i = 0; i < contacts.length; i++) {
> +        let contact = {};
> +        contact.properties = contacts[i];
> +        contact.id = idService.generateUUID().toString().replace('-', '', 'g').replace('{', '').replace('}', '');

Nit: This is pretty ugly, can you break this into multiple lines?

Comment 18

[Gregor Wagner [:gwagner]]

Attachment: patch

Comment 19

[Reuben Morais [:reuben]]

Comment on attachment 739000 [details] [diff] [review]
patch

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

::: dom/contacts/fallback/ContactDB.jsm
@@ +349,5 @@
> +        let contact = {};
> +        contact.properties = contacts[i];
> +        contact.id = idService.generateUUID().toString().replace('-', '', 'g')
> +                                                        .replace('{', '')
> +                                                        .replace('}', '');

Nit: " instead of ', and you can do do replace(/[{}-]/g, "").

Comment 20

[Gregor Wagner [:gwagner]]

(In reply to Reuben Morais [:reuben] from comment #19)
> Comment on attachment 739000 [details] [diff] [review]
> patch
> 
> Review of attachment 739000 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> ::: dom/contacts/fallback/ContactDB.jsm
> @@ +349,5 @@
> > +        let contact = {};
> > +        contact.properties = contacts[i];
> > +        contact.id = idService.generateUUID().toString().replace('-', '', 'g')
> > +                                                        .replace('{', '')
> > +                                                        .replace('}', '');
> 
> Nit: " instead of ', and you can do do replace(/[{}-]/g, "").

I don't trust our regexp implementation to be faster in this case :)

Comment 21

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/projects/birch/rev/e65a858b4235

Comment 22

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/e65a858b4235

Comment 23

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-b2g18/rev/42f118fcbcea
https://hg.mozilla.org/releases/mozilla-b2g18_v1_0_1/rev/4cdd542efe79

Comment 24

[Askeing Yen[:askeing][:fyen]]

Inari
Gaia v1.0.1: 2bcc625aecce5f111b70f7cf4d17523a09cb7d6c

Verified.