From 6a47c9778382446b685629fe4259982ef9a46882 Mon Sep 17 00:00:00 2001 From: mattreaganmozilla <145381717+mattreaganmozilla@users.noreply.github.com> Date: Thu, 9 May 2024 13:53:16 -0700 Subject: [PATCH] Refactor FXIOS-9050 [Multi-window] Throttler utility updates (#20172) * [9050] Updates to our Throttler and related unit tests * [9050] Use throttler in window tab sync coordinator --- .../WindowTabsSyncCoordinator.swift | 4 +- firefox-ios/Client/Utils/Throttler.swift | 14 ++-- .../ClientTests/Utils/ThrottlerTests.swift | 80 ++++++++++++------- 3 files changed, 58 insertions(+), 40 deletions(-) diff --git a/firefox-ios/Client/TabManagement/WindowTabsSyncCoordinator.swift b/firefox-ios/Client/TabManagement/WindowTabsSyncCoordinator.swift index 859a39ffd8b1..1c0bfd433fa3 100644 --- a/firefox-ios/Client/TabManagement/WindowTabsSyncCoordinator.swift +++ b/firefox-ios/Client/TabManagement/WindowTabsSyncCoordinator.swift @@ -29,9 +29,7 @@ final class WindowTabsSyncCoordinator { } func syncTabsToProfile() { - // throttler.throttle { [weak self] in self?.performSync() } - // TODO: [FXIOS-9050] Once 9050 issue is addressed, throttle calls to `performSync` - performSync() + throttler.throttle { [weak self] in self?.performSync() } } // MARK: - Utility diff --git a/firefox-ios/Client/Utils/Throttler.swift b/firefox-ios/Client/Utils/Throttler.swift index c865fa1dfbc8..69b471702841 100644 --- a/firefox-ios/Client/Utils/Throttler.swift +++ b/firefox-ios/Client/Utils/Throttler.swift @@ -8,24 +8,22 @@ import Common /// For any work that needs to be delayed, you can wrap it inside a throttler /// and specify the delay time, in seconds, and queue. class Throttler { - private var task = DispatchWorkItem(block: {}) private let defaultDelay = 0.35 - private let delay: Double + private let threshold: Double private var queue: DispatchQueueInterface + private var lastExecutationTime = Date.distantPast init(seconds delay: Double? = nil, on queue: DispatchQueueInterface = DispatchQueue.main) { - self.delay = delay ?? defaultDelay + self.threshold = delay ?? defaultDelay self.queue = queue } // This debounces; the task will not happen unless a duration of delay passes since the function was called func throttle(completion: @escaping () -> Void) { - // TODO: [FXIOS-9050] This can potentially infinitely delay the enqueued work which is not ideal. - task.cancel() - task = DispatchWorkItem { completion() } - - queue.asyncAfter(deadline: .now() + delay, execute: task) + guard lastExecutationTime.timeIntervalSinceNow < -threshold else { return } + lastExecutationTime = Date() + queue.async(execute: completion) } } diff --git a/firefox-ios/firefox-ios-tests/Tests/ClientTests/Utils/ThrottlerTests.swift b/firefox-ios/firefox-ios-tests/Tests/ClientTests/Utils/ThrottlerTests.swift index 235f98565c14..e4f687fcd070 100644 --- a/firefox-ios/firefox-ios-tests/Tests/ClientTests/Utils/ThrottlerTests.swift +++ b/firefox-ios/firefox-ios-tests/Tests/ClientTests/Utils/ThrottlerTests.swift @@ -7,46 +7,68 @@ import XCTest @testable import Client class ThrottlerTests: XCTestCase { - func testThrottle_1000SecondsThrottle_doesntCall() { - let subject = Throttler(seconds: 100000, on: DispatchQueue.global()) - var throttleCalled = 0 - subject.throttle { - throttleCalled += 1 - } + struct Timing { + static let veryLongDelay: Double = 100_000 + static let defaultTestMaxWaitTime: Double = 2 + } - subject.throttle { - throttleCalled += 1 - } + private let testQueue = DispatchQueue.global() + private var throttler: Throttler! + private var expectation: XCTestExpectation! + private var testValue = 0 + + func testMultipleFastConsecutiveCallsAreThrottledAndExecutedAtMostOneTime() { + prepareTest(timeout: Timing.veryLongDelay) - XCTAssertEqual(throttleCalled, 0, "Throttle isn't called since delay is high") + throttler.throttle { self.testValue += 1 } + throttler.throttle { self.testValue += 1 } + throttler.throttle { self.testValue += 1 } + + expect(value: 1) } - func testThrottle_zeroSecondThrottle_callsTwice() { - let subject = Throttler(seconds: 0, on: MockDispatchQueue()) - var throttleCalled = 0 - subject.throttle { - throttleCalled += 1 - } + func testThrottleZeroSecondThrottleExecutesAllClosures() { + prepareTest(timeout: 0) - subject.throttle { - throttleCalled += 1 - } + throttler.throttle { self.testValue += 1 } + throttler.throttle { self.testValue += 1 } - XCTAssertEqual(throttleCalled, 2, "Throttle twice is called since delay is zero") + expect(value: 2) } - func testThrottle_oneSecondThrottle_callsOnce() { - let subject = Throttler(seconds: 0.2, on: DispatchQueue.global()) - var throttleCalled = 0 - subject.throttle { - throttleCalled += 1 + func testSecondCallAfterDelayThresholdCallsBothClosures() { + let threshold: Double = 0.5 + let step: Double = (threshold / 2.0) + prepareTest(timeout: threshold) + + // Send one call to throttler + throttler.throttle { self.testValue = 1 } + DispatchQueue.main.asyncAfter(deadline: .now() + step) { + XCTAssertEqual(self.testValue, 1) } - subject.throttle { - throttleCalled += 1 + // Wait briefly after our threshold and send another call + DispatchQueue.main.asyncAfter(deadline: .now() + threshold + step) { + self.throttler.throttle { self.testValue = 2 } } - wait(0.5) - XCTAssertEqual(throttleCalled, 1, "Throttle is called once, one got canceled") + // Expect both calls to throttler have executed + self.expect(value: 2) + } + + // MARK: - Utility + + private func prepareTest(timeout: Double) { + testValue = 0 + expectation = XCTestExpectation(description: "Throttle value expectation") + throttler = Throttler(seconds: timeout, on: testQueue) + } + + private func expect(value expected: Int) { + DispatchQueue.main.asyncAfter(deadline: .now() + Timing.defaultTestMaxWaitTime) { + guard self.testValue == expected else { XCTFail("Expected value \(expected) != \(self.testValue)."); return } + self.expectation.fulfill() + } + wait(for: [expectation], timeout: Timing.defaultTestMaxWaitTime * 2.0) } }