From ee48f4babaeafae4b145f29ce33ac49f5a9c1ea8 Mon Sep 17 00:00:00 2001 From: Nate Weaver Date: Sat, 15 Feb 2020 08:22:51 -0600 Subject: [PATCH 1/3] Remove redundant nil check for homePageURL --- Shared/Favicons/FaviconDownloader.swift | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/Shared/Favicons/FaviconDownloader.swift b/Shared/Favicons/FaviconDownloader.swift index aeb42f960..604db246b 100644 --- a/Shared/Favicons/FaviconDownloader.swift +++ b/Shared/Favicons/FaviconDownloader.swift @@ -177,11 +177,9 @@ final class FaviconDownloader { remainingFaviconURLs[homePageURL] = nil - if let url = singleFaviconDownloader.homePageURL { - if self.homePageToFaviconURLCache[url] == nil { - self.homePageToFaviconURLCache[url] = singleFaviconDownloader.faviconURL - self.homePageToFaviconURLCacheDirty = true - } + if self.homePageToFaviconURLCache[homePageURL] == nil { + self.homePageToFaviconURLCache[homePageURL] = singleFaviconDownloader.faviconURL + self.homePageToFaviconURLCacheDirty = true } postFaviconDidBecomeAvailableNotification(singleFaviconDownloader.faviconURL) From b3f736f899dc0dbcce6dac90455dcf3287c362e2 Mon Sep 17 00:00:00 2001 From: Nate Weaver Date: Sat, 15 Feb 2020 08:22:59 -0600 Subject: [PATCH 2/3] Add a clarification comment --- Shared/Favicons/FaviconDownloader.swift | 1 + 1 file changed, 1 insertion(+) diff --git a/Shared/Favicons/FaviconDownloader.swift b/Shared/Favicons/FaviconDownloader.swift index 604db246b..0e65d4538 100644 --- a/Shared/Favicons/FaviconDownloader.swift +++ b/Shared/Favicons/FaviconDownloader.swift @@ -136,6 +136,7 @@ final class FaviconDownloader { findFaviconURLs(with: url) { (faviconURLs) in if let faviconURLs = faviconURLs { + // If the site explicitly specifies favicon.ico, it will appear twice. self.currentHomePageHasOnlyFaviconICO = faviconURLs.count == 1 if let firstIconURL = faviconURLs.first { From aad1fc4a0c75a54c8a8d459d2d95b9f168b574a8 Mon Sep 17 00:00:00 2001 From: Nate Weaver Date: Sat, 15 Feb 2020 08:53:56 -0600 Subject: [PATCH 3/3] Call the completion handler with nil if faviconURLs is nil Fixes #1791. --- Shared/Favicons/FaviconDownloader.swift | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/Shared/Favicons/FaviconDownloader.swift b/Shared/Favicons/FaviconDownloader.swift index 0e65d4538..25c83bcc1 100644 --- a/Shared/Favicons/FaviconDownloader.swift +++ b/Shared/Favicons/FaviconDownloader.swift @@ -211,21 +211,22 @@ private extension FaviconDownloader { } FaviconURLFinder.findFaviconURLs(with: homePageURL) { (faviconURLs) in + guard var faviconURLs = faviconURLs else { + completion(nil) + return + } + var defaultFaviconURL: String? = nil if let scheme = url.scheme, let host = url.host { defaultFaviconURL = "\(scheme)://\(host)/favicon.ico".lowercased(with: FaviconDownloader.localeForLowercasing) } - if var faviconURLs = faviconURLs { - if let defaultFaviconURL = defaultFaviconURL { - faviconURLs.append(defaultFaviconURL) - } - completion(faviconURLs) - return + if let defaultFaviconURL = defaultFaviconURL { + faviconURLs.append(defaultFaviconURL) } - completion(defaultFaviconURL != nil ? [defaultFaviconURL!] : nil) + completion(faviconURLs) } }