From 1a2cfe36bf32a8b3d8f155cf8e66e145fcfc3ba7 Mon Sep 17 00:00:00 2001 From: Sam Judd Date: Tue, 16 Aug 2022 13:26:29 -0700 Subject: [PATCH] Avoid always overwriting RunReason with one specific one in DecodeJob.reschedule. This seems to have been around for years. I think the only negative impact is that we call into the SourceGenerator and run a few extra lines, which shouldn't materially impact the result of the load or its performance. It's certainly confusing though. PiperOrigin-RevId: 468008961 --- .../bumptech/glide/load/engine/DecodeJob.java | 18 +++++++++++------- .../glide/load/engine/SourceGenerator.java | 3 ++- 2 files changed, 13 insertions(+), 8 deletions(-) diff --git a/library/src/main/java/com/bumptech/glide/load/engine/DecodeJob.java b/library/src/main/java/com/bumptech/glide/load/engine/DecodeJob.java index 74cfc90e9b..76726faf42 100644 --- a/library/src/main/java/com/bumptech/glide/load/engine/DecodeJob.java +++ b/library/src/main/java/com/bumptech/glide/load/engine/DecodeJob.java @@ -313,7 +313,7 @@ private void runGenerators() { currentGenerator = getNextGenerator(); if (stage == Stage.SOURCE) { - reschedule(); + reschedule(RunReason.SWITCH_TO_SOURCE_SERVICE); return; } } @@ -369,10 +369,16 @@ private Stage getNextStage(Stage current) { } } + private void reschedule(RunReason runReason) { + this.runReason = runReason; + callback.reschedule(this); + } + + // This is used by SourceGenerator to ask us to switch back to our thread. Internal methods in + // this class should call reschedule with a specific RunReason. @Override public void reschedule() { - runReason = RunReason.SWITCH_TO_SOURCE_SERVICE; - callback.reschedule(this); + reschedule(RunReason.SWITCH_TO_SOURCE_SERVICE); } @Override @@ -386,8 +392,7 @@ public void onDataFetcherReady( this.isLoadingFromAlternateCacheKey = sourceKey != decodeHelper.getCacheKeys().get(0); if (Thread.currentThread() != currentThread) { - runReason = RunReason.DECODE_DATA; - callback.reschedule(this); + reschedule(RunReason.DECODE_DATA); } else { GlideTrace.beginSection("DecodeJob.decodeFromRetrievedData"); try { @@ -406,8 +411,7 @@ public void onDataFetcherFailed( exception.setLoggingDetails(attemptedKey, dataSource, fetcher.getDataClass()); throwables.add(exception); if (Thread.currentThread() != currentThread) { - runReason = RunReason.SWITCH_TO_SOURCE_SERVICE; - callback.reschedule(this); + reschedule(RunReason.SWITCH_TO_SOURCE_SERVICE); } else { runGenerators(); } diff --git a/library/src/main/java/com/bumptech/glide/load/engine/SourceGenerator.java b/library/src/main/java/com/bumptech/glide/load/engine/SourceGenerator.java index 5273df49d0..9a3c63ce2d 100644 --- a/library/src/main/java/com/bumptech/glide/load/engine/SourceGenerator.java +++ b/library/src/main/java/com/bumptech/glide/load/engine/SourceGenerator.java @@ -204,7 +204,8 @@ void onDataReadyInternal(LoadData loadData, Object data) { if (data != null && diskCacheStrategy.isDataCacheable(loadData.fetcher.getDataSource())) { dataToCache = data; // We might be being called back on someone else's thread. Before doing anything, we should - // reschedule to get back onto Glide's thread. + // reschedule to get back onto Glide's thread. Then once we're back on Glide's thread, we'll + // get called again and we can write the retrieved data to cache. cb.reschedule(); } else { cb.onDataFetcherReady(