preload contacts will be removed after factory reset

VERIFIED FIXED

Status

defect
P1
normal
VERIFIED FIXED
6 years ago
6 years ago

People

(Reporter: Firefox_Mozilla, Assigned: gwagner)

Tracking

unspecified
Dependency tree / graph

Firefox Tracking Flags

(blocking-b2g:tef+, firefox21 wontfix, firefox22 wontfix, firefox23 fixed, b2g18 fixed, b2g18-v1.0.0 wontfix, b2g18-v1.0.1 fixed)

Details

(Whiteboard: IOT, Spain, [fixed-in-birch])

Attachments

(1 attachment, 1 obsolete attachment)

2.86 KB, patch
gwagner
: review+
Details | Diff | Splinter Review
Reporter

Description

6 years ago
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.
Reporter

Comment 1

6 years ago
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.
Hi, 

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

Thks!
David
Blocks: 855322
blocking-b2g: --- → tef?
Priority: -- → P1
blocking-b2g: tef? → tef+
yuren, mind taking a look at this? thanks
Assignee: nobody → yurenju.mozilla

Updated

6 years ago
QA Contact: atsai

Updated

6 years ago
Status: UNCONFIRMED → NEW
Ever confirmed: true
I can repoduce it, investigating...
Whiteboard: [status: under investigation]
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?
Flags: needinfo?(timdream)
also needinfo? to this feature's author. Albert, do you have any idea?
Flags: needinfo?(acperez)
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.)
Flags: needinfo?(timdream)
Tim is right, this is how to do it. Cc-ing the contact people...

Updated

6 years ago
Whiteboard: [status: under investigation] → IOT, Spain, [status: under investigation]
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.
Assignee: yurenju.mozilla → nobody
Assignee

Comment 10

6 years ago
We should just copy the behavior from the settingsDB.
Assignee

Updated

6 years ago
Assignee: nobody → anygregor
Assignee

Comment 11

6 years ago
Do we have a sample JSON file I can use for testing?
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?
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

6 years ago
I'm agree, Tim's proposal is the best approach,  like preloaded APNs.
Flags: needinfo?(acperez)
Assignee

Comment 16

6 years ago
Posted patch patch (obsolete) — Splinter Review
Assignee

Updated

6 years ago
Attachment #738097 - Flags: review?(bent.mozilla)
Whiteboard: IOT, Spain, [status: under investigation] → IOT, Spain, [status: needs review bent]
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?
Attachment #738097 - Flags: review?(bent.mozilla) → review+
Assignee

Comment 18

6 years ago
Posted patch patchSplinter Review
Attachment #738097 - Attachment is obsolete: true
Attachment #739000 - Flags: review+
Assignee

Updated

6 years ago
Whiteboard: IOT, Spain, [status: needs review bent] → IOT, Spain, checkin-needed
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, "").
Assignee

Comment 20

6 years ago
(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 :)
https://hg.mozilla.org/projects/birch/rev/e65a858b4235
Whiteboard: IOT, Spain, checkin-needed → IOT, Spain, [fixed-in-birch]
https://hg.mozilla.org/mozilla-central/rev/e65a858b4235
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Inari
Gaia v1.0.1: 2bcc625aecce5f111b70f7cf4d17523a09cb7d6c

Verified.
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.