diff --git a/CHANGELOG.md b/CHANGELOG.md index 63fa50ad56..917473258e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3,6 +3,7 @@ - [FEATURE] Add network instrumentation for async/await URLSession APIs. See [#1394][] - [FEATURE] Change default tracing headers for first party hosts to use both Datadog headers and W3C `tracecontext` headers. See [#1529][] - [FEATURE] Add tracestate headers when using W3C tracecontext. See [#1536][] +- [BUGFIX] Fix RUM ViewController leaks. See [#1533][] # 2.4.0 / 18-10-2023 @@ -551,6 +552,7 @@ Release `2.0` introduces breaking changes. Follow the [Migration Guide](MIGRATIO [#1394]: https://github.com/DataDog/dd-sdk-ios/pull/1394 [#1524]: https://github.com/DataDog/dd-sdk-ios/pull/1524 [#1529]: https://github.com/DataDog/dd-sdk-ios/pull/1529 +[#1533]: https://github.com/DataDog/dd-sdk-ios/pull/1533 [#1536]: https://github.com/DataDog/dd-sdk-ios/pull/1536 [@00fa9a]: https://github.com/00FA9A [@britton-earnin]: https://github.com/Britton-Earnin diff --git a/DatadogCore/Tests/Datadog/Mocks/RUMFeatureMocks.swift b/DatadogCore/Tests/Datadog/Mocks/RUMFeatureMocks.swift index 841d2a4ecc..60d07507ce 100644 --- a/DatadogCore/Tests/Datadog/Mocks/RUMFeatureMocks.swift +++ b/DatadogCore/Tests/Datadog/Mocks/RUMFeatureMocks.swift @@ -168,7 +168,7 @@ extension RUMStartViewCommand: AnyMockable, RandomMockable { return .mockWith( time: .mockRandomInThePast(), attributes: mockRandomAttributes(), - identity: String.mockRandom(), + identity: String.mockRandom().asRUMViewIdentity(), name: .mockRandom(), path: .mockRandom() ) @@ -177,7 +177,7 @@ extension RUMStartViewCommand: AnyMockable, RandomMockable { static func mockWith( time: Date = Date(), attributes: [AttributeKey: AttributeValue] = [:], - identity: RUMViewIdentifiable = mockView, + identity: RUMViewIdentity = mockViewIdentity, name: String = .mockAny(), path: String? = nil ) -> RUMStartViewCommand { @@ -198,14 +198,14 @@ extension RUMStopViewCommand: AnyMockable, RandomMockable { return .mockWith( time: .mockRandomInThePast(), attributes: mockRandomAttributes(), - identity: String.mockRandom() + identity: String.mockRandom().asRUMViewIdentity() ) } static func mockWith( time: Date = Date(), attributes: [AttributeKey: AttributeValue] = [:], - identity: RUMViewIdentifiable = mockView + identity: RUMViewIdentity = mockViewIdentity ) -> RUMStopViewCommand { return RUMStopViewCommand( time: time, attributes: attributes, identity: identity @@ -786,6 +786,7 @@ func createMockView(viewControllerClassName: String) -> UIViewController { ///// Holds the `mockView` object so it can be weakly referenced by `RUMViewScope` mocks. let mockView: UIViewController = createMockViewInWindow() +let mockViewIdentity = mockView.asRUMViewIdentity() extension RUMViewScope { static func mockAny() -> RUMViewScope { @@ -802,7 +803,7 @@ extension RUMViewScope { isInitialView: Bool = false, parent: RUMContextProvider = RUMContextProviderMock(), dependencies: RUMScopeDependencies = .mockAny(), - identity: RUMViewIdentifiable = mockView, + identity: RUMViewIdentity = mockViewIdentity, path: String = .mockAny(), name: String = .mockAny(), attributes: [AttributeKey: AttributeValue] = [:], diff --git a/DatadogCore/Tests/Datadog/RUM/RUMDebuggingTests.swift b/DatadogCore/Tests/Datadog/RUM/RUMDebuggingTests.swift index 555f41b0c6..b3ca57449d 100644 --- a/DatadogCore/Tests/Datadog/RUM/RUMDebuggingTests.swift +++ b/DatadogCore/Tests/Datadog/RUM/RUMDebuggingTests.swift @@ -22,7 +22,7 @@ class RUMDebuggingTests: XCTestCase { dependencies: .mockWith(rumApplicationID: "rum-123") ) _ = applicationScope.process( - command: RUMStartViewCommand.mockWith(identity: mockView, name: "FirstView"), + command: RUMStartViewCommand.mockWith(identity: mockViewIdentity, name: "FirstView"), context: .mockAny(), writer: FileWriterMock() ) @@ -56,7 +56,7 @@ class RUMDebuggingTests: XCTestCase { dependencies: .mockWith(rumApplicationID: "rum-123") ) _ = applicationScope.process( - command: RUMStartViewCommand.mockWith(identity: mockView, name: "FirstView"), + command: RUMStartViewCommand.mockWith(identity: mockViewIdentity, name: "FirstView"), context: context, writer: writer ) @@ -66,7 +66,7 @@ class RUMDebuggingTests: XCTestCase { writer: writer ) _ = applicationScope.process( - command: RUMStartViewCommand.mockWith(identity: mockView, name: "SecondView"), + command: RUMStartViewCommand.mockWith(identity: mockViewIdentity, name: "SecondView"), context: context, writer: writer ) diff --git a/DatadogRUM/Sources/Instrumentation/Views/RUMViewsHandler.swift b/DatadogRUM/Sources/Instrumentation/Views/RUMViewsHandler.swift index f8cb24c1c2..9dc5b80807 100644 --- a/DatadogRUM/Sources/Instrumentation/Views/RUMViewsHandler.swift +++ b/DatadogRUM/Sources/Instrumentation/Views/RUMViewsHandler.swift @@ -151,14 +151,14 @@ internal final class RUMViewsHandler { return } - guard let identity = view.identity.identifiable else { + guard view.identity.exists else { return } subscriber.process( command: RUMStartViewCommand( time: dateProvider.now, - identity: identity, + identity: view.identity, name: view.name, path: view.path, attributes: view.attributes @@ -167,7 +167,7 @@ internal final class RUMViewsHandler { } private func stop(view: View) { - guard let identity = view.identity.identifiable else { + guard view.identity.exists else { return } @@ -179,7 +179,7 @@ internal final class RUMViewsHandler { command: RUMStopViewCommand( time: dateProvider.now, attributes: [:], - identity: identity + identity: view.identity ) ) } @@ -201,14 +201,15 @@ internal final class RUMViewsHandler { extension RUMViewsHandler: UIViewControllerHandler { func notify_viewDidAppear(viewController: UIViewController, animated: Bool) { - if let view = stack.first(where: { $0.identity.equals(viewController) }) { + let identity = viewController.asRUMViewIdentity() + if let view = stack.first(where: { $0.identity.equals(identity) }) { // If the stack already contains the view controller, just restarts the view. // This prevents from calling the predicate when unnecessary. add(view: view) } else if let rumView = predicate?.rumView(for: viewController) { add( view: .init( - identity: viewController.asRUMViewIdentity(), + identity: identity, name: rumView.name, path: rumView.path, isUntrackedModal: rumView.isUntrackedModal, @@ -218,7 +219,7 @@ extension RUMViewsHandler: UIViewControllerHandler { } else if #available(iOS 13, tvOS 13, *), viewController.isModalInPresentation { add( view: .init( - identity: viewController.asRUMViewIdentity(), + identity: identity, name: "RUMUntrackedModal", path: nil, isUntrackedModal: true, diff --git a/DatadogRUM/Sources/RUMMonitor/Monitor.swift b/DatadogRUM/Sources/RUMMonitor/Monitor.swift index 8ff7c85a41..07730ee41a 100644 --- a/DatadogRUM/Sources/RUMMonitor/Monitor.swift +++ b/DatadogRUM/Sources/RUMMonitor/Monitor.swift @@ -221,7 +221,7 @@ extension Monitor: RUMMonitorProtocol { process( command: RUMStartViewCommand( time: dateProvider.now, - identity: viewController, + identity: viewController.asRUMViewIdentity(), name: name, path: nil, attributes: attributes @@ -234,7 +234,7 @@ extension Monitor: RUMMonitorProtocol { command: RUMStopViewCommand( time: dateProvider.now, attributes: attributes, - identity: viewController + identity: viewController.asRUMViewIdentity() ) ) } @@ -243,7 +243,7 @@ extension Monitor: RUMMonitorProtocol { process( command: RUMStartViewCommand( time: dateProvider.now, - identity: key, + identity: key.asRUMViewIdentity(), name: name ?? key, path: key, attributes: attributes @@ -256,7 +256,7 @@ extension Monitor: RUMMonitorProtocol { command: RUMStopViewCommand( time: dateProvider.now, attributes: attributes, - identity: key + identity: key.asRUMViewIdentity() ) ) } diff --git a/DatadogRUM/Sources/RUMMonitor/RUMCommand.swift b/DatadogRUM/Sources/RUMMonitor/RUMCommand.swift index 4c9475952f..c0b25c1365 100644 --- a/DatadogRUM/Sources/RUMMonitor/RUMCommand.swift +++ b/DatadogRUM/Sources/RUMMonitor/RUMCommand.swift @@ -54,7 +54,7 @@ internal struct RUMStartViewCommand: RUMCommand, RUMViewScopePropagatableAttribu let isUserInteraction = true // a new View means there was a navigation, it's considered a User interaction /// The value holding stable identity of the RUM View. - let identity: RUMViewIdentifiable + let identity: RUMViewIdentity /// The name of this View, rendered in RUM Explorer as `VIEW NAME`. let name: String @@ -64,7 +64,7 @@ internal struct RUMStartViewCommand: RUMCommand, RUMViewScopePropagatableAttribu init( time: Date, - identity: RUMViewIdentifiable, + identity: RUMViewIdentity, name: String?, path: String?, attributes: [AttributeKey: AttributeValue] @@ -84,7 +84,7 @@ internal struct RUMStopViewCommand: RUMCommand, RUMViewScopePropagatableAttribut let isUserInteraction = false // a view can be stopped and in most cases should not be considered an interaction (if it's stopped because the user navigate inside the same app, the startView will happen shortly after this) /// The value holding stable identity of the RUM View. - let identity: RUMViewIdentifiable + let identity: RUMViewIdentity } internal struct RUMAddCurrentViewErrorCommand: RUMCommand { diff --git a/DatadogRUM/Sources/RUMMonitor/Scopes/RUMSessionScope.swift b/DatadogRUM/Sources/RUMMonitor/Scopes/RUMSessionScope.swift index f6d55697d2..40c3567342 100644 --- a/DatadogRUM/Sources/RUMMonitor/Scopes/RUMSessionScope.swift +++ b/DatadogRUM/Sources/RUMMonitor/Scopes/RUMSessionScope.swift @@ -83,14 +83,13 @@ internal class RUMSessionScope: RUMScope, RUMContextProvider { didStartWithReplay: hasReplay ) - if let viewScope = resumingViewScope, - let viewIdentifiable = viewScope.identity.identifiable { + if let viewScope = resumingViewScope, viewScope.identity.exists { viewScopes.append( RUMViewScope( isInitialView: false, parent: self, dependencies: dependencies, - identity: viewIdentifiable, + identity: viewScope.identity, path: viewScope.viewPath, name: viewScope.viewName, attributes: viewScope.attributes, @@ -121,14 +120,14 @@ internal class RUMSessionScope: RUMScope, RUMContextProvider { // Transfer active Views by creating new `RUMViewScopes` for their identity objects: self.viewScopes = expiredSession.viewScopes.compactMap { expiredView in - guard let expiredViewIdentifiable = expiredView.identity.identifiable else { + guard expiredView.identity.exists else { return nil // if the underlying identifiable (`UIVIewController`) no longer exists, skip transferring its scope } return RUMViewScope( isInitialView: false, parent: self, dependencies: dependencies, - identity: expiredViewIdentifiable, + identity: expiredView.identity, path: expiredView.viewPath, name: expiredView.viewName, attributes: expiredView.attributes, @@ -233,7 +232,7 @@ internal class RUMSessionScope: RUMScope, RUMContextProvider { isInitialView: true, parent: self, dependencies: dependencies, - identity: RUMOffViewEventsHandlingRule.Constants.applicationLaunchViewURL, + identity: RUMOffViewEventsHandlingRule.Constants.applicationLaunchViewURL.asRUMViewIdentity(), path: RUMOffViewEventsHandlingRule.Constants.applicationLaunchViewURL, name: RUMOffViewEventsHandlingRule.Constants.applicationLaunchViewName, attributes: command.attributes, @@ -278,7 +277,7 @@ internal class RUMSessionScope: RUMScope, RUMContextProvider { isInitialView: isStartingInitialView, parent: self, dependencies: dependencies, - identity: RUMOffViewEventsHandlingRule.Constants.backgroundViewURL, + identity: RUMOffViewEventsHandlingRule.Constants.backgroundViewURL.asRUMViewIdentity(), path: RUMOffViewEventsHandlingRule.Constants.backgroundViewURL, name: RUMOffViewEventsHandlingRule.Constants.backgroundViewName, attributes: command.attributes, diff --git a/DatadogRUM/Sources/RUMMonitor/Scopes/RUMViewScope.swift b/DatadogRUM/Sources/RUMMonitor/Scopes/RUMViewScope.swift index 9cf49bdc7e..09eafc7926 100644 --- a/DatadogRUM/Sources/RUMMonitor/Scopes/RUMViewScope.swift +++ b/DatadogRUM/Sources/RUMMonitor/Scopes/RUMViewScope.swift @@ -96,7 +96,7 @@ internal class RUMViewScope: RUMScope, RUMContextProvider { isInitialView: Bool, parent: RUMContextProvider, dependencies: RUMScopeDependencies, - identity: RUMViewIdentifiable, + identity: RUMViewIdentity, path: String, name: String, attributes: [AttributeKey: AttributeValue], @@ -107,7 +107,7 @@ internal class RUMViewScope: RUMScope, RUMContextProvider { self.parent = parent self.dependencies = dependencies self.isInitialView = isInitialView - self.identity = identity.asRUMViewIdentity() + self.identity = identity self.attributes = attributes self.customTimings = customTimings self.viewUUID = dependencies.rumUUIDGenerator.generateUnique() diff --git a/DatadogRUM/Sources/RUMMonitor/Scopes/Utils/RUMViewIdentity.swift b/DatadogRUM/Sources/RUMMonitor/Scopes/Utils/RUMViewIdentity.swift index da756501cb..6a169030fc 100644 --- a/DatadogRUM/Sources/RUMMonitor/Scopes/Utils/RUMViewIdentity.swift +++ b/DatadogRUM/Sources/RUMMonitor/Scopes/Utils/RUMViewIdentity.swift @@ -10,7 +10,7 @@ import UIKit /// Based on the `equals(_:)` implementation, it decides if two `RUMViewIdentifiables` identify the same /// RUM View or not. Each implementation of the `RUMViewIdentifiable` decides by its own if it should use /// reference or value semantic for the comparison. -internal protocol RUMViewIdentifiable { +fileprivate protocol RUMViewIdentifiable { /// Compares the instance of this identifiable with another `RUMViewIdentifiable`. /// It returns `true` if both identify the same RUM View and `false` otherwise. func equals(_ otherIdentifiable: RUMViewIdentifiable?) -> Bool @@ -27,7 +27,7 @@ internal protocol RUMViewIdentifiable { /// Extends `UIViewController` with the ability to identify the RUM View. extension UIViewController: RUMViewIdentifiable { - func equals(_ otherIdentifiable: RUMViewIdentifiable?) -> Bool { + fileprivate func equals(_ otherIdentifiable: RUMViewIdentifiable?) -> Bool { if let otherViewController = otherIdentifiable as? UIViewController { // Two `UIViewController` identifiables indicate the same RUM View only if their references are equal. return self === otherViewController @@ -47,7 +47,7 @@ extension UIViewController: RUMViewIdentifiable { /// Extends `String` with the ability to identify the RUM View. extension String: RUMViewIdentifiable { - func equals(_ otherIdentifiable: RUMViewIdentifiable?) -> Bool { + fileprivate func equals(_ otherIdentifiable: RUMViewIdentifiable?) -> Bool { if let otherString = otherIdentifiable as? String { // Two `String` identifiables indicate the same RUM View only if their values are equal. return self == otherString @@ -85,14 +85,8 @@ internal struct RUMViewIdentity { } /// Returns `true` if a given identifiable indicates the same RUM View as the identifiable managed internally. - func equals(_ identifiable: RUMViewIdentifiable?) -> Bool { - if let selfObject = object as? RUMViewIdentifiable { - return selfObject.equals(identifiable) - } else if let selfValue = value as? RUMViewIdentifiable { - return selfValue.equals(identifiable) - } else { - return false - } + fileprivate func equals(_ identifiable: RUMViewIdentifiable?) -> Bool { + return self.identifiable?.equals(identifiable) ?? false } /// Returns `true` if a given identity indicates the same RUM View as the identifiable managed internally. @@ -100,8 +94,20 @@ internal struct RUMViewIdentity { return equals(identity?.identifiable) } + /// Return default path name for the managed identifiable. + var defaultViewPath: String { + return identifiable?.defaultViewPath ?? "" + } + + /// Returns `true` if the managed identifiable is still available. + /// Underlying `identfiable` is stored as a weak reference, so it may become `nil` at any time. + /// For example when the `UIViewController` is deallocated. + var exists: Bool { + return identifiable != nil + } + /// Returns the managed identifiable. - var identifiable: RUMViewIdentifiable? { + private var identifiable: RUMViewIdentifiable? { return (object as? RUMViewIdentifiable) ?? (value as? RUMViewIdentifiable) } } diff --git a/DatadogRUM/Tests/Mocks/RUMFeatureMocks.swift b/DatadogRUM/Tests/Mocks/RUMFeatureMocks.swift index 72a8ebacce..2f6887809c 100644 --- a/DatadogRUM/Tests/Mocks/RUMFeatureMocks.swift +++ b/DatadogRUM/Tests/Mocks/RUMFeatureMocks.swift @@ -165,7 +165,7 @@ extension RUMStartViewCommand: AnyMockable, RandomMockable { return .mockWith( time: .mockRandomInThePast(), attributes: mockRandomAttributes(), - identity: String.mockRandom(), + identity: String.mockRandom().asRUMViewIdentity(), name: .mockRandom(), path: .mockRandom() ) @@ -174,7 +174,7 @@ extension RUMStartViewCommand: AnyMockable, RandomMockable { static func mockWith( time: Date = Date(), attributes: [AttributeKey: AttributeValue] = [:], - identity: RUMViewIdentifiable = mockView, + identity: RUMViewIdentity = mockViewIdentity, name: String = .mockAny(), path: String? = nil ) -> RUMStartViewCommand { @@ -195,14 +195,14 @@ extension RUMStopViewCommand: AnyMockable, RandomMockable { return .mockWith( time: .mockRandomInThePast(), attributes: mockRandomAttributes(), - identity: String.mockRandom() + identity: String.mockRandom().asRUMViewIdentity() ) } static func mockWith( time: Date = Date(), attributes: [AttributeKey: AttributeValue] = [:], - identity: RUMViewIdentifiable = mockView + identity: RUMViewIdentity = mockViewIdentity ) -> RUMStopViewCommand { return RUMStopViewCommand( time: time, attributes: attributes, identity: identity @@ -766,6 +766,7 @@ func createMockView(viewControllerClassName: String) -> UIViewController { ///// Holds the `mockView` object so it can be weakly referenced by `RUMViewScope` mocks. let mockView: UIViewController = createMockViewInWindow() +let mockViewIdentity: RUMViewIdentity = mockView.asRUMViewIdentity() extension RUMViewScope { static func mockAny() -> RUMViewScope { @@ -782,7 +783,7 @@ extension RUMViewScope { isInitialView: Bool = false, parent: RUMContextProvider = RUMContextProviderMock(), dependencies: RUMScopeDependencies = .mockAny(), - identity: RUMViewIdentifiable = mockView, + identity: RUMViewIdentity = mockViewIdentity, path: String = .mockAny(), name: String = .mockAny(), attributes: [AttributeKey: AttributeValue] = [:], diff --git a/DatadogRUM/Tests/RUMMonitor/Scopes/RUMApplicationScopeTests.swift b/DatadogRUM/Tests/RUMMonitor/Scopes/RUMApplicationScopeTests.swift index 21fdcc87fb..9d9df1f16a 100644 --- a/DatadogRUM/Tests/RUMMonitor/Scopes/RUMApplicationScopeTests.swift +++ b/DatadogRUM/Tests/RUMMonitor/Scopes/RUMApplicationScopeTests.swift @@ -75,7 +75,7 @@ class RUMApplicationScopeTests: XCTestCase { let view = createMockViewInWindow() _ = scope.process( - command: RUMStartViewCommand.mockWith(time: currentTime, identity: view), + command: RUMStartViewCommand.mockWith(time: currentTime, identity: view.asRUMViewIdentity()), context: context, writer: writer ) @@ -116,12 +116,12 @@ class RUMApplicationScopeTests: XCTestCase { ) _ = scope.process( - command: RUMStartViewCommand.mockWith(time: currentTime, identity: mockView), + command: RUMStartViewCommand.mockWith(time: currentTime, identity: mockViewIdentity), context: context, writer: writer ) _ = scope.process( - command: RUMStopViewCommand.mockWith(time: currentTime, identity: mockView), + command: RUMStopViewCommand.mockWith(time: currentTime, identity: mockViewIdentity), context: context, writer: writer ) @@ -139,12 +139,12 @@ class RUMApplicationScopeTests: XCTestCase { ) _ = scope.process( - command: RUMStartViewCommand.mockWith(time: currentTime, identity: mockView), + command: RUMStartViewCommand.mockWith(time: currentTime, identity: mockViewIdentity), context: context, writer: writer ) _ = scope.process( - command: RUMStartViewCommand.mockWith(time: currentTime, identity: mockView), + command: RUMStartViewCommand.mockWith(time: currentTime, identity: mockViewIdentity), context: context, writer: writer ) @@ -163,12 +163,12 @@ class RUMApplicationScopeTests: XCTestCase { let simulatedSessionsCount = 400 (0.. Bool { + return equals(vc?.asRUMViewIdentity()) + } + + func equals(_ string: String?) -> Bool { + return equals(string?.asRUMViewIdentity()) } }