Favicons

DuckDuckGo/AppDelegate/AppDelegate.swift

@@ -79,6 +79,7 @@ final class AppDelegate: NSObject, NSApplicationDelegate {
 
         HTTPSUpgrade.shared.loadDataAsync()
         LocalBookmarkManager.shared.loadBookmarks()
+        FaviconManager.shared.loadFavicons()
         _=ConfigurationManager.shared
         _=DownloadListCoordinator.shared
 

DuckDuckGo/Bookmarks/Model/Bookmark.swift

@@ -80,7 +80,7 @@ internal class BaseBookmarkEntity {
             return Bookmark(id: id,
                             url: url,
                             title: title,
-                            favicon: managedObject.faviconEncrypted as? NSImage,
+                            oldFavicon: managedObject.faviconEncrypted as? NSImage,
                             isFavorite: managedObject.isFavorite,
                             parentFolderUUID: parentFolderUUID)
         }
@@ -134,21 +134,29 @@ final class Bookmark: BaseBookmarkEntity {
     }
 
     let url: URL
-    var favicon: NSImage?
     var isFavorite: Bool
     var parentFolderUUID: UUID?
 
+    // Property oldFavicon can be removed in future updates when favicon cache is built
+    var oldFavicon: NSImage?
+    let faviconManagement: FaviconManagement
+    func favicon(_ sizeCategory: Favicon.SizeCategory) -> NSImage? {
+        return faviconManagement.getCachedFavicon(for: url, sizeCategory: sizeCategory)?.image ?? oldFavicon
+    }
+
     init(id: UUID,
          url: URL,
          title: String,
-         favicon: NSImage? = nil,
+         oldFavicon: NSImage? = nil,
          isFavorite: Bool,
-         parentFolderUUID: UUID? = nil) {
+         parentFolderUUID: UUID? = nil,
+         faviconManagement: FaviconManagement = FaviconManager.shared) {
 
         self.url = url
-        self.favicon = favicon
+        self.oldFavicon = oldFavicon
         self.isFavorite = isFavorite
         self.parentFolderUUID = parentFolderUUID
+        self.faviconManagement = faviconManagement
 
         super.init(id: id, title: title, isFolder: false)
     }
@@ -157,7 +165,7 @@ final class Bookmark: BaseBookmarkEntity {
         self.init(id: bookmark.id,
                   url: newUrl,
                   title: bookmark.title,
-                  favicon: nil,
+                  oldFavicon: nil,
                   isFavorite: bookmark.isFavorite)
     }
 

DuckDuckGo/Bookmarks/Model/BookmarkList.swift

@@ -23,7 +23,7 @@ struct BookmarkList {
 
     var topLevelEntities: [BaseBookmarkEntity] = []
 
-    private var allBookmarkURLsOrdered: [URL]
+    private(set) var allBookmarkURLsOrdered: [URL]
     private var favoriteBookmarkURLsOrdered: [URL]
     private var itemsDict: [URL: Bookmark]
 

DuckDuckGo/Bookmarks/Model/BookmarkManager.swift

@@ -23,6 +23,7 @@ import Combine
 protocol BookmarkManager: AnyObject {
 
     func isUrlBookmarked(url: URL) -> Bool
+    func isHostInBookmarks(host: String) -> Bool
     func getBookmark(for url: URL) -> Bookmark?
     @discardableResult func makeBookmark(for url: URL, title: String, isFavorite: Bool) -> Bookmark?
     @discardableResult func makeFolder(for title: String, parent: BookmarkFolder?) -> BookmarkFolder
@@ -45,22 +46,18 @@ final class LocalBookmarkManager: BookmarkManager {
 
     static let shared = LocalBookmarkManager()
 
-    private init() {
-        subscribeToCachedFavicons()
-    }
+    private init() {}
 
-    init(bookmarkStore: BookmarkStore, faviconService: FaviconService) {
+    init(bookmarkStore: BookmarkStore, faviconManagement: FaviconManagement) {
         self.bookmarkStore = bookmarkStore
-        self.faviconService = faviconService
-
-        subscribeToCachedFavicons()
+        self.faviconManagement = faviconManagement
     }
 
     @Published private(set) var list: BookmarkList?
     var listPublisher: Published<BookmarkList?>.Publisher { $list }
 
     private lazy var bookmarkStore: BookmarkStore = LocalBookmarkStore()
-    private lazy var faviconService: FaviconService = LocalFaviconService.shared
+    private lazy var faviconManagement: FaviconManagement = FaviconManager.shared
 
     // MARK: - Bookmarks
 
@@ -86,6 +83,12 @@ final class LocalBookmarkManager: BookmarkManager {
         return list?[url] != nil
     }
 
+    func isHostInBookmarks(host: String) -> Bool {
+        return list?.allBookmarkURLsOrdered.contains(where: { url in
+            url.host == host
+        }) ?? false
+    }
+
     func getBookmark(for url: URL) -> Bookmark? {
         return list?[url]
     }
@@ -99,7 +102,7 @@ final class LocalBookmarkManager: BookmarkManager {
         }
 
         let id = UUID()
-        let bookmark = Bookmark(id: id, url: url, title: title, favicon: favicon(for: url.host), isFavorite: isFavorite)
+        let bookmark = Bookmark(id: id, url: url, title: title, isFavorite: isFavorite)
 
         list?.insert(bookmark)
         bookmarkStore.save(bookmark: bookmark, parent: nil) { [weak self] success, _  in
@@ -201,32 +204,9 @@ final class LocalBookmarkManager: BookmarkManager {
 
     // MARK: - Favicons
 
-    private var faviconCancellable: AnyCancellable?
-
-    private func subscribeToCachedFavicons() {
-        faviconCancellable = faviconService.cachedFaviconsPublisher
-            .sink(receiveValue: { [weak self] (host, favicon) in
-                self?.update(favicon: favicon, for: host)
-            })
-    }
-
-    private func update(favicon: NSImage, for host: String) {
-        guard let bookmarks = list?.bookmarks() else { return }
-
-        bookmarks
-            .filter { $0.url.host == host &&
-                $0.favicon?.size.isSmaller(than: favicon.size) ?? true
-            }
-            .forEach {
-                let bookmark = $0
-                bookmark.favicon = favicon
-                update(bookmark: bookmark)
-            }
-    }
-
     private func favicon(for host: String?) -> NSImage? {
         if let host = host {
-            return faviconService.getCachedFavicon(for: host, mustBeFromUserScript: false)
+            return faviconManagement.getCachedFavicon(for: host, sizeCategory: .small)?.image
         }
 
         return nil

DuckDuckGo/Bookmarks/Services/BookmarkStore.swift

@@ -122,7 +122,6 @@ final class LocalBookmarkStore: BookmarkStore {
             bookmarkMO.id = bookmark.id
             bookmarkMO.urlEncrypted = bookmark.url as NSURL?
             bookmarkMO.titleEncrypted = bookmark.title as NSString
-            bookmarkMO.faviconEncrypted = bookmark.favicon
             bookmarkMO.isFavorite = bookmark.isFavorite
             bookmarkMO.isFolder = bookmark.isFolder
             bookmarkMO.dateAdded = NSDate.now
@@ -485,7 +484,6 @@ fileprivate extension BookmarkManagedObject {
         id = bookmark.id
         urlEncrypted = bookmark.url as NSURL?
         titleEncrypted = bookmark.title as NSString
-        faviconEncrypted = bookmark.favicon
         isFavorite = bookmark.isFavorite
         isFolder = false
     }

DuckDuckGo/Bookmarks/View/BookmarkListViewController.swift

@@ -82,6 +82,12 @@ final class BookmarkListViewController: NSViewController {
         }.store(in: &cancellables)
     }
 
+    override func viewWillAppear() {
+        super.viewWillAppear()
+
+        reloadData()
+    }
+
     private func reloadData() {
         let selectedNodes = self.selectedNodes
 

DuckDuckGo/Bookmarks/View/BookmarkManagementSidebarViewController.swift

@@ -105,6 +105,8 @@ final class BookmarkManagementSidebarViewController: NSViewController {
 
     override func viewWillAppear() {
         super.viewWillAppear()
+        reloadData()
+
         tabSwitcherButton.select(tabType: .bookmarks)
     }
 

DuckDuckGo/Bookmarks/View/BookmarkOutlineViewCell.swift

@@ -48,7 +48,7 @@ final class BookmarkOutlineViewCell: NSTableCellView {
     private func commonInit() {}
 
     func update(from bookmark: Bookmark) {
-        faviconImageView.image = bookmark.favicon ?? Self.defaultBookmarkFavicon
+        faviconImageView.image = bookmark.favicon(.small) ?? Self.defaultBookmarkFavicon
         titleLabel.stringValue = bookmark.title
         countLabel.stringValue = ""
     }

DuckDuckGo/Bookmarks/View/BookmarkTableCellView.swift

@@ -169,7 +169,7 @@ final class BookmarkTableCellView: NSTableCellView, NibLoadable {
     func update(from bookmark: Bookmark) {
         self.entity = bookmark
 
-        faviconImageView.image = bookmark.favicon ?? Self.defaultBookmarkFavicon
+        faviconImageView.image = bookmark.favicon(.small)
         accessoryImageView.image = bookmark.isFavorite ? Self.favoriteAccessoryViewImage : nil
         favoriteButton.image = bookmark.isFavorite ? Self.favoriteFilledAccessoryViewImage : Self.favoriteAccessoryViewImage
         primaryTitleLabelValue = bookmark.title

DuckDuckGo/Bookmarks/ViewModel/BookmarkViewModel.swift

@@ -47,7 +47,7 @@ struct BookmarkViewModel {
         // bookmark.isFavorite ? bookmark.favicon?.makeFavoriteOverlay() : bookmark.favicon
 
         if let bookmark = entity as? Bookmark {
-            let favicon = bookmark.favicon?.copy() as? NSImage
+            let favicon = bookmark.favicon(.small)?.copy() as? NSImage
             favicon?.size = NSSize.faviconSize
             return favicon
         } else if entity is BookmarkFolder {

DuckDuckGo/BrowserTab/Model/Tab.swift

@@ -90,7 +90,7 @@ final class Tab: NSObject {
     weak var delegate: TabDelegate?
 
     init(content: TabContent,
-         faviconService: FaviconService = LocalFaviconService.shared,
+         faviconManagement: FaviconManagement = FaviconManager.shared,
          webCacheManager: WebCacheManager = WebCacheManager.shared,
          webViewConfiguration: WebViewConfiguration? = nil,
          historyCoordinating: HistoryCoordinating = HistoryCoordinator.shared,
@@ -105,7 +105,7 @@ final class Tab: NSObject {
          canBeClosedWithBack: Bool = false) {
 
         self.content = content
-        self.faviconService = faviconService
+        self.faviconManagement = faviconManagement
         self.historyCoordinating = historyCoordinating
         self.scriptsSource = scriptsSource
         self.visitedDomains = visitedDomains
@@ -125,12 +125,6 @@ final class Tab: NSObject {
         super.init()
 
         setupWebView(shouldLoadInBackground: shouldLoadInBackground)
-
-        // cache session-restored favicon if present
-        if let favicon = favicon,
-           let host = content.url?.host {
-            faviconService.cacheIfNeeded(favicon: favicon, for: host, isFromUserScript: false)
-        }
     }
 
     deinit {
@@ -382,32 +376,22 @@ final class Tab: NSObject {
     // MARK: - Favicon
 
     @Published var favicon: NSImage?
-    let faviconService: FaviconService
+    let faviconManagement: FaviconManagement
 
     private func handleFavicon(oldContent: TabContent) {
-        if !content.isUrl {
-            favicon = nil
-        }
-        if oldContent.url?.host != content.url?.host {
-            fetchFavicon(nil, for: content.url?.host, isFromUserScript: false)
-        }
-    }
+        guard faviconManagement.areFaviconsLoaded else { return }
 
-    private func fetchFavicon(_ faviconURL: URL?, for host: String?, isFromUserScript: Bool) {
-        if favicon != nil {
+        guard content.isUrl, let url = content.url else {
             favicon = nil
-        }
-
-        guard let host = host else {
             return
         }
 
-        faviconService.fetchFavicon(faviconURL, for: host, isFromUserScript: isFromUserScript) { (image, error) in
-            guard error == nil, let image = image else {
-                return
+        if let cachedFavicon = faviconManagement.getCachedFavicon(for: url, sizeCategory: .small)?.image {
+            if cachedFavicon != favicon {
+                favicon = cachedFavicon
             }
-
-            self.favicon = image
+        } else {
+            favicon = nil
         }
     }
 
@@ -588,20 +572,14 @@ extension Tab: ContextMenuDelegate {
 
 extension Tab: FaviconUserScriptDelegate {
 
-    func faviconUserScript(_ faviconUserScript: FaviconUserScript, didFindFavicon faviconUrl: URL) {
-        guard let host = self.content.url?.host else {
-            return
-        }
-
-        faviconService.fetchFavicon(faviconUrl, for: host, isFromUserScript: true) { (image, error) in
-            guard host == self.content.url?.host else {
-                return
-            }
-            guard error == nil, let image = image else {
+    func faviconUserScript(_ faviconUserScript: FaviconUserScript,
+                           didFindFaviconLinks faviconLinks: [FaviconUserScript.FaviconLink],
+                           for documentUrl: URL) {
+        faviconManagement.handleFaviconLinks(faviconLinks, documentUrl: documentUrl) { favicon in
+            guard documentUrl == self.content.url, let favicon = favicon else {
                 return
             }
-
-            self.favicon = image
+            self.favicon = favicon.image
         }
     }