1073684 - [Contacts] Closing contact app during facebook contact removal causes contacts not removed turn into regular contacts

Status: RESOLVED FIXED

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

Description

[[:RJ]]

Attachment: log cat

Description: 
Closing contact app during facebook contact removal causes imported number to be inaccurate, stating there are no friends imported when there are still leftover from the removal which seems to convert the left overs into normal contacts.  


Repro Steps: 
1) Update a Flame to 20140926000202 
2) Go to the  contacts app and import facebook contacts 
3) Toggle the sync friends to off
4) While contacts are being removed hold home button to go into card view
5) Remove the app wile it is removing > return to the contact settings 


Actual: 
status says 0 friends imported, when there are still imported contacts from facebook left on the phone


Expected: 
Display properly keeps tracks of imported friends 

Environmental Variables: 
Device: Flame (2.1 319mb) 
Build ID: 20140926000202 
Gaia: 6a6ed9433fce47e76c07fd35bc5952acb108f4c8 
Gecko: 7b09a378588c 
Version: 34.0a2 (2.1 319mb)
Firmware Version: v123 
User Agent: Mozilla/5.0 (Mobile; rv:34.0) Gecko/34.0 Firefox/34.0 

Repro frequency:100%
See attached:logcat, video clip: http://youtu.be/oNJIIIa3bPI

Comment 1

[[:RJ]]

This issue DOES occur on Flame 2.2 (319mb) kitkat Flame 2.1(512MB) kitkat 2.0 (319mb) kitkat


Closing contact app during facebook contact removal causes contacts not removed turn into regular contacts

Flame 2.2 KitKat Base (319mb)

Environmental Variables:
Device: Flame 2.2 Master
BuildID: 20140926040203
Gaia: a06714c555ca7068545f10b4437a16c14cd8e7f5
Gecko: 9e3d649b80a2
Version: 35.0a1 (2.2 Master)
Firmware: V180
User Agent: Mozilla/5.0 (Mobile; rv:35.0) Gecko/35.0 Firefox/35.0

Flame 2.1 KitKat Base (512mb)

Environmental Variables:
Device: Flame 2.1
BuildID: 20140926000202
Gaia: 6a6ed9433fce47e76c07fd35bc5952acb108f4c8
Gecko: 7b09a378588c
Version: 34.0a2 (2.1)
Firmware: V180
User Agent: Mozilla/5.0 (Mobile; rv:34.0) Gecko/34.0 Firefox/34.0


Flame 2.0 KitKat Base (319mb)

Device: Flame 2.0
BuildID: 20140926063008
Gaia: c1aa7829548e65360472c31544dbe2839eaf5be1
Gecko: 5a9f1f402425
Version: 32.0 (2.0)
Firmware: V180
User Agent: Mozilla/5.0 (Mobile; rv:32.0) Gecko/32.0 Firefox/32.0

Comment 2

[KTucker [:KTucker][Inactive 3/4/2016]]

This should probably be fixed but not going to nominate to block on his since the user is force closing the app while the contacts are being removed.

Comment 3

[Jose Manuel Cantera]

The root cause of this bug is that when we clean up data from Facebook, first of all we clear the FB datastore and then we remove the corresponding mozContacts. The rationale for doing this is that the process will speed up considerably. So here we have 2 options:

A/ Avoid the clear operation. This will make the process slower and on the other hand we will have the same inconsistency risk as we cannot guarantee to do the datastore record removal and the mozContact removal in an atomic step. Nonetheless, the number of affected contacts will be much more smaller. 

B/ Perform a sanity check in deferred_actions to remove all FB Contacts in mozContacts that are orphaned.

I'm more inclined to option B, as it is more elegant, safe and less intrusive to the current code and would not affect current performance.

Francisco, what do you think?

Comment 4

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

I totally agree with you, I would go for option B. We can setup kind of a cleaningfb flag and be sure in defered actions that we clean everything in case we were interrupted.

The only problem that I see is deferred actions is triggered once the list is ready, we will compete to fill the fb information, so if we manage to get to a point where we were interrupted and then we need to clean this info after restarting the app, probably we will need to rebuild the whole list. Or prevent any of that fb loading if that flag is still active.


Pretty good analysis Jose.

Comment 5

[Jose Manuel Cantera]

Attachment: 24938.html

Francisco,

I have fixed the issue about inconsistency in the contacts present. The patch also includes a fix that solves an issue with the cleaning process and linked contacts. 

thanks for the review

Comment 6

[Jose Manuel Cantera]

(In reply to Francisco Jordano [:arcturus] [:francisco] from comment #4)
> I totally agree with you, I would go for option B. We can setup kind of a
> cleaningfb flag and be sure in defered actions that we clean everything in
> case we were interrupted.
> 
> The only problem that I see is deferred actions is triggered once the list
> is ready, we will compete to fill the fb information, so if we manage to get
> to a point where we were interrupted and then we need to clean this info
> after restarting the app, probably we will need to rebuild the whole list.

In the proposed patch we clean the contacts using navigator.mozContacts.remove, so the oncontactchange events will force the list to reconfigure

> Or prevent any of that fb loading if that flag is still active.
> 
> 
> Pretty good analysis Jose.

Comment 7

[Jose Manuel Cantera]

Comment on attachment 8501684 [details]
24938.html

asking f? from Cristian as I've touched fb_contacts.js for fixing the aforementioned issue

Comment 8

[Cristian Rodriguez (:crdlc)]

Comment on attachment 8501684 [details]
24938.html

I left a comment, lgtm

Comment 9

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

Comment on attachment 8501684 [details]
24938.html

Left some minors comments, but the code looks pretty good.

As a general comment, not pretty important, to remove the output log. Not blocking for land this.

Thanks Jose!

Comment 10

[Jose Manuel Cantera]

landed with reviewer comments addressed:

https://github.com/mozilla-b2g/gaia/commit/fde9b0159b5063e8d9919ac47cf45df8e35bdf75

Comment 11

[Yeojin Chung [:YeojinC]]

This issue still occurs on Flame 2.2.

Result: "No friends imported (out of XXX)" message appears when removing Facebook contacts was cancelled.

Device: Flame 2.2 (319mb, KK, Shallow Flash)
BuildID: 20141120040205
Gaia: 1abe09b4925547699dfdb2d358aed019137c3aa6
Gecko: 6ce1b906c690
Version: 36.0a1 (2.2) 
Firmware Version: v188-1
User Agent: Mozilla/5.0 (Mobile; rv:36.0) Gecko/36.0 Firefox/36.0

Comment 12

[Yeojin Chung [:YeojinC]]

Bug 1103124 is filed as this bug failed verification.

Comment 13

[Jose Manuel Cantera]

(In reply to Yeojin Chung [:YeojinC] from comment #11)
> This issue still occurs on Flame 2.2.
> 
> Result: "No friends imported (out of XXX)" message appears when removing
> Facebook contacts was cancelled.
> 
> Device: Flame 2.2 (319mb, KK, Shallow Flash)
> BuildID: 20141120040205
> Gaia: 1abe09b4925547699dfdb2d358aed019137c3aa6
> Gecko: 6ce1b906c690
> Version: 36.0a1 (2.2) 
> Firmware Version: v188-1
> User Agent: Mozilla/5.0 (Mobile; rv:36.0) Gecko/36.0 Firefox/36.0

please verify correctly the bug. see comment #3.