Opened 2 years ago

Closed 22 months ago

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

Change History (16)

comment:1 Changed 2 years ago by dzhang

  • Description modified (diff)

comment:2 Changed 2 years ago by dzhang

  • Blocked By 5594 added

comment:3 Changed 2 years ago by dzhang

  • Cc mario tiago ashephard CraftyDeano added

comment:4 Changed 2 years ago 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 22 months ago by dzhang

  • Blocked By 6223 added

comment:6 Changed 22 months ago by dzhang

  • Blocked By 6225 added

comment:7 Changed 22 months ago by dzhang

  • Blocking 6232 added

comment:8 Changed 22 months ago 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 22 months ago by dzhang (previous) (diff)

comment:9 Changed 22 months ago by dzhang

  • Blocked By 6239 added

comment:10 Changed 22 months ago by dzhang

  • Blocked By 6239 removed
  • Blocking 6239 added

comment:11 Changed 22 months ago 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 22 months ago by dzhang

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

comment:13 Changed 22 months ago 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 22 months ago by dzhang (previous) (diff)

comment:14 Changed 22 months ago by dzhang

  • Blocking 4317 added

comment:15 Changed 22 months ago by dzhang

  • Description modified (diff)

comment:16 Changed 22 months ago by dzhang

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