1147804 - Facebook auto sync will not happen if you change the time 24 hours later

Status: RESOLVED FIXED

(Core :: DOM: Device Interfaces, defect)

Description

[Elie]

[1.Description]:
[Flame][v2.2][Contacts] Change the contact picture and birthday, and change system time from 2014/3/26 to2014/3/27, but the auto sync will not take place, the contact picture and birthday will not change.
Attchment:logcat(10).txt and VIDEO0310_Compress.MP4
Happen time:4:43

[2.Testing Steps]: 
1. Open the contacts app. 
2. Tap on the Settings icon.(Gear icon) 
3. Select import from facebook. 
4. Log in to the user′s facebook account. 
5. Select contacts to import. 
6. Tap import button. 
7. Make a noticeable change on the Facebook contact that was imported. (change the contact picture and birthday)
8. change system time from 2014/3/26 to2014/3/27 
wait for more than 5m


[3.Expected Result]: 
8. Auto sync will take place.

[4.Actual Result]: 
8.The automatic sync is not taking place, and the contact picture and birthday will not change.

[5.Reproduction build]: 
Flame 2.2 Affected
Build ID               20150325162500
Gaia Revision          2d42a7c1d99472780e3aeb2e16d7cb0a4adbd222
Gaia Date              2015-03-25 20:46:52
Gecko Revision         https://hg.mozilla.org/releases/mozilla-b2g37_v2_2/rev/ebb742bfe1a4
Gecko Version          37.0
Device Name            flame
Firmware(Release)      4.4.2
Firmware(Incremental)  eng.cltbld.20150325.200218
Firmware Date          Wed Mar 25 20:02:30 EDT 2015
Bootloader             L1TC000118D0
FLame 3.0:[Unaffect]
Build ID               20150325160204
Gaia Revision          508b8d48fb5ecf08bf0e5b4fef42bc48b770e7f2
Gaia Date              2015-03-25 16:54:53
Gecko Revision         https://hg.mozilla.org/mozilla-central/rev/f40ee067d081
Gecko Version          39.0a1
Device Name            flame
Firmware(Release)      4.4.2
Firmware(Incremental)  eng.cltbld.20150325.191758
Firmware Date          Wed Mar 25 19:18:07 EDT 2015
Bootloader             L1TC000118D0
[6.Reproduction Frequency]: 
Always Recurrence,5/5

[7.TCID]: 
5188

Comment 1

[Elie]

Attachment: VIDEO0310_Compress.MP4

Comment 2

[Johan Lorenzo [:jlorenzo]]

Do you know if this behavior is supported by mozAlarms, Fernando?

Comment 3

[Fernando Jiménez Moreno [:ferjm]]

Sorry, but I have no idea. And I am not even sure if this is the proper way to test fb sync. Francisco?

Maybe Fabrice knows if we update the alarms when the system clock changes.

Comment 4

[Fernando Jiménez Moreno [:ferjm]]

I can see that we react to timezone changes in the alarms API code [1] but this is not the case here. We are not changing timezones but the system time.

[1] https://mxr.mozilla.org/mozilla-central/search?find=%2Fdom%2Falarm%2F&string=change

Comment 5

[Fernando Jiménez Moreno [:ferjm]]

I think we should just fire all the expired alarms on system clock change.

Comment 6

[[:fabrice] Fabrice Desré]

(In reply to Fernando Jiménez Moreno [:ferjm] from comment #5)
> I think we should just fire all the expired alarms on system clock change.

Yep, looks fine to me.

Comment 7

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

I don't think this is the correct way to trying fb sync.

In case you want to try this feature, you'll need to create a new build specifying the time to perform each sync, you can change that here:

https://github.com/mozilla-b2g/gaia/blob/master/build/import-config.js#L70

take into account that you specify hours, so you could potentially specify a decimal number to make it sync in minutes.

Comment 8

[Johan Lorenzo [:jlorenzo]]

(In reply to Francisco Jordano [:arcturus] [:francisco] from comment #7)
> In case you want to try this feature, you'll need to create a new build
> specifying the time to perform each sync, you can change that here:

Sorry, our contracting QA teams work mainly with builds from pvtbuilds. This bug wasn't reported before because the test case says to wait 24 hours before noticing any change[1].

I agree this is not a high priority bug, but does firing expired alams sounds like a way to solve this issue, Francisco?

[1] https://moztrap.mozilla.org/manage/cases/?filter-id=5188

Comment 9

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

Agree :)

(In reply to Johan Lorenzo [:jlorenzo] (QA) from comment #8)
> (In reply to Francisco Jordano [:arcturus] [:francisco] from comment #7)
> > In case you want to try this feature, you'll need to create a new build
> > specifying the time to perform each sync, you can change that here:
> 
> Sorry, our contracting QA teams work mainly with builds from pvtbuilds. This
> bug wasn't reported before because the test case says to wait 24 hours
> before noticing any change[1].
> 
> I agree this is not a high priority bug, but does firing expired alams
> sounds like a way to solve this issue, Francisco?
> 
> [1] https://moztrap.mozilla.org/manage/cases/?filter-id=5188

Comment 10

[Fernando Jiménez Moreno [:ferjm]]

Attachment: v1

Comment 11

[Fernando Jiménez Moreno [:ferjm]]

Attachment: v1

Comment 12

[[:fabrice] Fabrice Desré]

Comment on attachment 8586082 [details] [diff] [review]
v1

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

::: dom/alarm/AlarmService.jsm
@@ +12,2 @@
>      dump("AlarmService: " + aStr + "\n");
> +  }

nit: DEBUG && dump(...)

::: dom/alarm/test/test_alarm_change_system_clock.js
@@ +29,5 @@
> +/* Tests */
> +
> +add_test(function test_getAll() {
> +  do_print("= There should not be any alarm =");
> +  AlarmService._db.getAll(MANIFEST_URL, (aAlarms) => {

nit: aAlarms => { ...

Comment 13

[Fernando Jiménez Moreno [:ferjm]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/3d38764239c2

Comment 14

[Daniel Holbert [:dholbert]]

This was missing an 'override' annotation, which causes clang 3.6 & newer to spam a "-Winconsistent-missing-override" build warning, which busts --enable-warnings-as-errors builds (with clang 3.6+).

I pushed a followup to add that keyword, with blanket r+ that ehsan granted me for fixes of this sort over in bug 1126447 comment 2:
https://hg.mozilla.org/integration/mozilla-inbound/rev/a45fe60bd40d

Comment 15

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/3d38764239c2

Comment 16

[Fernando Jiménez Moreno [:ferjm]]

(In reply to Daniel Holbert [:dholbert] from comment #14)
> This was missing an 'override' annotation, which causes clang 3.6 & newer to
> spam a "-Winconsistent-missing-override" build warning, which busts
> --enable-warnings-as-errors builds (with clang 3.6+).
> 
> I pushed a followup to add that keyword, with blanket r+ that ehsan granted
> me for fixes of this sort over in bug 1126447 comment 2:
> https://hg.mozilla.org/integration/mozilla-inbound/rev/a45fe60bd40d

Sorry about that. Thanks for fixing it Daniel!

Comment 17

[Carsten Book [:Tomcat]]

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