From 9d2c8f000f47e07dafdb95e90827dfa3580732e6 Mon Sep 17 00:00:00 2001 From: Brent Simmons Date: Sat, 8 May 2021 12:42:44 -0700 Subject: [PATCH] Create and use IconImageCache. It centralizes and de-dupes logic for getting feed/article images, and it caches the results, which helps performance. --- .../WebFeedInspectorViewController.swift | 13 +- .../Sidebar/SidebarViewController.swift | 2 +- .../Timeline/TimelineViewController.swift | 23 +--- .../Shared/Images/FeedIconImageLoader.swift | 16 +-- NetNewsWire.xcodeproj/project.pbxproj | 8 ++ Shared/Activity/ActivityManager.swift | 7 +- Shared/Extensions/ArticleUtilities.swift | 27 +--- Shared/IconImageCache.swift | 129 ++++++++++++++++++ iOS/AppDelegate.swift | 4 + .../WebFeedInspectorViewController.swift | 10 +- iOS/MasterFeed/MasterFeedViewController.swift | 77 +---------- iOS/SceneCoordinator.swift | 18 +-- 12 files changed, 160 insertions(+), 174 deletions(-) create mode 100644 Shared/IconImageCache.swift diff --git a/Mac/Inspector/WebFeedInspectorViewController.swift b/Mac/Inspector/WebFeedInspectorViewController.swift index d03fd135f..fb8d8ce75 100644 --- a/Mac/Inspector/WebFeedInspectorViewController.swift +++ b/Mac/Inspector/WebFeedInspectorViewController.swift @@ -146,18 +146,7 @@ private extension WebFeedInspectorViewController { guard let feed = feed, let iconView = iconView else { return } - - if let feedIcon = appDelegate.webFeedIconDownloader.icon(for: feed) { - iconView.iconImage = feedIcon - return - } - - if let favicon = appDelegate.faviconDownloader.favicon(for: feed) { - iconView.iconImage = favicon - return - } - - iconView.iconImage = feed.smallIcon + iconView.iconImage = IconImageCache.shared.imageForFeed(feed) } func updateName() { diff --git a/Mac/MainWindow/Sidebar/SidebarViewController.swift b/Mac/MainWindow/Sidebar/SidebarViewController.swift index 277be185e..0e89fc345 100644 --- a/Mac/MainWindow/Sidebar/SidebarViewController.swift +++ b/Mac/MainWindow/Sidebar/SidebarViewController.swift @@ -763,7 +763,7 @@ private extension SidebarViewController { } func imageFor(_ node: Node) -> IconImage? { - if let feed = node.representedObject as? WebFeed, let feedIcon = appDelegate.webFeedIconDownloader.icon(for: feed) { + if let feed = node.representedObject as? WebFeed, let feedIcon = IconImageCache.shared.imageForFeed(feed) { return feedIcon } if let smallIconProvider = node.representedObject as? SmallIconProvider { diff --git a/Mac/MainWindow/Timeline/TimelineViewController.swift b/Mac/MainWindow/Timeline/TimelineViewController.swift index 115a921f9..28f2b8729 100644 --- a/Mac/MainWindow/Timeline/TimelineViewController.swift +++ b/Mac/MainWindow/Timeline/TimelineViewController.swift @@ -886,28 +886,7 @@ extension TimelineViewController: NSTableViewDelegate { if !showIcons { return nil } - - if let authors = article.authors { - for author in authors { - if let image = avatarForAuthor(author) { - return image - } - } - } - - guard let feed = article.webFeed else { - return nil - } - - if let feedIcon = appDelegate.webFeedIconDownloader.icon(for: feed) { - return feedIcon - } - - if let favicon = appDelegate.faviconDownloader.faviconAsIcon(for: feed) { - return favicon - } - - return FaviconGenerator.favicon(feed) + return IconImageCache.shared.imageForArticle(article) } private func avatarForAuthor(_ author: Author) -> IconImage? { diff --git a/Multiplatform/Shared/Images/FeedIconImageLoader.swift b/Multiplatform/Shared/Images/FeedIconImageLoader.swift index 9ee4d4bef..d4c041bd5 100644 --- a/Multiplatform/Shared/Images/FeedIconImageLoader.swift +++ b/Multiplatform/Shared/Images/FeedIconImageLoader.swift @@ -41,20 +41,6 @@ final class FeedIconImageLoader: ObservableObject { private extension FeedIconImageLoader { func fetchImage() { - if let webFeed = feed as? WebFeed { - if let feedIconImage = appDelegate.webFeedIconDownloader.icon(for: webFeed) { - image = feedIconImage - return - } - if let faviconImage = appDelegate.faviconDownloader.faviconAsIcon(for: webFeed) { - image = faviconImage - return - } - } - - if let smallIconProvider = feed as? SmallIconProvider { - image = smallIconProvider.smallIcon - } + image = IconImageCache.shared.imageForFeed(feed) } - } diff --git a/NetNewsWire.xcodeproj/project.pbxproj b/NetNewsWire.xcodeproj/project.pbxproj index dc761f156..6ddaf69ac 100644 --- a/NetNewsWire.xcodeproj/project.pbxproj +++ b/NetNewsWire.xcodeproj/project.pbxproj @@ -1011,6 +1011,9 @@ 844B5B691FEA20DF00C7C76A /* SidebarKeyboardShortcuts.plist in Resources */ = {isa = PBXBuildFile; fileRef = 844B5B681FEA20DF00C7C76A /* SidebarKeyboardShortcuts.plist */; }; 845213231FCA5B11003B6E93 /* ImageDownloader.swift in Sources */ = {isa = PBXBuildFile; fileRef = 845213221FCA5B10003B6E93 /* ImageDownloader.swift */; }; 845479881FEB77C000AD8B59 /* TimelineKeyboardShortcuts.plist in Resources */ = {isa = PBXBuildFile; fileRef = 845479871FEB77C000AD8B59 /* TimelineKeyboardShortcuts.plist */; }; + 8454C3F3263F2D8700E3F9C7 /* IconImageCache.swift in Sources */ = {isa = PBXBuildFile; fileRef = 8454C3F2263F2D8700E3F9C7 /* IconImageCache.swift */; }; + 8454C3F8263F3AD400E3F9C7 /* IconImageCache.swift in Sources */ = {isa = PBXBuildFile; fileRef = 8454C3F2263F2D8700E3F9C7 /* IconImageCache.swift */; }; + 8454C3FD263F3AD600E3F9C7 /* IconImageCache.swift in Sources */ = {isa = PBXBuildFile; fileRef = 8454C3F2263F2D8700E3F9C7 /* IconImageCache.swift */; }; 845A29091FC74B8E007B49E3 /* SingleFaviconDownloader.swift in Sources */ = {isa = PBXBuildFile; fileRef = 845A29081FC74B8E007B49E3 /* SingleFaviconDownloader.swift */; }; 845A29221FC9251E007B49E3 /* SidebarCellLayout.swift in Sources */ = {isa = PBXBuildFile; fileRef = 845A29211FC9251E007B49E3 /* SidebarCellLayout.swift */; }; 845A29241FC9255E007B49E3 /* SidebarCellAppearance.swift in Sources */ = {isa = PBXBuildFile; fileRef = 845A29231FC9255E007B49E3 /* SidebarCellAppearance.swift */; }; @@ -1885,6 +1888,7 @@ 844B5B681FEA20DF00C7C76A /* SidebarKeyboardShortcuts.plist */ = {isa = PBXFileReference; lastKnownFileType = text.plist.xml; path = SidebarKeyboardShortcuts.plist; sourceTree = ""; }; 845213221FCA5B10003B6E93 /* ImageDownloader.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = ImageDownloader.swift; sourceTree = ""; }; 845479871FEB77C000AD8B59 /* TimelineKeyboardShortcuts.plist */ = {isa = PBXFileReference; lastKnownFileType = text.plist.xml; path = TimelineKeyboardShortcuts.plist; sourceTree = ""; }; + 8454C3F2263F2D8700E3F9C7 /* IconImageCache.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = IconImageCache.swift; sourceTree = ""; }; 845A29081FC74B8E007B49E3 /* SingleFaviconDownloader.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = SingleFaviconDownloader.swift; sourceTree = ""; }; 845A29211FC9251E007B49E3 /* SidebarCellLayout.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = SidebarCellLayout.swift; sourceTree = ""; }; 845A29231FC9255E007B49E3 /* SidebarCellAppearance.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = SidebarCellAppearance.swift; sourceTree = ""; }; @@ -3418,6 +3422,7 @@ 842E45CD1ED8C308000A8B52 /* AppNotifications.swift */, 51C4CFEF24D37D1F00AF9874 /* Secrets.swift */, 511B9805237DCAC90028BCAA /* UserInfoKey.swift */, + 8454C3F2263F2D8700E3F9C7 /* IconImageCache.swift */, 51C452AD2265102800C03939 /* Timeline */, 84702AB31FA27AE8006B8943 /* Commands */, 51934CCC231078DC006127BE /* Activity */, @@ -5127,6 +5132,7 @@ 65ED4007235DEF6C0081F399 /* AddFeedController.swift in Sources */, 65ED4008235DEF6C0081F399 /* AccountRefreshTimer.swift in Sources */, 65ED4009235DEF6C0081F399 /* SidebarStatusBarView.swift in Sources */, + 8454C3FD263F3AD600E3F9C7 /* IconImageCache.swift in Sources */, 65ED400A235DEF6C0081F399 /* SearchTimelineFeedDelegate.swift in Sources */, 65ED400B235DEF6C0081F399 /* TodayFeedDelegate.swift in Sources */, 65ED400C235DEF6C0081F399 /* FolderInspectorViewController.swift in Sources */, @@ -5352,6 +5358,7 @@ 516AE9B32371C372007DEEAA /* MasterFeedTableViewSectionHeaderLayout.swift in Sources */, 51DC370B2405BC9A0095D371 /* PreloadedWebView.swift in Sources */, D3555BF524664566005E48C3 /* ArticleSearchBar.swift in Sources */, + 8454C3F3263F2D8700E3F9C7 /* IconImageCache.swift in Sources */, B24E9ADE245AB88400DA5718 /* NSAttributedString+NetNewsWire.swift in Sources */, C5A6ED5223C9AF4300AB6BE2 /* TitleActivityItemSource.swift in Sources */, 51DC37092402F1470095D371 /* MasterFeedDataSourceOperation.swift in Sources */, @@ -5522,6 +5529,7 @@ 849A97771ED9EC04007D329B /* TimelineCellData.swift in Sources */, 841ABA6020145EC100980E11 /* BuiltinSmartFeedInspectorViewController.swift in Sources */, D5E4CC54202C1361009B4FFC /* AppDelegate+Scriptability.swift in Sources */, + 8454C3F8263F3AD400E3F9C7 /* IconImageCache.swift in Sources */, 518651B223555EB20078E021 /* NNW3Document.swift in Sources */, D5F4EDB5200744A700B9E363 /* ScriptingObject.swift in Sources */, D5F4EDB920074D7C00B9E363 /* Folder+Scriptability.swift in Sources */, diff --git a/Shared/Activity/ActivityManager.swift b/Shared/Activity/ActivityManager.swift index 300aaa255..266911e5d 100644 --- a/Shared/Activity/ActivityManager.swift +++ b/Shared/Activity/ActivityManager.swift @@ -248,12 +248,11 @@ private extension ActivityManager { attributeSet.title = feed.nameForDisplay attributeSet.keywords = makeKeywords(feed.nameForDisplay) attributeSet.relatedUniqueIdentifier = ActivityManager.identifer(for: feed) - if let iconImage = appDelegate.webFeedIconDownloader.icon(for: feed) { - attributeSet.thumbnailData = iconImage.image.dataRepresentation() - } else if let iconImage = appDelegate.faviconDownloader.faviconAsIcon(for: feed) { + + if let iconImage = IconImageCache.shared.imageForFeed(feed) { attributeSet.thumbnailData = iconImage.image.dataRepresentation() } - + selectingActivity!.contentAttributeSet = attributeSet selectingActivity!.needsSave = true diff --git a/Shared/Extensions/ArticleUtilities.swift b/Shared/Extensions/ArticleUtilities.swift index 1ad6a0f6f..ace388e04 100644 --- a/Shared/Extensions/ArticleUtilities.swift +++ b/Shared/Extensions/ArticleUtilities.swift @@ -96,32 +96,7 @@ extension Article { } func iconImage() -> IconImage? { - if let authors = authors, authors.count == 1, let author = authors.first { - if let image = appDelegate.authorAvatarDownloader.image(for: author) { - return image - } - } - - if let authors = webFeed?.authors, authors.count == 1, let author = authors.first { - if let image = appDelegate.authorAvatarDownloader.image(for: author) { - return image - } - } - - guard let webFeed = webFeed else { - return nil - } - - let feedIconImage = appDelegate.webFeedIconDownloader.icon(for: webFeed) - if feedIconImage != nil { - return feedIconImage - } - - if let faviconImage = appDelegate.faviconDownloader.faviconAsIcon(for: webFeed) { - return faviconImage - } - - return FaviconGenerator.favicon(webFeed) + return IconImageCache.shared.imageForArticle(self) } func iconImageUrl(webFeed: WebFeed) -> URL? { diff --git a/Shared/IconImageCache.swift b/Shared/IconImageCache.swift new file mode 100644 index 000000000..1a7bc1826 --- /dev/null +++ b/Shared/IconImageCache.swift @@ -0,0 +1,129 @@ +// +// IconImageCache.swift +// NetNewsWire-iOS +// +// Created by Brent Simmons on 5/2/21. +// Copyright © 2021 Ranchero Software. All rights reserved. +// + +import Foundation +import Account +import Articles + +class IconImageCache { + + static var shared = IconImageCache() + + private var smartFeedIconImageCache = [FeedIdentifier: IconImage]() + private var webFeedIconImageCache = [FeedIdentifier: IconImage]() + private var faviconImageCache = [FeedIdentifier: IconImage]() + private var smallIconImageCache = [FeedIdentifier: IconImage]() + private var authorIconImageCache = [Author: IconImage]() + + func imageFor(_ feedID: FeedIdentifier) -> IconImage? { + if let smartFeed = SmartFeedsController.shared.find(by: feedID) { + return imageForFeed(smartFeed) + } + if let feed = AccountManager.shared.existingFeed(with: feedID) { + return imageForFeed(feed) + } + return nil + } + + func imageForFeed(_ feed: Feed) -> IconImage? { + guard let feedID = feed.feedID else { + return nil + } + + if let smartFeed = feed as? PseudoFeed { + return imageForSmartFeed(smartFeed, feedID) + } + if let webFeed = feed as? WebFeed, let iconImage = imageForWebFeed(webFeed, feedID) { + return iconImage + } + if let smallIconProvider = feed as? SmallIconProvider { + return imageForSmallIconProvider(smallIconProvider, feedID) + } + + return nil + } + + func imageForArticle(_ article: Article) -> IconImage? { + if let iconImage = imageForAuthors(article.authors) { + return iconImage + } + guard let feed = article.webFeed else { + return nil + } + return imageForFeed(feed) + } + + func emptyCache() { + smartFeedIconImageCache = [FeedIdentifier: IconImage]() + webFeedIconImageCache = [FeedIdentifier: IconImage]() + faviconImageCache = [FeedIdentifier: IconImage]() + smallIconImageCache = [FeedIdentifier: IconImage]() + authorIconImageCache = [Author: IconImage]() + } +} + +private extension IconImageCache { + + func imageForSmartFeed(_ smartFeed: PseudoFeed, _ feedID: FeedIdentifier) -> IconImage? { + if let iconImage = smartFeedIconImageCache[feedID] { + return iconImage + } + if let iconImage = smartFeed.smallIcon { + smartFeedIconImageCache[feedID] = iconImage + return iconImage + } + return nil + } + + func imageForWebFeed(_ webFeed: WebFeed, _ feedID: FeedIdentifier) -> IconImage? { + if let iconImage = webFeedIconImageCache[feedID] { + return iconImage + } + if let iconImage = appDelegate.webFeedIconDownloader.icon(for: webFeed) { + webFeedIconImageCache[feedID] = iconImage + return iconImage + } + if let faviconImage = faviconImageCache[feedID] { + return faviconImage + } + if let faviconImage = appDelegate.faviconDownloader.faviconAsIcon(for: webFeed) { + faviconImageCache[feedID] = faviconImage + return faviconImage + } + return nil + } + + func imageForSmallIconProvider(_ provider: SmallIconProvider, _ feedID: FeedIdentifier) -> IconImage? { + if let iconImage = smallIconImageCache[feedID] { + return iconImage + } + if let iconImage = provider.smallIcon { + smallIconImageCache[feedID] = iconImage + return iconImage + } + return nil + } + + func imageForAuthors(_ authors: Set?) -> IconImage? { + guard let authors = authors, authors.count == 1, let author = authors.first else { + return nil + } + return imageForAuthor(author) + } + + func imageForAuthor(_ author: Author) -> IconImage? { + if let iconImage = authorIconImageCache[author] { + return iconImage + } + if let iconImage = appDelegate.authorAvatarDownloader.image(for: author) { + authorIconImageCache[author] = iconImage + return iconImage + } + return nil + } +} diff --git a/iOS/AppDelegate.swift b/iOS/AppDelegate.swift index 6e1bab493..d4fa54768 100644 --- a/iOS/AppDelegate.swift +++ b/iOS/AppDelegate.swift @@ -132,6 +132,10 @@ class AppDelegate: UIResponder, UIApplicationDelegate, UNUserNotificationCenterD func applicationWillTerminate(_ application: UIApplication) { shuttingDown = true } + + func applicationDidEnterBackground(_ application: UIApplication) { + IconImageCache.shared.emptyCache() + } // MARK: Notifications diff --git a/iOS/Inspector/WebFeedInspectorViewController.swift b/iOS/Inspector/WebFeedInspectorViewController.swift index c52bfeedf..4716f575d 100644 --- a/iOS/Inspector/WebFeedInspectorViewController.swift +++ b/iOS/Inspector/WebFeedInspectorViewController.swift @@ -23,14 +23,8 @@ class WebFeedInspectorViewController: UITableViewController { @IBOutlet weak var feedURLLabel: InteractiveLabel! private var headerView: InspectorIconHeaderView? - private var iconImage: IconImage { - if let feedIcon = appDelegate.webFeedIconDownloader.icon(for: webFeed) { - return feedIcon - } - if let favicon = appDelegate.faviconDownloader.faviconAsIcon(for: webFeed) { - return favicon - } - return FaviconGenerator.favicon(webFeed) + private var iconImage: IconImage? { + return IconImageCache.shared.imageForFeed(webFeed) } private let homePageIndexPath = IndexPath(row: 0, section: 1) diff --git a/iOS/MasterFeed/MasterFeedViewController.swift b/iOS/MasterFeed/MasterFeedViewController.swift index bf9e5cf45..8687fe012 100644 --- a/iOS/MasterFeed/MasterFeedViewController.swift +++ b/iOS/MasterFeed/MasterFeedViewController.swift @@ -42,11 +42,6 @@ class MasterFeedViewController: UITableViewController, UndoableCommandRunner { return true } - private var smartFeedIconImageCache = [FeedIdentifier: IconImage]() - private var webFeedIconImageCache = [FeedIdentifier: IconImage]() - private var faviconImageCache = [FeedIdentifier: IconImage]() - private var smallIconImageCache = [FeedIdentifier: IconImage]() - override func viewDidLoad() { super.viewDidLoad() @@ -91,12 +86,7 @@ class MasterFeedViewController: UITableViewController, UndoableCommandRunner { } override func traitCollectionDidChange(_ previousTraitCollection: UITraitCollection?) { - // Empty IconImage caches - smartFeedIconImageCache = [FeedIdentifier: IconImage]() - webFeedIconImageCache = [FeedIdentifier: IconImage]() - faviconImageCache = [FeedIdentifier: IconImage]() - smallIconImageCache = [FeedIdentifier: IconImage]() - + IconImageCache.shared.emptyCache() super.traitCollectionDidChange(previousTraitCollection) reloadAllVisibleCells() } @@ -858,69 +848,12 @@ private extension MasterFeedViewController { } func configureIcon(_ cell: MasterFeedTableViewCell, _ identifier: MasterFeedTableViewIdentifier) { - cell.iconImage = imageFor(identifier) + guard let feedID = identifier.feedID else { + return + } + cell.iconImage = IconImageCache.shared.imageFor(feedID) } - func imageFor(_ identifier: MasterFeedTableViewIdentifier) -> IconImage? { - guard let feedID = identifier.feedID else { return nil } - - if let smartFeed = SmartFeedsController.shared.find(by: feedID) { - return imageForSmartFeed(smartFeed, feedID) - } - - guard let feed = AccountManager.shared.existingFeed(with: feedID) else { return nil } - - if let webFeed = feed as? WebFeed, let iconImage = imageForWebFeed(webFeed, feedID) { - return iconImage - } - - if let smallIconProvider = feed as? SmallIconProvider, let iconImage = imageForSmallIconProvider(smallIconProvider, feedID) { - return iconImage - } - - return nil - } - - func imageForSmartFeed(_ smartFeed: PseudoFeed, _ feedID: FeedIdentifier) -> IconImage? { - if let iconImage = smartFeedIconImageCache[feedID] { - return iconImage - } - if let iconImage = smartFeed.smallIcon { - smartFeedIconImageCache[feedID] = iconImage - return iconImage - } - return nil - } - - func imageForWebFeed(_ webFeed: WebFeed, _ feedID: FeedIdentifier) -> IconImage? { - if let iconImage = webFeedIconImageCache[feedID] { - return iconImage - } - if let iconImage = appDelegate.webFeedIconDownloader.icon(for: webFeed) { - webFeedIconImageCache[feedID] = iconImage - return iconImage - } - if let faviconImage = faviconImageCache[feedID] { - return faviconImage - } - if let faviconImage = appDelegate.faviconDownloader.faviconAsIcon(for: webFeed) { - faviconImageCache[feedID] = faviconImage - return faviconImage - } - return nil - } - - func imageForSmallIconProvider(_ provider: SmallIconProvider, _ feedID: FeedIdentifier) -> IconImage? { - if let iconImage = smallIconImageCache[feedID] { - return iconImage - } - if let iconImage = provider.smallIcon { - smallIconImageCache[feedID] = iconImage - return iconImage - } - return nil - } - func nameFor(_ node: Node) -> String { if let displayNameProvider = node.representedObject as? DisplayNameProvider { return displayNameProvider.nameForDisplay diff --git a/iOS/SceneCoordinator.swift b/iOS/SceneCoordinator.swift index c273d2354..edb8f31e3 100644 --- a/iOS/SceneCoordinator.swift +++ b/iOS/SceneCoordinator.swift @@ -146,22 +146,12 @@ class SceneCoordinator: NSObject, UndoableCommandRunner, UnreadCountProvider { // At some point we should refactor the current Feed IndexPath out and only use the timeline feed private(set) var currentFeedIndexPath: IndexPath? - + var timelineIconImage: IconImage? { - if let feed = timelineFeed as? WebFeed { - - let feedIconImage = appDelegate.webFeedIconDownloader.icon(for: feed) - if feedIconImage != nil { - return feedIconImage - } - - if let faviconIconImage = appDelegate.faviconDownloader.faviconAsIcon(for: feed) { - return faviconIconImage - } - + guard let timelineFeed = timelineFeed else { + return nil } - - return (timelineFeed as? SmallIconProvider)?.smallIcon + return IconImageCache.shared.imageForFeed(timelineFeed) } private var exceptionArticleFetcher: ArticleFetcher?