From 09ec0fe2582551d153f2872c13081986af315d53 Mon Sep 17 00:00:00 2001 From: Chris Brind Date: Wed, 4 Dec 2024 18:18:40 +0000 Subject: [PATCH 1/2] clean up clearing with fire button --- DuckDuckGo/AutoClear.swift | 16 +++--- DuckDuckGo/FireButtonAnimator.swift | 61 ++++++++++++++++++++++- DuckDuckGo/MainViewController.swift | 76 ++++++++++++++++------------- DuckDuckGo/SettingsViewModel.swift | 8 +-- 4 files changed, 111 insertions(+), 50 deletions(-) diff --git a/DuckDuckGo/AutoClear.swift b/DuckDuckGo/AutoClear.swift index 2e1abbc02b..cc81fc726f 100644 --- a/DuckDuckGo/AutoClear.swift +++ b/DuckDuckGo/AutoClear.swift @@ -23,13 +23,13 @@ import Core protocol AutoClearWorker { - func clearNavigationStack() + func clearNavigationStack() async func forgetData() async func forgetData(applicationState: DataStoreWarmup.ApplicationState) async - func forgetTabs() + func forgetTabs() async - func willStartClearing(_: AutoClear) - func autoClearDidFinishClearing(_: AutoClear, isLaunching: Bool) + func willStartClearing(_: AutoClear) async + func autoClearDidFinishClearing(_: AutoClear, isLaunching: Bool) async } class AutoClear { @@ -52,17 +52,17 @@ class AutoClear { func clearDataIfEnabled(launching: Bool = false, applicationState: DataStoreWarmup.ApplicationState = .unknown) async { guard let settings = AutoClearSettingsModel(settings: appSettings) else { return } - worker.willStartClearing(self) + await worker.willStartClearing(self) if settings.action.contains(.clearTabs) { - worker.forgetTabs() + await worker.forgetTabs() } if settings.action.contains(.clearData) { await worker.forgetData(applicationState: applicationState) } - worker.autoClearDidFinishClearing(self, isLaunching: launching) + await worker.autoClearDidFinishClearing(self, isLaunching: launching) } /// Note: function is parametrised because of tests. @@ -99,7 +99,7 @@ class AutoClear { shouldClearData(elapsedTime: baseTimeInterval - timestamp) else { return } self.timestamp = nil - worker.clearNavigationStack() + await worker.clearNavigationStack() await clearDataIfEnabled(applicationState: applicationState) } } diff --git a/DuckDuckGo/FireButtonAnimator.swift b/DuckDuckGo/FireButtonAnimator.swift index 84b9f82a39..59f0d07baf 100644 --- a/DuckDuckGo/FireButtonAnimator.swift +++ b/DuckDuckGo/FireButtonAnimator.swift @@ -109,8 +109,65 @@ class FireButtonAnimator { name: AppUserDefaults.Notifications.currentFireButtonAnimationChange, object: nil) } - - func animate(onAnimationStart: @escaping () async -> Void, onTransitionCompleted: @escaping () async -> Void, completion: @escaping () async -> Void) { + + /// Shows the selected clearing animation while managing a screenshot of the screen in order to allow UI updates happen while the animation is running (e.g. tabs closing). + /// + /// To use this start clearing immediately in a separate task. Wait for this task to finish and if cleaning is still happening then show the indeterminte progress. + /// + /// @param afterScreenUpdates Mainly provided for the preview in settings. From the fire button we want the screen to be captured immediately. + @MainActor + func animate(afterScreenUpdates: Bool = false) async { + print("***", #function, "IN") + + guard let window = UIApplication.shared.firstKeyWindow, + let snapshot = window.snapshotView(afterScreenUpdates: afterScreenUpdates), + let composition = preLoadedComposition else { + return + } + + window.addSubview(snapshot) + + let animationView = LottieAnimationView(animation: composition) + let currentAnimation = appSettings.currentFireButtonAnimation + let speed = currentAnimation.speed + animationView.contentMode = .scaleAspectFill + animationView.animationSpeed = CGFloat(speed) + animationView.frame = window.frame + window.addSubview(animationView) + + let duration = Double(composition.duration) / speed + let delay = duration * currentAnimation.transition + + var animationFinished = false + print("***", #function, " play IN") + animationView.play(fromProgress: 0, toProgress: 1) { _ in + animationFinished = true + print("***", #function, " play OUT") + } + + await transition(snapshot, withDelay: delay) + + while !animationFinished { + await Task.yield() // Give the system chance to decide if something higher priority should run + try? await Task.sleep(interval: 0.01) // Either way, wait a small amount of time for the animation to finish in case this is the highest priority task + } + + animationView.removeFromSuperview() + print("***", #function, "OUT") + } + + func transition(_ snapshot: UIView, withDelay delay: TimeInterval) async { + print("***", #function, "IN") + do { + try await Task.sleep(interval: delay) + } catch { + // TODO log this + } + await snapshot.removeFromSuperview() + print("***", #function, "OUT") + } + + func legacy_animate(onAnimationStart: @escaping () async -> Void, onTransitionCompleted: @escaping () async -> Void, completion: @escaping () async -> Void) { guard let window = UIApplication.shared.firstKeyWindow, let snapshot = window.snapshotView(afterScreenUpdates: false) else { diff --git a/DuckDuckGo/MainViewController.swift b/DuckDuckGo/MainViewController.swift index 506727e17f..37b19e6d8b 100644 --- a/DuckDuckGo/MainViewController.swift +++ b/DuckDuckGo/MainViewController.swift @@ -164,8 +164,7 @@ class MainViewController: UIViewController { } var keyModifierFlags: UIKeyModifierFlags? - var showKeyboardAfterFireButton: DispatchWorkItem? - + // Skip SERP flow (focusing on autocomplete logic) and prepare for new navigation when selecting search bar private var skipSERPFlow = true @@ -856,7 +855,7 @@ class MainViewController: UIViewController { func showClearDataAlert() { let alert = ForgetDataAlert.buildAlert(forgetTabsAndDataHandler: { [weak self] in - self?.forgetAllWithAnimation {} + self?.forgetAllWithAnimation() }) self.present(controller: alert, fromView: self.viewCoordinator.toolbar) } @@ -883,7 +882,7 @@ class MainViewController: UIViewController { func onQuickFirePressed() { wakeLazyFireButtonAnimator() - forgetAllWithAnimation {} + forgetAllWithAnimation() dismiss(animated: true) if KeyboardSettings().onAppLaunch { enterSearch() @@ -2197,8 +2196,6 @@ extension MainViewController: AutocompleteViewControllerDelegate { extension MainViewController { private func handleRequestedURL(_ url: URL) { - showKeyboardAfterFireButton?.cancel() - if url.isBookmarklet() { executeBookmarklet(url) } else { @@ -2537,9 +2534,8 @@ extension MainViewController: TabSwitcherDelegate { } func tabSwitcherDidRequestForgetAll(tabSwitcher: TabSwitcherViewController) { - self.forgetAllWithAnimation { - tabSwitcher.dismiss(animated: false, completion: nil) - } + self.forgetAllWithAnimation() + tabSwitcher.dismiss(animated: false, completion: nil) } } @@ -2702,36 +2698,48 @@ extension MainViewController: AutoClearWorker { AppDependencyProvider.shared.downloadManager.cancelAllDownloads() } - func forgetAllWithAnimation(transitionCompletion: (() -> Void)? = nil, showNextDaxDialog: Bool = false) { - let spid = Instruments.shared.startTimedEvent(.clearingData) - Pixel.fire(pixel: .forgetAllExecuted) + func forgetAllWithAnimation(showNextDaxDialog: Bool = false) { + Task { @MainActor in + + var dataClearingFinished = false + var preClearingUIUpdatesFinished = false + + // Run this in parallel + Task { + print("***", #function, "clearing, IN") + self.tabManager.prepareCurrentTabForDataClearing() + self.stopAllOngoingDownloads() + self.forgetTabs() + preClearingUIUpdatesFinished = true + print("***", #function, "clearing, pre clean UI cleaned up") + + self.refreshUIAfterClear() + await self.forgetData() + + // Add some sleep here to test the indterimnate state + // try? await Task.sleep(interval: 5.0) + + dataClearingFinished = true + print("***", #function, "clearing, OUT") + } + + await fireButtonAnimator.animate() + + print("***", #function, "animation complete", preClearingUIUpdatesFinished, dataClearingFinished) + + // TODO if clearing not finished yet show UI and wait for it to finish - tabManager.prepareAllTabsExceptCurrentForDataClearing() - - fireButtonAnimator.animate { - self.tabManager.prepareCurrentTabForDataClearing() - self.stopAllOngoingDownloads() - self.forgetTabs() - await self.forgetData() - Instruments.shared.endTimedEvent(for: spid) - DaxDialogs.shared.resumeRegularFlow() - } onTransitionCompleted: { ActionMessageView.present(message: UserText.actionForgetAllDone, - presentationLocation: .withBottomBar(andAddressBarBottom: self.appSettings.currentAddressBarPosition.isBottom)) - transitionCompletion?() - self.refreshUIAfterClear() - } completion: { - self.privacyProDataReporter.saveFireCount() + presentationLocation: .withBottomBar(andAddressBarBottom: appSettings.currentAddressBarPosition.isBottom)) + + privacyProDataReporter.saveFireCount() - // Ideally this should happen once data clearing has finished AND the animation is finished if showNextDaxDialog { self.newTabPageViewController?.showNextDaxDialog() - } else if KeyboardSettings().onNewTab && !self.contextualOnboardingLogic.isShowingAddToDockDialog { // If we're showing the Add to Dock dialog prevent address bar to become first responder. We want to make sure the user focues on the Add to Dock instructions. - let showKeyboardAfterFireButton = DispatchWorkItem { - self.enterSearch() - } - DispatchQueue.main.asyncAfter(deadline: .now() + 0.3, execute: showKeyboardAfterFireButton) - self.showKeyboardAfterFireButton = showKeyboardAfterFireButton + } else if KeyboardSettings().onNewTab && + // If we're showing the Add to Dock dialog prevent address bar to become first responder. + !self.contextualOnboardingLogic.isShowingAddToDockDialog { + self.enterSearch() } if self.variantManager.isContextualDaxDialogsEnabled { diff --git a/DuckDuckGo/SettingsViewModel.swift b/DuckDuckGo/SettingsViewModel.swift index a97700ad72..a4841cecc9 100644 --- a/DuckDuckGo/SettingsViewModel.swift +++ b/DuckDuckGo/SettingsViewModel.swift @@ -119,12 +119,8 @@ final class SettingsViewModel: ObservableObject { self.appSettings.currentFireButtonAnimation = $0 self.state.fireButtonAnimation = $0 NotificationCenter.default.post(name: AppUserDefaults.Notifications.currentFireButtonAnimationChange, object: self) - self.animator.animate { - // no op - } onTransitionCompleted: { - // no op - } completion: { - // no op + Task { @MainActor in + await self.animator.animate(afterScreenUpdates: true) } } ) From 120eaf09fa78ee59a2ec888f2dc088dd23c2624f Mon Sep 17 00:00:00 2001 From: Chris Brind Date: Wed, 4 Dec 2024 19:01:42 +0000 Subject: [PATCH 2/2] remove print statements and add pixels --- Core/PixelEvent.swift | 4 ++++ DuckDuckGo/MainViewController.swift | 19 +++++++++++++------ 2 files changed, 17 insertions(+), 6 deletions(-) diff --git a/Core/PixelEvent.swift b/Core/PixelEvent.swift index b98773df96..4bbcbfe74e 100644 --- a/Core/PixelEvent.swift +++ b/Core/PixelEvent.swift @@ -593,6 +593,8 @@ extension Pixel { case debugWebsiteDataStoresNotClearedOne case debugWebsiteDataStoresCleared + case debugAnimationFinishedBeforeClearing + case debugBookmarksMigratedMoreThanOnce // Return user measurement @@ -1452,6 +1454,8 @@ extension Pixel.Event { case .debugWebsiteDataStoresNotClearedOne: return "m_d_wkwebsitedatastoresnotcleared_one" case .debugWebsiteDataStoresCleared: return "m_d_wkwebsitedatastorescleared" + case .debugAnimationFinishedBeforeClearing: return "m_debug_animation-finished-before-clearing" + // MARK: Ad Attribution case .adAttributionGlobalAttributedRulesDoNotExist: return "m_attribution_global_attributed_rules_do_not_exist" diff --git a/DuckDuckGo/MainViewController.swift b/DuckDuckGo/MainViewController.swift index 37b19e6d8b..1c5cfe447e 100644 --- a/DuckDuckGo/MainViewController.swift +++ b/DuckDuckGo/MainViewController.swift @@ -2699,6 +2699,9 @@ extension MainViewController: AutoClearWorker { } func forgetAllWithAnimation(showNextDaxDialog: Bool = false) { + let spid = Instruments.shared.startTimedEvent(.clearingData) + Pixel.fire(pixel: .forgetAllExecuted) + Task { @MainActor in var dataClearingFinished = false @@ -2706,28 +2709,32 @@ extension MainViewController: AutoClearWorker { // Run this in parallel Task { - print("***", #function, "clearing, IN") self.tabManager.prepareCurrentTabForDataClearing() self.stopAllOngoingDownloads() self.forgetTabs() + self.refreshUIAfterClear() preClearingUIUpdatesFinished = true - print("***", #function, "clearing, pre clean UI cleaned up") - self.refreshUIAfterClear() await self.forgetData() // Add some sleep here to test the indterimnate state // try? await Task.sleep(interval: 5.0) + Instruments.shared.endTimedEvent(for: spid) dataClearingFinished = true - print("***", #function, "clearing, OUT") } await fireButtonAnimator.animate() - print("***", #function, "animation complete", preClearingUIUpdatesFinished, dataClearingFinished) + if !preClearingUIUpdatesFinished { + Pixel.fire(pixel: .debugAnimationFinishedBeforeClearing) + } + + if !dataClearingFinished { + Pixel.fire(pixel: .debugAnimationFinishedBeforeClearing) + } - // TODO if clearing not finished yet show UI and wait for it to finish + // MARK: post-clearing animation tasks ActionMessageView.present(message: UserText.actionForgetAllDone, presentationLocation: .withBottomBar(andAddressBarBottom: appSettings.currentAddressBarPosition.isBottom))