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): |
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: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
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.
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.
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
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.