Opened on 03/16/2019 at 04:19:44 PM
Closed on 07/26/2019 at 02:27:15 PM
Last modified on 09/23/2019 at 07:14:13 AM
#7375 closed change (fixed)
Replace Ci.nsITimer with setTimeout()
| Reported by: | mjethani | Assignee: | mjethani |
|---|---|---|---|
| Priority: | P3 | Milestone: | |
| Module: | Core | Keywords: | |
| Cc: | kzar, sergz | Blocked By: | |
| Blocking: | #5702 | Platform: | Unknown / Cross platform |
| Ready: | yes | Confidential: | no |
| Tester: | Ross | Verified working: | yes |
| Review URL(s): |
https://gitlab.com/eyeo/adblockplus/adblockpluscore/merge_requests/109 |
||
Description (last modified by mjethani)
Background
For #5702, we can replace Ci.nsITimer directly with setTimeout().
What to change
In lib/downloader.js, replace Ci.nsITimer with setTimeout(). Update test/_common.js to override setTimeout() to use the fake timer.
Integration notes
In adblockpluschrome, remove nsITimer and related code (e.g. FakeTimer) from lib/compat.js.
Note: combined with other changes, this should mean the removal of lib/compat.js entirely.
In libadblockplus, remove anything related to Ci.nsITimer and add an implementation of setTimeout() that is compatible with the JS version.
Hints for testers
Try this test:
- Add a subscription with an Expires: value of 1h and wait for it to be synced once
- Shut down the browser
- Turn the system clock forward by a little over 72 minutes (1.2 times an hour)
- Restart the browser and wait for ~1 minute
- The subscription should be synced once (see server logs)
- Turn the system clock forward by a little over 2 hours
- The subscription should be synced again (see server logs)
Follow-up work
See core#43.
Attachments (0)
Change History (10)
comment:1 Changed on 03/17/2019 at 06:44:14 AM by mjethani
- Blocking 5702 added
- Description modified (diff)
comment:2 Changed on 07/26/2019 at 11:10:19 AM by mjethani
- Description modified (diff)
- Owner set to mjethani
- Priority changed from Unknown to P3
- Ready set
- Summary changed from Implement Timer class to Replace Ci.nsITimer with setTimeout()
comment:3 Changed on 07/26/2019 at 01:38:34 PM by abpbot
comment:4 Changed on 07/26/2019 at 01:43:50 PM by mjethani
- Cc kzar sergz added
- Description modified (diff)
comment:5 Changed on 07/26/2019 at 01:44:22 PM by mjethani
- Status changed from new to reviewing
comment:7 Changed on 07/26/2019 at 02:27:15 PM by mjethani
- Description modified (diff)
- Resolution set to fixed
- Status changed from reviewing to closed
comment:10 Changed on 09/23/2019 at 07:14:13 AM by Ross
- Tester changed from Unknown to Ross
- Verified working set
Done. Working as described.
ABP 3.6.3.2391
Chrome 52.0.2743.82 / Windows 10 1903
Chrome 77.0.3865.90 / Windows 10 1903
Edge Chrome 78.0.276.2 / Windows 10 1903
Firefox 51.0 / Windows 10 1903
Firefox 69.0.1 / Windows 10 1903
Firefox Mobile 68.1.1 / Windows 10 1903
Opera 38.0.2220.41 / Windows 10 1903
Opera 63.0.3368.94 / Windows 10 1903
ABP 0.9.17.2391
Edge 44.18362.329.0 / Windows 10 1903
A commit referencing this issue has landed:
Issue 7375 - Replace Ci.nsITimer with setTimeout()