Opened 3 years ago

Closed 3 years ago

#4125 closed change (fixed)

[emscripten] Convert filter classes to C++

Reported by: trev Assignee: trev
Priority: P2 Milestone:
Module: Core Keywords:
Cc: Blocked By:
Blocking: #4122 Platform: Unknown / Cross platform
Ready: yes Confidential: no
Tester: Unknown Verified working: no
Review URL(s):

https://codereview.adblockplus.org/29333474/

Description

Background

See #4122 for the rationale, with Emscripten we should get a significantly more compact representation of the filters data.

What to change

Reimplement most of the current API in C++. Quite a few properties don't need to be exposed however, these are only used internally anyway. Some other properties are read-only now even though these technically allowed write access before. Other differences:

  • JavaScript code obtaining references to filters has to call filter.delete() in order to release the reference.
  • No support for legacy "simplified" element hiding syntax, it is deprecated and should be removed now.
  • No Filter.subscriptions property at this point (to be added once subscription classes are implemented).
  • Filter.serialize() returns a string instead of adding to a buffer (to become an internal method anyway once FilterStorage has been converted).
  • Filter.normalize() no longer exists, normalization is a mandatory part of the filter creation process.
  • Filter.fromObject() in unimplemented (to become an internal method once FilterStorage has been converted).
  • Filter.knownFilters lookup table replaced by static Filter.getKnownFilter() method.

Change History (4)

comment:1 Changed 3 years ago by trev

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

comment:2 Changed 3 years ago by trev

  • Priority changed from P3 to P2

comment:3 Changed 3 years ago by abpbot

A commit referencing this issue has landed:
Issue 4125 - [emscripten Convert filter classes to C++]

comment:4 Changed 3 years ago by trev

  • Resolution set to fixed
  • Status changed from reviewing to closed
Note: See TracTickets for help on using tickets.