View-independent network activity

[webmaster128]

On iOS, we use a global object counting network activity jobs to show the network activity indicator of job count >= 1. The interface is as sibple as

NetworkActivityIndicatorManager.shared.incrementActivityCount()
NetworkActivityIndicatorManager.shared.decrementActivityCount()

I tried to hack these calls into Kingfisher by setting an invisible custom Indivator

    fileprivate struct InvisibleIndicator: Indicator {
        func startAnimatingView() {
            log.debug(" + 1")
            NetworkActivityIndicatorManager.shared.incrementActivityCount()
        }

        func stopAnimatingView() {
            log.debug(" - 1")
            NetworkActivityIndicatorManager.shared.decrementActivityCount()
        }

        var view: IndicatorView {
            return IndicatorView(frame: CGRect.zero)
        }

        var viewCenter: CGPoint {
            return CGPoint.zero
        }
    }

Now I am using this in a View of a UICollectionView, where views are reused. It turns out, that not for all network jobs, stopAnimatingView is called. I guess this is because Views are reused before the network job ends.

2017-02-23 10:58:26.574 [Debug] [main] [Cell.swift:10] startAnimatingView() >  + 1
2017-02-23 10:58:26.578 [Debug] [main] [Cell.swift:10] startAnimatingView() >  + 1
2017-02-23 10:58:26.581 [Debug] [main] [Cell.swift:10] startAnimatingView() >  + 1
2017-02-23 10:58:26.582 [Debug] [main] [Cell.swift:10] startAnimatingView() >  + 1
2017-02-23 10:58:26.583 [Debug] [main] [Cell.swift:10] startAnimatingView() >  + 1
2017-02-23 10:58:26.585 [Debug] [main] [Cell.swift:10] startAnimatingView() >  + 1
2017-02-23 10:58:26.784 [Debug] [main] [Cell.swift:15] stopAnimatingView() >  - 1
2017-02-23 10:58:26.828 [Debug] [main] [Cell.swift:15] stopAnimatingView() >  - 1
2017-02-23 10:58:26.881 [Debug] [main] [Cell.swift:15] stopAnimatingView() >  - 1
2017-02-23 10:58:27.427 [Debug] [main] [Cell.swift:10] startAnimatingView() >  + 1
2017-02-23 10:58:27.430 [Debug] [main] [Cell.swift:10] startAnimatingView() >  + 1
2017-02-23 10:58:27.435 [Debug] [main] [Cell.swift:10] startAnimatingView() >  + 1
2017-02-23 10:58:27.437 [Debug] [main] [Cell.swift:10] startAnimatingView() >  + 1
2017-02-23 10:58:27.440 [Debug] [main] [Cell.swift:10] startAnimatingView() >  + 1
2017-02-23 10:58:27.442 [Debug] [main] [Cell.swift:10] startAnimatingView() >  + 1
2017-02-23 10:58:27.487 [Debug] [main] [Cell.swift:15] stopAnimatingView() >  - 1
2017-02-23 10:58:27.491 [Debug] [main] [Cell.swift:15] stopAnimatingView() >  - 1
2017-02-23 10:58:27.510 [Debug] [main] [Cell.swift:15] stopAnimatingView() >  - 1

This is 12x startAnimatingView and only 6x stopAnimatingView.

It makes sense of course to skip stopAnimatingView when this only changes a subview of the reused view. But as soon as you have side-effects, this causes problems.

I guess it makes no sense to further misuse the View-based indivator API. But is there a way to get delegate calls for network activity even when target views are destroyed?

[onevcat]

Hi, @webmaster128

Yes, the problem here is related to the reusing. There is a mechanism in Kingfisher's image view extension that if the target URL is not the same as the url of currently loaded image, it will neither think the image setting would finish nor call the completion handler. Instead, it just keep trying to wait the "correct" image to be back.

It is a reasonable behavior for it working with UIKit and from an end-user stand. The way of Kingfisher's is not quite the same as you thought.

For your specified situation, what you really need is to hook up to the downloading process. I suggest you to subclass the ImageDownloader and override the downloadImage method as you need. Then, use the customized downloader as an option to make your indicator working.

Maybe some code snippet would help: (I didn't test them myself, so just for you as a reference)

// MyDownloader.swift
class MyDownloader: ImageDownloader {
    override func downloadImage(with url: URL, options: KingfisherOptionsInfo?, progressBlock: ImageDownloaderProgressBlock?, completionHandler: ImageDownloaderCompletionHandler?) -> RetrieveImageDownloadTask? {
        
        NetworkActivityIndicatorManager.shared.incrementActivityCount()
        
        return super.downloadImage(with: url, options: options, progressBlock: progressBlock) { (image, error, url, data) in
            NetworkActivityIndicatorManager.shared.decrementActivityCount()
            completionHandler?(image, error, url, data)
        }
    }
}

// In some global scope
let myDownloader = MyDownloader(name: "my_downloader")

// Using the new downloader
imageView.kf.setImage(with: url, options: [.downloader(myDownloader)],

[tomaskraina]

@onevcat I'm trying to use the solution you proposed above with my own image downloader used as the default on KingfisherManager:

KingfisherManager.shared.downloader = MyImageDownloader(name: "MyImageDownloader")

However, the method downloadImage gets never called on MyImageDownloader.

Do have any suggestion how to set MyImageDownloader globally?

[webmaster128]

Thanks a lot for the detailed answer. Unfortunately we did choose another library and I did not get the chance to test the suggested solution. But it looks like this is of interest for other users :) From my side, this issue can be closed. I leave it open for now as long as other users need it. Feel free to close this whenever you want.

[onevcat]

@tomaskraina

The extension methods are using ImageDownloader.default instead of the KingfisherManager.shared.downloader. The one in KingfisherManager is a legacy way and now should not be used outside (I will add deprecated annotation to it later.)

I strongly suggest to use the options way in the sample above to pass the downloader for each downloading. It could reduce risk of unintended code sharing. If you need to use your own downloader more than once, you could create your own wrapper of existing code like this:

extension Kingfisher where Base: UIImageView {
    func setImageWithMyOwnDownloader(with resource: Resource?,
                                     placeholder: Image? = nil,
                                     options: KingfisherOptionsInfo? = nil,
                                     progressBlock: DownloadProgressBlock? = nil,
                                     completionHandler: CompletionHandler? = nil) -> RetrieveImageTask
    {
        var options = options ?? []
        options.append(.downloader(ImageDownloader.default))
        
        return setImage(with: resource,
                        placeholder: placeholder,
                        options: options,
                        progressBlock: progressBlock,
                        completionHandler: completionHandler)
    }
}

Then you could use anywhere else.

[tomaskraina]

Awesome. Thanks, I will try that out.

[tomaskraina]

@onevcat I have tried that and it does not work, unfortunately.

The problem seems two-fold.

First, the overridden method downloadImage does not get called when using the kf extension on UIImageView. What get's called instead is the other downloadImage method. However, this method is not open and thus, can't be overridden.

I have sold this issue by adding another ImageDownloaderDelegate method - imageDownloader(_:willDownloadImageForURL:with:) that gets called when the ImageDownloader object will start downloading an image from specified URL.

The second issue is that the imageDownloader(_:, didDownload:, for:, with) method seems to get called more times than the number of actual network requests. I didn't find the root cause, but I was able to implement a workaround in my implementation of ImageDownloaderDelegate - see gist: https://gist.github.com/tomaskraina/ef2c84e568f07491fe4d7f0480b65602

[onevcat]

@tomaskraina

Oops, you are right, the one actually called by extension method is not the overridable one.

I think it is much clearer and is a correct direction to use a delegate to implement it! But be careful that the didDownload callback will only get fired when the downloading finished without problem as well as an image object could be created from data. You have also to handle the case of downloading or creating error. Currently there is no callback for that and maybe we could add them later.

However, I've tried the imageDownloader(_:didDownload:for:with), it seems that this method works fine for me, with correct count. Could you give some more information on it (like the url you are trying to download)? It is strange to me since it should definitely be called once per image downloading.

Thanks!