Opened 2 years ago

Closed 2 years ago

#6217 closed change (fixed)

Correct behavior of whitelisting a domain

Reported by: dzhang Assignee: dzhang
Priority: Unknown Milestone: Adblock-Plus-for-iOS-next
Module: Adblock-Plus-for-iOS/macOS Keywords:
Cc: Blocked By:
Blocking: Platform: iOS
Ready: yes Confidential: no
Tester: Unknown Verified working: no
Review URL(s):

https://codereview.adblockplus.org/29647688/

Description (last modified by dzhang)

Background

It has been observed that the time it takes for whitelisting to take effect is inconsistent when performing whitelisting using the Safari action extension.

This has been traced to an implementation error where the content blocker was incorrectly being reloaded at the wrong time.

It appears that calling reload on the content blocker more than once concurrently can cause unexpected effects such as a crash during content blocker access or code execution being halted.

Removing extraneous content blocker reload calls appears to greatly improve the consistency of whitelisting by causing the whitelisted site to be recognized almost immediately after whitelisting.

A future improvement can involve a notification to let the user know that when the whitelisting has been completed.

What to change

Prevent extraneous reloads of the content blocker so that the reloads do not interefere with whitelisting operations.

Change History (4)

comment:1 Changed 2 years ago by dzhang

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

comment:2 Changed 2 years ago by dzhang

  • Description modified (diff)

comment:3 Changed 2 years ago by dzhang

When testing on the simulator, there are Main Thread Checker errors reporting during the first whitelisting of a website using the Safari action extension. These are due to KVO observers, and their subsequent UI operations, being in effect in AdblockPlusControllerBase. The success of the whitelisting operation is not affected by these errors reported in Xcode 9.

Last edited 2 years ago by dzhang (previous) (diff)

comment:4 Changed 2 years ago by dzhang

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