Skip to content
This repository has been archived by the owner on May 10, 2024. It is now read-only.

Commit

Permalink
Fix #7352: Add ad-block level to advanced shield settings (#7437)
Browse files Browse the repository at this point in the history
  • Loading branch information
cuba authored and iccub committed May 16, 2023
1 parent dfb645d commit 1f97760
Show file tree
Hide file tree
Showing 14 changed files with 220 additions and 66 deletions.
3 changes: 2 additions & 1 deletion Sources/Brave/Frontend/Browser/BrowserViewController.swift
Original file line number Diff line number Diff line change
Expand Up @@ -513,6 +513,7 @@ public class BrowserViewController: UIViewController {
Preferences.Playlist.enablePlaylistMenuBadge.observe(from: self)
Preferences.Playlist.enablePlaylistURLBarButton.observe(from: self)
Preferences.Playlist.syncSharedFoldersAutomatically.observe(from: self)
ShieldPreferences.blockAdsAndTrackingLevelRaw.observe(from: self)

pageZoomListener = NotificationCenter.default.addObserver(forName: PageZoomView.notificationName, object: nil, queue: .main) { [weak self] _ in
self?.tabManager.allTabs.forEach({
Expand Down Expand Up @@ -2994,7 +2995,7 @@ extension BrowserViewController: PreferencesObserver {
tabManager.reloadSelectedTab()
case Preferences.General.enablePullToRefresh.key:
tabManager.selectedTab?.updatePullToRefreshVisibility()
case Preferences.Shields.blockAdsAndTracking.key,
case ShieldPreferences.blockAdsAndTrackingLevelRaw.key,
Preferences.Shields.blockScripts.key,
Preferences.Shields.blockImages.key,
Preferences.Shields.fingerprintingProtection.key,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import Combine
import BraveCore
import BraveNews
import Preferences
import BraveShields
import Data
import Growth
import os
Expand Down Expand Up @@ -42,6 +43,14 @@ import os
p3aUtilities.isP3AEnabled = isP3AEnabled
}
}
@Published var adBlockAndTrackingPreventionLevel: ShieldLevel {
didSet {
ShieldPreferences.blockAdsAndTrackingLevel = adBlockAndTrackingPreventionLevel
if adBlockAndTrackingPreventionLevel != oldValue {
recordGlobalAdBlockShieldsP3A()
}
}
}

typealias ClearDataCallback = @MainActor (Bool) -> Void
@Published var clearableSettings: [ClearableSetting]
Expand All @@ -61,6 +70,7 @@ import os
self.tabManager = tabManager
self.isP3AEnabled = p3aUtilities.isP3AEnabled
self.clearDataCallback = clearDataCallback
self.adBlockAndTrackingPreventionLevel = ShieldPreferences.blockAdsAndTrackingLevel

cookieConsentBlocking = FilterListStorage.shared.isEnabled(
for: FilterList.cookieConsentNoticesComponentID
Expand Down Expand Up @@ -189,12 +199,6 @@ import os
}
.store(in: &subscriptions)

Preferences.Shields.blockAdsAndTracking.$value
.sink { [weak self] _ in
self?.recordGlobalAdBlockShieldsP3A()
}
.store(in: &subscriptions)

Preferences.Shields.fingerprintingProtection.$value
.sink { [weak self] _ in
self?.recordGlobalFingerprintingShieldsP3A()
Expand All @@ -211,7 +215,15 @@ import os
case standard = 1
case aggressive = 2
}
let answer: Answer = Preferences.Shields.blockAdsAndTracking.value ? .standard : .disabled

let answer = { () -> Answer in
switch ShieldPreferences.blockAdsAndTrackingLevel {
case .disabled: return .disabled
case .standard: return .standard
case .aggressive: return .aggressive
}
}()

UmaHistogramEnumeration("Brave.Shields.AdBlockSetting", sample: answer)
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ import Data
import DesignSystem
import BraveUI
import Preferences
import Strings
import BraveShields

struct DefaultShieldsViewView: View {
enum CookieAlertType: String, Identifiable {
Expand All @@ -24,11 +24,20 @@ struct DefaultShieldsViewView: View {

var body: some View {
Section {
OptionToggleView(
title: Strings.blockAdsAndTracking,
subtitle: Strings.blockAdsAndTrackingDescription,
option: Preferences.Shields.blockAdsAndTracking
)
Picker(selection: $settings.adBlockAndTrackingPreventionLevel) {
ForEach(ShieldLevel.allCases) { level in
Text(level.localizedTitle)
.foregroundColor(Color(.secondaryBraveLabel))
.tag(level)
}
} label: {
ShieldLabelView(
title: Strings.Shields.trackersAndAdsBlocking,
subtitle: Strings.Shields.trackersAndAdsBlockingDescription
)
}
.listRowBackground(Color(.secondaryBraveGroupedBackground))

OptionToggleView(
title: Strings.HTTPSEverywhere,
subtitle: Strings.HTTPSEverywhereDescription,
Expand Down Expand Up @@ -134,3 +143,17 @@ struct DefaultShieldsViewView: View {
}
}
}

extension ShieldLevel: Identifiable {
public var id: String {
return rawValue
}

public var localizedTitle: String {
switch self {
case .aggressive: return Strings.Shields.trackersAndAdsBlockingAggressive
case .disabled: return Strings.Shields.trackersAndAdsBlockingDisabled
case .standard: return Strings.Shields.trackersAndAdsBlockingStandard
}
}
}
3 changes: 2 additions & 1 deletion Sources/Brave/Frontend/Shields/AdvancedShieldsView.swift
Original file line number Diff line number Diff line change
Expand Up @@ -6,10 +6,11 @@ import UIKit
import Shared
import BraveShared
import BraveUI
import BraveShields

class AdvancedShieldsView: UIStackView {
let siteTitle = HeaderTitleView()
let adsTrackersControl = ToggleView(title: Strings.blockAdsAndTracking)
let adsTrackersControl = ToggleView(title: Strings.Shields.trackersAndAdsBlocking)
let blockScriptsControl = ToggleView(title: Strings.blockScripts)
let fingerprintingControl = ToggleView(title: Strings.fingerprintingProtection)
let globalControlsTitleView = HeaderTitleView().then {
Expand Down
42 changes: 21 additions & 21 deletions Sources/Brave/Frontend/Shields/ShieldsViewController.swift
Original file line number Diff line number Diff line change
Expand Up @@ -66,23 +66,23 @@ class ShieldsViewController: UIViewController, PopoverContentComponent {
domain = Domain.getOrCreate(forUrl: url, persistent: !isPrivateBrowsing)
}

if let domain = domain {
shieldsUpSwitch.isOn = !domain.isShieldExpected(.AllOff, considerAllShieldsOption: false)
} else {
shieldsUpSwitch.isOn = true
}

shieldControlMapping.forEach { shield, view, option in
// Updating based on global settings
if let option = option {
// Sets the default setting
view.toggleSwitch.isOn = option.value
}
// Domain specific overrides after defaults have already been setup
shieldsUpSwitch.isOn = domain?.isShieldExpected(.AllOff, considerAllShieldsOption: false) == false

shieldControlMapping.forEach { shield, view in
if let domain = domain {
// site-specific shield has been overridden, update
view.toggleSwitch.isOn = domain.isShieldExpected(shield, considerAllShieldsOption: false)
} else {
switch shield {
case .AdblockAndTp:
view.toggleSwitch.isOn = ShieldPreferences.blockAdsAndTrackingLevel.isEnabled
case .AllOff:
assertionFailure()
case .FpProtection:
view.toggleSwitch.isOn = Preferences.Shields.fingerprintingProtection.value
case .NoScript:
view.toggleSwitch.isOn = Preferences.Shields.blockScripts.value
}
}
}
updateGlobalShieldState(shieldsUpSwitch.isOn)
Expand All @@ -94,7 +94,7 @@ class ShieldsViewController: UIViewController, PopoverContentComponent {
)
}

private func updateBraveShieldState(shield: BraveShield, on: Bool, option: Preferences.Option<Bool>?) {
private func updateBraveShieldState(shield: BraveShield, on: Bool) {
guard let url = url else { return }
let allOff = shield == .AllOff
// `.AllOff` uses inverse logic. Technically we set "all off" when the switch is OFF, unlike all the others
Expand Down Expand Up @@ -211,10 +211,10 @@ class ShieldsViewController: UIViewController, PopoverContentComponent {
// MARK: -

/// Groups the shield types with their control and global preference
private lazy var shieldControlMapping: [(BraveShield, AdvancedShieldsView.ToggleView, Preferences.Option<Bool>?)] = [
(.AdblockAndTp, shieldsView.advancedShieldView.adsTrackersControl, Preferences.Shields.blockAdsAndTracking),
(.NoScript, shieldsView.advancedShieldView.blockScriptsControl, Preferences.Shields.blockScripts),
(.FpProtection, shieldsView.advancedShieldView.fingerprintingControl, Preferences.Shields.fingerprintingProtection),
private lazy var shieldControlMapping: [(BraveShield, AdvancedShieldsView.ToggleView)] = [
(.AdblockAndTp, shieldsView.advancedShieldView.adsTrackersControl),
(.NoScript, shieldsView.advancedShieldView.blockScriptsControl),
(.FpProtection, shieldsView.advancedShieldView.fingerprintingControl),
]

var shieldsView: View {
Expand Down Expand Up @@ -264,11 +264,11 @@ class ShieldsViewController: UIViewController, PopoverContentComponent {
updatePreferredContentSize()
}

shieldControlMapping.forEach { shield, toggle, option in
shieldControlMapping.forEach { shield, toggle in
toggle.valueToggled = { [weak self] on in
guard let self = self else { return }
// Localized / per domain toggles triggered here
self.updateBraveShieldState(shield: shield, on: on, option: option)
self.updateBraveShieldState(shield: shield, on: on)
// Wait a fraction of a second to allow DB write to complete otherwise it will not use the
// updated shield settings when reloading the page
DispatchQueue.main.asyncAfter(deadline: .now() + 0.3) {
Expand All @@ -281,7 +281,7 @@ class ShieldsViewController: UIViewController, PopoverContentComponent {
@objc private func shieldsOverrideSwitchValueChanged() {
let isOn = shieldsUpSwitch.isOn
self.updateGlobalShieldState(isOn, animated: true)
self.updateBraveShieldState(shield: .AllOff, on: isOn, option: nil)
self.updateBraveShieldState(shield: .AllOff, on: isOn)
// Wait a fraction of a second to allow DB write to complete otherwise it will not use the updated
// shield settings when reloading the page
DispatchQueue.main.asyncAfter(deadline: .now() + 0.3) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
import Foundation
import WebKit
import Shared
import BraveShields
import os.log

class SiteStateListenerScriptHandler: TabContentScript {
Expand Down Expand Up @@ -67,7 +68,7 @@ class SiteStateListenerScriptHandler: TabContentScript {
if domain.areAllShieldsOff { return }

let models = await AdBlockStats.shared.cosmeticFilterModels(forFrameURL: frameURL, domain: domain)
let args = try self.makeArgs(from: models, frameURL: frameURL)
let args = try self.makeArgs(from: models, frameURL: frameURL, isAggressive: ShieldPreferences.blockAdsAndTrackingLevel.isAggressive)
let source = try ScriptFactory.shared.makeScriptSource(of: .selectorsPoller).replacingOccurrences(of: "$<args>", with: args)

let secureSource = CosmeticFiltersScriptHandler.secureScript(
Expand Down Expand Up @@ -97,7 +98,7 @@ class SiteStateListenerScriptHandler: TabContentScript {
}
}

@MainActor private func makeArgs(from modelTuples: [CachedAdBlockEngine.CosmeticFilterModelTuple], frameURL: URL) throws -> String {
@MainActor private func makeArgs(from modelTuples: [CachedAdBlockEngine.CosmeticFilterModelTuple], frameURL: URL, isAggressive: Bool) throws -> String {
var standardSelectors: Set<String> = []
var agressiveSelectors: Set<String> = []
var styleSelectors: [String: Set<String>] = [:]
Expand Down Expand Up @@ -126,7 +127,7 @@ class SiteStateListenerScriptHandler: TabContentScript {
// (i.e. we don't hide first party content on standard mode)
let setup = UserScriptType.SelectorsPollerSetup(
frameURL: frameURL,
hideFirstPartyContent: false,
hideFirstPartyContent: isAggressive,
genericHide: modelTuples.contains { $0.model.genericHide },
firstSelectorsPollingDelayMs: nil,
switchToSelectorsPollingThreshold: 1000,
Expand Down
50 changes: 47 additions & 3 deletions Sources/Brave/Migration/Migration.swift
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
import Foundation
import Shared
import Preferences
import BraveShields
import SwiftKeychainWrapper
import Data
import BraveCore
Expand All @@ -22,8 +23,8 @@ public class Migration {

public func launchMigrations(keyPrefix: String, profile: Profile) {
Preferences.migratePreferences(keyPrefix: keyPrefix)

Preferences.migrateWalletPreferences()
Preferences.migrateAdAndTrackingProtection()

if !Preferences.Migration.documentsDirectoryCleanupCompleted.value {
documentsDirectoryCleanup()
Expand Down Expand Up @@ -182,6 +183,10 @@ public class Migration {
}

fileprivate extension Preferences {
private final class DeprecatedPreferences {
static let blockAdsAndTracking = Option<Bool>(key: "shields.block-ads-and-tracking", default: true)
}

/// Migration preferences
final class Migration {
static let completed = Option<Bool>(key: "migration.completed", default: false)
Expand All @@ -202,6 +207,12 @@ fileprivate extension Preferences {
Option<Bool>(key: "migration.wallet-provider-account-request-completed", default: false)

static let tabMigrationToInteractionStateCompleted = Option<Bool>(key: "migration.tab-to-interaction-state", default: false)

/// A more complicated ad blocking and tracking protection preference in `1.52.x`
/// allows a user to select between `standard`, `aggressive` and `disabled` instead of a simple on/off `Bool`
static let adBlockAndTrackingProtectionShieldLevelCompleted = Option<Bool>(
key: "migration.ad-block-and-tracking-protection-shield-level-completed", default: false
)
}

/// Migrate a given key from `Prefs` into a specific option
Expand Down Expand Up @@ -282,7 +293,7 @@ fileprivate extension Preferences {
}

// Shields
migrate(key: "braveBlockAdsAndTracking", to: Preferences.Shields.blockAdsAndTracking)
migrate(key: "braveBlockAdsAndTracking", to: DeprecatedPreferences.blockAdsAndTracking)
migrate(key: "braveHttpsEverywhere", to: Preferences.Shields.httpsEverywhere)
migrate(key: "noscript_on", to: Preferences.Shields.blockScripts)
migrate(key: "fingerprintprotection_on", to: Preferences.Shields.fingerprintingProtection)
Expand All @@ -309,10 +320,23 @@ fileprivate extension Preferences {
// On 1.6 lastLaunchInfo is used to check if it's first app launch or not.
// This needs to be translated to our new preference.
Preferences.General.isFirstLaunch.value = Preferences.DAU.lastLaunchInfo.value == nil

Preferences.Migration.completed.value = true
}

class func migrateAdAndTrackingProtection() {
guard !Migration.adBlockAndTrackingProtectionShieldLevelCompleted.value else { return }

// Migrate old tracking protection setting to new BraveShields setting
DeprecatedPreferences.blockAdsAndTracking.migrate() { isEnabled in
if !isEnabled {
// We only need to migrate `disabled`. `standard` is the default.
ShieldPreferences.blockAdsAndTrackingLevel = .disabled
}
}

Migration.adBlockAndTrackingProtectionShieldLevelCompleted.value = true
}

/// Migrate Wallet Preferences from version <1.43
class func migrateWalletPreferences() {
guard Preferences.Migration.walletProviderAccountRequestCompleted.value != true else { return }
Expand All @@ -323,3 +347,23 @@ fileprivate extension Preferences {
Preferences.Migration.walletProviderAccountRequestCompleted.value = true
}
}

private extension Preferences.Option {
/// Migrate this preference to another one using the given transform
///
/// This method will return any stored value (if it is available). If nothing is stored, the callback is not triggered.
/// Any stored value is then removed from the container.
func migrate(onStoredValue: ((ValueType) -> Void)) {
// Have to do two checks because T may be an Optional, since object(forKey:) returns Any? it will succeed
// as casting to T if T is Optional even if the key doesnt exist.
if let value = container.object(forKey: key) {
if let value = value as? ValueType {
onStoredValue(value)
}

container.removeObject(forKey: key)
} else {
Logger.module.info("Could not migrate legacy pref with key: \"\(self.key)\".")
}
}
}
2 changes: 1 addition & 1 deletion Sources/BraveShields/BraveShield.swift
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ public enum BraveShield {
case .AllOff:
return false
case .AdblockAndTp:
return Preferences.Shields.blockAdsAndTracking.value
return ShieldPreferences.blockAdsAndTrackingLevel.isEnabled
case .FpProtection:
return Preferences.Shields.fingerprintingProtection.value
case .NoScript:
Expand Down
Loading

0 comments on commit 1f97760

Please sign in to comment.