Adjust Request.Listener.onStart() timing. Add EventListener.onDispatc…

…h(). (coil-kt#348)

coil-base/api/coil-base.api

@@ -69,8 +69,9 @@ public abstract interface class coil/EventListener : coil/request/Request$Listen
 	public abstract fun fetchEnd (Lcoil/request/Request;Lcoil/fetch/Fetcher;Lcoil/decode/Options;)V
 	public abstract fun fetchStart (Lcoil/request/Request;Lcoil/fetch/Fetcher;Lcoil/decode/Options;)V
 	public abstract fun mapEnd (Lcoil/request/Request;Ljava/lang/Object;)V
-	public abstract fun mapStart (Lcoil/request/Request;)V
+	public abstract fun mapStart (Lcoil/request/Request;Ljava/lang/Object;)V
 	public abstract fun onCancel (Lcoil/request/Request;)V
+	public abstract fun onDispatch (Lcoil/request/Request;)V
 	public abstract fun onError (Lcoil/request/Request;Ljava/lang/Throwable;)V
 	public abstract fun onStart (Lcoil/request/Request;)V
 	public abstract fun onSuccess (Lcoil/request/Request;Lcoil/decode/DataSource;)V
@@ -91,8 +92,9 @@ public final class coil/EventListener$DefaultImpls {
 	public static fun fetchEnd (Lcoil/EventListener;Lcoil/request/Request;Lcoil/fetch/Fetcher;Lcoil/decode/Options;)V
 	public static fun fetchStart (Lcoil/EventListener;Lcoil/request/Request;Lcoil/fetch/Fetcher;Lcoil/decode/Options;)V
 	public static fun mapEnd (Lcoil/EventListener;Lcoil/request/Request;Ljava/lang/Object;)V
-	public static fun mapStart (Lcoil/EventListener;Lcoil/request/Request;)V
+	public static fun mapStart (Lcoil/EventListener;Lcoil/request/Request;Ljava/lang/Object;)V
 	public static fun onCancel (Lcoil/EventListener;Lcoil/request/Request;)V
+	public static fun onDispatch (Lcoil/EventListener;Lcoil/request/Request;)V
 	public static fun onError (Lcoil/EventListener;Lcoil/request/Request;Ljava/lang/Throwable;)V
 	public static fun onStart (Lcoil/EventListener;Lcoil/request/Request;)V
 	public static fun onSuccess (Lcoil/EventListener;Lcoil/request/Request;Lcoil/decode/DataSource;)V
@@ -108,7 +110,7 @@ public abstract interface class coil/EventListener$Factory {
 	public static final field Companion Lcoil/EventListener$Factory$Companion;
 	public static final field NONE Lcoil/EventListener$Factory;
 	public static fun create (Lcoil/EventListener;)Lcoil/EventListener$Factory;
-	public abstract fun newListener (Lcoil/request/Request;)Lcoil/EventListener;
+	public abstract fun create (Lcoil/request/Request;)Lcoil/EventListener;
 }
 
 public final class coil/EventListener$Factory$Companion {

coil-base/src/androidTest/java/coil/EventListenerTest.kt

@@ -107,6 +107,7 @@ class EventListenerTest {
     @Test
     fun error() {
         val eventListener = TestEventListener(
+            onStart = MethodChecker(false),
             resolveSizeStart = MethodChecker(false),
             resolveSizeEnd = MethodChecker(false),
             fetchStart = MethodChecker(false),
@@ -173,9 +174,10 @@ class EventListenerTest {
     }
 
     private class TestEventListener(
-        val onStart: MethodChecker = MethodChecker(true),
+        val onDispatch: MethodChecker = MethodChecker(true),
         val mapStart: MethodChecker = MethodChecker(true),
         val mapEnd: MethodChecker = MethodChecker(true),
+        val onStart: MethodChecker = MethodChecker(true),
         val resolveSizeStart: MethodChecker = MethodChecker(true),
         val resolveSizeEnd: MethodChecker = MethodChecker(true),
         val fetchStart: MethodChecker = MethodChecker(true),
@@ -191,9 +193,10 @@ class EventListenerTest {
         val onError: MethodChecker = MethodChecker(true)
     ) : EventListener {
 
-        override fun onStart(request: Request) = onStart.call()
-        override fun mapStart(request: Request) = mapStart.call()
+        override fun onDispatch(request: Request) = onDispatch.call()
+        override fun mapStart(request: Request, data: Any) = mapStart.call()
         override fun mapEnd(request: Request, mappedData: Any) = mapEnd.call()
+        override fun onStart(request: Request) = onStart.call()
         override fun resolveSizeStart(request: Request) = resolveSizeStart.call()
         override fun resolveSizeEnd(request: Request, size: Size) = resolveSizeEnd.call()
         override fun fetchStart(request: Request, fetcher: Fetcher<*>, options: Options) = fetchStart.call()
@@ -209,9 +212,10 @@ class EventListenerTest {
         override fun onError(request: Request, throwable: Throwable) = onError.call()
 
         fun complete() {
-            onStart.complete("onStart")
+            onDispatch.complete("onDispatch")
             mapStart.complete("mapStart")
             mapEnd.complete("mapEnd")
+            onStart.complete("onStart")
             resolveSizeStart.complete("resolveSizeStart")
             resolveSizeEnd.complete("resolveSizeEnd")
             fetchStart.complete("fetchStart")

coil-base/src/main/java/coil/EventListener.kt

@@ -31,16 +31,18 @@ interface EventListener : Request.Listener {
     }
 
     /**
-     * @see Request.Listener.onStart
+     * Called immediately after the request is dispatched.
      */
     @MainThread
-    override fun onStart(request: Request) {}
+    fun onDispatch(request: Request) {}
 
     /**
      * Called before any [Mapper]s and/or [MeasuredMapper]s are called to convert the request's data.
+     *
+     * @param data The data that will be converted.
      */
     @MainThread
-    fun mapStart(request: Request) {}
+    fun mapStart(request: Request, data: Any) {}
 
     /**
      * Called after the request's data has been converted by all applicable [Mapper]s and/or [MeasuredMapper]s.
@@ -51,6 +53,12 @@ interface EventListener : Request.Listener {
     @MainThread
     fun mapEnd(request: Request, mappedData: Any) {}
 
+    /**
+     * @see Request.Listener.onStart
+     */
+    @MainThread
+    override fun onStart(request: Request) {}
+
     /**
      * Called before [SizeResolver.size].
      */
@@ -124,15 +132,17 @@ interface EventListener : Request.Listener {
     /**
      * Called before [Transition.transition].
      *
-     * This is skipped if [Request.transition] is null or [Request.target] does not implement [TransitionTarget].
+     * This is skipped if [Request.transition] is [Transition.NONE]
+     * or [Request.target] does not implement [TransitionTarget].
      */
     @MainThread
     fun transitionStart(request: Request) {}
 
     /**
      * Called after [Transition.transition].
      *
-     * This is skipped if [Request.transition] is null or [Request.target] does not implement [TransitionTarget].
+     * This is skipped if [Request.transition] is [Transition.NONE]
+     * or [Request.target] does not implement [TransitionTarget].
      */
     @MainThread
     fun transitionEnd(request: Request) {}
@@ -161,17 +171,17 @@ interface EventListener : Request.Listener {
         companion object {
             @JvmField val NONE = Factory(EventListener.NONE)
 
-            /** Create an [EventListener.Factory] that returns the same [listener]. */
+            /** Create an [EventListener.Factory] that always returns [listener]. */
             @JvmStatic
             @JvmName("create")
             operator fun invoke(listener: EventListener): Factory {
                 return object : Factory {
-                    override fun newListener(request: Request) = listener
+                    override fun create(request: Request) = listener
                 }
             }
         }
 
         /** Return a new [EventListener]. */
-        fun newListener(request: Request): EventListener
+        fun create(request: Request): EventListener
     }
 }

coil-base/src/main/java/coil/RealImageLoader.kt

@@ -50,7 +50,6 @@ import coil.request.ViewTargetRequestDisposable
 import coil.size.Scale
 import coil.size.Size
 import coil.size.SizeResolver
-import coil.target.Target
 import coil.target.ViewTarget
 import coil.transform.Transformation
 import coil.util.ComponentCallbacks
@@ -156,7 +155,7 @@ internal class RealImageLoader(
         check(!isShutdown) { "The image loader is shutdown." }
 
         // Create a new event listener.
-        val eventListener = eventListenerFactory.newListener(request)
+        val eventListener = eventListenerFactory.create(request)
 
         // Compute lifecycle info on the main thread.
         val (lifecycle, mainDispatcher) = requestService.lifecycleInfo(request)
@@ -168,17 +167,16 @@ internal class RealImageLoader(
             // Fail before starting if data is null.
             val data = request.data ?: throw NullRequestDataException()
 
-            // Notify the listener that the request has started.
-            eventListener.onStart(request)
-            request.listener?.onStart(request)
+            // Notify the event listener that the request has been dispatched.
+            eventListener.onDispatch(request)
 
             // Invalidate the bitmap if it was provided as input.
             when (data) {
                 is BitmapDrawable -> referenceCounter.invalidate(data.bitmap)
                 is Bitmap -> referenceCounter.invalidate(data)
             }
 
-            // Add the target as a lifecycle observer, if necessary.
+            // Add the target as a lifecycle observer if necessary.
             val target = request.target
             if (target is ViewTarget<*> && target is LifecycleObserver) {
                 lifecycle.addObserver(target)
@@ -189,7 +187,7 @@ internal class RealImageLoader(
             val lazySizeResolver = LazySizeResolver(this, sizeResolver, targetDelegate, request, defaults, eventListener)
 
             // Perform any data mapping.
-            eventListener.mapStart(request)
+            eventListener.mapStart(request, data)
             val mappedData = mapData(data, lazySizeResolver)
             eventListener.mapEnd(request, mappedData)
 
@@ -329,7 +327,7 @@ internal class RealImageLoader(
                     decodeResult
                 } catch (rethrown: Exception) {
                     // NOTE: We only close the stream automatically if there is an uncaught exception.
-                    // This allows custom decoders to continue to read the source after returning a Drawable.
+                    // This allows custom decoders to continue to read the source after returning a drawable.
                     fetchResult.source.closeQuietly()
                     throw rethrown
                 }
@@ -416,7 +414,7 @@ internal class RealImageLoader(
         clearMemory()
     }
 
-    /** Lazily resolves and caches a request's size. Responsible for calling [Target.onStart]. */
+    /** Lazily resolves and caches a request's size. Responsible for calling onStart. */
     class LazySizeResolver(
         private val scope: CoroutineScope,
         private val sizeResolver: SizeResolver,
@@ -432,9 +430,12 @@ internal class RealImageLoader(
         suspend inline fun size(cached: BitmapDrawable? = null): Size = scope.run {
             size?.let { return@run it }
 
-            // Call the target's onStart before resolving the size.
+            // Call onStart before resolving the request's size.
             targetDelegate.start(cached, cached ?: request.placeholderOrDefault(defaults))
+            eventListener.onStart(request)
+            request.listener?.onStart(request)
 
+            // Resolve the request's size and cache it.
             eventListener.resolveSizeStart(request)
             val size = sizeResolver.size().also { size = it }
             eventListener.resolveSizeEnd(request, size)

coil-base/src/main/java/coil/request/Request.kt

@@ -77,25 +77,25 @@ sealed class Request {
     interface Listener {
 
         /**
-         * Called when the request is dispatched and starts loading the image.
+         * Called immediately after [Target.onStart].
          */
         @MainThread
         fun onStart(request: Request) {}
 
         /**
-         * Called when the request successfully loads the image.
+         * Called if the request completes successfully.
          */
         @MainThread
         fun onSuccess(request: Request, source: DataSource) {}
 
         /**
-         * Called when the request is cancelled.
+         * Called if the request is cancelled.
          */
         @MainThread
         fun onCancel(request: Request) {}
 
         /**
-         * Called when the request fails to load the image.
+         * Called if an error occurs while executing the request.
          */
         @MainThread
         fun onError(request: Request, throwable: Throwable) {}

coil-base/src/main/java/coil/target/PoolableViewTarget.kt

@@ -30,7 +30,7 @@ import androidx.annotation.MainThread
 interface PoolableViewTarget<T : View> : ViewTarget<T> {
 
     /**
-     * Called when the current Drawable is no longer usable. Targets **must** stop using the current Drawable.
+     * Called when the current drawable is no longer usable. Targets **must** stop using the current Drawable.
      *
      * In practice, this will only be called when the view is detached or about to be destroyed.
      */

coil-base/src/main/java/coil/target/Target.kt

@@ -4,26 +4,24 @@ import android.graphics.drawable.Drawable
 import androidx.annotation.MainThread
 
 /**
- * A listener that accepts the result of an image load.
- *
- * Each lifecycle method is called at most once. [onSuccess] and [onError] are mutually exclusive.
+ * A listener that accepts the result of an image request.
  */
 interface Target {
 
     /**
-     * Called when the image request starts.
+     * Called when the request starts.
      */
     @MainThread
     fun onStart(placeholder: Drawable?) {}
 
     /**
-     * Called if the image request is successful.
+     * Called if the request completes successfully.
      */
     @MainThread
     fun onSuccess(result: Drawable) {}
 
     /**
-     * Called if the image request fails.
+     * Called if an error occurs while executing the request.
      */
     @MainThread
     fun onError(error: Drawable?) {}

coil-base/src/main/java/coil/target/ViewTarget.kt

@@ -9,7 +9,7 @@ import androidx.lifecycle.LifecycleObserver
  *
  * If the loaded [Drawable] will only be used with one [View], prefer this to [Target].
  *
- * Unlike [Target]s, [ViewTarget]s can have their lifecycle methods called multiple times.
+ * [ViewTarget]s can have their lifecycle methods called multiple times.
  *
  * Optionally, a [ViewTarget] can be declared as a [LifecycleObserver]. It is automatically registered when the request
  * starts and unregistered when the request is disposed.

coil-video/src/main/java/coil/fetch/VideoFrameFetcher.kt

@@ -125,7 +125,7 @@ abstract class VideoFrameFetcher<T : Any>(private val context: Context) : Fetche
                     } else {
                         // We were unable to decode the video's dimensions.
                         // Fall back to decoding the video frame at the original size.
-                        // We'll scale the resulting bitmap after decoding, if necessary.
+                        // We'll scale the resulting bitmap after decoding if necessary.
                         OriginalSize
                     }
                 }