Skip to content

Commit

Permalink
Display Fire window downloads per Fire Window (#3250)
Browse files Browse the repository at this point in the history
Task/Issue URL:
https://app.asana.com/0/1199230911884351/1207052331808912/f

**Description**:
- Separate downloads from Fire window and Regular windows

**Steps to test this PR**:
1. Start a download in a regular window
2. Open a Fire Window, validate its downloads list is empty
3. Start a download in the Fire Window; Validate the download stays in
the list after it‘s completed.
4. Close the Fire Window with active downloads; Validate the alert is
displayed (with "this file" or "these files" for 1 or many files
downloaded)
5. Close the Fire Window, validate the downloads aren‘t displayed in the
Regular window

<!--
Tagging instructions
If this PR isn't ready to be merged for whatever reason it should be
marked with the `DO NOT MERGE` label (particularly if it's a draft)
If it's pending Product Review/PFR, please add the `Pending Product
Review` label.

If at any point it isn't actively being worked on/ready for
review/otherwise moving forward (besides the above PR/PFR exception)
strongly consider closing it (or not opening it in the first place). If
you decide not to close it, make sure it's labelled to make it clear the
PRs state and comment with more information.
-->

**Definition of Done**:

* [ ] Does this PR satisfy our [Definition of
Done](https://app.asana.com/0/1202500774821704/1207634633537039/f)?

---
###### Internal references:
[Pull Request Review
Checklist](https://app.asana.com/0/1202500774821704/1203764234894239/f)
[Software Engineering
Expectations](https://app.asana.com/0/59792373528535/199064865822552)
[Technical Design
Template](https://app.asana.com/0/59792373528535/184709971311943)
[Pull Request
Documentation](https://app.asana.com/0/1202500774821704/1204012835277482/f)

---------

Co-authored-by: Juan Manuel Pereira <[email protected]>
  • Loading branch information
mallexxx and jotaemepereira authored Oct 18, 2024
1 parent 6de0fcc commit 8acf064
Show file tree
Hide file tree
Showing 29 changed files with 694 additions and 134 deletions.
6 changes: 6 additions & 0 deletions DuckDuckGo.xcodeproj/project.pbxproj
Original file line number Diff line number Diff line change
Expand Up @@ -1810,6 +1810,8 @@
843D73BC2C786E5400E4F9DC /* BookmarkListPopover.swift in Sources */ = {isa = PBXBuildFile; fileRef = 84F1C8DA2C774CA900716446 /* BookmarkListPopover.swift */; };
844D7DA42C9443EA00BE61D4 /* NSPrintInfoExtension.swift in Sources */ = {isa = PBXBuildFile; fileRef = 844D7DA32C9443E500BE61D4 /* NSPrintInfoExtension.swift */; };
844D7DA52C9443EA00BE61D4 /* NSPrintInfoExtension.swift in Sources */ = {isa = PBXBuildFile; fileRef = 844D7DA32C9443E500BE61D4 /* NSPrintInfoExtension.swift */; };
84537A032C998C28008723BC /* FireWindowSession.swift in Sources */ = {isa = PBXBuildFile; fileRef = 84537A022C998C24008723BC /* FireWindowSession.swift */; };
84537A042C998C28008723BC /* FireWindowSession.swift in Sources */ = {isa = PBXBuildFile; fileRef = 84537A022C998C24008723BC /* FireWindowSession.swift */; };
84537A082C99C203008723BC /* DeallocationTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = B6C2C9EE276081AB005B7F0A /* DeallocationTests.swift */; };
84537A092C99C203008723BC /* DeallocationTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = B6C2C9EE276081AB005B7F0A /* DeallocationTests.swift */; };
848648A12C76F4B20082282D /* BookmarksBarMenuViewController.swift in Sources */ = {isa = PBXBuildFile; fileRef = 848648A02C76F4B20082282D /* BookmarksBarMenuViewController.swift */; };
Expand Down Expand Up @@ -3897,6 +3899,7 @@
843965112C6F2FFE004C8899 /* NSDragOperationExtension.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = NSDragOperationExtension.swift; sourceTree = "<group>"; };
843965142C737022004C8899 /* NSPasteboardExtension.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = NSPasteboardExtension.swift; sourceTree = "<group>"; };
844D7DA32C9443E500BE61D4 /* NSPrintInfoExtension.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = NSPrintInfoExtension.swift; sourceTree = "<group>"; };
84537A022C998C24008723BC /* FireWindowSession.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = FireWindowSession.swift; sourceTree = "<group>"; };
848648A02C76F4B20082282D /* BookmarksBarMenuViewController.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = BookmarksBarMenuViewController.swift; sourceTree = "<group>"; };
84DC71582C1C1E8A00033B8C /* UserDefaultsWrapperTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = UserDefaultsWrapperTests.swift; sourceTree = "<group>"; };
84DDB9092C92B667008C997B /* WKVisitedLinkStoreWrapper.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = WKVisitedLinkStoreWrapper.swift; sourceTree = "<group>"; };
Expand Down Expand Up @@ -7557,6 +7560,7 @@
AA585DB02490E6FA00E9A3E2 /* MainWindow */ = {
isa = PBXGroup;
children = (
84537A022C998C24008723BC /* FireWindowSession.swift */,
AA7412BC24D2BEEE00D22FE0 /* MainWindow.swift */,
AA7412B424D1536B00D22FE0 /* MainWindowController.swift */,
AA585DAE2490E6E600E9A3E2 /* MainViewController.swift */,
Expand Down Expand Up @@ -10896,6 +10900,7 @@
3706FB62293F65D500E42796 /* CSVParser.swift in Sources */,
31C26A0E2CBE9DFE00FFF462 /* AIChatPreferences.swift in Sources */,
B626A75B29921FAA00053070 /* NavigationActionPolicyExtension.swift in Sources */,
84537A032C998C28008723BC /* FireWindowSession.swift in Sources */,
4B9DB02A2A983B24000927DB /* WaitlistStorage.swift in Sources */,
BB470EBC2C5A66D6002EE91D /* BookmarkManagementDetailViewModel.swift in Sources */,
3706FB65293F65D500E42796 /* PrivacyDashboardWebView.swift in Sources */,
Expand Down Expand Up @@ -12120,6 +12125,7 @@
B60C6F8D29B200AB007BFAA8 /* SavePanelAccessoryView.swift in Sources */,
F1D042992BFBABA100A31506 /* SubscriptionManager+StandardConfiguration.swift in Sources */,
37534CA3281132CB002621E7 /* TabLazyLoaderDataSource.swift in Sources */,
84537A042C998C28008723BC /* FireWindowSession.swift in Sources */,
3158B15C2B0BF76D00AF130C /* DataBrokerProtectionAppEvents.swift in Sources */,
4B723E0E26B0006300E14D75 /* LoginImport.swift in Sources */,
4B9DB03E2A983B24000927DB /* JoinWaitlistView.swift in Sources */,
Expand Down
11 changes: 9 additions & 2 deletions DuckDuckGo/Application/AppDelegate.swift
Original file line number Diff line number Diff line change
Expand Up @@ -457,9 +457,16 @@ final class AppDelegate: NSObject, NSApplicationDelegate {
func applicationShouldTerminate(_ sender: NSApplication) -> NSApplication.TerminateReply {
if !FileDownloadManager.shared.downloads.isEmpty {
// if there‘re downloads without location chosen yet (save dialog should display) - ignore them
if FileDownloadManager.shared.downloads.contains(where: { $0.state.isDownloading }) {
let activeDownloads = Set(FileDownloadManager.shared.downloads.filter { $0.state.isDownloading })
if !activeDownloads.isEmpty {
let alert = NSAlert.activeDownloadsTerminationAlert(for: FileDownloadManager.shared.downloads)
if alert.runModal() == .cancel {
let downloadsFinishedCancellable = FileDownloadManager.observeDownloadsFinished(activeDownloads) {
// close alert and burn the window when all downloads finished
NSApp.stopModal(withCode: .OK)
}
let response = alert.runModal()
downloadsFinishedCancellable.cancel()
if response == .cancel {
return .terminateCancel
}
}
Expand Down
11 changes: 11 additions & 0 deletions DuckDuckGo/Common/Extensions/NSAlertExtension.swift
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,17 @@ import Cocoa

extension NSAlert {

@discardableResult
func addButton(withTitle title: String, response: NSApplication.ModalResponse, keyEquivalent: NSEvent.KeyEquivalent? = nil) -> NSButton {
let button = addButton(withTitle: title)
button.tag = response.rawValue
if let keyEquivalent {
button.keyEquivalent = keyEquivalent.charCode
button.keyEquivalentModifierMask = keyEquivalent.modifierMask
}
return button
}

static func fireproofAlert(with domain: String) -> NSAlert {
let alert = NSAlert()
alert.messageText = UserText.fireproofConfirmationTitle(domain: domain)
Expand Down
7 changes: 7 additions & 0 deletions DuckDuckGo/Common/Extensions/NSEventExtension.swift
Original file line number Diff line number Diff line change
Expand Up @@ -104,6 +104,7 @@ enum KeyEquivalentElement: ExpressibleByStringLiteral, Hashable {
static let tab = KeyEquivalentElement.charCode("\t")
static let left = KeyEquivalentElement.charCode("\u{2190}")
static let right = KeyEquivalentElement.charCode("\u{2192}")
static let escape = KeyEquivalentElement.charCode("\u{1B}")

init(stringLiteral value: String) {
self = .charCode(value)
Expand All @@ -113,6 +114,12 @@ enum KeyEquivalentElement: ExpressibleByStringLiteral, Hashable {
extension NSEvent.KeyEquivalent: ExpressibleByStringLiteral, ExpressibleByUnicodeScalarLiteral, ExpressibleByExtendedGraphemeClusterLiteral {
public typealias StringLiteralType = String

static let backspace: Self = [.backspace]
static let tab: Self = [.tab]
static let left: Self = [.left]
static let right: Self = [.right]
static let escape: Self = [.escape]

public init(stringLiteral value: String) {
self = [.charCode(value)]
}
Expand Down
9 changes: 9 additions & 0 deletions DuckDuckGo/Common/Extensions/StringExtension.swift
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,15 @@ extension String {
return (self.count > length) ? self.prefix(length) + trailing : self
}

func truncated(length: Int, middle: String) -> String {
guard self.count > length else { return self }

let halfLength = length / 2
let start = self.prefix(halfLength).trimmingCharacters(in: .whitespaces)
let end = self.suffix(halfLength).trimmingCharacters(in: .whitespaces)
return "\(start)\(middle)\(end)"
}

func escapedJavaScriptString() -> String {
self.replacingOccurrences(of: "\\", with: "\\\\")
.replacingOccurrences(of: "\"", with: "\\\"")
Expand Down
7 changes: 6 additions & 1 deletion DuckDuckGo/Common/Localizables/UserText.swift
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ struct UserText {
static let neverForThisSite = NSLocalizedString("never.for.this.site", value: "Never Ask for This Site", comment: "Never ask to save login credentials for this site button")
static let open = NSLocalizedString("open", value: "Open", comment: "Open button")
static let close = NSLocalizedString("close", value: "Close", comment: "Close button")
static let dontClose = NSLocalizedString("dont.close", value: "Don’t Close", comment: "Don’t Close the window button title")
static let save = NSLocalizedString("save", value: "Save", comment: "Save button")
static let dontSave = NSLocalizedString("dont.save", value: "Don't Save", comment: "Don't Save button")
static let update = NSLocalizedString("update", value: "Update", comment: "Update button")
Expand Down Expand Up @@ -809,8 +810,12 @@ struct UserText {
static let revealToolTip = NSLocalizedString("downloads.tooltip.reveal", value: "Show in Finder", comment: "Mouse-over tooltip for Show in Finder button")

static let downloadsActiveAlertTitle = NSLocalizedString("downloads.active.alert.title", value: "A download is in progress.", comment: "Alert title when trying to quit application while files are being downloaded")
static let downloadsActiveAlertMessageFormat = NSLocalizedString("downloads.active.alert.message.format", value: "Are you sure you want to quit? DuckDuckGo Privacy Browser is currently downloading “%@”%@. If you quit now DuckDuckGo Privacy Browser wont finish downloading this file.", comment: "Alert text format when trying to quit application while file “filename”[, and others] are being downloaded")
static let downloadsActiveAlertMessageFormat = NSLocalizedString("downloads.active.alert.message.format", value: "Are you sure you want to quit?\n\nDuckDuckGo is currently downloading “%@”%@. If you quit now, DuckDuckGo wont finish downloading %@.", comment: "Alert text format when trying to quit application while file “filename (%@)”[, and others (%@)] are being downloaded; If you quit now, DuckDuckGo won‘t finish downloading [this file|these files](%@).")
static let downloadsActiveAlertMessageAndOthers = NSLocalizedString("downloads.active.alert.message.and.others", value: ", and other files", comment: "Alert text format element for “, and other files”")
static let downloadsActiveAlertMessageThisFile = NSLocalizedString("downloads.active.alert.message.this.file", value: "this file", comment: "Alert text format element for “DuckDuckGo won‘t finish downloading ->this file<-”")
static let downloadsActiveAlertMessageTheseFiles = NSLocalizedString("downloads.active.alert.message.these.files", value: "these files", comment: "Alert text format element for “DuckDuckGo won‘t finish downloading ->these file<-”")

static let downloadsActiveInFireWindowAlertMessageFormat = NSLocalizedString("fire-window.downloads.active.alert.message.format", value: "Are you sure you want to close the Fire Window?\n\nDuckDuckGo is currently downloading “%@”%@. If you close the Fire Window, DuckDuckGo will delete %@.", comment: "Alert text format when trying to close a Fire Window while file “filename (%@)”[, and others (%@)] are being downloaded in it. If you close the Fire Window, DuckDuckGo will delete [this file|these files](%@).")

static let exportLoginsFailedMessage = NSLocalizedString("export.logins.failed.message", value: "Failed to Export Passwords", comment: "Alert title when exporting login data fails")
static let exportLoginsFailedInformative = NSLocalizedString("export.logins.failed.informative", value: "Please check that no file exists at the location you selected.", comment: "Alert message when exporting login data fails")
Expand Down
5 changes: 4 additions & 1 deletion DuckDuckGo/FileDownload/Model/DownloadListItem.swift
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,10 @@ struct DownloadListItem: Equatable {
}
}

let isBurner: Bool
let fireWindowSession: FireWindowSessionRef?
var isBurner: Bool {
fireWindowSession != nil
}

/// final download destination url, will match actual file url when download is finished
var destinationURL: URL? {
Expand Down
16 changes: 10 additions & 6 deletions DuckDuckGo/FileDownload/Model/DownloadListViewModel.swift
Original file line number Diff line number Diff line change
Expand Up @@ -24,16 +24,20 @@ import os.log
@MainActor
final class DownloadListViewModel {

private let fireWindowSession: FireWindowSessionRef?
private let coordinator: DownloadListCoordinator
private var viewModels: [UUID: DownloadViewModel]
private var cancellable: AnyCancellable?

@Published private(set) var items: [DownloadViewModel]

init(coordinator: DownloadListCoordinator = DownloadListCoordinator.shared) {
init(fireWindowSession: FireWindowSessionRef?, coordinator: DownloadListCoordinator = DownloadListCoordinator.shared) {
self.fireWindowSession = fireWindowSession
self.coordinator = coordinator

let items = coordinator.downloads(sortedBy: \.added, ascending: false).map(DownloadViewModel.init)
let items = coordinator.downloads(sortedBy: \.added, ascending: false)
.filter { $0.fireWindowSession == fireWindowSession }
.map(DownloadViewModel.init)
self.items = items
self.viewModels = items.reduce(into: [:]) { $0[$1.id] = $1 }
cancellable = coordinator.updates.receive(on: DispatchQueue.main).sink { [weak self] update in
Expand All @@ -47,22 +51,22 @@ final class DownloadListViewModel {
dispatchPrecondition(condition: .onQueue(.main))
switch kind {
case .added:
guard item.fireWindowSession == self.fireWindowSession else { return }

let viewModel = DownloadViewModel(item: item)
self.viewModels[item.identifier] = viewModel
self.items.insert(viewModel, at: 0)
case .updated:
self.viewModels[item.identifier]?.update(with: item)
case .removed:
guard let index = self.items.firstIndex(where: { $0.id == item.identifier }) else {
return
}
guard let index = self.items.firstIndex(where: { $0.id == item.identifier }) else { return }
self.viewModels[item.identifier] = nil
self.items.remove(at: index)
}
}

func cleanupInactiveDownloads() {
coordinator.cleanupInactiveDownloads()
coordinator.cleanupInactiveDownloads(for: fireWindowSession)
}

func cancelDownload(at index: Int) {
Expand Down
31 changes: 26 additions & 5 deletions DuckDuckGo/FileDownload/Model/FileDownloadManager.swift
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ protocol FileDownloadManagerProtocol: AnyObject {

@discardableResult
@MainActor
func add(_ download: WebKitDownload, fromBurnerWindow: Bool, delegate: DownloadTaskDelegate?, destination: WebKitDownloadTask.DownloadDestination) -> WebKitDownloadTask
func add(_ download: WebKitDownload, fireWindowSession: FireWindowSessionRef?, delegate: DownloadTaskDelegate?, destination: WebKitDownloadTask.DownloadDestination) -> WebKitDownloadTask

func cancelAll(waitUntilDone: Bool)
}
Expand All @@ -38,8 +38,8 @@ extension FileDownloadManagerProtocol {

@discardableResult
@MainActor
func add(_ download: WebKitDownload, fromBurnerWindow: Bool, destination: WebKitDownloadTask.DownloadDestination) -> WebKitDownloadTask {
add(download, fromBurnerWindow: fromBurnerWindow, delegate: nil, destination: destination)
func add(_ download: WebKitDownload, fireWindowSession: FireWindowSessionRef?, destination: WebKitDownloadTask.DownloadDestination) -> WebKitDownloadTask {
add(download, fireWindowSession: fireWindowSession, delegate: nil, destination: destination)
}

}
Expand All @@ -63,15 +63,15 @@ final class FileDownloadManager: FileDownloadManagerProtocol {

@discardableResult
@MainActor
func add(_ download: WebKitDownload, fromBurnerWindow: Bool, delegate: DownloadTaskDelegate?, destination: WebKitDownloadTask.DownloadDestination) -> WebKitDownloadTask {
func add(_ download: WebKitDownload, fireWindowSession: FireWindowSessionRef?, delegate: DownloadTaskDelegate?, destination: WebKitDownloadTask.DownloadDestination) -> WebKitDownloadTask {
dispatchPrecondition(condition: .onQueue(.main))

var destination = destination
// always prompt when "downloading" a local file
if download.originalRequest?.url?.isFileURL ?? true, case .auto = destination {
destination = .prompt
}
let task = WebKitDownloadTask(download: download, destination: destination, isBurner: fromBurnerWindow)
let task = WebKitDownloadTask(download: download, destination: destination, fireWindowSession: fireWindowSession)
Logger.fileDownload.debug("add \(String(describing: download)): \(download.originalRequest?.url?.absoluteString ?? "<nil>") -> \(destination.debugDescription): \(task)")

let shouldCancelDownloadIfDelegateIsGone = delegate != nil
Expand Down Expand Up @@ -247,6 +247,27 @@ extension FileDownloadManager: WebKitDownloadTaskDelegate {

}

extension FileDownloadManager {

static func observeDownloadsFinished(_ downloads: Set<WebKitDownloadTask>, callback: @escaping () -> Void) -> AnyCancellable {
var cancellables = [WebKitDownloadTask: AnyCancellable]()
for download in downloads {
cancellables[download] = download.$state.sink {
if !$0.isDownloading {
cancellables[download] = nil
if cancellables.isEmpty {
callback()
}
}
}
}
return AnyCancellable {
cancellables.removeAll()
}
}

}

protocol DownloadTaskDelegate: AnyObject {

@MainActor
Expand Down
21 changes: 12 additions & 9 deletions DuckDuckGo/FileDownload/Model/WebKitDownloadTask.swift
Original file line number Diff line number Diff line change
Expand Up @@ -100,8 +100,8 @@ final class WebKitDownloadTask: NSObject, ProgressReporting, @unchecked Sendable
}
}

/// downloads initiated from a Burner Window won‘t stay in the Downloads panel after completion
let isBurner: Bool
/// downloads initiated from a Burner Window will be kept in the window
let fireWindowSession: FireWindowSessionRef?

private weak var delegate: WebKitDownloadTaskDelegate?

Expand Down Expand Up @@ -131,12 +131,12 @@ final class WebKitDownloadTask: NSObject, ProgressReporting, @unchecked Sendable
}

@MainActor(unsafe)
init(download: WebKitDownload, destination: DownloadDestination, isBurner: Bool) {
init(download: WebKitDownload, destination: DownloadDestination, fireWindowSession: FireWindowSessionRef?) {
self.download = download
self.progress = DownloadProgress(download: download)
self.fileProgressPresenter = FileProgressPresenter(progress: progress)
self.state = .initial(destination)
self.isBurner = isBurner
self.fireWindowSession = fireWindowSession
super.init()

progress.cancellationHandler = { [weak self, taskDescr=self.debugDescription] in
Expand Down Expand Up @@ -512,12 +512,15 @@ final class WebKitDownloadTask: NSObject, ProgressReporting, @unchecked Sendable
return
}

let tempURL = tempFile.url
// disable retrying download for user-removed/trashed files
let isRetryable = if tempURL == nil || tempURL.map({ !FileManager.default.fileExists(atPath: $0.path) || FileManager.default.isInTrash($0) }) == true {
false
// disable retrying download for user-removed/trashed files or fire windows downloads
let isRetryable: Bool
if let url = tempFile.url {
let fileExists = FileManager.default.fileExists(atPath: url.path)
let isInTrash = FileManager.default.isInTrash(url)
let isFromFireWindow = fireWindowSession != nil
isRetryable = fileExists && !isInTrash && !isFromFireWindow
} else {
true
isRetryable = false
}

Logger.fileDownload.debug("❗️ downloadDidFail \(self): \(error), retryable: \(isRetryable)")
Expand Down
Loading

0 comments on commit 8acf064

Please sign in to comment.