Opened on 08/11/2018 at 02:37:19 PM

Closed on 08/29/2018 at 01:04:55 PM

Last modified on 10/24/2018 at 01:35:07 PM

#6854 closed change (fixed)

Remove deprecated FilterNotifier methods

Reported by: mjethani Assignee: mjethani
Priority: P3 Milestone:
Module: Core Keywords:
Cc: kzar, greiner, sebastian, wspee Blocked By:
Blocking: Platform: Unknown / Cross platform
Ready: yes Confidential: no
Tester: Ross Verified working: yes
Review URL(s):

https://codereview.adblockplus.org/29853561/

Description (last modified by mjethani)

Background

The following methods of FilterNotifier are deprecated: addListener, removeListener, and triggerListeners. They should be removed now.

FilterNotifier.triggerListeners, in particular, is bad for performance, because it calls every single listener rather than only the ones that are interested in the specific event.

What to change

Replace FilterNotifier.addListener, FilterNotifier.removeListener, and FilterNotifier.triggerListeners with FilterNotifier.on, FilterNotifier.off, and FilterNotifier.emit, respectively, as appropriate.

Update the tests for any change in behavior.

Integration notes

The deprecated methods are not being used by any other module outside of core; the respective module owners might want to verify this.

Hints for testers

For the dependency update in #6933: The deprecated methods were already not being used by the extension, therefore there should be nothing to test here from the point of view of the extension.

Attachments (0)

Change History (10)

comment:1 Changed on 08/11/2018 at 02:40:12 PM by mjethani

  • Description modified (diff)

comment:2 Changed on 08/11/2018 at 02:40:20 PM by mjethani

  • Owner set to mjethani

comment:3 Changed on 08/11/2018 at 02:43:46 PM by mjethani

  • Review URL(s) modified (diff)
  • Status changed from new to reviewing

comment:4 Changed on 08/29/2018 at 12:59:44 PM by abpbot

A commit referencing this issue has landed:
Issue 6854 - Remove deprecated FilterNotifier methods

comment:5 Changed on 08/29/2018 at 01:04:05 PM by mjethani

  • Cc greiner sebastian wspee added
  • Description modified (diff)

comment:6 Changed on 08/29/2018 at 01:04:55 PM by mjethani

  • Resolution set to fixed
  • Status changed from reviewing to closed

comment:7 Changed on 09/11/2018 at 01:49:46 PM by mjethani

  • Description modified (diff)

comment:8 Changed on 09/11/2018 at 02:36:47 PM by greiner

the respective module owners might want to verify this

I did a quick search through adblockplusui and can confirm that none of the deprecated methods are being used.

comment:9 Changed on 09/11/2018 at 04:47:20 PM by mjethani

  • Description modified (diff)

comment:10 Changed on 10/24/2018 at 01:35:07 PM by Ross

  • Tester changed from Unknown to Ross
  • Verified working set

Done. Doesn't look to have caused any issues or regressions in the extension.

ABP 3.3.2.2175
Firefox 62 / 51 / Windows 10
Chrome 69 / 49 / Windows 10
Opera 56 / 36 / Windows 10

Add Comment

Modify Ticket

Change Properties
Action
as closed .
The resolution will be deleted. Next status will be 'reopened'.
to The owner will be changed from mjethani.
 
Note: See TracTickets for help on using tickets.