Skip to content

Commit edd96d8

Browse files
sjuddglide-copybara-robot
authored andcommittedSep 10, 2020
Wait for the app to render at least one frame before enabling Hardware 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: 330840252
1 parent e7d16d7 commit edd96d8

File tree

8 files changed

+93
-26
lines changed

8 files changed

+93
-26
lines changed
 

‎annotation/compiler/test/src/test/resources/EmptyAppGlideModuleTest/GlideApp.java

+9
Original file line numberDiff line numberDiff line change
@@ -69,6 +69,15 @@ public static void init(@NonNull Context context, @NonNull GlideBuilder builder)
6969
Glide.init(context, builder);
7070
}
7171

72+
/**
73+
* @see Glide#enableHardwareBitmaps()
74+
*/
75+
@VisibleForTesting
76+
@SuppressLint("VisibleForTests")
77+
public static void enableHardwareBitmaps() {
78+
Glide.enableHardwareBitmaps();
79+
}
80+
7281
/**
7382
* @see Glide#tearDown()
7483
*/

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

+1
Original file line numberDiff line numberDiff line change
@@ -121,6 +121,7 @@ public void loadTransparentGifResource_asHardware_withNoOtherLoaders_decodesReso
121121
assumeTrue(
122122
"Hardware Bitmaps are only supported on O+",
123123
Build.VERSION.SDK_INT >= Build.VERSION_CODES.O);
124+
Glide.enableHardwareBitmaps();
124125

125126
Glide.get(context)
126127
.getRegistry()

‎library/src/main/java/com/bumptech/glide/Glide.java

+14-1
Original file line numberDiff line numberDiff line change
@@ -63,6 +63,7 @@
6363
import com.bumptech.glide.load.resource.bitmap.DefaultImageHeaderParser;
6464
import com.bumptech.glide.load.resource.bitmap.Downsampler;
6565
import com.bumptech.glide.load.resource.bitmap.ExifInterfaceImageHeaderParser;
66+
import com.bumptech.glide.load.resource.bitmap.HardwareConfigState;
6667
import com.bumptech.glide.load.resource.bitmap.InputStreamBitmapImageDecoderResourceDecoder;
6768
import com.bumptech.glide.load.resource.bitmap.ParcelFileDescriptorBitmapDecoder;
6869
import com.bumptech.glide.load.resource.bitmap.ResourceBitmapDecoder;
@@ -235,6 +236,19 @@ public static void init(@NonNull Context context, @NonNull GlideBuilder builder)
235236
}
236237
}
237238

239+
/**
240+
* Allows hardware Bitmaps to be used prior to the first frame in the app being drawn as soon as
241+
* this method is called.
242+
*
243+
* <p>If you use this method in non-test code, your app will experience native crashes on some
244+
* versions of Android if you try to decode a hardware Bitmap. This method is only useful for
245+
* testing.
246+
*/
247+
@VisibleForTesting
248+
public static void enableHardwareBitmaps() {
249+
HardwareConfigState.getInstance().unblockHardwareBitmaps();
250+
}
251+
238252
@VisibleForTesting
239253
public static void tearDown() {
240254
synchronized (Glide.class) {
@@ -373,7 +387,6 @@ private static void throwIncorrectGlideModule(Exception e) {
373387
@NonNull List<RequestListener<Object>> defaultRequestListeners,
374388
boolean isLoggingRequestOriginsEnabled,
375389
boolean isImageDecoderEnabledForBitmaps,
376-
boolean blockHardwareBitmaps,
377390
int manualOverrideHardwareBitmapMaxFdCount) {
378391
this.engine = engine;
379392
this.bitmapPool = bitmapPool;

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

+1-8
Original file line numberDiff line numberDiff line change
@@ -66,9 +66,7 @@ public RequestOptions build() {
6666
private boolean isLoggingRequestOriginsEnabled;
6767

6868
private boolean isImageDecoderEnabledForBitmaps;
69-
private boolean waitForFirstFrameBeforeEnablingHardwareBitmaps;
7069
private int manualOverrideHardwareBitmapMaxFdCount = HardwareConfigState.NO_MAX_FD_COUNT;
71-
private boolean waitForCallBeforeEnablingHardwareBitmaps;
7270

7371
/**
7472
* Sets the {@link com.bumptech.glide.load.engine.bitmap_recycle.BitmapPool} implementation to use
@@ -563,11 +561,7 @@ Glide build(@NonNull Context context) {
563561
}
564562

565563
RequestManagerRetriever requestManagerRetriever =
566-
new RequestManagerRetriever(
567-
requestManagerFactory, waitForFirstFrameBeforeEnablingHardwareBitmaps);
568-
569-
boolean blockHardwareBitmaps =
570-
waitForFirstFrameBeforeEnablingHardwareBitmaps || waitForCallBeforeEnablingHardwareBitmaps;
564+
new RequestManagerRetriever(requestManagerFactory);
571565

572566
return new Glide(
573567
context,
@@ -583,7 +577,6 @@ Glide build(@NonNull Context context) {
583577
defaultRequestListeners,
584578
isLoggingRequestOriginsEnabled,
585579
isImageDecoderEnabledForBitmaps,
586-
blockHardwareBitmaps,
587580
manualOverrideHardwareBitmapMaxFdCount);
588581
}
589582
}

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

+18-2
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,15 @@
1515
* Android O+.
1616
*/
1717
public final class HardwareConfigState {
18+
/**
19+
* Force the state to wait until a call to allow hardware Bitmaps to be used when they'd otherwise
20+
* be eligible to work around a framework issue pre Q that can cause a native crash when
21+
* allocating a hardware Bitmap in this specific circumstance. See b/126573603#comment12 for
22+
* details.
23+
*/
24+
private static final boolean BLOCK_HARDWARE_BITMAPS_BY_DEFAULT =
25+
Build.VERSION.SDK_INT < Build.VERSION_CODES.Q;
26+
1827
/**
1928
* The minimum size in pixels a {@link Bitmap} must be in both dimensions to be created with the
2029
* {@link Bitmap.Config#HARDWARE} configuration.
@@ -66,7 +75,6 @@ public final class HardwareConfigState {
6675
public static final int NO_MAX_FD_COUNT = -1;
6776

6877
private static volatile HardwareConfigState instance;
69-
private static volatile boolean blockHardwareBitmapsByDefault;
7078
private static volatile int manualOverrideMaxFdCount = NO_MAX_FD_COUNT;
7179

7280
private final boolean isHardwareConfigAllowedByDeviceModel;
@@ -104,6 +112,10 @@ public static HardwareConfigState getInstance() {
104112
}
105113
}
106114

115+
public void unblockHardwareBitmaps() {
116+
areHardwareBitmapsUnblocked = true;
117+
}
118+
107119
public boolean isHardwareConfigAllowed(
108120
int targetWidth,
109121
int targetHeight,
@@ -112,7 +124,7 @@ public boolean isHardwareConfigAllowed(
112124
if (!isHardwareConfigAllowed
113125
|| !isHardwareConfigAllowedByDeviceModel
114126
|| Build.VERSION.SDK_INT < Build.VERSION_CODES.O
115-
|| (blockHardwareBitmapsByDefault && !areHardwareBitmapsUnblocked)
127+
|| areHardwareBitmapsBlockedByAppState()
116128
|| isExifOrientationRequired) {
117129
return false;
118130
}
@@ -123,6 +135,10 @@ public boolean isHardwareConfigAllowed(
123135
&& isFdSizeBelowHardwareLimit();
124136
}
125137

138+
private boolean areHardwareBitmapsBlockedByAppState() {
139+
return BLOCK_HARDWARE_BITMAPS_BY_DEFAULT && !areHardwareBitmapsUnblocked;
140+
}
141+
126142
@TargetApi(Build.VERSION_CODES.O)
127143
boolean setHardwareConfigIfAllowed(
128144
int targetWidth,

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

+18-14
Original file line numberDiff line numberDiff line change
@@ -70,12 +70,10 @@ public class RequestManagerRetriever implements Handler.Callback {
7070
// This is really misplaced here, but to put it anywhere else means duplicating all of the
7171
// Fragment/Activity extraction logic that already exists here. It's gross, but less likely to
7272
// break.
73-
@Nullable private final FirstFrameWaiter firstFrameWaiter;
73+
private final FirstFrameWaiter firstFrameWaiter = new FirstFrameWaiter();
7474

75-
public RequestManagerRetriever(
76-
@Nullable RequestManagerFactory factory, boolean addFirstFrameWaiter) {
75+
public RequestManagerRetriever(@Nullable RequestManagerFactory factory) {
7776
this.factory = factory != null ? factory : DEFAULT_FACTORY;
78-
firstFrameWaiter = addFirstFrameWaiter ? new FirstFrameWaiter() : null;
7977
handler = new Handler(Looper.getMainLooper(), this /* Callback */);
8078
}
8179

@@ -126,19 +124,13 @@ public RequestManager get(@NonNull Context context) {
126124
return getApplicationManager(context);
127125
}
128126

129-
private void maybeRegisterFirstFrameWaiter(@NonNull Activity activity) {
130-
if (firstFrameWaiter != null) {
131-
firstFrameWaiter.registerSelf(activity);
132-
}
133-
}
134-
135127
@NonNull
136128
public RequestManager get(@NonNull FragmentActivity activity) {
137129
if (Util.isOnBackgroundThread()) {
138130
return get(activity.getApplicationContext());
139131
} else {
140132
assertNotDestroyed(activity);
141-
maybeRegisterFirstFrameWaiter(activity);
133+
firstFrameWaiter.registerSelf(activity);
142134
FragmentManager fm = activity.getSupportFragmentManager();
143135
return supportFragmentGet(activity, fm, /*parentHint=*/ null, isActivityVisible(activity));
144136
}
@@ -152,7 +144,13 @@ public RequestManager get(@NonNull Fragment fragment) {
152144
if (Util.isOnBackgroundThread()) {
153145
return get(fragment.getContext().getApplicationContext());
154146
} else {
155-
maybeRegisterFirstFrameWaiter(fragment.getActivity());
147+
// In some unusual cases, it's possible to have a Fragment not hosted by an activity. There's
148+
// not all that much we can do here. Most apps will be started with a standard activity. If
149+
// we manage not to register the first frame waiter for a while, the consequences are not
150+
// catastrophic, we'll just use some extra memory.
151+
if (fragment.getActivity() != null) {
152+
firstFrameWaiter.registerSelf(fragment.getActivity());
153+
}
156154
FragmentManager fm = fragment.getChildFragmentManager();
157155
return supportFragmentGet(fragment.getContext(), fm, fragment, fragment.isVisible());
158156
}
@@ -167,7 +165,7 @@ public RequestManager get(@NonNull Activity activity) {
167165
return get((FragmentActivity) activity);
168166
} else {
169167
assertNotDestroyed(activity);
170-
maybeRegisterFirstFrameWaiter(activity);
168+
firstFrameWaiter.registerSelf(activity);
171169
android.app.FragmentManager fm = activity.getFragmentManager();
172170
return fragmentGet(activity, fm, /*parentHint=*/ null, isActivityVisible(activity));
173171
}
@@ -347,7 +345,13 @@ public RequestManager get(@NonNull android.app.Fragment fragment) {
347345
if (Util.isOnBackgroundThread() || Build.VERSION.SDK_INT < Build.VERSION_CODES.JELLY_BEAN_MR1) {
348346
return get(fragment.getActivity().getApplicationContext());
349347
} else {
350-
maybeRegisterFirstFrameWaiter(fragment.getActivity());
348+
// In some unusual cases, it's possible to have a Fragment not hosted by an activity. There's
349+
// not all that much we can do here. Most apps will be started with a standard activity. If
350+
// we manage not to register the first frame waiter for a while, the consequences are not
351+
// catastrophic, we'll just use some extra memory.
352+
if (fragment.getActivity() != null) {
353+
firstFrameWaiter.registerSelf(fragment.getActivity());
354+
}
351355
android.app.FragmentManager fm = fragment.getChildFragmentManager();
352356
return fragmentGet(fragment.getActivity(), fm, fragment, fragment.isVisible());
353357
}

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

+31
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@ public class HardwareConfigStateTest {
2121
public void
2222
setHardwareConfigIfAllowed_withAllowedState_setsInPreferredConfigAndMutable_returnsFalse() {
2323
HardwareConfigState state = new HardwareConfigState();
24+
state.unblockHardwareBitmaps();
2425
BitmapFactory.Options options = new BitmapFactory.Options();
2526
boolean result =
2627
state.setHardwareConfigIfAllowed(
@@ -39,6 +40,7 @@ public class HardwareConfigStateTest {
3940
@Test
4041
public void setHardwareConfigIfAllowed_withSmallerThanMinWidth_returnsFalse_doesNotSetValues() {
4142
HardwareConfigState state = new HardwareConfigState();
43+
state.unblockHardwareBitmaps();
4244
BitmapFactory.Options options = new BitmapFactory.Options();
4345
options.inPreferredConfig = null;
4446
options.inMutable = true;
@@ -60,6 +62,7 @@ public void setHardwareConfigIfAllowed_withSmallerThanMinWidth_returnsFalse_does
6062
@Test
6163
public void setHardwareConfigIfAllowed_withSmallerThanMinHeight_returnsFalse_doesNotSetValues() {
6264
HardwareConfigState state = new HardwareConfigState();
65+
state.unblockHardwareBitmaps();
6366
BitmapFactory.Options options = new BitmapFactory.Options();
6467
options.inPreferredConfig = null;
6568
options.inMutable = true;
@@ -82,6 +85,7 @@ public void setHardwareConfigIfAllowed_withSmallerThanMinHeight_returnsFalse_doe
8285
public void
8386
setHardwareConfigIfAllowed_withHardwareConfigDisallowed_returnsFalse_doesNotSetValues() {
8487
HardwareConfigState state = new HardwareConfigState();
88+
state.unblockHardwareBitmaps();
8589
BitmapFactory.Options options = new BitmapFactory.Options();
8690
options.inPreferredConfig = null;
8791
options.inMutable = true;
@@ -104,6 +108,7 @@ public void setHardwareConfigIfAllowed_withSmallerThanMinHeight_returnsFalse_doe
104108
public void
105109
setHardwareConfigIfAllowed_withExifOrientationRequired_returnsFalse_doesNotSetValues() {
106110
HardwareConfigState state = new HardwareConfigState();
111+
state.unblockHardwareBitmaps();
107112
BitmapFactory.Options options = new BitmapFactory.Options();
108113
options.inPreferredConfig = null;
109114
options.inMutable = true;
@@ -125,6 +130,29 @@ public void setHardwareConfigIfAllowed_withSmallerThanMinHeight_returnsFalse_doe
125130
@Test
126131
public void setHardwareConfigIfAllowed_withOsLessThanO_returnsFalse_doesNotSetValues() {
127132
HardwareConfigState state = new HardwareConfigState();
133+
state.unblockHardwareBitmaps();
134+
BitmapFactory.Options options = new BitmapFactory.Options();
135+
options.inPreferredConfig = null;
136+
options.inMutable = true;
137+
138+
boolean result =
139+
state.setHardwareConfigIfAllowed(
140+
/*targetWidth=*/ HardwareConfigState.MIN_HARDWARE_DIMENSION_O,
141+
/*targetHeight=*/ HardwareConfigState.MIN_HARDWARE_DIMENSION_O,
142+
options,
143+
/*isHardwareConfigAllowed=*/ true,
144+
/*isExifOrientationRequired=*/ false);
145+
146+
assertThat(result).isFalse();
147+
assertThat(options.inMutable).isTrue();
148+
assertThat(options.inPreferredConfig).isNull();
149+
}
150+
151+
@Config(sdk = Build.VERSION_CODES.P)
152+
@Test
153+
public void
154+
setHardwareConfigIfAllowed_withOsLessThanQ_beforeUnblockingHardwareBitmaps_returnsFalseAndDoesNotSetValues() {
155+
HardwareConfigState state = new HardwareConfigState();
128156
BitmapFactory.Options options = new BitmapFactory.Options();
129157
options.inPreferredConfig = null;
130158
options.inMutable = true;
@@ -152,6 +180,7 @@ public void setHardwareConfigIfAllowed_withOsLessThanO_returnsFalse_doesNotSetVa
152180
}) {
153181
ShadowBuild.setModel(model);
154182
HardwareConfigState state = new HardwareConfigState();
183+
state.unblockHardwareBitmaps();
155184
BitmapFactory.Options options = new BitmapFactory.Options();
156185
options.inPreferredConfig = null;
157186
options.inMutable = true;
@@ -179,6 +208,7 @@ public void setHardwareConfigIfAllowed_withDisallowedSamsungDevices_OMR1_returns
179208
}) {
180209
ShadowBuild.setModel(model);
181210
HardwareConfigState state = new HardwareConfigState();
211+
state.unblockHardwareBitmaps();
182212
BitmapFactory.Options options = new BitmapFactory.Options();
183213
options.inPreferredConfig = null;
184214
options.inMutable = true;
@@ -206,6 +236,7 @@ public void setHardwareConfigIfAllowed_withShortEmptyOrNullModelNames_returnsTru
206236
new String[] {null, ".", "-", "", "S", "SM", "SM-", "SM-G", "SM-G9.", "SM-G93"}) {
207237
ShadowBuild.setModel(model);
208238
HardwareConfigState state = new HardwareConfigState();
239+
state.unblockHardwareBitmaps();
209240
BitmapFactory.Options options = new BitmapFactory.Options();
210241
options.inPreferredConfig = null;
211242
options.inMutable = true;

‎library/test/src/test/java/com/bumptech/glide/manager/RequestManagerRetrieverTest.java

+1-1
Original file line numberDiff line numberDiff line change
@@ -58,7 +58,7 @@ public class RequestManagerRetrieverTest {
5858
public void setUp() {
5959
appContext = ApplicationProvider.getApplicationContext();
6060

61-
retriever = new RequestManagerRetriever(/*factory=*/ null, /* addFirstFrameWaiter= */ false);
61+
retriever = new RequestManagerRetriever(/*factory=*/ null);
6262

6363
harnesses =
6464
new RetrieverHarness[] {new DefaultRetrieverHarness(), new SupportRetrieverHarness()};

0 commit comments

Comments
 (0)
Please sign in to comment.