Closed Bug 1136434 Opened 10 years ago Closed 10 years ago

request-sync still occurs after sync task has been unregistered


(Core :: DOM: Core & HTML, defect)

Not set



blocking-b2g 2.2+
Tracking Status
firefox37 --- wontfix
firefox38 --- wontfix
firefox39 --- fixed
b2g-v2.2 --- fixed
b2g-master --- fixed


(Reporter: jrburke, Assigned: baku)



(1 file)

This was found on master/trunk pvt nightlies on the Flame device, using the email app. Once an email account is configured with the email app:

* Open email app, tap menu button in upper left corner.
* Select gear icon to go to settings.
* Tap 5 times fairly quickly *below* the Add Account button to bring up secret debug menu.
* Tap the "Enable faster sync options" button, then press Back button.
* Tap an account to go to the account settings.
* For the "Check for new messages" item, select 100 seconds.
* Then long-tap the home button to bring up the app switcher and completely close the email app.
* Wait for the sync to occur, you can watch the adb logcat to see happen.

Once a sync does happen, go back to the account settings, and set the "Check for new messages" to "Manually". This will unregister the sync task. Long tap home to completely kill the email app.

You can verify that no syncs are registered, or at least visible to the APIs exposed to apps, by installing the requestsyncmanager app:

Or by using the secret debug menu to list the sync tasks in the logcat.

However, if you wait another 100 seconds or so, the logcat shows the email app starting up again and syncing.

So there is something that is still registered with the requestsync plumbing, even though the requestsync APIs exposed to the apps do not show any task.

The disconcerting part is a restart of the phone does not fix the problem, syncs still occur, with no mechanism to turn them off.

This affects master and 2.2/the mozilla37-b2g branch. So once fixed on master, it should be uplifted for the branch used by b2g 2.2.
Flags: needinfo?(amarchesini)
Can I reproduce this issue with the latest gecko+gaia?
Flags: needinfo?(amarchesini) → needinfo?(jrburke)
I was seeing this with yesterday's pvt flashes, and I saw it even before then, so yes, I expect it still to be there in the latest gecko+gaia.
Flags: needinfo?(jrburke) → needinfo?(amarchesini)
Assignee: nobody → amarchesini
Flags: needinfo?(amarchesini)
OS: Mac OS X → All
Hardware: x86 → All
Attached patch rs.patchSplinter Review
When I removed the timer from the _registrations object, I forgot this one.
Attachment #8570183 - Flags: review?(ehsan)
Comment on attachment 8570183 [details] [diff] [review]

Let's add fabrice too. Fabrice reviewed the previous code.
Attachment #8570183 - Flags: review?(fabrice)
Comment on attachment 8570183 [details] [diff] [review]

Review of attachment 8570183 [details] [diff] [review]:

Please add a test!
Attachment #8570183 - Flags: review?(ehsan) → review+
Attachment #8570183 - Flags: review?(fabrice)
The funny thing is that we have a test. But we don't check if the task is re-triggered because we don't use a setTimeout just for this. I don't think we want to add this, because timers are usually a problem in mochitests.
Timers as such aren't the problem.
It is that they are occasionally used in a way which makes the tests flaky.
If you for example need to test that something *does not happen*, you need to use a timer
(and hope the timer value is long enough to catch the theoretical "does happen" case).
I agree with Olli.
Closed: 10 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla39
triage: it's affecting email app. we'll need the fix for 2.2 (gecko37)
blocking-b2g: 2.2? → 2.2+
Flags: needinfo?(amarchesini)
Comment on attachment 8570183 [details] [diff] [review]

NOTE: Please see to better understand the B2G approval process and landings.

[Approval Request Comment]
Bug caused by (feature/regressing bug #): bug 1018320
User impact if declined: deleting tasks from requestSyncService will not remove the timers, so the tasks are still schedulated once.
Testing completed: on device
Risk to taking this patch (and alternatives if risky): no risks. 
String or UUID changes made by this patch: none
Flags: needinfo?(amarchesini)
Attachment #8570183 - Flags: approval-mozilla-b2g37?
Attachment #8570183 - Flags: approval-mozilla-b2g37? → approval-mozilla-b2g37+
Component: DOM → DOM: Core & HTML
You need to log in before you can comment on or make changes to this bug.