diff --git a/Client/Frontend/Browser/Favicons/FaviconFetcher.swift b/Client/Frontend/Browser/Favicons/FaviconFetcher.swift index b7289a7fa71..6ab0f5163f5 100644 --- a/Client/Frontend/Browser/Favicons/FaviconFetcher.swift +++ b/Client/Frontend/Browser/Favicons/FaviconFetcher.swift @@ -258,7 +258,7 @@ class FaviconFetcher { downloadIcon(url: url, addingToDatabase: false) { [weak self] image in guard let self = self else { return } if let image = image { - self.isIconBackgroundTransparentAroundEdges(image) { isTransparent in + Self.isIconBackgroundTransparentAroundEdges(image) { isTransparent in completion(self.url, FaviconAttributes(image: image, includePadding: isTransparent)) } } else { @@ -410,12 +410,21 @@ class FaviconFetcher { /// Determines if the downloaded image should be padded because its edges /// are for the most part transparent - private func isIconBackgroundTransparentAroundEdges(_ icon: UIImage, completion: @escaping (_ isTransparent: Bool) -> Void) { + static func isIconBackgroundTransparentAroundEdges(_ icon: UIImage, completion: @escaping (_ isTransparent: Bool) -> Void) { + if icon.size.width.isZero || icon.size.height.isZero { + DispatchQueue.main.async { + completion(false) + } + return + } DispatchQueue.global(qos: .utility).async { guard let cgImage = icon.createScaled(CGSize(width: 48, height: 48)).cgImage else { - completion(false) + DispatchQueue.main.async { + completion(false) + } return } + let iconSize = CGSize(width: cgImage.width, height: cgImage.height) let alphaInfo = cgImage.alphaInfo let hasAlphaChannel = alphaInfo == .first || alphaInfo == .last || alphaInfo == .premultipliedFirst || alphaInfo == .premultipliedLast @@ -429,7 +438,7 @@ class FaviconFetcher { // are transparent and the image should be padded slightly var score: Int = 0 func updateScore(x: Int, y: Int) { - let location = ((Int(icon.size.width) * y) + x) * 4 + let location = ((Int(iconSize.width) * y) + x) * 4 guard location + 3 < length else { return } let alpha = data[location + 3] if alpha == 255 { @@ -438,24 +447,28 @@ class FaviconFetcher { score += 1 } } - for x in 0.. 0) } - completion(score > 0) } } else { - completion(false) + DispatchQueue.main.async { + completion(false) + } } } } diff --git a/ClientTests/TestFavicons.swift b/ClientTests/TestFavicons.swift index 381ca28ad10..317f048f81a 100644 --- a/ClientTests/TestFavicons.swift +++ b/ClientTests/TestFavicons.swift @@ -27,4 +27,53 @@ class TestFavicons: ProfileTest { } wait(for: [expectation], timeout: 5.0) } + + func testIconTransparentAroundEdges() { + let expectation = XCTestExpectation(description: "favicon.transparent.1") + var image: UIImage! + let size = CGSize(width: 48, height: 48) + UIGraphicsBeginImageContext(size) + let context = UIGraphicsGetCurrentContext()! + context.setFillColor(UIColor.black.cgColor) + context.addEllipse(in: CGRect(size: size).insetBy(dx: 5, dy: 5)) + context.fillPath() + image = UIGraphicsGetImageFromCurrentImageContext() + UIGraphicsEndImageContext() + FaviconFetcher.isIconBackgroundTransparentAroundEdges(image) { isTransparent in + dispatchPrecondition(condition: .onQueue(.main)) + XCTAssert(isTransparent) + expectation.fulfill() + } + wait(for: [expectation], timeout: 5.0) + } + + func testIconNotTransparentAroundEdges() { + let expectation = XCTestExpectation(description: "favicon.transparent.2") + var image: UIImage! + let size = CGSize(width: 48, height: 48) + UIGraphicsBeginImageContext(size) + let context = UIGraphicsGetCurrentContext()! + context.setFillColor(UIColor.black.cgColor) + context.addRect(CGRect(size: size)) + context.fillPath() + image = UIGraphicsGetImageFromCurrentImageContext() + UIGraphicsEndImageContext() + FaviconFetcher.isIconBackgroundTransparentAroundEdges(image) { isTransparent in + dispatchPrecondition(condition: .onQueue(.main)) + XCTAssertFalse(isTransparent) + expectation.fulfill() + } + wait(for: [expectation], timeout: 5.0) + } + + func testIconTransparencyDoesntCrashWithEmptyImage() { + let expectation = XCTestExpectation(description: "favicon.transparent.3") + let image = UIImage() + FaviconFetcher.isIconBackgroundTransparentAroundEdges(image) { isTransparent in + dispatchPrecondition(condition: .onQueue(.main)) + XCTAssertFalse(isTransparent) + expectation.fulfill() + } + wait(for: [expectation], timeout: 5.0) + } }