Opened on 06/07/2016 at 02:58:09 PM

Closed on 03/21/2017 at 12:10:00 PM

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

Attachments (0)

Change History (4)

comment:1 Changed on 06/07/2016 at 02:59:01 PM by trev

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

comment:2 Changed on 06/07/2016 at 03:19:56 PM by trev

  • Priority changed from P3 to P2

comment:3 Changed on 03/21/2017 at 12:09:15 PM by abpbot

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

comment:4 Changed on 03/21/2017 at 12:10:00 PM by trev

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

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 trev.
 
Note: See TracTickets for help on using tickets.