Fix #6591, Fix #6592: Shields P3A question fixes

Client/Frontend/Browser/BrowserViewController.swift

@@ -576,6 +576,8 @@ public class BrowserViewController: UIViewController {
     // Eliminate the older usage days
     // Used in App Rating criteria
     AppReviewManager.shared.processMainCriteria(for: .daysInUse)
+
+    maybeRecordInitialShieldsP3A()
   }
 
   private func setupAdsNotificationHandler() {

Client/Frontend/Browser/BrowserViewController/BrowserViewController+P3A.swift

@@ -0,0 +1,48 @@
+// Copyright 2022 The Brave Authors. All rights reserved.
+// This Source Code Form is subject to the terms of the Mozilla Public
+// License, v. 2.0. If a copy of the MPL was not distributed with this
+// file, You can obtain one at http://mozilla.org/MPL/2.0/.
+
+import Foundation
+import BraveShared
+import Growth
+import Data
+
+extension BrowserViewController {
+
+  func maybeRecordInitialShieldsP3A() {
+    if Preferences.Shields.initialP3AStateReported.value { return }
+    defer { Preferences.Shields.initialP3AStateReported.value = true }
+    recordShieldsUpdateP3A(shield: .AdblockAndTp)
+    recordShieldsUpdateP3A(shield: .FpProtection)
+  }
+
+  func recordShieldsUpdateP3A(shield: BraveShield) {
+    let buckets: [Bucket] = [
+      0,
+      .r(1...5),
+      .r(6...10),
+      .r(11...20),
+      .r(21...30),
+      .r(31...),
+    ]
+    switch shield {
+    case .AdblockAndTp:
+      // Q51 On how many domains has the user set the adblock setting to be lower (block less) than the default?
+      let adsBelowGlobalCount = Domain.totalDomainsWithAdblockShieldsLoweredFromGlobal()
+      UmaHistogramRecordValueToBucket("Brave.Shields.DomainAdsSettingsBelowGlobal", buckets: buckets, value: adsBelowGlobalCount)
+      // Q52 On how many domains has the user set the adblock setting to be higher (block more) than the default?
+      let adsAboveGlobalCount = Domain.totalDomainsWithAdblockShieldsIncreasedFromGlobal()
+      UmaHistogramRecordValueToBucket("Brave.Shields.DomainAdsSettingsAboveGlobal", buckets: buckets, value: adsAboveGlobalCount)
+    case .FpProtection:
+      // Q53 On how many domains has the user set the FP setting to be lower (block less) than the default?
+      let fingerprintingBelowGlobalCount = Domain.totalDomainsWithFingerprintingProtectionLoweredFromGlobal()
+      UmaHistogramRecordValueToBucket("Brave.Shields.DomainFingerprintSettingsBelowGlobal", buckets: buckets, value: fingerprintingBelowGlobalCount)
+      // Q54 On how many domains has the user set the FP setting to be higher (block more) than the default?
+      let fingerprintingAboveGlobalCount = Domain.totalDomainsWithFingerprintingProtectionIncreasedFromGlobal()
+      UmaHistogramRecordValueToBucket("Brave.Shields.DomainFingerprintSettingsAboveGlobal", buckets: buckets, value: fingerprintingAboveGlobalCount)
+    case .AllOff, .NoScript, .SafeBrowsing:
+      break
+    }
+  }
+}

Client/Frontend/Browser/BrowserViewController/BrowserViewController+ToolbarDelegate.swift

@@ -292,13 +292,19 @@ extension BrowserViewController: TopToolbarDelegate {
     }
 
     let shields = ShieldsViewController(tab: selectedTab)
-    shields.shieldsSettingsChanged = { [unowned self] _ in
+    shields.shieldsSettingsChanged = { [unowned self] _, shield in
       // Update the shields status immediately
       self.topToolbar.refreshShieldsStatus()
 
       // Reload this tab. This will also trigger an update of the brave icon in `TabLocationView` if
       // the setting changed is the global `.AllOff` shield
       self.tabManager.selectedTab?.reload()
+
+      // Record P3A shield changes
+      DispatchQueue.main.asyncAfter(deadline: .now() + 1) {
+        // Record shields & FP related hisotgrams, wait a sec for CoreData to sync contexts
+        self.recordShieldsUpdateP3A(shield: shield)
+      }
 
       // In 1.6 we "reload" the whole web view state, dumping caches, etc. (reload():BraveWebView.swift:495)
       // BRAVE TODO: Port over proper tab reloading with Shields

Client/Frontend/Shields/ShieldsViewController.swift

@@ -27,7 +27,7 @@ class ShieldsViewController: UIViewController, PopoverContentComponent {
     return _url
   }()
 
-  var shieldsSettingsChanged: ((ShieldsViewController) -> Void)?
+  var shieldsSettingsChanged: ((ShieldsViewController, BraveShield) -> Void)?
   var showGlobalShieldsSettings: ((ShieldsViewController) -> Void)?
 
   private var statsUpdateObservable: AnyObject?
@@ -105,10 +105,6 @@ class ShieldsViewController: UIViewController, PopoverContentComponent {
     Domain.setBraveShield(
       forUrl: url, shield: shield, isOn: isOn,
       isPrivateBrowsing: PrivateBrowsingManager.shared.isPrivateBrowsing)
-    DispatchQueue.main.asyncAfter(deadline: .now() + 1) {
-      // Record shields & FP related hisotgrams, wait a sec for CoreData to sync contexts
-      self.recordShieldsUpdateP3A(shield: shield)
-    }
   }
 
   private func updateGlobalShieldState(_ on: Bool, animated: Bool = false) {
@@ -273,7 +269,7 @@ class ShieldsViewController: UIViewController, PopoverContentComponent {
         // 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) {
-          self.shieldsSettingsChanged?(self)
+          self.shieldsSettingsChanged?(self, shield)
         }
       }
     }
@@ -286,7 +282,7 @@ class ShieldsViewController: UIViewController, PopoverContentComponent {
     // 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) {
-      self.shieldsSettingsChanged?(self)
+      self.shieldsSettingsChanged?(self, .AllOff)
     }
   }
 
@@ -344,40 +340,4 @@ class ShieldsViewController: UIViewController, PopoverContentComponent {
   required init?(coder aDecoder: NSCoder) {
     fatalError()
   }
-
-  // MARK: - P3A
-
-  private enum P3AShieldUpdateKind {
-    case adblock
-    case fingerprintingProtection
-  }
-
-  private func recordShieldsUpdateP3A(shield: BraveShield) {
-    let buckets: [Bucket] = [
-      0,
-      .r(1...5),
-      .r(6...10),
-      .r(11...20),
-      .r(21...30),
-      .r(31...),
-    ]
-    switch shield {
-    case .AdblockAndTp:
-      // Q51 On how many domains has the user set the adblock setting to be lower (block less) than the default?
-      let adsBelowGlobalCount = Domain.totalDomainsWithAdblockShieldsLoweredFromGlobal()
-      UmaHistogramRecordValueToBucket("Brave.Shields.DomainAdsSettingsBelowGlobal", buckets: buckets, value: adsBelowGlobalCount)
-      // Q52 On how many domains has the user set the adblock setting to be higher (block more) than the default?
-      let adsAboveGlobalCount = Domain.totalDomainsWithAdblockShieldsIncreasedFromGlobal()
-      UmaHistogramRecordValueToBucket("Brave.Shields.DomainAdsSettingsAboveGlobal", buckets: buckets, value: adsBelowGlobalCount)
-    case .FpProtection:
-      // Q53 On how many domains has the user set the FP setting to be lower (block less) than the default?
-      let fingerprintingBelowGlobalCount = Domain.totalDomainsWithFingerprintingProtectionLoweredFromGlobal()
-      UmaHistogramRecordValueToBucket("Brave.Shields.DomainFingerprintSettingsBelowGlobal", buckets: buckets, value: fingerprintingBelowGlobalCount)
-      // Q54 On how many domains has the user set the FP setting to be higher (block more) than the default?
-      let fingerprintingAboveGlobalCount = Domain.totalDomainsWithFingerprintingProtectionIncreasedFromGlobal()
-      UmaHistogramRecordValueToBucket("Brave.Shields.DomainFingerprintSettingsAboveGlobal", buckets: buckets, value: fingerprintingAboveGlobalCount)
-    case .AllOff, .NoScript, .SafeBrowsing:
-      break
-    }
-  }
 }

Sources/BraveShared/Preferences.swift

@@ -87,6 +87,8 @@ extension Preferences {
     public static let adblockStatsDataVersion = Option<Int?>(key: "stats.adblock-data-version", default: nil)
     /// Whether or not advanced controls in the shields UI are visible by default
     public static let advancedControlsVisible = Option<Bool>(key: "shields.advanced-controls-visible", default: false)
+    /// Whether or not we've reported the initial state of shields for p3a
+    public static let initialP3AStateReported = Option<Bool>(key: "shields.initial-p3a-state-reported", default: false)
   }
 
   public final class Rewards {