From d18e5e8595daff24356fa4fe36833cc8bfc7ca46 Mon Sep 17 00:00:00 2001 From: judds Date: Thu, 23 Aug 2018 17:09:05 -0700 Subject: [PATCH] Automated g4 rollback of changelist 209077692. *** Reason for rollback *** All requests still end up failing. *** Original change description *** Add back assertion on empty resource classes in ResourceCacheGenerator Currently we're failing to load some images that are present in cache due to a race condition. I haven't had any luck looking through the code to see why this happens. I suspect a race of some sort. I'm hoping that adding the exception back in with additional logging will help me move forward. I'm also hoping that this change avoids the worst of the behavior in the bug by failing only one request, rather than all requests.... *** ------------- Created by MOE: https://github.com/google/moe MOE_MIGRATED_REVID=210027336 --- .../java/com/bumptech/glide/Registry.java | 33 ++----------------- .../load/engine/ResourceCacheGenerator.java | 6 ++-- 2 files changed, 5 insertions(+), 34 deletions(-) diff --git a/library/src/main/java/com/bumptech/glide/Registry.java b/library/src/main/java/com/bumptech/glide/Registry.java index ac31980b4b..62f135d7d1 100644 --- a/library/src/main/java/com/bumptech/glide/Registry.java +++ b/library/src/main/java/com/bumptech/glide/Registry.java @@ -25,13 +25,10 @@ import com.bumptech.glide.provider.ResourceDecoderRegistry; import com.bumptech.glide.provider.ResourceEncoderRegistry; import com.bumptech.glide.util.pool.FactoryPools; -import java.io.File; import java.util.ArrayList; import java.util.Arrays; import java.util.Collections; -import java.util.HashMap; import java.util.List; -import java.util.Map; /** * Manages component registration to extend or replace Glide's default loading, decoding, and @@ -533,43 +530,17 @@ public List> getRegisteredResourceClasses if (result == null) { result = new ArrayList<>(); List> dataClasses = modelLoaderRegistry.getDataClasses(modelClass); - - Map, List>> dataClassToResourceClasses = new HashMap<>(); - Map, List>> resourceClassToTranscodeClasses = new HashMap<>(); - for (Class dataClass : dataClasses) { List> registeredResourceClasses = decoderRegistry.getResourceClasses(dataClass, resourceClass); - - dataClassToResourceClasses.put(dataClass, registeredResourceClasses); - for (Class registeredResourceClass : registeredResourceClasses) { - - List> registeredTranscodeClasses = - transcoderRegistry.getTranscodeClasses(registeredResourceClass, transcodeClass); - - resourceClassToTranscodeClasses.put(registeredResourceClass, registeredTranscodeClasses); - + List> registeredTranscodeClasses = transcoderRegistry + .getTranscodeClasses(registeredResourceClass, transcodeClass); if (!registeredTranscodeClasses.isEmpty() && !result.contains(registeredResourceClass)) { result.add(registeredResourceClass); } } } - - // Throw the exception before populating the cache in the hopes that a subsequent attempt will - // succeed and only one request will randomly fail. This is really debugging logic that should - // go away when we find the actual cause for b/73882030. - if (result.isEmpty() && !File.class.equals(transcodeClass)) { - if (dataClasses.isEmpty()) { - throw new IllegalStateException("Failed to find any data classes for: " + modelClass); - } - - throw new IllegalStateException( - "Failed to find any resource or transcode classes for model: " + modelClass - + ", data to resource classes: " + dataClassToResourceClasses - + ", resource to transcode classes: " + resourceClassToTranscodeClasses); - } - modelToResourceClassCache.put(modelClass, resourceClass, Collections.unmodifiableList(result)); } diff --git a/library/src/main/java/com/bumptech/glide/load/engine/ResourceCacheGenerator.java b/library/src/main/java/com/bumptech/glide/load/engine/ResourceCacheGenerator.java index cd6c13a3e5..86cdfb5063 100644 --- a/library/src/main/java/com/bumptech/glide/load/engine/ResourceCacheGenerator.java +++ b/library/src/main/java/com/bumptech/glide/load/engine/ResourceCacheGenerator.java @@ -53,9 +53,9 @@ public boolean startNext() { // TODO(b/73882030): This case gets triggered when it shouldn't. With this assertion it causes // all loads to fail. Without this assertion it causes loads to miss the disk cache // unnecessarily - throw new IllegalStateException( - "Failed to find any load path from " + helper.getModelClass() + " to " - + helper.getTranscodeClass()); + // throw new IllegalStateException( + // "Failed to find any load path from " + helper.getModelClass() + " to " + // + helper.getTranscodeClass()); } while (modelLoaders == null || !hasNextModelLoader()) { resourceClassIndex++;