fix(breadcrumbs): Unsubscribe system event breadcrumbs during background

CHANGELOG.md

@@ -5,6 +5,7 @@
 ### Fixes
 
 - Detect development builds via provisioning profile and debugger attachment (#7702)
+- Unsubscribe system event breadcrumbs during background (#7702)
 
 ## 9.10.0
 

SentryTestUtils/Sources/TestNSNotificationCenterWrapper.swift

@@ -82,10 +82,14 @@ public typealias CrossPlatformApplication = NSApplication
             observers.removeAll { item in
                 switch item {
                 case .observerWithObject(let weakObserver, _, let name, let object):
-                    return (weakObserver.value === observer as? NSObject) || 
-                           (name == aName && ((object == nil && anObject == nil) || (object as AnyObject === anObject as AnyObject)))
+                    guard weakObserver.value === observer as AnyObject else { return false }
+                    if let aName = aName, name != aName { return false }
+                    if anObject != nil && object as AnyObject? !== anObject as AnyObject? { return false }
+                    return true
                 case .observerWithBlock(let weakObserver, let name, _):
-                    return (weakObserver.value === observer as? NSObject) || name == aName
+                    guard weakObserver.value === observer as AnyObject else { return false }
+                    if let aName = aName, name != aName { return false }
+                    return true
                 default:
                     return false
                 }

Sources/Swift/Integrations/Breadcrumbs/SentrySystemEventBreadcrumbs.swift

@@ -17,6 +17,9 @@ final class SentrySystemEventBreadcrumbs: NSObject {
     private var didBeginGeneratingOrientationNotifications = false
     #endif
 
+    /// Whether system event observers are currently registered.
+    private var isSubscribedToSystemEvents = false
+
     init(
         currentDeviceProvider: SentryUIDeviceWrapperProvider,
         fileManager: SentryFileManager,
@@ -38,6 +41,48 @@ final class SentrySystemEventBreadcrumbs: NSObject {
 
     func start(with delegate: SentryBreadcrumbDelegate) {
         self.delegate = delegate
+
+        notificationCenterWrapper.addObserver(
+            self,
+            selector: #selector(didEnterBackground),
+            name: UIApplication.didEnterBackgroundNotification,
+            object: nil
+        )
+        notificationCenterWrapper.addObserver(
+            self,
+            selector: #selector(willEnterForeground),
+            name: UIApplication.willEnterForegroundNotification,
+            object: nil
+        )
+
+        subscribeToSystemEvents()
+    }
+
+    func timezoneEventTriggered() {
+        timezoneEventTriggered(storedTimezoneOffset: nil)
+    }
+
+    func stop() {
+        notificationCenterWrapper.removeObserver(self, name: UIApplication.didEnterBackgroundNotification, object: nil)
+        notificationCenterWrapper.removeObserver(self, name: UIApplication.willEnterForegroundNotification, object: nil)
+
+        unsubscribeFromSystemEvents()
+    }
+
+    // MARK: - Lifecycle
+
+    @objc private func didEnterBackground() {
+        unsubscribeFromSystemEvents()
+    }
+
+    @objc private func willEnterForeground() {
+        subscribeToSystemEvents()
+    }
+
+    private func subscribeToSystemEvents() {
+        guard !isSubscribedToSystemEvents else { return }
+        isSubscribedToSystemEvents = true
+
         #if os(iOS)
         initBatteryObserver(currentDeviceProvider.uiDeviceWrapper.currentDevice)
         initOrientationObserver(currentDeviceProvider.uiDeviceWrapper.currentDevice)
@@ -48,14 +93,12 @@ final class SentrySystemEventBreadcrumbs: NSObject {
         initSignificantTimeChangeObserver()
     }
 
-    func timezoneEventTriggered() {
-        timezoneEventTriggered(storedTimezoneOffset: nil)
-    }
+    private func unsubscribeFromSystemEvents() {
+        guard isSubscribedToSystemEvents else { return }
+        isSubscribedToSystemEvents = false
 
-    func stop() {
         // Remove the observers with the most specific detail possible, see
         // https://developer.apple.com/documentation/foundation/nsnotificationcenter/1413994-removeobserver
-
         notificationCenterWrapper.removeObserver(self, name: UIApplication.userDidTakeScreenshotNotification, object: nil)
         notificationCenterWrapper.removeObserver(self, name: UIApplication.significantTimeChangeNotification, object: nil)
         notificationCenterWrapper.removeObserver(self, name: NSNotification.Name.NSSystemTimeZoneDidChange, object: nil)

Tests/SentryTests/Integrations/Breadcrumbs/SentrySystemEventBreadcrumbsTest.swift

@@ -373,8 +373,64 @@ class SentrySystemEventBreadcrumbsTest: XCTestCase {
     func testStopCallsSpecificRemoveObserverMethods() {
         sut = fixture.getSut(currentDevice: nil)
         sut.stop()
+        // 8 system event observers + 2 lifecycle observers (didEnterBackground, willEnterForeground)
+        XCTAssertEqual(fixture.notificationCenterWrapper.removeObserverWithNameAndObjectInvocations.count, 10)
+    }
+
+    // MARK: - Lifecycle Tests
+
+    func testBackgroundUnsubscribesFromSystemEvents() {
+        sut = fixture.getSut(currentDevice: nil)
+        fixture.notificationCenterWrapper.removeObserverWithNameAndObjectInvocations.removeAll()
+
+        fixture.notificationCenterWrapper.post(Notification(name: UIApplication.didEnterBackgroundNotification))
+
+        // Should have removed all 8 system event observers
         XCTAssertEqual(fixture.notificationCenterWrapper.removeObserverWithNameAndObjectInvocations.count, 8)
     }
+
+    func testForegroundResubscribesToSystemEvents() {
+        sut = fixture.getSut(currentDevice: nil)
+
+        // Go to background first
+        fixture.notificationCenterWrapper.post(Notification(name: UIApplication.didEnterBackgroundNotification))
+        fixture.notificationCenterWrapper.addObserverWithObjectInvocations.removeAll()
+
+        // Then come back to foreground
+        fixture.notificationCenterWrapper.post(Notification(name: UIApplication.willEnterForegroundNotification))
+
+        // Should have re-registered system event observers (8 on iOS: battery x2, orientation, keyboard x2, screenshot, timezone, significant time)
+        XCTAssertEqual(fixture.notificationCenterWrapper.addObserverWithObjectInvocations.count, 8)
+    }
+
+    func testRepeatedBackgroundDoesNotDoubleUnsubscribe() {
+        sut = fixture.getSut(currentDevice: nil)
+        fixture.notificationCenterWrapper.removeObserverWithNameAndObjectInvocations.removeAll()
+
+        fixture.notificationCenterWrapper.post(Notification(name: UIApplication.didEnterBackgroundNotification))
+        let firstCount = fixture.notificationCenterWrapper.removeObserverWithNameAndObjectInvocations.count
+
+        fixture.notificationCenterWrapper.post(Notification(name: UIApplication.didEnterBackgroundNotification))
+        let secondCount = fixture.notificationCenterWrapper.removeObserverWithNameAndObjectInvocations.count
+
+        XCTAssertEqual(firstCount, secondCount, "Second background notification should be a no-op")
+    }
+
+    func testRepeatedForegroundDoesNotDoubleSubscribe() {
+        sut = fixture.getSut(currentDevice: nil)
+
+        // Go to background and back
+        fixture.notificationCenterWrapper.post(Notification(name: UIApplication.didEnterBackgroundNotification))
+        fixture.notificationCenterWrapper.addObserverWithObjectInvocations.removeAll()
+
+        fixture.notificationCenterWrapper.post(Notification(name: UIApplication.willEnterForegroundNotification))
+        let firstCount = fixture.notificationCenterWrapper.addObserverWithObjectInvocations.count
+
+        fixture.notificationCenterWrapper.post(Notification(name: UIApplication.willEnterForegroundNotification))
+        let secondCount = fixture.notificationCenterWrapper.addObserverWithObjectInvocations.count
+
+        XCTAssertEqual(firstCount, secondCount, "Second foreground notification should be a no-op")
+    }
 
     private func postBatteryLevelNotification(uiDevice: UIDevice?) {
         Dynamic(sut).batteryStateChanged(Notification(name: UIDevice.batteryLevelDidChangeNotification, object: uiDevice))