NetworkImageView does not handle cancellation properly in all cases

[etanhatch]

If an old image request is ongoing and a new one happens, the cancellation does not happen properly in this case:

1. The old image was already loaded before the new request happens, but the callback is delayed in the onResponse handler due to: "if (isImmediate && isInLayoutPass)"

2. The new image is loaded instantly (from a cache), but "if (isImmediate && isInLayoutPass)" does not happen for this request

Now the setImageBitmap() method is called first with the new image, then called again right afterwards with the old image.

[etanhatch]

This issue is fixed e.g. by:

void loadImageIfNecessary(final boolean isInLayoutPass) {
     String myUrl = mUrl;
     // Existing code...
                            @Override
                            public void onResponse(
                                    final ImageContainer response, boolean isImmediate) {
                                
                                if (!mUrl.equals(myUrl)) {
                                    return; // A new request was already made
                                }
     // Existing code...

[jpd236]

Thanks for the analysis. The proposed fix seems simple and safe enough, but I'm a little unsure about the full order of events here.

The interleaving implies that onLayout is first called (on the main thread) with a particular image URL set. This results in an immediate load of that URL and a delayed runnable to set the image (again, on the main thread) after the layout pass completes. For the URL to change in that time, something must have called setImageUrl in the middle. But setImageUrl is marked as @MainThread as it must be invoked on the main thread to provide the correct thread safety guarantees (and indeed will throw if not invoked on that thread). So it seems impossible that you could invoke setImageUrl on the main thread during this time as it's busy doing layout.

Just to check - are you using Volley 1.1.1? The main thread check was added in 2858832, so if you're using an older version, and invoking setImageUrl on a background thread, that could explain why you're seeing this. Otherwise, I'm curious what interleaving is happening here between setImageUrl and loadImageIfNecessary.