Merge pull request #12 from bookingcom/gtarasov/fix_async_experiment

Fix a few issues in `observersOnBackgroundQueue` experiment

Author: pilot34

PerformanceSuite/Sources/TTIObserver+Extensions.swift

@@ -29,7 +29,7 @@ public extension UIViewController {
     /// }
     /// ```
     @objc func screenIsReady() {
-        let observer = ViewControllerObserverFactoryHelper.existingObserver(for: self, identifier: TTIObserverHelper.identifier) as? ScreenIsReadyProvider
+        let observer = ViewControllerObserverFactoryHelper.existingObserver(forChild: self, identifier: TTIObserverHelper.identifier) as? ScreenIsReadyProvider
         observer?.screenIsReady()
     }
 

PerformanceSuite/Sources/ViewControllerObserver.swift

@@ -40,14 +40,22 @@ final class ViewControllerObserverFactory<T: ViewControllerObserver, S: ScreenMe
             precondition(Thread.isMainThread)
         }
 
-        if let observer = ViewControllerObserverFactoryHelper.existingObserver(for: viewController, identifier: T.identifier) as? T {
-            return observer
+        if PerformanceMonitoring.experiments.observersOnBackgroundQueue {
+            if let observer = ViewControllerObserverFactoryHelper.existingObserver(for: viewController, identifier: T.identifier) as? T {
+                return observer
+            }
         }
 
         guard let screen = metricsReceiver.screenIdentifier(for: viewController) else {
             return nil
         }
 
+        if !PerformanceMonitoring.experiments.observersOnBackgroundQueue {
+            if let observer = ViewControllerObserverFactoryHelper.existingObserver(for: viewController, identifier: T.identifier) as? T {
+                return observer
+            }
+        }
+
         let tPointer = unsafeBitCast(T.identifier, to: UnsafeRawPointer.self)
         let observer = observerMaker(screen)
         objc_setAssociatedObject(viewController, tPointer, observer, .OBJC_ASSOCIATION_RETAIN_NONATOMIC)
@@ -192,4 +200,16 @@ final class ViewControllerObserverFactoryHelper {
 
         return nil
     }
+
+    static func existingObserver(forChild viewController: UIViewController, identifier: AnyObject) -> Any? {
+        var vc: UIViewController? = viewController
+        while let current = vc {
+            let tPointer = unsafeBitCast(identifier, to: UnsafeRawPointer.self)
+            if let result = objc_getAssociatedObject(current, tPointer) {
+                return result
+            }
+            vc = current.parent
+        }
+        return nil
+    }
 }

PerformanceSuite/Tests/TTIObserverExtensionTests.swift

@@ -21,7 +21,7 @@ class TTIObserverExtensionTests: XCTestCase {
         try super.setUpWithError()
         defaultTimeProvider = timeProvider
         metricsReceiver = TTIMetricsReceiverStub()
-        try PerformanceMonitoring.enable(config: [.screenLevelTTI(metricsReceiver)], experiments: Experiments())
+        try PerformanceMonitoring.enable(config: [.screenLevelTTI(metricsReceiver)], experiments: Experiments(observersOnBackgroundQueue: true))
     }
 
     override func tearDownWithError() throws {
@@ -308,6 +308,44 @@ class TTIObserverExtensionTests: XCTestCase {
         XCTAssertEqual(metricsReceiver.ttiMetrics?.tti, .seconds(2))
         XCTAssertEqual(metricsReceiver.lastController?.title, "vc3")
     }
+
+    func testScreenIsReadyForChildViewController() {
+        let now = DispatchTime(uptimeNanoseconds: 10_000_000)
+        timeProvider.time = now
+
+        let vc1 = MyViewController()
+        vc1.title = "vc1"
+
+        let vc2 = UIViewController()
+        vc2.title = "vc2"
+
+        vc1.addChild(vc2)
+
+        timeProvider.time = now.advanced(by: .seconds(1))
+
+        let exp1 = expectation(description: "viewDidAppear vc1")
+        vc1.viewAppeared = {
+            DispatchQueue.main.async {
+                exp1.fulfill()
+            }
+        }
+
+        let window = makeWindow()
+        window.rootViewController = vc1
+        window.makeKeyAndVisible()
+
+        waitForExpectations(timeout: 3, handler: nil)
+
+        // we call screenIsReady for the child, but it should work for the parent
+        vc2.screenIsReady()
+
+        PerformanceMonitoring.queue.sync {}
+        PerformanceMonitoring.consumerQueue.sync {}
+
+        XCTAssertNotNil(metricsReceiver.ttiMetrics)
+        XCTAssertEqual(metricsReceiver.ttiMetrics?.tti, .seconds(1))
+        XCTAssertEqual(metricsReceiver.lastController?.title, "vc1")
+    }
 }
 
 private class MyHostingController<T: View>: UIHostingController<T> {

PerformanceSuite/Tests/TTIObserverTests.swift

@@ -29,22 +29,22 @@ class TTIObserverTests: XCTestCase {
     }
 
     func testTTIObserverForViewController() throws {
-        let vc1 = UIViewController()
+        let vc1 = MyViewController()
         waitForTheNextRunLoop()
         XCTAssertNil(ViewControllerObserverFactoryHelper.existingObserver(for: vc1, identifier: TTIObserverHelper.identifier))
 
         try PerformanceMonitoring.enable(config: [.screenLevelTTI(TTIMetricsReceiverStub())])
-        let vc2 = UIViewController()
+        let vc2 = MyViewController()
         waitForTheNextRunLoop()
         XCTAssertNotNil(ViewControllerObserverFactoryHelper.existingObserver(for: vc2, identifier: TTIObserverHelper.identifier))
 
         try PerformanceMonitoring.disable()
-        let vc3 = UIViewController()
+        let vc3 = MyViewController()
         waitForTheNextRunLoop()
         XCTAssertNil(ViewControllerObserverFactoryHelper.existingObserver(for: vc3, identifier: TTIObserverHelper.identifier))
 
         try PerformanceMonitoring.enable(config: [])
-        let vc4 = UIViewController()
+        let vc4 = MyViewController()
         waitForTheNextRunLoop()
         XCTAssertNil(ViewControllerObserverFactoryHelper.existingObserver(for: vc4, identifier: TTIObserverHelper.identifier))
         try PerformanceMonitoring.disable()
@@ -539,7 +539,8 @@ class TTIMetricsReceiverStub: TTIMetricsReceiver {
 
     func screenIdentifier(for viewController: UIViewController) -> UIViewController? {
         if viewController is UINavigationController
-            || viewController is UITabBarController {
+            || viewController is UITabBarController
+            || type(of: viewController) == UIViewController.self {
             return nil
         }
         return viewController
@@ -549,3 +550,5 @@ class TTIMetricsReceiverStub: TTIMetricsReceiver {
     var ttiMetrics: TTIMetrics?
     var lastController: UIViewController?
 }
+
+private class MyViewController: UIViewController { }

PerformanceSuite/Tests/ViewControllerObserverTests.swift

@@ -79,7 +79,7 @@ class ViewControllerObserverTests: XCTestCase {
 
         lastObserverCreated = nil
 
-        let vc1 = UIViewController()
+        let vc1 = MyViewController()
         factory.beforeInit(viewController: vc1)
         PerformanceMonitoring.queue.sync { }
 
@@ -100,7 +100,7 @@ class ViewControllerObserverTests: XCTestCase {
         observer?.clear()
 
 
-        let vc2 = UIViewController()
+        let vc2 = MyViewController()
         factory.afterViewDidAppear(viewController: vc2)
         PerformanceMonitoring.queue.sync { }
 
@@ -158,3 +158,5 @@ private class MetricsConsumerForSwiftUITest: ScreenMetricsReceiver {
     func renderingMetricsReceived(metrics: RenderingMetrics, screen: UIViewController) {}
     func appRenderingMetricsReceived(metrics: RenderingMetrics) {}
 }
+
+private class MyViewController: UIViewController { }