Wait for the app to render at least one frame before enabling Hardwar…

…e bitmaps. There appears to be a bug on versions of Android < Q where trying to create a hardware bitmap before the application has rendered a single frame causes a native crash. This works around the issue by making sure Glide only uses regular Bitmaps until the application has drawn at least one frame. PiperOrigin-RevId: 331202490
bumptech · Sep 11, 2020 · 8584b45 · 8584b45
1 parent edd96d8
commit 8584b45
Show file tree

Hide file tree

Showing 8 changed files with 26 additions and 93 deletions.
diff --git a/annotation/compiler/test/src/test/resources/EmptyAppGlideModuleTest/GlideApp.java b/annotation/compiler/test/src/test/resources/EmptyAppGlideModuleTest/GlideApp.java
@@ -69,15 +69,6 @@ public static void init(@NonNull Context context, @NonNull GlideBuilder builder)
     Glide.init(context, builder);
   }
 
-  /**
-   * @see Glide#enableHardwareBitmaps()
-   */
-  @VisibleForTesting
-  @SuppressLint("VisibleForTests")
-  public static void enableHardwareBitmaps() {
-    Glide.enableHardwareBitmaps();
-  }
-
   /**
    * @see Glide#tearDown()
    */

diff --git a/...rumentation/src/androidTest/java/com/bumptech/glide/LoadResourcesWithDownsamplerTest.java b/...rumentation/src/androidTest/java/com/bumptech/glide/LoadResourcesWithDownsamplerTest.java
@@ -121,7 +121,6 @@ public void loadTransparentGifResource_asHardware_withNoOtherLoaders_decodesReso
     assumeTrue(
         "Hardware Bitmaps are only supported on O+",
         Build.VERSION.SDK_INT >= Build.VERSION_CODES.O);
-    Glide.enableHardwareBitmaps();
 
     Glide.get(context)
         .getRegistry()

diff --git a/library/src/main/java/com/bumptech/glide/Glide.java b/library/src/main/java/com/bumptech/glide/Glide.java
@@ -63,7 +63,6 @@
 import com.bumptech.glide.load.resource.bitmap.DefaultImageHeaderParser;
 import com.bumptech.glide.load.resource.bitmap.Downsampler;
 import com.bumptech.glide.load.resource.bitmap.ExifInterfaceImageHeaderParser;
-import com.bumptech.glide.load.resource.bitmap.HardwareConfigState;
 import com.bumptech.glide.load.resource.bitmap.InputStreamBitmapImageDecoderResourceDecoder;
 import com.bumptech.glide.load.resource.bitmap.ParcelFileDescriptorBitmapDecoder;
 import com.bumptech.glide.load.resource.bitmap.ResourceBitmapDecoder;
@@ -236,19 +235,6 @@ public static void init(@NonNull Context context, @NonNull GlideBuilder builder)
     }
   }
 
-  /**
-   * Allows hardware Bitmaps to be used prior to the first frame in the app being drawn as soon as
-   * this method is called.
-   *
-   * <p>If you use this method in non-test code, your app will experience native crashes on some
-   * versions of Android if you try to decode a hardware Bitmap. This method is only useful for
-   * testing.
-   */
-  @VisibleForTesting
-  public static void enableHardwareBitmaps() {
-    HardwareConfigState.getInstance().unblockHardwareBitmaps();
-  }
-
   @VisibleForTesting
   public static void tearDown() {
     synchronized (Glide.class) {
@@ -387,6 +373,7 @@ private static void throwIncorrectGlideModule(Exception e) {
       @NonNull List<RequestListener<Object>> defaultRequestListeners,
       boolean isLoggingRequestOriginsEnabled,
       boolean isImageDecoderEnabledForBitmaps,
+      boolean blockHardwareBitmaps,
       int manualOverrideHardwareBitmapMaxFdCount) {
     this.engine = engine;
     this.bitmapPool = bitmapPool;

diff --git a/library/src/main/java/com/bumptech/glide/GlideBuilder.java b/library/src/main/java/com/bumptech/glide/GlideBuilder.java
@@ -66,7 +66,9 @@ public RequestOptions build() {
   private boolean isLoggingRequestOriginsEnabled;
 
   private boolean isImageDecoderEnabledForBitmaps;
+  private boolean waitForFirstFrameBeforeEnablingHardwareBitmaps;
   private int manualOverrideHardwareBitmapMaxFdCount = HardwareConfigState.NO_MAX_FD_COUNT;
+  private boolean waitForCallBeforeEnablingHardwareBitmaps;
 
   /**
    * Sets the {@link com.bumptech.glide.load.engine.bitmap_recycle.BitmapPool} implementation to use
@@ -561,7 +563,11 @@ Glide build(@NonNull Context context) {
     }
 
     RequestManagerRetriever requestManagerRetriever =
-        new RequestManagerRetriever(requestManagerFactory);
+        new RequestManagerRetriever(
+            requestManagerFactory, waitForFirstFrameBeforeEnablingHardwareBitmaps);
+
+    boolean blockHardwareBitmaps =
+        waitForFirstFrameBeforeEnablingHardwareBitmaps || waitForCallBeforeEnablingHardwareBitmaps;
 
     return new Glide(
         context,
@@ -577,6 +583,7 @@ Glide build(@NonNull Context context) {
         defaultRequestListeners,
         isLoggingRequestOriginsEnabled,
         isImageDecoderEnabledForBitmaps,
+        blockHardwareBitmaps,
         manualOverrideHardwareBitmapMaxFdCount);
   }
 }
diff --git a/library/src/main/java/com/bumptech/glide/load/resource/bitmap/HardwareConfigState.java b/library/src/main/java/com/bumptech/glide/load/resource/bitmap/HardwareConfigState.java
@@ -15,15 +15,6 @@
  * Android O+.
  */
 public final class HardwareConfigState {
-  /**
-   * 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 =
-      Build.VERSION.SDK_INT < Build.VERSION_CODES.Q;
-
   /**
    * The minimum size in pixels a {@link Bitmap} must be in both dimensions to be created with the
    * {@link Bitmap.Config#HARDWARE} configuration.
@@ -75,6 +66,7 @@ public final class HardwareConfigState {
   public static final int NO_MAX_FD_COUNT = -1;
 
   private static volatile HardwareConfigState instance;
+  private static volatile boolean blockHardwareBitmapsByDefault;
   private static volatile int manualOverrideMaxFdCount = NO_MAX_FD_COUNT;
 
   private final boolean isHardwareConfigAllowedByDeviceModel;
@@ -112,10 +104,6 @@ public static HardwareConfigState getInstance() {
     }
   }
 
-  public void unblockHardwareBitmaps() {
-    areHardwareBitmapsUnblocked = true;
-  }
-
   public boolean isHardwareConfigAllowed(
       int targetWidth,
       int targetHeight,
@@ -124,7 +112,7 @@ public boolean isHardwareConfigAllowed(
     if (!isHardwareConfigAllowed
         || !isHardwareConfigAllowedByDeviceModel
         || Build.VERSION.SDK_INT < Build.VERSION_CODES.O
-        || areHardwareBitmapsBlockedByAppState()
+        || (blockHardwareBitmapsByDefault && !areHardwareBitmapsUnblocked)
         || isExifOrientationRequired) {
       return false;
     }
@@ -135,10 +123,6 @@ public boolean isHardwareConfigAllowed(
         && isFdSizeBelowHardwareLimit();
   }
 
-  private boolean areHardwareBitmapsBlockedByAppState() {
-    return BLOCK_HARDWARE_BITMAPS_BY_DEFAULT && !areHardwareBitmapsUnblocked;
-  }
-
   @TargetApi(Build.VERSION_CODES.O)
   boolean setHardwareConfigIfAllowed(
       int targetWidth,

diff --git a/library/src/main/java/com/bumptech/glide/manager/RequestManagerRetriever.java b/library/src/main/java/com/bumptech/glide/manager/RequestManagerRetriever.java
@@ -70,10 +70,12 @@ 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 = new FirstFrameWaiter();
+  @Nullable private final FirstFrameWaiter firstFrameWaiter;
 
-  public RequestManagerRetriever(@Nullable RequestManagerFactory factory) {
+  public RequestManagerRetriever(
+      @Nullable RequestManagerFactory factory, boolean addFirstFrameWaiter) {
     this.factory = factory != null ? factory : DEFAULT_FACTORY;
+    firstFrameWaiter = addFirstFrameWaiter ? new FirstFrameWaiter() : null;
     handler = new Handler(Looper.getMainLooper(), this /* Callback */);
   }
 
@@ -124,13 +126,19 @@ public RequestManager get(@NonNull Context context) {
     return getApplicationManager(context);
   }
 
+  private void maybeRegisterFirstFrameWaiter(@NonNull Activity activity) {
+    if (firstFrameWaiter != null) {
+      firstFrameWaiter.registerSelf(activity);
+    }
+  }
+
   @NonNull
   public RequestManager get(@NonNull FragmentActivity activity) {
     if (Util.isOnBackgroundThread()) {
       return get(activity.getApplicationContext());
     } else {
       assertNotDestroyed(activity);
-      firstFrameWaiter.registerSelf(activity);
+      maybeRegisterFirstFrameWaiter(activity);
       FragmentManager fm = activity.getSupportFragmentManager();
       return supportFragmentGet(activity, fm, /*parentHint=*/ null, isActivityVisible(activity));
     }
@@ -144,13 +152,7 @@ public RequestManager get(@NonNull Fragment fragment) {
     if (Util.isOnBackgroundThread()) {
       return get(fragment.getContext().getApplicationContext());
     } else {
-      // In some unusual cases, it's possible to have a Fragment not hosted by an activity. There's
-      // not all that much we can do here. Most apps will be started with a standard activity. If
-      // 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());
-      }
+      maybeRegisterFirstFrameWaiter(fragment.getActivity());
       FragmentManager fm = fragment.getChildFragmentManager();
       return supportFragmentGet(fragment.getContext(), fm, fragment, fragment.isVisible());
     }
@@ -165,7 +167,7 @@ public RequestManager get(@NonNull Activity activity) {
       return get((FragmentActivity) activity);
     } else {
       assertNotDestroyed(activity);
-      firstFrameWaiter.registerSelf(activity);
+      maybeRegisterFirstFrameWaiter(activity);
       android.app.FragmentManager fm = activity.getFragmentManager();
       return fragmentGet(activity, fm, /*parentHint=*/ null, isActivityVisible(activity));
     }
@@ -345,13 +347,7 @@ public RequestManager get(@NonNull android.app.Fragment fragment) {
     if (Util.isOnBackgroundThread() || Build.VERSION.SDK_INT < Build.VERSION_CODES.JELLY_BEAN_MR1) {
       return get(fragment.getActivity().getApplicationContext());
     } else {
-      // In some unusual cases, it's possible to have a Fragment not hosted by an activity. There's
-      // not all that much we can do here. Most apps will be started with a standard activity. If
-      // 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());
-      }
+      maybeRegisterFirstFrameWaiter(fragment.getActivity());
       android.app.FragmentManager fm = fragment.getChildFragmentManager();
       return fragmentGet(fragment.getActivity(), fm, fragment, fragment.isVisible());
     }

diff --git a/...y/test/src/test/java/com/bumptech/glide/load/resource/bitmap/HardwareConfigStateTest.java b/...y/test/src/test/java/com/bumptech/glide/load/resource/bitmap/HardwareConfigStateTest.java
@@ -21,7 +21,6 @@ public class HardwareConfigStateTest {
   public void
       setHardwareConfigIfAllowed_withAllowedState_setsInPreferredConfigAndMutable_returnsFalse() {
     HardwareConfigState state = new HardwareConfigState();
-    state.unblockHardwareBitmaps();
     BitmapFactory.Options options = new BitmapFactory.Options();
     boolean result =
         state.setHardwareConfigIfAllowed(
@@ -40,7 +39,6 @@ public class HardwareConfigStateTest {
   @Test
   public void setHardwareConfigIfAllowed_withSmallerThanMinWidth_returnsFalse_doesNotSetValues() {
     HardwareConfigState state = new HardwareConfigState();
-    state.unblockHardwareBitmaps();
     BitmapFactory.Options options = new BitmapFactory.Options();
     options.inPreferredConfig = null;
     options.inMutable = true;
@@ -62,7 +60,6 @@ public void setHardwareConfigIfAllowed_withSmallerThanMinWidth_returnsFalse_does
   @Test
   public void setHardwareConfigIfAllowed_withSmallerThanMinHeight_returnsFalse_doesNotSetValues() {
     HardwareConfigState state = new HardwareConfigState();
-    state.unblockHardwareBitmaps();
     BitmapFactory.Options options = new BitmapFactory.Options();
     options.inPreferredConfig = null;
     options.inMutable = true;
@@ -85,7 +82,6 @@ public void setHardwareConfigIfAllowed_withSmallerThanMinHeight_returnsFalse_doe
   public void
       setHardwareConfigIfAllowed_withHardwareConfigDisallowed_returnsFalse_doesNotSetValues() {
     HardwareConfigState state = new HardwareConfigState();
-    state.unblockHardwareBitmaps();
     BitmapFactory.Options options = new BitmapFactory.Options();
     options.inPreferredConfig = null;
     options.inMutable = true;
@@ -108,7 +104,6 @@ public void setHardwareConfigIfAllowed_withSmallerThanMinHeight_returnsFalse_doe
   public void
       setHardwareConfigIfAllowed_withExifOrientationRequired_returnsFalse_doesNotSetValues() {
     HardwareConfigState state = new HardwareConfigState();
-    state.unblockHardwareBitmaps();
     BitmapFactory.Options options = new BitmapFactory.Options();
     options.inPreferredConfig = null;
     options.inMutable = true;
@@ -130,29 +125,6 @@ public void setHardwareConfigIfAllowed_withSmallerThanMinHeight_returnsFalse_doe
   @Test
   public void setHardwareConfigIfAllowed_withOsLessThanO_returnsFalse_doesNotSetValues() {
     HardwareConfigState state = new HardwareConfigState();
-    state.unblockHardwareBitmaps();
-    BitmapFactory.Options options = new BitmapFactory.Options();
-    options.inPreferredConfig = null;
-    options.inMutable = true;
-
-    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.inMutable).isTrue();
-    assertThat(options.inPreferredConfig).isNull();
-  }
-
-  @Config(sdk = Build.VERSION_CODES.P)
-  @Test
-  public void
-      setHardwareConfigIfAllowed_withOsLessThanQ_beforeUnblockingHardwareBitmaps_returnsFalseAndDoesNotSetValues() {
-    HardwareConfigState state = new HardwareConfigState();
     BitmapFactory.Options options = new BitmapFactory.Options();
     options.inPreferredConfig = null;
     options.inMutable = true;
@@ -180,7 +152,6 @@ public void setHardwareConfigIfAllowed_withOsLessThanO_returnsFalse_doesNotSetVa
         }) {
       ShadowBuild.setModel(model);
       HardwareConfigState state = new HardwareConfigState();
-      state.unblockHardwareBitmaps();
       BitmapFactory.Options options = new BitmapFactory.Options();
       options.inPreferredConfig = null;
       options.inMutable = true;
@@ -208,7 +179,6 @@ public void setHardwareConfigIfAllowed_withDisallowedSamsungDevices_OMR1_returns
         }) {
       ShadowBuild.setModel(model);
       HardwareConfigState state = new HardwareConfigState();
-      state.unblockHardwareBitmaps();
       BitmapFactory.Options options = new BitmapFactory.Options();
       options.inPreferredConfig = null;
       options.inMutable = true;
@@ -236,7 +206,6 @@ public void setHardwareConfigIfAllowed_withShortEmptyOrNullModelNames_returnsTru
         new String[] {null, ".", "-", "", "S", "SM", "SM-", "SM-G", "SM-G9.", "SM-G93"}) {
       ShadowBuild.setModel(model);
       HardwareConfigState state = new HardwareConfigState();
-      state.unblockHardwareBitmaps();
       BitmapFactory.Options options = new BitmapFactory.Options();
       options.inPreferredConfig = null;
       options.inMutable = true;

diff --git a/library/test/src/test/java/com/bumptech/glide/manager/RequestManagerRetrieverTest.java b/library/test/src/test/java/com/bumptech/glide/manager/RequestManagerRetrieverTest.java
@@ -58,7 +58,7 @@ public class RequestManagerRetrieverTest {
   public void setUp() {
     appContext = ApplicationProvider.getApplicationContext();
 
-    retriever = new RequestManagerRetriever(/*factory=*/ null);
+    retriever = new RequestManagerRetriever(/*factory=*/ null, /* addFirstFrameWaiter= */ false);
 
     harnesses =
         new RetrieverHarness[] {new DefaultRetrieverHarness(), new SupportRetrieverHarness()};