Add a method of frame waiting that resets when we get certain trim memory callbacks.

The GL context may be destroyed on TRIM_MEMORY_MODERATE or higher. We could strictly listen to that callback, but we'd end up racing because we only get trimMemory on the UI thread and decodes happen on multiple background threads. Hopefully by resetting when the app is backgrounded we can minimize those races.

PiperOrigin-RevId: 337927814

Author: sjudd

instrumentation/src/androidTest/java/com/bumptech/glide/LoadResourcesWithDownsamplerTest.java

@@ -28,11 +28,14 @@
 import com.bumptech.glide.test.GlideApp;
 import com.bumptech.glide.test.ResourceIds;
 import com.bumptech.glide.test.TearDownGlide;
+import com.bumptech.glide.util.Util;
 import java.io.ByteArrayOutputStream;
 import java.io.FileNotFoundException;
 import java.io.IOException;
 import java.io.InputStream;
 import java.util.Locale;
+import java.util.concurrent.CountDownLatch;
+import java.util.concurrent.TimeUnit;
 import org.junit.Rule;
 import org.junit.Test;
 import org.junit.runner.RunWith;
@@ -117,11 +120,22 @@ public void loadTransparentGifResource_withNoOtherLoaders_decodesResource() {
   }
 
   @Test
-  public void loadTransparentGifResource_asHardware_withNoOtherLoaders_decodesResource() {
+  public void loadTransparentGifResource_asHardware_withNoOtherLoaders_decodesResource()
+      throws InterruptedException {
     assumeTrue(
         "Hardware Bitmaps are only supported on O+",
         Build.VERSION.SDK_INT >= Build.VERSION_CODES.O);
-    Glide.enableHardwareBitmaps();
+    // enableHardwareBitmaps must be called on the main thread.
+    final CountDownLatch latch = new CountDownLatch(1);
+    Util.postOnUiThread(
+        new Runnable() {
+          @Override
+          public void run() {
+            Glide.enableHardwareBitmaps();
+            latch.countDown();
+          }
+        });
+    latch.await(5, TimeUnit.SECONDS);
 
     Glide.get(context)
         .getRegistry()

library/src/main/java/com/bumptech/glide/GlideBuilder.java

@@ -41,7 +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 final GlideExperiments.Builder glideExperimentsBuilder = new GlideExperiments.Builder();
   private Engine engine;
   private BitmapPool bitmapPool;
   private ArrayPool arrayPool;
@@ -448,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) {
-    glideExperiments.update(new LogRequestOrigins(), isEnabled);
+    glideExperimentsBuilder.update(new LogRequestOrigins(), isEnabled);
     return this;
   }
 
@@ -479,7 +479,7 @@ public GlideBuilder setLogRequestOrigins(boolean isEnabled) {
    * which may not agree.
    */
   public GlideBuilder setImageDecoderEnabledForBitmaps(boolean isEnabled) {
-    glideExperiments.update(
+    glideExperimentsBuilder.update(
         new EnableImageDecoderForBitmaps(),
         /*isEnabled=*/ isEnabled && Build.VERSION.SDK_INT >= Build.VERSION_CODES.Q);
     return this;
@@ -556,8 +556,9 @@ Glide build(@NonNull Context context) {
       defaultRequestListeners = Collections.unmodifiableList(defaultRequestListeners);
     }
 
+    GlideExperiments experiments = glideExperimentsBuilder.build();
     RequestManagerRetriever requestManagerRetriever =
-        new RequestManagerRetriever(requestManagerFactory);
+        new RequestManagerRetriever(requestManagerFactory, experiments);
 
     return new Glide(
         context,
@@ -571,7 +572,7 @@ Glide build(@NonNull Context context) {
         defaultRequestOptionsFactory,
         defaultTransitionOptions,
         defaultRequestListeners,
-        glideExperiments.build());
+        experiments);
   }
 
   static final class ManualOverrideHardwareBitmapMaxFdCount implements Experiment {
@@ -583,6 +584,11 @@ static final class ManualOverrideHardwareBitmapMaxFdCount implements Experiment
     }
   }
 
+  /** See {@link #setWaitForFramesAfterTrimMemory(boolean)}. */
+  public static final class WaitForFramesAfterTrimMemory implements Experiment {
+    private WaitForFramesAfterTrimMemory() {}
+  }
+
   static final class EnableImageDecoderForBitmaps implements Experiment {}
 
   /** See {@link #setLogRequestOrigins(boolean)}. */

library/src/main/java/com/bumptech/glide/load/resource/bitmap/HardwareConfigState.java

@@ -7,21 +7,25 @@
 import android.util.Log;
 import androidx.annotation.GuardedBy;
 import androidx.annotation.VisibleForTesting;
+import com.bumptech.glide.util.Util;
 import java.io.File;
 import java.util.Arrays;
+import java.util.concurrent.atomic.AtomicBoolean;
 
 /**
  * State and constants for interacting with {@link android.graphics.Bitmap.Config#HARDWARE} on
  * Android O+.
  */
 public final class HardwareConfigState {
+  private static final String TAG = "HardwareConfig";
+
   /**
    * Force the state to wait until a call to allow hardware Bitmaps to be used when they'd otherwise
    * be eligible to work around a framework issue pre Q that can cause a native crash when
    * allocating a hardware Bitmap in this specific circumstance. See b/126573603#comment12 for
    * details.
    */
-  private static final boolean BLOCK_HARDWARE_BITMAPS_BY_DEFAULT =
+  private static final boolean BLOCK_HARDWARE_BITMAPS_WHEN_GL_CONTEXT_MIGHT_NOT_BE_INITIALIZED =
       Build.VERSION.SDK_INT < Build.VERSION_CODES.Q;
 
   /**
@@ -87,7 +91,13 @@ public final class HardwareConfigState {
   @GuardedBy("this")
   private boolean isFdSizeBelowHardwareLimit = true;
 
-  private volatile boolean areHardwareBitmapsUnblocked;
+  /**
+   * Only mutated on the main thread. Read by any number of background threads concurrently.
+   *
+   * <p>Defaults to {@code false} because we need to wait for the GL context to be initialized and
+   * it defaults to not initialized (https://b.corp.google.com/issues/126573603#comment12).
+   */
+  private final AtomicBoolean isHardwareConfigAllowedByAppState = new AtomicBoolean(false);
 
   public static HardwareConfigState getInstance() {
     if (instance == null) {
@@ -112,31 +122,77 @@ public static HardwareConfigState getInstance() {
     }
   }
 
-  public void unblockHardwareBitmaps() {
-    areHardwareBitmapsUnblocked = true;
+  public synchronized void blockHardwareBitmaps() {
+    Util.assertMainThread();
+    isHardwareConfigAllowedByAppState.set(false);
+  }
+
+  public synchronized void unblockHardwareBitmaps() {
+    Util.assertMainThread();
+    isHardwareConfigAllowedByAppState.set(true);
   }
 
   public boolean isHardwareConfigAllowed(
       int targetWidth,
       int targetHeight,
       boolean isHardwareConfigAllowed,
       boolean isExifOrientationRequired) {
-    if (!isHardwareConfigAllowed
-        || !isHardwareConfigAllowedByDeviceModel
-        || Build.VERSION.SDK_INT < Build.VERSION_CODES.O
-        || areHardwareBitmapsBlockedByAppState()
-        || isExifOrientationRequired) {
+    if (!isHardwareConfigAllowed) {
+      if (Log.isLoggable(TAG, Log.VERBOSE)) {
+        Log.v(TAG, "Hardware config disallowed by caller");
+      }
+      return false;
+    }
+    if (!isHardwareConfigAllowedByDeviceModel) {
+      if (Log.isLoggable(TAG, Log.VERBOSE)) {
+        Log.v(TAG, "Hardware config disallowed by device model");
+      }
+      return false;
+    }
+    if (Build.VERSION.SDK_INT < Build.VERSION_CODES.O) {
+      if (Log.isLoggable(TAG, Log.VERBOSE)) {
+        Log.v(TAG, "Hardware config disallowed by sdk");
+      }
+      return false;
+    }
+    if (areHardwareBitmapsBlockedByAppState()) {
+      if (Log.isLoggable(TAG, Log.VERBOSE)) {
+        Log.v(TAG, "Hardware config disallowed by app state");
+      }
+      return false;
+    }
+    if (isExifOrientationRequired) {
+      if (Log.isLoggable(TAG, Log.VERBOSE)) {
+        Log.v(TAG, "Hardware config disallowed because exif orientation is required");
+      }
+      return false;
+    }
+    if (targetWidth < minHardwareDimension) {
+      if (Log.isLoggable(TAG, Log.VERBOSE)) {
+        Log.v(TAG, "Hardware config disallowed because width is too small");
+      }
+      return false;
+    }
+    if (targetHeight < minHardwareDimension) {
+      if (Log.isLoggable(TAG, Log.VERBOSE)) {
+        Log.v(TAG, "Hardware config disallowed because height is too small");
+      }
+      return false;
+    }
+    // Make sure to call isFdSizeBelowHardwareLimit last because it has side affects.
+    if (!isFdSizeBelowHardwareLimit()) {
+      if (Log.isLoggable(TAG, Log.VERBOSE)) {
+        Log.v(TAG, "Hardware config disallowed because there are insufficient FDs");
+      }
       return false;
     }
 
-    return targetWidth >= minHardwareDimension
-        && targetHeight >= minHardwareDimension
-        // Make sure to call isFdSizeBelowHardwareLimit last because it has side affects.
-        && isFdSizeBelowHardwareLimit();
+    return true;
   }
 
   private boolean areHardwareBitmapsBlockedByAppState() {
-    return BLOCK_HARDWARE_BITMAPS_BY_DEFAULT && !areHardwareBitmapsUnblocked;
+    return BLOCK_HARDWARE_BITMAPS_WHEN_GL_CONTEXT_MIGHT_NOT_BE_INITIALIZED
+        && !isHardwareConfigAllowedByAppState.get();
   }
 
   @TargetApi(Build.VERSION_CODES.O)

library/src/main/java/com/bumptech/glide/manager/DoNothingFirstFrameWaiter.java

@@ -2,7 +2,7 @@
 
 import android.app.Activity;
 
-final class DoNothingFirstFrameWaiter implements FirstFrameWaiter {
+final class DoNothingFirstFrameWaiter implements FrameWaiter {
 
   @Override
   public void registerSelf(Activity activity) {

library/src/main/java/com/bumptech/glide/manager/FirstFrameAndAfterTrimMemoryWaiter.java

@@ -0,0 +1,24 @@
+package com.bumptech.glide.manager;
+
+import android.app.Activity;
+import android.content.ComponentCallbacks2;
+import android.content.res.Configuration;
+import android.os.Build;
+import androidx.annotation.NonNull;
+import androidx.annotation.RequiresApi;
+
+@RequiresApi(Build.VERSION_CODES.O)
+final class FirstFrameAndAfterTrimMemoryWaiter implements FrameWaiter, ComponentCallbacks2 {
+
+  @Override
+  public void registerSelf(Activity activity) {}
+
+  @Override
+  public void onTrimMemory(int level) {}
+
+  @Override
+  public void onConfigurationChanged(@NonNull Configuration newConfig) {}
+
+  @Override
+  public void onLowMemory() {}
+}

library/src/main/java/com/bumptech/glide/manager/FirstFrameWaiter.java

@@ -1,7 +1,12 @@
 package com.bumptech.glide.manager;
 
 import android.app.Activity;
+import android.os.Build;
+import androidx.annotation.RequiresApi;
 
-interface FirstFrameWaiter {
-  void registerSelf(Activity activity);
+@RequiresApi(Build.VERSION_CODES.O)
+final class FirstFrameWaiter implements FrameWaiter {
+
+  @Override
+  public void registerSelf(Activity activity) {}
 }

library/src/main/java/com/bumptech/glide/manager/FirstFrameWaiterO.java

@@ -0,0 +1,7 @@
+package com.bumptech.glide.manager;
+
+import android.app.Activity;
+
+interface FrameWaiter {
+  void registerSelf(Activity activity);
+}

library/src/main/java/com/bumptech/glide/manager/FrameWaiter.java

@@ -22,6 +22,8 @@
 import androidx.fragment.app.FragmentActivity;
 import androidx.fragment.app.FragmentManager;
 import com.bumptech.glide.Glide;
+import com.bumptech.glide.GlideBuilder.WaitForFramesAfterTrimMemory;
+import com.bumptech.glide.GlideExperiments;
 import com.bumptech.glide.RequestManager;
 import com.bumptech.glide.util.Preconditions;
 import com.bumptech.glide.util.Util;
@@ -70,14 +72,23 @@ public class RequestManagerRetriever implements Handler.Callback {
   // This is really misplaced here, but to put it anywhere else means duplicating all of the
   // Fragment/Activity extraction logic that already exists here. It's gross, but less likely to
   // break.
-  private final FirstFrameWaiter firstFrameWaiter =
-      Build.VERSION.SDK_INT >= Build.VERSION_CODES.O
-          ? new FirstFrameWaiterO()
-          : new DoNothingFirstFrameWaiter();
+  private final FrameWaiter frameWaiter;
 
-  public RequestManagerRetriever(@Nullable RequestManagerFactory factory) {
+  public RequestManagerRetriever(
+      @Nullable RequestManagerFactory factory, GlideExperiments experiments) {
     this.factory = factory != null ? factory : DEFAULT_FACTORY;
     handler = new Handler(Looper.getMainLooper(), this /* Callback */);
+
+    frameWaiter = buildFrameWaiter(experiments);
+  }
+
+  private static FrameWaiter buildFrameWaiter(GlideExperiments experiments) {
+    if (Build.VERSION.SDK_INT < Build.VERSION_CODES.O) {
+      return new DoNothingFirstFrameWaiter();
+    }
+    return experiments.isEnabled(WaitForFramesAfterTrimMemory.class)
+        ? new FirstFrameAndAfterTrimMemoryWaiter()
+        : new FirstFrameWaiter();
   }
 
   @NonNull
@@ -133,7 +144,7 @@ public RequestManager get(@NonNull FragmentActivity activity) {
       return get(activity.getApplicationContext());
     } else {
       assertNotDestroyed(activity);
-      firstFrameWaiter.registerSelf(activity);
+      frameWaiter.registerSelf(activity);
       FragmentManager fm = activity.getSupportFragmentManager();
       return supportFragmentGet(activity, fm, /*parentHint=*/ null, isActivityVisible(activity));
     }
@@ -152,7 +163,7 @@ public RequestManager get(@NonNull Fragment fragment) {
       // we manage not to register the first frame waiter for a while, the consequences are not
       // catastrophic, we'll just use some extra memory.
       if (fragment.getActivity() != null) {
-        firstFrameWaiter.registerSelf(fragment.getActivity());
+        frameWaiter.registerSelf(fragment.getActivity());
       }
       FragmentManager fm = fragment.getChildFragmentManager();
       return supportFragmentGet(fragment.getContext(), fm, fragment, fragment.isVisible());
@@ -168,7 +179,7 @@ public RequestManager get(@NonNull Activity activity) {
       return get((FragmentActivity) activity);
     } else {
       assertNotDestroyed(activity);
-      firstFrameWaiter.registerSelf(activity);
+      frameWaiter.registerSelf(activity);
       android.app.FragmentManager fm = activity.getFragmentManager();
       return fragmentGet(activity, fm, /*parentHint=*/ null, isActivityVisible(activity));
     }
@@ -353,7 +364,7 @@ public RequestManager get(@NonNull android.app.Fragment fragment) {
       // we manage not to register the first frame waiter for a while, the consequences are not
       // catastrophic, we'll just use some extra memory.
       if (fragment.getActivity() != null) {
-        firstFrameWaiter.registerSelf(fragment.getActivity());
+        frameWaiter.registerSelf(fragment.getActivity());
       }
       android.app.FragmentManager fm = fragment.getChildFragmentManager();
       return fragmentGet(fragment.getActivity(), fm, fragment, fragment.isVisible());

library/src/main/java/com/bumptech/glide/manager/RequestManagerRetriever.java

@@ -19,7 +19,7 @@ public class HardwareConfigStateTest {
   @Config(sdk = Build.VERSION_CODES.O)
   @Test
   public void
-      setHardwareConfigIfAllowed_withAllowedState_setsInPreferredConfigAndMutable_returnsFalse() {
+      setHardwareConfigIfAllowed_withAllowedState_setsInPreferredConfigAndMutable_returnsTrue() {
     HardwareConfigState state = new HardwareConfigState();
     state.unblockHardwareBitmaps();
     BitmapFactory.Options options = new BitmapFactory.Options();
@@ -32,10 +32,29 @@ public class HardwareConfigStateTest {
             /*isExifOrientationRequired=*/ false);
 
     assertThat(result).isTrue();
-    assertThat(options.inMutable).isFalse();
     assertThat(options.inPreferredConfig).isEqualTo(Bitmap.Config.HARDWARE);
   }
 
+  @Config(sdk = Build.VERSION_CODES.O)
+  @Test
+  public void
+      setHardwareConfigIfAllowed_withAllowedState_afterReblock_returnsFalseAndDoesNotSetValues() {
+    HardwareConfigState state = new HardwareConfigState();
+    state.unblockHardwareBitmaps();
+    state.blockHardwareBitmaps();
+    BitmapFactory.Options options = new BitmapFactory.Options();
+    boolean result =
+        state.setHardwareConfigIfAllowed(
+            /*targetWidth=*/ HardwareConfigState.MIN_HARDWARE_DIMENSION_O,
+            /*targetHeight=*/ HardwareConfigState.MIN_HARDWARE_DIMENSION_O,
+            options,
+            /*isHardwareConfigAllowed=*/ true,
+            /*isExifOrientationRequired=*/ false);
+
+    assertThat(result).isFalse();
+    assertThat(options.inPreferredConfig).isNotEqualTo(Bitmap.Config.HARDWARE);
+  }
+
   @Config(sdk = Build.VERSION_CODES.O)
   @Test
   public void setHardwareConfigIfAllowed_withSmallerThanMinWidth_returnsFalse_doesNotSetValues() {

library/test/src/test/java/com/bumptech/glide/load/resource/bitmap/HardwareConfigStateTest.java

@@ -24,6 +24,7 @@
 import androidx.fragment.app.FragmentController;
 import androidx.fragment.app.FragmentHostCallback;
 import androidx.test.core.app.ApplicationProvider;
+import com.bumptech.glide.GlideExperiments;
 import com.bumptech.glide.RequestManager;
 import com.bumptech.glide.tests.BackgroundUtil.BackgroundTester;
 import com.bumptech.glide.tests.GlideShadowLooper;
@@ -58,7 +59,7 @@ public class RequestManagerRetrieverTest {
   public void setUp() {
     appContext = ApplicationProvider.getApplicationContext();
 
-    retriever = new RequestManagerRetriever(/*factory=*/ null);
+    retriever = new RequestManagerRetriever(/*factory=*/ null, mock(GlideExperiments.class));
 
     harnesses =
         new RetrieverHarness[] {new DefaultRetrieverHarness(), new SupportRetrieverHarness()};