Add Experiments class to Glide.

We seem to keep adding more and more experiments. Hopefully this makes them a bit easier to keep track of and reduces the pass through work required each time one is added or removed. PiperOrigin-RevId: 337556489
bumptech · Oct 16, 2020 · 6d7c484 · 6d7c484
1 parent 9c6eae7
commit 6d7c484
Show file tree

Hide file tree

Showing 8 changed files with 105 additions and 32 deletions.
diff --git a/checkstyle_suppressions.xml b/checkstyle_suppressions.xml
@@ -5,6 +5,7 @@
     "http://www.puppycrawl.com/dtds/suppressions_1_1.dtd">
 
 <suppressions>
+    <suppress files=".*[/\\]library[/\\]src[/\\]main[/\\]java[/\\]com[/\\]bumptech[/\\]glide[/\\]GlideExperiments.java" checks="FinalClass"/>
     <suppress files=".*[/\\]library[/\\]src[/\\]main[/\\]java[/\\]com[/\\]bumptech[/\\]glide[/\\]util[/\\]CachedHashCodeArrayMap.java" checks="EqualsHashCodeCheck"/>
     <suppress files=".*[/\\]library[/\\]test[/\\]src[/\\]test[/\\].*" checks="Javadoc.*"/>
     <suppress files=".*[/\\]annotation[/\\]compiler[/\\]test[/\\]src[/\\]test[/\\]resources[/\\].*" checks=".*"/>

diff --git a/library/src/main/java/com/bumptech/glide/Glide.java b/library/src/main/java/com/bumptech/glide/Glide.java
@@ -22,6 +22,7 @@
 import androidx.annotation.VisibleForTesting;
 import androidx.fragment.app.Fragment;
 import androidx.fragment.app.FragmentActivity;
+import com.bumptech.glide.GlideBuilder.EnableImageDecoderForBitmaps;
 import com.bumptech.glide.gifdecoder.GifDecoder;
 import com.bumptech.glide.load.DecodeFormat;
 import com.bumptech.glide.load.ImageHeaderParser;
@@ -384,9 +385,7 @@ private static void throwIncorrectGlideModule(Exception e) {
       @NonNull RequestOptionsFactory defaultRequestOptionsFactory,
       @NonNull Map<Class<?>, TransitionOptions<?, ?>> defaultTransitionOptions,
       @NonNull List<RequestListener<Object>> defaultRequestListeners,
-      boolean isLoggingRequestOriginsEnabled,
-      boolean isImageDecoderEnabledForBitmaps,
-      int manualOverrideHardwareBitmapMaxFdCount) {
+      GlideExperiments experiments) {
     this.engine = engine;
     this.bitmapPool = bitmapPool;
     this.arrayPool = arrayPool;
@@ -419,7 +418,8 @@ private static void throwIncorrectGlideModule(Exception e) {
 
     ResourceDecoder<ByteBuffer, Bitmap> byteBufferBitmapDecoder;
     ResourceDecoder<InputStream, Bitmap> streamBitmapDecoder;
-    if (isImageDecoderEnabledForBitmaps && Build.VERSION.SDK_INT >= Build.VERSION_CODES.P) {
+    if (experiments.isEnabled(EnableImageDecoderForBitmaps.class)
+        && Build.VERSION.SDK_INT >= Build.VERSION_CODES.P) {
       streamBitmapDecoder = new InputStreamBitmapImageDecoderResourceDecoder();
       byteBufferBitmapDecoder = new ByteBufferBitmapImageDecoderResourceDecoder();
     } else {
@@ -604,7 +604,7 @@ Uri.class, Bitmap.class, new ResourceBitmapDecoder(resourceDrawableDecoder, bitm
             defaultTransitionOptions,
             defaultRequestListeners,
             engine,
-            isLoggingRequestOriginsEnabled,
+            experiments,
             logLevel);
   }
 

diff --git a/library/src/main/java/com/bumptech/glide/GlideBuilder.java b/library/src/main/java/com/bumptech/glide/GlideBuilder.java
@@ -2,12 +2,13 @@
 
 import android.content.Context;
 import android.graphics.Bitmap;
+import android.os.Build;
 import android.util.Log;
 import androidx.annotation.NonNull;
 import androidx.annotation.Nullable;
 import androidx.collection.ArrayMap;
-import androidx.core.os.BuildCompat;
 import com.bumptech.glide.Glide.RequestOptionsFactory;
+import com.bumptech.glide.GlideExperiments.Experiment;
 import com.bumptech.glide.load.DataSource;
 import com.bumptech.glide.load.engine.Engine;
 import com.bumptech.glide.load.engine.GlideException;
@@ -22,7 +23,6 @@
 import com.bumptech.glide.load.engine.cache.MemoryCache;
 import com.bumptech.glide.load.engine.cache.MemorySizeCalculator;
 import com.bumptech.glide.load.engine.executor.GlideExecutor;
-import com.bumptech.glide.load.resource.bitmap.HardwareConfigState;
 import com.bumptech.glide.manager.ConnectivityMonitorFactory;
 import com.bumptech.glide.manager.DefaultConnectivityMonitorFactory;
 import com.bumptech.glide.manager.RequestManagerRetriever;
@@ -41,6 +41,7 @@
 @SuppressWarnings("PMD.ImmutableField")
 public final class GlideBuilder {
   private final Map<Class<?>, TransitionOptions<?, ?>> defaultTransitionOptions = new ArrayMap<>();
+  private final GlideExperiments.Builder glideExperiments = new GlideExperiments.Builder();
   private Engine engine;
   private BitmapPool bitmapPool;
   private ArrayPool arrayPool;
@@ -63,10 +64,6 @@ public RequestOptions build() {
   private GlideExecutor animationExecutor;
   private boolean isActiveResourceRetentionAllowed;
   @Nullable private List<RequestListener<Object>> defaultRequestListeners;
-  private boolean isLoggingRequestOriginsEnabled;
-
-  private boolean isImageDecoderEnabledForBitmaps;
-  private int manualOverrideHardwareBitmapMaxFdCount = HardwareConfigState.NO_MAX_FD_COUNT;
 
   /**
    * Sets the {@link com.bumptech.glide.load.engine.bitmap_recycle.BitmapPool} implementation to use
@@ -451,7 +448,7 @@ public GlideBuilder addGlobalRequestListener(@NonNull RequestListener<Object> li
    * <p>This is an experimental API that may be removed in the future.
    */
   public GlideBuilder setLogRequestOrigins(boolean isEnabled) {
-    isLoggingRequestOriginsEnabled = isEnabled;
+    glideExperiments.update(new LogRequestOrigins(), isEnabled);
     return this;
   }
 
@@ -482,10 +479,9 @@ public GlideBuilder setLogRequestOrigins(boolean isEnabled) {
    * which may not agree.
    */
   public GlideBuilder setImageDecoderEnabledForBitmaps(boolean isEnabled) {
-    if (!BuildCompat.isAtLeastQ()) {
-      return this;
-    }
-    isImageDecoderEnabledForBitmaps = isEnabled;
+    glideExperiments.update(
+        new EnableImageDecoderForBitmaps(),
+        /*isEnabled=*/ isEnabled && Build.VERSION.SDK_INT >= Build.VERSION_CODES.Q);
     return this;
   }
 
@@ -575,8 +571,20 @@ Glide build(@NonNull Context context) {
         defaultRequestOptionsFactory,
         defaultTransitionOptions,
         defaultRequestListeners,
-        isLoggingRequestOriginsEnabled,
-        isImageDecoderEnabledForBitmaps,
-        manualOverrideHardwareBitmapMaxFdCount);
+        glideExperiments.build());
   }
+
+  static final class ManualOverrideHardwareBitmapMaxFdCount implements Experiment {
+
+    final int fdCount;
+
+    ManualOverrideHardwareBitmapMaxFdCount(int fdCount) {
+      this.fdCount = fdCount;
+    }
+  }
+
+  static final class EnableImageDecoderForBitmaps implements Experiment {}
+
+  /** See {@link #setLogRequestOrigins(boolean)}. */
+  public static final class LogRequestOrigins implements Experiment {}
 }
diff --git a/library/src/main/java/com/bumptech/glide/GlideContext.java b/library/src/main/java/com/bumptech/glide/GlideContext.java
@@ -22,6 +22,7 @@
  * Global context for all loads in Glide containing and exposing the various registries and classes
  * required to load resources.
  */
+@SuppressWarnings("PMD.DataClass")
 public class GlideContext extends ContextWrapper {
   @VisibleForTesting
   static final TransitionOptions<?, ?> DEFAULT_TRANSITION_OPTIONS =
@@ -34,7 +35,7 @@ public class GlideContext extends ContextWrapper {
   private final List<RequestListener<Object>> defaultRequestListeners;
   private final Map<Class<?>, TransitionOptions<?, ?>> defaultTransitionOptions;
   private final Engine engine;
-  private final boolean isLoggingRequestOriginsEnabled;
+  private final GlideExperiments experiments;
   private final int logLevel;
 
   @Nullable
@@ -50,7 +51,7 @@ public GlideContext(
       @NonNull Map<Class<?>, TransitionOptions<?, ?>> defaultTransitionOptions,
       @NonNull List<RequestListener<Object>> defaultRequestListeners,
       @NonNull Engine engine,
-      boolean isLoggingRequestOriginsEnabled,
+      @NonNull GlideExperiments experiments,
       int logLevel) {
     super(context.getApplicationContext());
     this.arrayPool = arrayPool;
@@ -60,7 +61,7 @@ public GlideContext(
     this.defaultRequestListeners = defaultRequestListeners;
     this.defaultTransitionOptions = defaultTransitionOptions;
     this.engine = engine;
-    this.isLoggingRequestOriginsEnabled = isLoggingRequestOriginsEnabled;
+    this.experiments = experiments;
     this.logLevel = logLevel;
   }
 
@@ -118,13 +119,7 @@ public ArrayPool getArrayPool() {
     return arrayPool;
   }
 
-  /**
-   * Returns {@code true} if Glide should populate {@link
-   * com.bumptech.glide.load.engine.GlideException#setOrigin(Exception)} for failed requests.
-   *
-   * <p>This is an experimental API that may be removed in the future.
-   */
-  public boolean isLoggingRequestOriginsEnabled() {
-    return isLoggingRequestOriginsEnabled;
+  public GlideExperiments getExperiments() {
+    return experiments;
   }
 }
diff --git a/library/src/main/java/com/bumptech/glide/GlideExperiments.java b/library/src/main/java/com/bumptech/glide/GlideExperiments.java
@@ -0,0 +1,66 @@
+package com.bumptech.glide;
+
+import androidx.annotation.Nullable;
+import com.bumptech.glide.util.Synthetic;
+import java.util.Collections;
+import java.util.HashMap;
+import java.util.Map;
+
+/**
+ * Keeps track of a set of Experimental features that may be enabled in Glide, simplifying the
+ * process of adding and removing them.
+ *
+ * <p>This is an experimental API, it may be removed at any point without deprecation or other
+ * notice.
+ */
+// non-final for mocking
+public class GlideExperiments {
+
+  private final Map<Class<?>, Experiment> experiments;
+
+  @Synthetic
+  GlideExperiments(Builder builder) {
+    this.experiments =
+        Collections.unmodifiableMap(new HashMap<Class<?>, Experiment>(builder.experiments));
+  }
+
+  @SuppressWarnings("unchecked")
+  @Nullable
+  <T extends Experiment> T get(Class<T> clazz) {
+    return (T) experiments.get(clazz);
+  }
+
+  /**
+   * Returns {@code true} if the given experiment is enabled.
+   *
+   * <p>This is an experimental API, it may be removed at any point without deprecation or other
+   * notice.
+   */
+  public boolean isEnabled(Class<? extends Experiment> clazz) {
+    return experiments.containsKey(clazz);
+  }
+
+  interface Experiment {}
+
+  static final class Builder {
+    private final Map<Class<?>, Experiment> experiments = new HashMap<>();
+
+    Builder update(Experiment experiment, boolean isEnabled) {
+      if (isEnabled) {
+        add(experiment);
+      } else {
+        experiments.remove(experiment.getClass());
+      }
+      return this;
+    }
+
+    Builder add(Experiment experiment) {
+      experiments.put(experiment.getClass(), experiment);
+      return this;
+    }
+
+    GlideExperiments build() {
+      return new GlideExperiments(this);
+    }
+  }
+}
diff --git a/library/src/main/java/com/bumptech/glide/request/SingleRequest.java b/library/src/main/java/com/bumptech/glide/request/SingleRequest.java
@@ -8,6 +8,7 @@
 import androidx.annotation.GuardedBy;
 import androidx.annotation.NonNull;
 import androidx.annotation.Nullable;
+import com.bumptech.glide.GlideBuilder.LogRequestOrigins;
 import com.bumptech.glide.GlideContext;
 import com.bumptech.glide.Priority;
 import com.bumptech.glide.load.DataSource;
@@ -203,7 +204,7 @@ private SingleRequest(
     this.callbackExecutor = callbackExecutor;
     status = Status.PENDING;
 
-    if (requestOrigin == null && glideContext.isLoggingRequestOriginsEnabled()) {
+    if (requestOrigin == null && glideContext.getExperiments().isEnabled(LogRequestOrigins.class)) {
       requestOrigin = new RuntimeException("Glide request origin trace");
     }
   }

diff --git a/library/test/src/test/java/com/bumptech/glide/GlideContextTest.java b/library/test/src/test/java/com/bumptech/glide/GlideContextTest.java
@@ -52,7 +52,7 @@ public RequestOptions build() {
             transitionOptions,
             /*defaultRequestListeners=*/ Collections.<RequestListener<Object>>emptyList(),
             mock(Engine.class),
-            /*isLoggingRequestOriginsEnabled=*/ false,
+            mock(GlideExperiments.class),
             Log.DEBUG);
   }
 

diff --git a/library/test/src/test/java/com/bumptech/glide/request/SingleRequestTest.java b/library/test/src/test/java/com/bumptech/glide/request/SingleRequestTest.java
@@ -23,6 +23,7 @@
 import androidx.annotation.NonNull;
 import androidx.annotation.Nullable;
 import com.bumptech.glide.GlideContext;
+import com.bumptech.glide.GlideExperiments;
 import com.bumptech.glide.Priority;
 import com.bumptech.glide.load.DataSource;
 import com.bumptech.glide.load.Key;
@@ -944,6 +945,7 @@ static final class SingleRequestBuilder {
     private final Map<Class<?>, Transformation<?>> transformations = new HashMap<>();
 
     SingleRequestBuilder() {
+      when(glideContext.getExperiments()).thenReturn(mock(GlideExperiments.class));
       when(requestCoordinator.getRoot()).thenReturn(requestCoordinator);
       when(requestCoordinator.canSetImage(any(Request.class))).thenReturn(true);
       when(requestCoordinator.canNotifyCleared(any(Request.class))).thenReturn(true);