Lazily register component callbacks and the network observer on Android.

coil-core/src/androidInstrumentedTest/kotlin/coil3/util/SystemCallbacksTest.kt

@@ -17,11 +17,18 @@ class SystemCallbacksTest {
 
     @Test
     fun imageLoaderIsFreedWithoutShutdown() {
-        val systemCallbacks = SystemCallbacks() as AndroidSystemCallbacks
-        systemCallbacks.register(ImageLoader(context) as RealImageLoader)
+        var imageLoader: RealImageLoader?
+        imageLoader = ImageLoader(context) as RealImageLoader
+        val systemCallbacks = SystemCallbacks(imageLoader) as AndroidSystemCallbacks
+        systemCallbacks.registerMemoryPressureCallbacks()
+        systemCallbacks.isOnline
+
+        // Clear the local reference.
+        @Suppress("UNUSED_VALUE")
+        imageLoader = null
 
         val bitmaps = mutableListOf<Bitmap>()
-        while (systemCallbacks.imageLoader?.get() != null) {
+        while (systemCallbacks.imageLoader.get() != null) {
             // Request that garbage collection occur.
             Runtime.getRuntime().gc()
 
@@ -33,7 +40,7 @@ class SystemCallbacksTest {
         // Ensure that the next system callback is called.
         systemCallbacks.onTrimMemory(TRIM_MEMORY_BACKGROUND)
 
-        assertTrue(systemCallbacks.isShutdown)
+        assertTrue(systemCallbacks.shutdown)
     }
 
     @Test
@@ -43,9 +50,9 @@ class SystemCallbacksTest {
             .build()
         val imageLoader = ImageLoader.Builder(context)
             .memoryCache(memoryCache)
-            .build()
-        val systemCallbacks = SystemCallbacks() as AndroidSystemCallbacks
-        systemCallbacks.register(imageLoader as RealImageLoader)
+            .build() as RealImageLoader
+        val systemCallbacks = SystemCallbacks(imageLoader) as AndroidSystemCallbacks
+        systemCallbacks.registerMemoryPressureCallbacks()
 
         memoryCache[MemoryCache.Key("1")] = MemoryCache.Value(
             image = createBitmap(1000, 1000, Bitmap.Config.ARGB_8888)

coil-core/src/androidMain/kotlin/coil3/request/RequestService.kt

@@ -69,7 +69,7 @@ internal class AndroidRequestService(
             request.fileSystem,
             request.memoryCachePolicy,
             request.diskCachePolicy,
-            request.resolveNetworkCachePolicy(),
+            request.networkCachePolicy,
             request.resolveExtras(size),
         )
     }
@@ -83,15 +83,6 @@ internal class AndroidRequestService(
         }
     }
 
-    private fun ImageRequest.resolveNetworkCachePolicy(): CachePolicy {
-        // Disable fetching from the network if we know we're offline.
-        if (systemCallbacks.isOnline) {
-            return networkCachePolicy
-        } else {
-            return CachePolicy.DISABLED
-        }
-    }
-
     private fun ImageRequest.resolveExtras(size: Size): Extras {
         var bitmapConfig = bitmapConfig
         var allowRgb565 = allowRgb565
@@ -119,11 +110,27 @@ internal class AndroidRequestService(
     }
 
     override fun updateOptionsOnWorkerThread(options: Options): Options {
+        var changed = false
+        var networkCachePolicy = options.networkCachePolicy
+        var extras = options.extras
+
         if (!isBitmapConfigValidWorkerThread(options)) {
+            extras = extras.newBuilder()
+                .set(Extras.Key.bitmapConfig, Bitmap.Config.ARGB_8888)
+                .build()
+            changed = true
+        }
+
+        if (options.networkCachePolicy.readEnabled && !systemCallbacks.isOnline) {
+            // Disable fetching from the network if we know we're offline.
+            networkCachePolicy = CachePolicy.DISABLED
+            changed = true
+        }
+
+        if (changed) {
             return options.copy(
-                extras = options.extras.newBuilder()
-                    .set(Extras.Key.bitmapConfig, Bitmap.Config.ARGB_8888)
-                    .build(),
+                networkCachePolicy = networkCachePolicy,
+                extras = extras,
             )
         } else {
             return options

coil-core/src/androidMain/kotlin/coil3/util/SystemCallbacks.kt

@@ -7,12 +7,13 @@ import android.content.ComponentCallbacks2.TRIM_MEMORY_RUNNING_LOW
 import android.content.ComponentCallbacks2.TRIM_MEMORY_UI_HIDDEN
 import android.content.Context
 import android.content.res.Configuration
-import androidx.annotation.VisibleForTesting
 import coil3.RealImageLoader
+import coil3.annotation.VisibleForTesting
 import coil3.networkObserverEnabled
-import java.lang.ref.WeakReference
 
-internal actual fun SystemCallbacks(): SystemCallbacks = AndroidSystemCallbacks()
+internal actual fun SystemCallbacks(
+    imageLoader: RealImageLoader,
+): SystemCallbacks = AndroidSystemCallbacks(imageLoader)
 
 /**
  * Proxies [ComponentCallbacks2] and [NetworkObserver.Listener] calls to a weakly referenced
@@ -22,40 +23,53 @@ internal actual fun SystemCallbacks(): SystemCallbacks = AndroidSystemCallbacks(
  * it be freed automatically by the garbage collector. If the [imageLoader] is freed, it unregisters
  * its callbacks.
  */
-internal class AndroidSystemCallbacks : SystemCallbacks, ComponentCallbacks2, NetworkObserver.Listener {
-
+internal class AndroidSystemCallbacks(
+    imageLoader: RealImageLoader,
+) : SystemCallbacks, ComponentCallbacks2, NetworkObserver.Listener {
+    @VisibleForTesting val imageLoader = WeakReference(imageLoader)
     private var application: Context? = null
     private var networkObserver: NetworkObserver? = null
-    @VisibleForTesting var imageLoader: WeakReference<RealImageLoader>? = null
+    @VisibleForTesting var shutdown = false
 
-    @Volatile override var isOnline = true
-    @Volatile override var isShutdown = false
+    private var _isOnline = true
+    override val isOnline: Boolean
+        @Synchronized get() {
+            // Register the network observer lazily.
+            registerNetworkObserver()
+            return _isOnline
+        }
 
     @Synchronized
-    override fun register(imageLoader: RealImageLoader) {
+    override fun registerMemoryPressureCallbacks() = withImageLoader { imageLoader ->
+        if (application != null) return@withImageLoader
+
         val application = imageLoader.options.application
         this.application = application
-        this.imageLoader = WeakReference(imageLoader)
         application.registerComponentCallbacks(this)
+    }
+
+    @Synchronized
+    private fun registerNetworkObserver() = withImageLoader { imageLoader ->
+        if (networkObserver != null) return@withImageLoader
 
-        val networkObserver = if (imageLoader.options.networkObserverEnabled) {
-            NetworkObserver(application, this, imageLoader.options.logger)
+        val options = imageLoader.options
+        val networkObserver = if (options.networkObserverEnabled) {
+            NetworkObserver(options.application, this, options.logger)
         } else {
             EmptyNetworkObserver()
         }
         this.networkObserver = networkObserver
-        this.isOnline = networkObserver.isOnline
+        this._isOnline = networkObserver.isOnline
     }
 
     @Synchronized
     override fun shutdown() {
-        if (isShutdown) return
-        isShutdown = true
+        if (shutdown) return
+        shutdown = true
 
         application?.unregisterComponentCallbacks(this)
         networkObserver?.shutdown()
-        imageLoader?.clear()
-        imageLoader = null
+        imageLoader.clear()
     }
 
     @Synchronized
@@ -79,17 +93,13 @@ internal class AndroidSystemCallbacks : SystemCallbacks, ComponentCallbacks2, Ne
     @Synchronized
     override fun onConnectivityChange(isOnline: Boolean) = withImageLoader { imageLoader ->
         imageLoader.options.logger?.log(TAG, Logger.Level.Info) {
-            if (isOnline) {
-                "onConnectivityChange: The device is online."
-            } else {
-                "onConnectivityChange: The device is offline."
-            }
+            "onConnectivityChange: The device is ${if (isOnline) "online" else "offline"}."
         }
-        this.isOnline = isOnline
+        _isOnline = isOnline
     }
 
     private inline fun withImageLoader(block: (RealImageLoader) -> Unit) {
-        imageLoader?.get()?.let(block) ?: shutdown()
+        imageLoader.get()?.let(block) ?: shutdown()
     }
 
     private companion object {

coil-core/src/androidUnitTest/kotlin/coil3/intercept/EngineInterceptorTest.kt

@@ -5,6 +5,7 @@ import android.graphics.drawable.BitmapDrawable
 import android.graphics.drawable.ColorDrawable
 import coil3.EventListener
 import coil3.ImageLoader
+import coil3.RealImageLoader
 import coil3.asCoilImage
 import coil3.decode.DataSource
 import coil3.intercept.EngineInterceptor.ExecuteResult
@@ -79,10 +80,12 @@ class EngineInterceptorTest : RobolectricTest() {
             .components {
                 add(Keyer { _: Any, _ -> key })
             }
-            .build()
+            .build() as RealImageLoader
+        val systemCallbacks = SystemCallbacks(imageLoader)
         return EngineInterceptor(
             imageLoader = imageLoader,
-            requestService = RequestService(imageLoader, SystemCallbacks(), null),
+            systemCallbacks = systemCallbacks,
+            requestService = RequestService(imageLoader, systemCallbacks, null),
             logger = null,
         )
     }

coil-core/src/androidUnitTest/kotlin/coil3/memory/MemoryCacheServiceTest.kt

@@ -3,6 +3,7 @@ package coil3.memory
 import android.graphics.Bitmap
 import coil3.EventListener
 import coil3.ImageLoader
+import coil3.RealImageLoader
 import coil3.asCoilImage
 import coil3.key.Keyer
 import coil3.memory.MemoryCacheService.Companion.EXTRA_IS_SAMPLED
@@ -517,10 +518,11 @@ class MemoryCacheServiceTest : RobolectricTest() {
             .components {
                 add(Keyer { _: Any, _ -> key })
             }
-            .build()
+            .build() as RealImageLoader
+        val systemCallbacks = SystemCallbacks(imageLoader)
         return MemoryCacheService(
             imageLoader = imageLoader,
-            requestService = RequestService(imageLoader, SystemCallbacks(), null),
+            requestService = RequestService(imageLoader, systemCallbacks, null),
             logger = null
         )
     }

coil-core/src/androidUnitTest/kotlin/coil3/request/RequestServiceTest.kt

@@ -28,7 +28,7 @@ class RequestServiceTest : RobolectricTest() {
     @Before
     fun before() {
         val imageLoader = ImageLoader(context) as RealImageLoader
-        service = RequestService(imageLoader, SystemCallbacks(), null)
+        service = RequestService(imageLoader, SystemCallbacks(imageLoader), null)
     }
 
     @Test

coil-core/src/commonMain/kotlin/coil3/RealImageLoader.common.kt

@@ -45,9 +45,8 @@ import kotlinx.coroutines.withContext
 internal class RealImageLoader(
     val options: Options,
 ) : ImageLoader {
-
     private val scope = CoroutineScope(options.logger)
-    private val systemCallbacks = SystemCallbacks()
+    private val systemCallbacks = SystemCallbacks(this)
     private val requestService = RequestService(this, systemCallbacks, options.logger)
     override val defaults get() = options.defaults
     override val memoryCache by options.memoryCacheLazy
@@ -58,15 +57,10 @@ internal class RealImageLoader(
         .addJvmComponents(options)
         .addAppleComponents(options)
         .addCommonComponents(options)
-        .add(EngineInterceptor(this, requestService, options.logger))
+        .add(EngineInterceptor(this, systemCallbacks, requestService, options.logger))
         .build()
     private val shutdown = atomic(false)
 
-    init {
-        // Must be called after the image loader is fully initialized.
-        systemCallbacks.register(this)
-    }
-
     override fun enqueue(request: ImageRequest): Disposable {
         // Start executing the request on the main thread.
         val job = scope.async {

coil-core/src/commonMain/kotlin/coil3/intercept/EngineInterceptor.kt

@@ -18,6 +18,7 @@ import coil3.request.Options
 import coil3.request.RequestService
 import coil3.request.SuccessResult
 import coil3.util.Logger
+import coil3.util.SystemCallbacks
 import coil3.util.addFirst
 import coil3.util.closeQuietly
 import coil3.util.eventListener
@@ -28,10 +29,10 @@ import kotlinx.coroutines.withContext
 /** The last interceptor in the chain which executes the [ImageRequest]. */
 internal class EngineInterceptor(
     private val imageLoader: ImageLoader,
+    private val systemCallbacks: SystemCallbacks,
     private val requestService: RequestService,
     private val logger: Logger?,
 ) : Interceptor {
-
     private val memoryCacheService = MemoryCacheService(imageLoader, requestService, logger)
 
     override suspend fun intercept(chain: Interceptor.Chain): ImageResult {
@@ -62,6 +63,9 @@ internal class EngineInterceptor(
                 // Fetch and decode the image.
                 val result = execute(request, mappedData, options, eventListener)
 
+                // Register memory pressure callbacks.
+                systemCallbacks.registerMemoryPressureCallbacks()
+
                 // Write the result to the memory cache.
                 val isCached = memoryCacheService.setCacheValue(cacheKey, request, result)
 

coil-core/src/commonMain/kotlin/coil3/util/SystemCallbacks.kt

@@ -2,12 +2,13 @@ package coil3.util
 
 import coil3.RealImageLoader
 
-internal expect fun SystemCallbacks(): SystemCallbacks
+internal expect fun SystemCallbacks(
+    imageLoader: RealImageLoader,
+): SystemCallbacks
 
 internal interface SystemCallbacks {
     val isOnline: Boolean
-    val isShutdown: Boolean
 
-    fun register(imageLoader: RealImageLoader)
+    fun registerMemoryPressureCallbacks()
     fun shutdown()
 }

coil-core/src/nonAndroidMain/kotlin/coil3/util/SystemCallbacks.kt

@@ -1,18 +1,15 @@
 package coil3.util
 
 import coil3.RealImageLoader
-import kotlinx.atomicfu.atomic
 
-internal actual fun SystemCallbacks(): SystemCallbacks = NoopSystemCallbacks()
+internal actual fun SystemCallbacks(
+    imageLoader: RealImageLoader,
+): SystemCallbacks = NoopSystemCallbacks()
 
-// TODO: Listen for memory-pressure events to trim the memory cache on non-Android platforms.
 private class NoopSystemCallbacks : SystemCallbacks {
     override val isOnline get() = true
-    override var isShutdown by atomic(false)
 
-    override fun register(imageLoader: RealImageLoader) {}
-
-    override fun shutdown() {
-        isShutdown = true
-    }
+    // TODO: Listen for memory-pressure events to trim the memory cache on non-Android platforms.
+    override fun registerMemoryPressureCallbacks() {}
+    override fun shutdown() {}
 }