Opened on 08/21/2017 at 03:41:40 AM

Closed on 01/17/2018 at 02:07:38 AM

#5551 closed change (fixed)

Create ContentBlockerManager and FilterListsUpdater to replace methods from AdblockPlusExtras

Reported by: dzhang Assignee: dzhang
Priority: Unknown Milestone: Adblock-Plus-for-iOS-next
Module: Adblock-Plus-for-iOS/macOS Keywords: ios, swift, rxswift
Cc: mario, tiago, ashephard, CraftyDeano Blocked By: #5550, #5594, #6223, #6225
Blocking: #4317, #6232, #6239 Platform: iOS
Ready: no Confidential: no
Tester: Unknown Verified working: no
Review URL(s):

https://codereview.adblockplus.org/29660659/

Description (last modified by dzhang)

Background

A class with equivalent functionality to ContentBlockerManager and AdblockPlusExtras will be created using Swift and RxSwift. The classes will replace AdblockPlusExtras. These steps are part of converting ABP to Swift and follow from #5550.

Almost all operations related to filter list updating and content blocker reloading will be converted to Swift.

What to change

  • Add RxSwift to the project
  • Create ContentBlockerManager to include functionality found in the class ContentBlockerManager
  • Create FilterListsUpdater to include functionality found in AdblockPlusExtras
    • Make the Swift version more modular by separating distinct operations into separate functions. Use Observables where it is sensible to do so. The intention is to reduce large blocks of code such as found in the init function.

RxSwift code here will be used for #5514.

Attachments (0)

Change History (16)

comment:1 Changed on 08/21/2017 at 03:44:02 AM by dzhang

  • Description modified (diff)

comment:2 Changed on 08/30/2017 at 03:25:53 AM by dzhang

  • Blocked By 5594 added

comment:3 Changed on 09/02/2017 at 12:18:37 AM by dzhang

  • Cc mario tiago ashephard CraftyDeano added

comment:4 Changed on 10/24/2017 at 01:22:22 AM by dzhang

There is an implementation error in function outdatedFilterListNames. It is only looking at the active filter list rather than the entire set for the user. Operations dependent on receiving all outdated filter list names cannot function correctly as a result. This function will be corrected during creation of FilterListsUpdater.

comment:5 Changed on 12/28/2017 at 08:58:57 PM by dzhang

  • Blocked By 6223 added

comment:6 Changed on 12/29/2017 at 11:46:06 PM by dzhang

  • Blocked By 6225 added

comment:7 Changed on 01/06/2018 at 01:01:03 AM by dzhang

  • Blocking 6232 added

comment:8 Changed on 01/09/2018 at 09:00:19 AM by dzhang

A major problem appears to have been solved regarding detecting the activation state of ABP. It was found that concurrent content blocker reloads could cause the activation state to be detected incorrectly.

A few remedies were implemented.

  • On iOS >= 10, detect content blocker activation state using getStateOfContentBlocker.
  • Dispose of content blocker reload subscriptions before checking the activation state thereby preventing delayed effects from ongoing single or concurrent operations.
  • Only check activation state on the start of the app as the activation state will almost never change while the app is in the foreground - This was being done before, but there may have been redundant calls. It only needs to be performed once when the app becomes active.
Last edited on 01/17/2018 at 02:07:28 AM by dzhang

comment:9 Changed on 01/10/2018 at 02:44:51 AM by dzhang

  • Blocked By 6239 added

comment:10 Changed on 01/10/2018 at 02:48:05 AM by dzhang

  • Blocked By 6239 removed
  • Blocking 6239 added

comment:11 Changed on 01/10/2018 at 03:11:33 AM by dzhang

  • Summary changed from Create ContentBlockerManager and FilterListsUpdater in Swift with RxSwift to Create ContentBlockerManager and FilterListsUpdater to replace methods from AdblockPlusExtras

comment:12 Changed on 01/10/2018 at 03:37:40 AM by dzhang

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

comment:13 Changed on 01/10/2018 at 03:49:42 AM by dzhang

The previous Salsita implementation seems to work but the method is based on timing the Content Blocker reload and that is not going to be as reliable as the API in iOS >= 10. We'll use the legacy implementation for iOS < 10 and phase it out when we no longer support iOS 9.

Last edited on 01/10/2018 at 03:50:14 AM by dzhang

comment:14 Changed on 01/12/2018 at 01:36:30 AM by dzhang

  • Blocking 4317 added

comment:15 Changed on 01/15/2018 at 03:25:44 PM by dzhang

  • Description modified (diff)

comment:16 Changed on 01/17/2018 at 02:07:38 AM by dzhang

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