Opened on 04/14/2017 at 06:44:53 AM

Closed on 10/12/2017 at 10:23:29 AM

#5137 closed change (fixed)

[emscripten] Add implementation of basic filter storage functionality

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

https://codereview.adblockplus.org/29426559/

Description (last modified by trev)

Background

In preparation for #4128, we should move filter list management into Emscripten.

What to change

  • Update bindings generation to allow producing singletons, with this syntax:
    singleton_("FilterStorage")
        .function("addFilter", &FilterStorage::addFilter)
        .property("initialized", &FilterStorage::getInitialized);
    
  • Create compiled/storage/FilterStorage.{cpp|h} defining the FilterStorage singleton class.
  • Expose subscription list as subscriptionCount property and methods subscriptionAt, indexOfSubscription. In JavaScript, implement an iteratable FilterStorage.subscription property that will delete references automatically once the loop iteration is done.
  • Also implement methods getSubscriptionForFilter (aliased to getGroupForFilter for backwards compatibility), addSubscription, removeSubscription, moveSubscription.
  • Add Subscription.listed flag, set by FilterStorage.addSubscription and unset by FilterStorage.removeSubscription - this replaces FilterStorage.knownSubscriptions map.
  • FilterStorage.moveFilter should not be implemented, the same effect can be achieved by calling UserDefinedSubscription.removeFilterAt and UserDefinedSubscription.insertFilterAt. These methods should trigger notifications for listed subscriptions however.
  • Implement FilterStorage.addFilter and FilterStorage.removeFilter helpers without subscription parameter - these are only to be used if the filter location is unknown, otherwise the respective UserDefinedSubscription methods are preferred.

Note

Current filter storage API has considerably more functionality, it should be added in the follow-up issues.

Attachments (0)

Change History (17)

comment:1 Changed on 04/14/2017 at 07:11:35 AM by trev

  • Blocking 5138 added

comment:2 Changed on 04/14/2017 at 08:56:54 AM by trev

  • Description modified (diff)

comment:3 Changed on 04/15/2017 at 08:25:56 AM by trev

  • Blocking 5144 added

comment:4 Changed on 04/18/2017 at 03:04:35 PM by trev

  • Owner set to trev

comment:5 Changed on 04/25/2017 at 02:38:48 PM by trev

  • Description modified (diff)

comment:6 Changed on 05/01/2017 at 02:48:12 PM by trev

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

comment:7 Changed on 05/06/2017 at 08:33:56 AM by trev

  • Blocked By 5216 added

comment:8 Changed on 05/10/2017 at 02:06:09 PM by trev

  • Description modified (diff)

comment:9 Changed on 05/18/2017 at 02:17:51 PM by trev

  • Blocked By 5258 added

comment:10 Changed on 05/18/2017 at 02:27:27 PM by trev

  • Blocking 5259 added

comment:11 Changed on 05/18/2017 at 02:34:20 PM by trev

  • Blocking 5146 added

comment:12 Changed on 05/18/2017 at 02:37:26 PM by trev

  • Description modified (diff)
  • Summary changed from [emscripten] Convert filter storage to C++ to [emscripten] Add implementation of basic filter storage functionality

I've updated description to describe the current state of affairs. Also, hit counts functionality moved into #5259 and FilterStorage.updateSubscriptionFilters() replacement into #5146.

comment:13 Changed on 08/31/2017 at 01:30:41 PM by abpbot

A commit referencing this issue has landed:
Issue 5137 - [emscripten Added basic filter storage implementation]

comment:14 Changed on 09/12/2017 at 02:12:17 PM by sergz

  • Blocked By 5661 added

comment:15 Changed on 09/12/2017 at 02:13:48 PM by sergz

  • Blocked By 5662 added

comment:16 Changed on 10/12/2017 at 10:21:51 AM by trev

  • Blocked By 5258 removed

comment:17 Changed on 10/12/2017 at 10:23:29 AM by trev

  • Blocked By 5661 removed
  • Resolution set to fixed
  • Status changed from reviewing to closed

This isn't really blocked by 5661. Basic functionality has been implemented, parsing and serialization as well as hit counts are addressed by other issues.

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.