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 · Aug 16, 2022 · 1a2cfe3 · 1a2cfe3
1 parent 10bffe3
commit 1a2cfe3
Show file tree

Hide file tree

Showing 2 changed files with 13 additions and 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
@@ -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
@@ -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(