Upstream #2104 to 2.x. (#2110)

* Upstream [#2104] to 2.x.

* Delay isOnline request.

coil-base/src/androidTest/java/coil/util/SystemCallbacksTest.kt

@@ -27,7 +27,7 @@ class SystemCallbacksTest {
 
     @Test
     fun imageLoaderIsFreedWithoutShutdown() {
-        val systemCallbacks = SystemCallbacks(ImageLoader(context) as RealImageLoader, context, true)
+        val systemCallbacks = SystemCallbacks(ImageLoader(context) as RealImageLoader)
 
         val bitmaps = mutableListOf<Bitmap>()
         while (systemCallbacks.imageLoader.get() != null) {
@@ -42,7 +42,7 @@ class SystemCallbacksTest {
         // Ensure that the next system callback is called.
         systemCallbacks.onTrimMemory(TRIM_MEMORY_BACKGROUND)
 
-        assertTrue(systemCallbacks.isShutdown)
+        assertTrue(systemCallbacks.shutdown)
     }
 
     @Test
@@ -52,7 +52,7 @@ class SystemCallbacksTest {
             .memoryCache(memoryCache)
             .diskCache(null)
             .build()
-        val systemCallbacks = SystemCallbacks(imageLoader as RealImageLoader, context, true)
+        val systemCallbacks = SystemCallbacks(imageLoader as RealImageLoader)
 
         memoryCache[Key("1")] = Value(createBitmap(1000, 1000, Bitmap.Config.ARGB_8888))
         memoryCache[Key("2")] = Value(createBitmap(1000, 1000, Bitmap.Config.ARGB_8888))

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

@@ -76,7 +76,7 @@ internal class RealImageLoader(
 
     private val scope = CoroutineScope(SupervisorJob() + Dispatchers.Main.immediate +
         CoroutineExceptionHandler { _, throwable -> logger?.log(TAG, throwable) })
-    private val systemCallbacks = SystemCallbacks(this, context, options.networkObserverEnabled)
+    private val systemCallbacks = SystemCallbacks(this)
     private val requestService = RequestService(this, systemCallbacks, logger)
     override val memoryCache by memoryCacheLazy
     override val diskCache by diskCacheLazy
@@ -103,14 +103,10 @@ internal class RealImageLoader(
         // Decoders
         .add(BitmapFactoryDecoder.Factory(options.bitmapFactoryMaxParallelism, options.bitmapFactoryExifOrientationPolicy))
         .build()
-    private val interceptors = components.interceptors + EngineInterceptor(this, requestService, logger)
+    private val interceptors = components.interceptors +
+        EngineInterceptor(this, systemCallbacks, requestService, logger)
     private val shutdown = AtomicBoolean(false)
 
-    init {
-        // Must be called only after the image loader is fully initialized.
-        systemCallbacks.register()
-    }
-
     override fun enqueue(request: ImageRequest): Disposable {
         // Start executing the request on the main thread.
         val job = scope.async {

coil-base/src/main/java/coil/intercept/EngineInterceptor.kt

@@ -24,6 +24,7 @@ import coil.request.SuccessResult
 import coil.transform.Transformation
 import coil.util.DrawableUtils
 import coil.util.Logger
+import coil.util.SystemCallbacks
 import coil.util.VALID_TRANSFORMATION_CONFIGS
 import coil.util.addFirst
 import coil.util.closeQuietly
@@ -40,6 +41,7 @@ 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 {
@@ -74,6 +76,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)
 
@@ -108,9 +113,8 @@ internal class EngineInterceptor(
         var components = imageLoader.components
         var fetchResult: FetchResult? = null
         val executeResult = try {
-            if (!requestService.allowHardwareWorkerThread(options)) {
-                options = options.copy(config = Bitmap.Config.ARGB_8888)
-            }
+            options = requestService.updateOptionsOnWorkerThread(options)
+
             if (request.fetcherFactory != null || request.decoderFactory != null) {
                 components = components.newBuilder()
                     .addFirst(request.fetcherFactory)

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

@@ -60,13 +60,6 @@ internal class RequestService(
             isConfigValidForHardwareAllocation(request, size)
         val config = if (isValidConfig) request.bitmapConfig else Bitmap.Config.ARGB_8888
 
-        // Disable fetching from the network if we know we're offline.
-        val networkCachePolicy = if (systemCallbacks.isOnline) {
-            request.networkCachePolicy
-        } else {
-            CachePolicy.DISABLED
-        }
-
         // Use `Scale.FIT` if either dimension is undefined.
         val scale = if (size.width == Dimension.Undefined || size.height == Dimension.Undefined) {
             Scale.FIT
@@ -96,7 +89,7 @@ internal class RequestService(
             parameters = request.parameters,
             memoryCachePolicy = request.memoryCachePolicy,
             diskCachePolicy = request.diskCachePolicy,
-            networkCachePolicy = networkCachePolicy
+            networkCachePolicy = request.networkCachePolicy,
         )
     }
 
@@ -124,9 +117,35 @@ internal class RequestService(
         return true
     }
 
+    fun updateOptionsOnWorkerThread(options: Options): Options {
+        var changed = false
+        var bitmapConfig = options.config
+        var networkCachePolicy = options.networkCachePolicy
+
+        if (!isBitmapConfigValidWorkerThread(options)) {
+            bitmapConfig = Bitmap.Config.ARGB_8888
+            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(
+                config = bitmapConfig,
+                networkCachePolicy = networkCachePolicy,
+            )
+        } else {
+            return options
+        }
+    }
+
     /** Return 'true' if we can allocate a hardware bitmap. */
     @WorkerThread
-    fun allowHardwareWorkerThread(options: Options): Boolean {
+    private fun isBitmapConfigValidWorkerThread(options: Options): Boolean {
         return !options.config.isHardware || hardwareBitmapService.allowHardwareWorkerThread()
     }
 

coil-base/src/main/java/coil/util/SystemCallbacks.kt

@@ -10,61 +10,81 @@ import coil.RealImageLoader
 import coil.network.EmptyNetworkObserver
 import coil.network.NetworkObserver
 import java.lang.ref.WeakReference
-import java.util.concurrent.atomic.AtomicBoolean
 
 /**
- * Proxies [ComponentCallbacks2] and [NetworkObserver.Listener] calls to a
- * weakly referenced [imageLoader].
+ * Proxies [ComponentCallbacks2] and [NetworkObserver.Listener] calls to a weakly referenced
+ * [imageLoader].
  *
  * This prevents the system from having a strong reference to the [imageLoader], which allows
- * it be freed naturally by the garbage collector. If the [imageLoader] is freed, it unregisters
+ * it be freed automatically by the garbage collector. If the [imageLoader] is freed, it unregisters
  * its callbacks.
  */
 internal class SystemCallbacks(
     imageLoader: RealImageLoader,
-    private val context: Context,
-    isNetworkObserverEnabled: Boolean
 ) : ComponentCallbacks2, NetworkObserver.Listener {
+    @VisibleForTesting val imageLoader = WeakReference(imageLoader)
+    private var application: Context? = null
+    private var networkObserver: NetworkObserver? = null
+    @VisibleForTesting var shutdown = false
 
-    @VisibleForTesting internal val imageLoader = WeakReference(imageLoader)
-    private val networkObserver = if (isNetworkObserverEnabled) {
-        NetworkObserver(context, this, imageLoader.logger)
-    } else {
-        EmptyNetworkObserver()
-    }
+    private var _isOnline = true
+    val isOnline: Boolean
+        @Synchronized get() {
+            // Register the network observer lazily.
+            registerNetworkObserver()
+            return _isOnline
+        }
+
+    @Synchronized
+    fun registerMemoryPressureCallbacks() = withImageLoader { imageLoader ->
+        if (application != null) return@withImageLoader
 
-    @Volatile private var _isOnline = networkObserver.isOnline
-    private val _isShutdown = AtomicBoolean(false)
+        val application = imageLoader.context
+        this.application = application
+        application.registerComponentCallbacks(this)
+    }
 
-    val isOnline get() = _isOnline
-    val isShutdown get() = _isShutdown.get()
+    @Synchronized
+    private fun registerNetworkObserver() = withImageLoader { imageLoader ->
+        if (networkObserver != null) return@withImageLoader
 
-    fun register() {
-        context.registerComponentCallbacks(this)
+        val networkObserver = if (imageLoader.options.networkObserverEnabled) {
+            NetworkObserver(imageLoader.context, this, imageLoader.logger)
+        } else {
+            EmptyNetworkObserver()
+        }
+        this.networkObserver = networkObserver
+        this._isOnline = networkObserver.isOnline
     }
 
-    override fun onConfigurationChanged(newConfig: Configuration) {
-        imageLoader.get() ?: shutdown()
+    @Synchronized
+    fun shutdown() {
+        if (shutdown) return
+        shutdown = true
+
+        application?.unregisterComponentCallbacks(this)
+        networkObserver?.shutdown()
+        imageLoader.clear()
     }
 
+    @Synchronized
+    override fun onConfigurationChanged(newConfig: Configuration) = withImageLoader {}
+
+    @Synchronized
     override fun onTrimMemory(level: Int) = withImageLoader { imageLoader ->
         imageLoader.logger?.log(TAG, Log.VERBOSE) { "trimMemory, level=$level" }
         imageLoader.onTrimMemory(level)
     }
 
+    @Synchronized
     override fun onLowMemory() = onTrimMemory(TRIM_MEMORY_COMPLETE)
 
+    @Synchronized
     override fun onConnectivityChange(isOnline: Boolean) = withImageLoader { imageLoader ->
         imageLoader.logger?.log(TAG, Log.INFO) { if (isOnline) ONLINE else OFFLINE }
         _isOnline = isOnline
     }
 
-    fun shutdown() {
-        if (_isShutdown.getAndSet(true)) return
-        context.unregisterComponentCallbacks(this)
-        networkObserver.shutdown()
-    }
-
     private inline fun withImageLoader(block: (RealImageLoader) -> Unit) {
         imageLoader.get()?.let(block) ?: shutdown()
     }

coil-base/src/test/java/coil/intercept/EngineInterceptorTest.kt

@@ -88,9 +88,10 @@ class EngineInterceptorTest {
                 add(Keyer { _: Any, _ -> key })
             }
             .build()
-        val systemCallbacks = SystemCallbacks(imageLoader as RealImageLoader, context, true)
+        val systemCallbacks = SystemCallbacks(imageLoader as RealImageLoader)
         return EngineInterceptor(
             imageLoader = imageLoader,
+            systemCallbacks = systemCallbacks,
             requestService = RequestService(imageLoader, systemCallbacks, null),
             logger = null
         )

coil-base/src/test/java/coil/memory/MemoryCacheServiceTest.kt

@@ -516,7 +516,7 @@ class MemoryCacheServiceTest {
                 add(Keyer { _: Any, _ -> key })
             }
             .build()
-        val systemCallbacks = SystemCallbacks(imageLoader as RealImageLoader, context, true)
+        val systemCallbacks = SystemCallbacks(imageLoader as RealImageLoader)
         return MemoryCacheService(
             imageLoader = imageLoader,
             requestService = RequestService(imageLoader, systemCallbacks, null),

coil-base/src/test/java/coil/request/RequestServiceTest.kt

@@ -33,7 +33,7 @@ class RequestServiceTest {
     fun before() {
         context = ApplicationProvider.getApplicationContext()
         val imageLoader = ImageLoader(context) as RealImageLoader
-        val systemCallbacks = SystemCallbacks(imageLoader, context, true)
+        val systemCallbacks = SystemCallbacks(imageLoader)
         service = RequestService(imageLoader, systemCallbacks, null)
     }