RUMM-1278 PR comments addressed

DataDog · Jun 24, 2021 · c7f0310 · c7f0310
1 parent 67f4237
commit c7f0310
Show file tree

Hide file tree

Showing 2 changed files with 40 additions and 78 deletions.
diff --git a/Sources/Datadog/RUM/RUMVitals/VitalRefreshRateReader.swift b/Sources/Datadog/RUM/RUMVitals/VitalRefreshRateReader.swift
@@ -12,7 +12,6 @@ internal class VitalRefreshRateReader {
     private var observers = [VitalObserver]()
     private var displayLink: CADisplayLink?
     private var lastFrameTimestamp: CFTimeInterval?
-    private(set) var isRunning = false
 
     init(notificationCenter: NotificationCenter = .default) {
         notificationCenter.addObserver(
@@ -27,6 +26,8 @@ internal class VitalRefreshRateReader {
             name: UIApplication.didBecomeActiveNotification,
             object: nil
         )
+
+        start()
     }
 
     deinit {
@@ -51,19 +52,6 @@ internal class VitalRefreshRateReader {
         }
     }
 
-    /// Starts listening to frame paints.
-    /// - Throws: only if `UIScreen.main` cannot generate its `CADisplayLink`
-    func start() throws {
-        try private_start()
-        isRunning = true
-    }
-
-    /// Stops listening frame paints. Automatically called at `deinit()`.
-    func stop() {
-        private_stop()
-        isRunning = false
-    }
-
     // MARK: - Private
 
     @objc
@@ -79,34 +67,28 @@ internal class VitalRefreshRateReader {
         lastFrameTimestamp = link.timestamp
     }
 
-    @objc
-    private func appWillResignActive() {
-        private_stop()
-    }
-
-    @objc
-    private func appDidBecomeActive() {
-        if isRunning {
-            try? private_start()
+    private func start() {
+        if displayLink != nil {
+            return
         }
-    }
 
-    private func private_start() throws {
-        stop()
-
-        guard let link = UIScreen.main.displayLink(
-            withTarget: self,
-            selector: #selector(displayTick(link:))
-        ) else {
-            throw InternalError(description: "CADisplayLink could not be created!")
-        }
-        link.add(to: .main, forMode: .default)
-        self.displayLink = link
+        displayLink = CADisplayLink(target: self, selector: #selector(displayTick(link:)))
+        displayLink?.add(to: .main, forMode: .default)
     }
 
-    private func private_stop() {
+    private func stop() {
         displayLink?.invalidate()
         displayLink = nil
         lastFrameTimestamp = nil
     }
+
+    @objc
+    private func appWillResignActive() {
+        stop()
+    }
+
+    @objc
+    private func appDidBecomeActive() {
+        start()
+    }
 }
diff --git a/Tests/DatadogTests/Datadog/RUM/RUMVitals/VitalRefreshRateReaderTests.swift b/Tests/DatadogTests/Datadog/RUM/RUMVitals/VitalRefreshRateReaderTests.swift
@@ -11,94 +11,74 @@ import UIKit
 class VitalRefreshRateReaderTests: XCTestCase {
     private let mockNotificationCenter = NotificationCenter()
 
-    func testRefreshRateReader() throws {
+    func testHighAndLowRefreshRates() {
         let reader = VitalRefreshRateReader(notificationCenter: mockNotificationCenter)
-        XCTAssertFalse(reader.isRunning)
-
         let observer_view1 = VitalObserver(listener: VitalListenerMock())
         let observer_view2 = VitalObserver(listener: VitalListenerMock())
 
-        XCTAssertNoThrow(try reader.start())
-        XCTAssertTrue(reader.isRunning)
-
+        // View1 has simple UI, high FPS expected
         reader.register(observer_view1)
 
+        // Wait without blocking UI thread
         let expectation1 = expectation(description: "async expectation for first observer")
         DispatchQueue.global().async {
-            Thread.sleep(forTimeInterval: 1.0)
+            Thread.sleep(forTimeInterval: 0.5)
             expectation1.fulfill()
         }
 
-        waitForExpectations(timeout: 3.0) { _ in
+        waitForExpectations(timeout: 1.0) { _ in
             XCTAssertGreaterThan(observer_view1.vitalInfo.sampleCount, 0)
             XCTAssertGreaterThan(UIScreen.main.maximumFramesPerSecond, Int(observer_view1.vitalInfo.maxValue))
             XCTAssertGreaterThan(observer_view1.vitalInfo.minValue, 0.0)
         }
 
-        reader.register(observer_view1)
+        reader.unregister(observer_view1)
+        // View2 has complex UI, lower FPS expected
+        reader.register(observer_view2)
 
+        // Wait without blocking UI thread
         let expectation2 = expectation(description: "async expectation for second observer")
         DispatchQueue.global().async {
-            Thread.sleep(forTimeInterval: 1.0)
+            Thread.sleep(forTimeInterval: 0.5)
             expectation2.fulfill()
         }
+        waitForExpectations(timeout: 1.0) { _ in }
 
-        waitForExpectations(timeout: 3.0) { _ in
-            XCTAssertGreaterThan(observer_view1.vitalInfo.sampleCount, observer_view2.vitalInfo.sampleCount)
-        }
+        // Block UI thread
+        Thread.sleep(forTimeInterval: 0.5)
+
+        XCTAssertGreaterThan(observer_view2.vitalInfo.sampleCount, 0)
+        XCTAssertGreaterThan(observer_view1.vitalInfo.meanValue, observer_view2.vitalInfo.meanValue)
     }
 
-    func testAppStateHandling() throws {
+    func testAppStateHandling() {
         let reader = VitalRefreshRateReader(notificationCenter: mockNotificationCenter)
-        XCTAssertFalse(reader.isRunning)
-
         let observer = VitalObserver(listener: VitalListenerMock())
 
-        XCTAssertNoThrow(try reader.start())
-        XCTAssertTrue(reader.isRunning)
-
+        mockNotificationCenter.post(name: UIApplication.didBecomeActiveNotification, object: nil)
         mockNotificationCenter.post(name: UIApplication.willResignActiveNotification, object: nil)
         reader.register(observer)
 
         let expectation1 = expectation(description: "async expectation for first observer")
         DispatchQueue.global().async {
-            Thread.sleep(forTimeInterval: 1.0)
+            Thread.sleep(forTimeInterval: 0.5)
             expectation1.fulfill()
         }
 
-        waitForExpectations(timeout: 3.0) { _ in
+        waitForExpectations(timeout: 1.0) { _ in
             XCTAssertEqual(observer.vitalInfo.sampleCount, 0)
         }
 
         mockNotificationCenter.post(name: UIApplication.didBecomeActiveNotification, object: nil)
 
         let expectation2 = expectation(description: "async expectation for second observer")
         DispatchQueue.global().async {
-            Thread.sleep(forTimeInterval: 1.0)
+            Thread.sleep(forTimeInterval: 0.5)
             expectation2.fulfill()
         }
 
-        waitForExpectations(timeout: 3.0) { _ in
+        waitForExpectations(timeout: 1.0) { _ in
             XCTAssertGreaterThan(observer.vitalInfo.sampleCount, 0)
         }
     }
-
-    func testReaderNotRestartIfNotAlreadyRunning() throws {
-        let reader = VitalRefreshRateReader(notificationCenter: mockNotificationCenter)
-        XCTAssertFalse(reader.isRunning)
-
-        let observer = VitalObserver(listener: VitalListenerMock())
-
-        mockNotificationCenter.post(name: UIApplication.didBecomeActiveNotification, object: nil)
-
-        let expectation1 = expectation(description: "async expectation for second observer")
-        DispatchQueue.global().async {
-            Thread.sleep(forTimeInterval: 1.0)
-            expectation1.fulfill()
-        }
-
-        waitForExpectations(timeout: 3.0) { _ in
-            XCTAssertEqual(observer.vitalInfo.sampleCount, 0)
-        }
-    }
 }