Opened 7 months ago

Closed 3 months ago

Last modified 4 weeks ago

#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:

  1. Add a subscription with an Expires: value of 1h and wait for it to be synced once
  2. Shut down the browser
  3. Turn the system clock forward by a little over 72 minutes (1.2 times an hour)
  4. Restart the browser and wait for ~1 minute
  5. The subscription should be synced once (see server logs)
  6. Turn the system clock forward by a little over 2 hours
  7. The subscription should be synced again (see server logs)

Follow-up work

See core#43.

Change History (10)

comment:1 Changed 7 months ago by mjethani

  • Blocking 5702 added
  • Description modified (diff)

comment:2 Changed 3 months ago 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 3 months ago by abpbot

A commit referencing this issue has landed:
Issue 7375 - Replace Ci.nsITimer with setTimeout()

comment:4 Changed 3 months ago by mjethani

  • Cc kzar sergz added
  • Description modified (diff)

comment:5 Changed 3 months ago by mjethani

  • Status changed from new to reviewing

comment:6 Changed 3 months ago by mjethani

  • Description modified (diff)

comment:7 Changed 3 months ago by mjethani

  • Description modified (diff)
  • Resolution set to fixed
  • Status changed from reviewing to closed

comment:8 Changed 3 months ago by mjethani

  • Review URL(s) modified (diff)

comment:9 Changed 3 months ago by mjethani

  • Description modified (diff)

comment:10 Changed 4 weeks ago 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

Note: See TracTickets for help on using tickets.