From 0ad5d2ac1e5688acfeaab6ed3b9ec5129f852d40 Mon Sep 17 00:00:00 2001 From: Alexey Martemyanov Date: Mon, 26 Feb 2024 21:12:42 +0600 Subject: [PATCH 1/9] Add estimated download time --- DuckDuckGo/Common/Localizables/UserText.swift | 1 + .../Model/WebKitDownloadTask.swift | 44 ++++++++++++++--- .../FileDownload/View/Downloads.storyboard | 8 ++-- .../FileDownload/View/DownloadsCellView.swift | 48 ++++++++++++++----- .../View/DownloadsViewController.swift | 2 +- DuckDuckGo/Localizable.xcstrings | 11 +++++ 6 files changed, 90 insertions(+), 24 deletions(-) diff --git a/DuckDuckGo/Common/Localizables/UserText.swift b/DuckDuckGo/Common/Localizables/UserText.swift index eee5b3f4e5..bcb8f2d2b0 100644 --- a/DuckDuckGo/Common/Localizables/UserText.swift +++ b/DuckDuckGo/Common/Localizables/UserText.swift @@ -668,6 +668,7 @@ struct UserText { static let downloadCanceled = NSLocalizedString("downloads.error.canceled", value: "Canceled", comment: "Short error description when downloaded file download was canceled") static let downloadFailedToMoveFileToDownloads = NSLocalizedString("downloads.error.move.failed", value: "Could not move file to Downloads", comment: "Short error description when could not move downloaded file to the Downloads folder") static let downloadFailed = NSLocalizedString("downloads.error.other", value: "Error", comment: "Short error description when Download failed") + static let downloadBytesLoadedFormat = NSLocalizedString("%@ of %@", comment: "Number of bytes out of total bytes downloaded (1Mb of 2Mb)") static let cancelDownloadToolTip = NSLocalizedString("downloads.tooltip.cancel", value: "Cancel Download", comment: "Mouse-over tooltip for Cancel Download button") static let restartDownloadToolTip = NSLocalizedString("downloads.tooltip.restart", value: "Restart Download", comment: "Mouse-over tooltip for Restart Download button") diff --git a/DuckDuckGo/FileDownload/Model/WebKitDownloadTask.swift b/DuckDuckGo/FileDownload/Model/WebKitDownloadTask.swift index 4e96b1f482..b94250c2e2 100644 --- a/DuckDuckGo/FileDownload/Model/WebKitDownloadTask.swift +++ b/DuckDuckGo/FileDownload/Model/WebKitDownloadTask.swift @@ -33,6 +33,10 @@ protocol WebKitDownloadTaskDelegate: AnyObject { final class WebKitDownloadTask: NSObject, ProgressReporting, @unchecked Sendable { static let downloadExtension = "duckload" + private enum Constants { + static let remainingDownloadTimeEstimationDelay: TimeInterval = 1 + static let downloadSpeedSmoothingFactor = 0.1 + } let progress: Progress let shouldPromptForLocation: Bool @@ -72,7 +76,7 @@ final class WebKitDownloadTask: NSObject, ProgressReporting, @unchecked Sendable private weak var delegate: WebKitDownloadTaskDelegate? private let download: WebKitDownload - private var cancellables = Set() + private var progressCancellable: AnyCancellable? private var decideDestinationCompletionHandler: ((URL?) -> Void)? @@ -114,12 +118,38 @@ final class WebKitDownloadTask: NSObject, ProgressReporting, @unchecked Sendable private func start() { self.progress.fileDownloadingSourceURL = download.originalRequest?.url if let progress = (self.download as? ProgressReporting)?.progress { - progress.publisher(for: \.totalUnitCount) - .assign(to: \.totalUnitCount, onWeaklyHeld: self.progress) - .store(in: &self.cancellables) - progress.publisher(for: \.completedUnitCount) - .assign(to: \.completedUnitCount, onWeaklyHeld: self.progress) - .store(in: &self.cancellables) + + var startTime: Date? + progressCancellable = progress.publisher(for: \.totalUnitCount) + .combineLatest(progress.publisher(for: \.completedUnitCount)) + .sink { [weak progress=self.progress] total, completed in + guard let progress else { return } + if progress.totalUnitCount != total { + progress.totalUnitCount = total + } + progress.completedUnitCount = completed + + if total > 0, completed > 0 { + guard let startTime else { + startTime = Date() + return + } + let elapsedTime = Date().timeIntervalSince(startTime) + guard elapsedTime > Constants.remainingDownloadTimeEstimationDelay else { return } + + // Calculate instantaneous download speed + var throughput = Double(completed) / elapsedTime + + // Calculate the moving average of download speed + if let oldThroughput = progress.throughput.map(Double.init) { + throughput = Constants.downloadSpeedSmoothingFactor * throughput + (1 - Constants.downloadSpeedSmoothingFactor) * oldThroughput + } + progress.throughput = Int(throughput) + + // only update estimated time after initial delay + progress.estimatedTimeRemaining = Double(total - completed) / throughput + } + } } } diff --git a/DuckDuckGo/FileDownload/View/Downloads.storyboard b/DuckDuckGo/FileDownload/View/Downloads.storyboard index 411575e7a9..673a1fcaaa 100644 --- a/DuckDuckGo/FileDownload/View/Downloads.storyboard +++ b/DuckDuckGo/FileDownload/View/Downloads.storyboard @@ -123,7 +123,7 @@ - + @@ -134,7 +134,7 @@ - + @@ -247,12 +247,12 @@ - + - + diff --git a/DuckDuckGo/FileDownload/View/DownloadsCellView.swift b/DuckDuckGo/FileDownload/View/DownloadsCellView.swift index b891bbb620..5eadf9a99d 100644 --- a/DuckDuckGo/FileDownload/View/DownloadsCellView.swift +++ b/DuckDuckGo/FileDownload/View/DownloadsCellView.swift @@ -54,6 +54,22 @@ final class DownloadsCellView: NSTableCellView { private var progressCancellable: AnyCancellable? private static let byteFormatter = ByteCountFormatter() + private static let estimatedMinutesRemainingFormatter: DateComponentsFormatter = { + let formatter = DateComponentsFormatter() + formatter.allowedUnits = [.hour, .minute] + formatter.unitsStyle = .full + formatter.includesApproximationPhrase = true + formatter.includesTimeRemainingPhrase = true + return formatter + }() + private static let estimatedSecondsRemainingFormatter: DateComponentsFormatter = { + let formatter = DateComponentsFormatter() + formatter.allowedUnits = [.second] + formatter.unitsStyle = .full + formatter.includesApproximationPhrase = true + formatter.includesTimeRemainingPhrase = true + return formatter + }() var isSelected: Bool = false { didSet { @@ -140,26 +156,34 @@ final class DownloadsCellView: NSTableCellView { private var onButtonMouseOverChange: ((Bool) -> Void)? private func updateDetails(with progress: Progress) { + self.detailLabel.toolTip = progress.localizedAdditionalDescription ?? "" + var details: String if cancelButton.isMouseOver { details = UserText.cancelDownloadToolTip } else { - details = progress.localizedAdditionalDescription ?? "" - if details.isEmpty { - if progress.fractionCompleted == 0 { - details = UserText.downloadStarting - } else if progress.fractionCompleted == 1.0 { - details = UserText.downloadFinishing - } else { - assertionFailure("Unexpected empty description") - details = "Downloading…" - } + if progress.fractionCompleted == 0 { + details = UserText.downloadStarting + } else if progress.fractionCompleted == 1.0 { + details = UserText.downloadFinishing + } else if progress.totalUnitCount > 0 { + let completed = Self.byteFormatter.string(fromByteCount: progress.completedUnitCount) + let total = Self.byteFormatter.string(fromByteCount: progress.totalUnitCount) + details = String(format: UserText.downloadBytesLoadedFormat, completed, total) + } else { + details = Self.byteFormatter.string(fromByteCount: progress.completedUnitCount) + } + + if let estimatedTimeRemaining = progress.estimatedTimeRemaining, + let estimatedTimeStr = (estimatedTimeRemaining < 60 ? Self.estimatedSecondsRemainingFormatter : Self.estimatedMinutesRemainingFormatter) + .string(from: estimatedTimeRemaining) { + + details += " – " + estimatedTimeStr } - self.detailLabel.toolTip = progress.localizedDescription + self.detailLabel.stringValue = details } - self.detailLabel.stringValue = details } private func subscribe(to progress: Progress) { diff --git a/DuckDuckGo/FileDownload/View/DownloadsViewController.swift b/DuckDuckGo/FileDownload/View/DownloadsViewController.swift index 21c286ce61..bc124e9d2a 100644 --- a/DuckDuckGo/FileDownload/View/DownloadsViewController.swift +++ b/DuckDuckGo/FileDownload/View/DownloadsViewController.swift @@ -148,7 +148,7 @@ final class DownloadsViewController: NSViewController { return } self.dismiss() - NSWorkspace.shared.selectFile(nil, inFileViewerRootedAtPath: url.path) + NSWorkspace.shared.selectFile(itemToSelect?.path, inFileViewerRootedAtPath: url.path) } @IBAction func clearDownloadsAction(_ sender: Any) { diff --git a/DuckDuckGo/Localizable.xcstrings b/DuckDuckGo/Localizable.xcstrings index ddf6e62331..d7519d72cb 100644 --- a/DuckDuckGo/Localizable.xcstrings +++ b/DuckDuckGo/Localizable.xcstrings @@ -37,6 +37,17 @@ "%@ does not support storing passwords" : { "comment" : "Data Import disabled checkbox message about a browser (%@) not supporting storing passwords" }, + "%@ of %@" : { + "comment" : "Number of bytes out of total bytes downloaded (1Mb of 2Mb)", + "localizations" : { + "en" : { + "stringUnit" : { + "state" : "new", + "value" : "%1$@ of %2$@" + } + } + } + }, "%lld" : { }, From 1dccea2090448dcb796cc52c480bd4f87f0e89a5 Mon Sep 17 00:00:00 2001 From: Alexey Martemyanov Date: Mon, 26 Feb 2024 21:21:05 +0600 Subject: [PATCH 2/9] fix build --- DuckDuckGo/FileDownload/View/DownloadsViewController.swift | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/DuckDuckGo/FileDownload/View/DownloadsViewController.swift b/DuckDuckGo/FileDownload/View/DownloadsViewController.swift index bc124e9d2a..21c286ce61 100644 --- a/DuckDuckGo/FileDownload/View/DownloadsViewController.swift +++ b/DuckDuckGo/FileDownload/View/DownloadsViewController.swift @@ -148,7 +148,7 @@ final class DownloadsViewController: NSViewController { return } self.dismiss() - NSWorkspace.shared.selectFile(itemToSelect?.path, inFileViewerRootedAtPath: url.path) + NSWorkspace.shared.selectFile(nil, inFileViewerRootedAtPath: url.path) } @IBAction func clearDownloadsAction(_ sender: Any) { From 8503f767784f05a729b57cf95521107e2fccfc8c Mon Sep 17 00:00:00 2001 From: Alexey Martemyanov Date: Tue, 27 Feb 2024 18:28:54 +0600 Subject: [PATCH 3/9] fix address bar tests --- DuckDuckGo/MainWindow/MainViewController.swift | 8 ++++---- DuckDuckGo/Tab/Model/Tab.swift | 10 ++++++++-- IntegrationTests/Tab/AddressBarTests.swift | 1 + 3 files changed, 13 insertions(+), 6 deletions(-) diff --git a/DuckDuckGo/MainWindow/MainViewController.swift b/DuckDuckGo/MainWindow/MainViewController.swift index 94f4ac7953..afeee4c7e3 100644 --- a/DuckDuckGo/MainWindow/MainViewController.swift +++ b/DuckDuckGo/MainWindow/MainViewController.swift @@ -102,7 +102,7 @@ final class MainViewController: NSViewController { super.viewDidAppear() mainView.setMouseAboveWebViewTrackingAreaEnabled(true) registerForBookmarkBarPromptNotifications() - adjustFirstResponder() + adjustFirstResponder(force: true) } var bookmarkBarPromptObserver: Any? @@ -417,7 +417,7 @@ final class MainViewController: NSViewController { // MARK: - First responder - func adjustFirstResponder(selectedTabViewModel: TabViewModel? = nil, tabContent: Tab.TabContent? = nil) { + func adjustFirstResponder(selectedTabViewModel: TabViewModel? = nil, tabContent: Tab.TabContent? = nil, force: Bool = false) { guard let selectedTabViewModel = selectedTabViewModel ?? tabCollectionViewModel.selectedTabViewModel else { assertionFailure("No tab view model selected") return @@ -430,8 +430,8 @@ final class MainViewController: NSViewController { } else { // ignore published tab switch: BrowserTabViewController // adjusts first responder itself - guard selectedTabViewModel === tabCollectionViewModel.selectedTabViewModel else { return } - browserTabViewController.adjustFirstResponder(tabContent: tabContent) + guard selectedTabViewModel === tabCollectionViewModel.selectedTabViewModel || force else { return } + browserTabViewController.adjustFirstResponder(force: force, tabContent: tabContent) } } diff --git a/DuckDuckGo/Tab/Model/Tab.swift b/DuckDuckGo/Tab/Model/Tab.swift index a8dc0f6c89..b56b7c6d4b 100644 --- a/DuckDuckGo/Tab/Model/Tab.swift +++ b/DuckDuckGo/Tab/Model/Tab.swift @@ -565,12 +565,18 @@ protocol NewWindowPolicyDecisionMaker { webView.stopAllMedia(shouldStopLoading: true) userContentController?.cleanUpBeforeClosing() - webView.assertObjectDeallocated(after: 4.0) +#if DEBUG + if case .normal = NSApp.runType { + webView.assertObjectDeallocated(after: 4.0) + } +#endif } - if !onDeinit { +#if DEBUG + if !onDeinit, case .normal = NSApp.runType { // Tab should be deallocated shortly after burning self.assertObjectDeallocated(after: 4.0) } +#endif guard Thread.isMainThread else { DispatchQueue.main.async { job() } return diff --git a/IntegrationTests/Tab/AddressBarTests.swift b/IntegrationTests/Tab/AddressBarTests.swift index 6494b2c3d5..7894d6d0cb 100644 --- a/IntegrationTests/Tab/AddressBarTests.swift +++ b/IntegrationTests/Tab/AddressBarTests.swift @@ -451,6 +451,7 @@ class AddressBarTests: XCTestCase { window = WindowsManager.openNewWindow(with: viewModel)! try await tab.webViewDidFinishNavigationPublisher.timeout(5).first().promise().value + try await Task.sleep(interval: 10) XCTAssertEqual(window.firstResponder, tab.webView) try await tab.setContent(.url(.makeSearchUrl(from: "cats")!, credential: nil, source: .bookmark))?.result.get() From 50a7b65a5b2591e607b7ed232d6c1415a1792b97 Mon Sep 17 00:00:00 2001 From: Alexey Martemyanov Date: Tue, 27 Feb 2024 19:07:26 +0600 Subject: [PATCH 4/9] cleanup --- IntegrationTests/Tab/AddressBarTests.swift | 1 - 1 file changed, 1 deletion(-) diff --git a/IntegrationTests/Tab/AddressBarTests.swift b/IntegrationTests/Tab/AddressBarTests.swift index 7894d6d0cb..6494b2c3d5 100644 --- a/IntegrationTests/Tab/AddressBarTests.swift +++ b/IntegrationTests/Tab/AddressBarTests.swift @@ -451,7 +451,6 @@ class AddressBarTests: XCTestCase { window = WindowsManager.openNewWindow(with: viewModel)! try await tab.webViewDidFinishNavigationPublisher.timeout(5).first().promise().value - try await Task.sleep(interval: 10) XCTAssertEqual(window.firstResponder, tab.webView) try await tab.setContent(.url(.makeSearchUrl(from: "cats")!, credential: nil, source: .bookmark))?.result.get() From d2fb6391aadd09fc65fa2b70dfe4ad090ce89104 Mon Sep 17 00:00:00 2001 From: Alexey Martemyanov Date: Tue, 27 Feb 2024 19:08:03 +0600 Subject: [PATCH 5/9] add speed and estimated time on extra line --- DuckDuckGo/Common/Localizables/UserText.swift | 5 +- .../FileDownload/View/Downloads.storyboard | 71 ++++++++++++------- .../FileDownload/View/DownloadsCellView.swift | 45 +++++++----- DuckDuckGo/Localizable.xcstrings | 38 +++++++++- 4 files changed, 113 insertions(+), 46 deletions(-) diff --git a/DuckDuckGo/Common/Localizables/UserText.swift b/DuckDuckGo/Common/Localizables/UserText.swift index b9ec8409e7..13b777a8ee 100644 --- a/DuckDuckGo/Common/Localizables/UserText.swift +++ b/DuckDuckGo/Common/Localizables/UserText.swift @@ -671,7 +671,10 @@ struct UserText { static let downloadCanceled = NSLocalizedString("downloads.error.canceled", value: "Canceled", comment: "Short error description when downloaded file download was canceled") static let downloadFailedToMoveFileToDownloads = NSLocalizedString("downloads.error.move.failed", value: "Could not move file to Downloads", comment: "Short error description when could not move downloaded file to the Downloads folder") static let downloadFailed = NSLocalizedString("downloads.error.other", value: "Error", comment: "Short error description when Download failed") - static let downloadBytesLoadedFormat = NSLocalizedString("%@ of %@", comment: "Number of bytes out of total bytes downloaded (1Mb of 2Mb)") + static let downloadBytesLoadedFormat = NSLocalizedString("downloads.bytes.format", value: "%@ of %@", comment: "Number of bytes out of total bytes downloaded (1Mb of 2Mb)") + static let downloadSpeedFormat = NSLocalizedString("downloads.speed.format", value: "%@/sec", comment: "Download speed format (1Mb/sec)") + + static let downloadFewSecondsRemaining = NSLocalizedString("downloads.few.seconds.remaining", value: "A few seconds remaining", comment: "Estimated time info displayed when a file is almost downloaded") static let cancelDownloadToolTip = NSLocalizedString("downloads.tooltip.cancel", value: "Cancel Download", comment: "Mouse-over tooltip for Cancel Download button") static let restartDownloadToolTip = NSLocalizedString("downloads.tooltip.restart", value: "Restart Download", comment: "Mouse-over tooltip for Restart Download button") diff --git a/DuckDuckGo/FileDownload/View/Downloads.storyboard b/DuckDuckGo/FileDownload/View/Downloads.storyboard index 673a1fcaaa..77e3e22cd5 100644 --- a/DuckDuckGo/FileDownload/View/Downloads.storyboard +++ b/DuckDuckGo/FileDownload/View/Downloads.storyboard @@ -122,25 +122,48 @@ - - - - - - - - - - - - - - - - - - - + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + - + - + - + - + @@ -111,7 +111,7 @@ - + @@ -122,50 +122,27 @@ - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - + + + + + + + + + + + + + + + + + + + - + - + + - - + + + - + + @@ -280,7 +260,6 @@ - @@ -291,11 +270,11 @@ - + - + @@ -303,7 +282,7 @@