Skip to content

Commit 8f1ea5c

Browse files
sjuddsjudd
committed
Allow starting requests on background threads without posting to the main thread.
------------- Avoid posting to the main thread when clearing in RequestFutureTarget. ------------- Created by MOE: https://github.com/google/moe MOE_MIGRATED_REVID=221803963
1 parent 617edc3 commit 8f1ea5c

File tree

29 files changed

+1208
-844
lines changed

29 files changed

+1208
-844
lines changed

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

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -48,19 +48,19 @@ public <ResourceType> GlideRequest<ResourceType> as(@NonNull Class<ResourceType>
4848

4949
@Override
5050
@NonNull
51-
public GlideRequests applyDefaultRequestOptions(@NonNull RequestOptions options) {
51+
public synchronized GlideRequests applyDefaultRequestOptions(@NonNull RequestOptions options) {
5252
return (GlideRequests) super.applyDefaultRequestOptions(options);
5353
}
5454

5555
@Override
5656
@NonNull
57-
public GlideRequests setDefaultRequestOptions(@NonNull RequestOptions options) {
57+
public synchronized GlideRequests setDefaultRequestOptions(@NonNull RequestOptions options) {
5858
return (GlideRequests) super.setDefaultRequestOptions(options);
5959
}
6060

6161
@Override
6262
@NonNull
63-
public GlideRequests addDefaultRequestListener(RequestListener<Object> listener) {
63+
public synchronized GlideRequests addDefaultRequestListener(RequestListener<Object> listener) {
6464
return (GlideRequests) super.addDefaultRequestListener(listener);
6565
}
6666

‎annotation/compiler/test/src/test/resources/GlideExtensionWithTypeTest/GlideRequests.java‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -58,19 +58,19 @@ public GlideRequest<Number> asNumber() {
5858

5959
@Override
6060
@NonNull
61-
public GlideRequests applyDefaultRequestOptions(@NonNull RequestOptions options) {
61+
public synchronized GlideRequests applyDefaultRequestOptions(@NonNull RequestOptions options) {
6262
return (GlideRequests) super.applyDefaultRequestOptions(options);
6363
}
6464

6565
@Override
6666
@NonNull
67-
public GlideRequests setDefaultRequestOptions(@NonNull RequestOptions options) {
67+
public synchronized GlideRequests setDefaultRequestOptions(@NonNull RequestOptions options) {
6868
return (GlideRequests) super.setDefaultRequestOptions(options);
6969
}
7070

7171
@Override
7272
@NonNull
73-
public GlideRequests addDefaultRequestListener(RequestListener<Object> listener) {
73+
public synchronized GlideRequests addDefaultRequestListener(RequestListener<Object> listener) {
7474
return (GlideRequests) super.addDefaultRequestListener(listener);
7575
}
7676

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

Lines changed: 56 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,8 @@
2525
import com.bumptech.glide.load.engine.DiskCacheStrategy;
2626
import com.bumptech.glide.load.engine.cache.LruResourceCache;
2727
import com.bumptech.glide.load.engine.cache.MemoryCacheAdapter;
28+
import com.bumptech.glide.load.engine.executor.GlideExecutor;
29+
import com.bumptech.glide.load.engine.executor.MockGlideExecutor;
2830
import com.bumptech.glide.request.FutureTarget;
2931
import com.bumptech.glide.request.RequestListener;
3032
import com.bumptech.glide.request.target.Target;
@@ -67,12 +69,11 @@ public class CachingTest {
6769
private Context context;
6870

6971
@Before
70-
public void setUp() throws InterruptedException {
72+
public void setUp() {
7173
MockitoAnnotations.initMocks(this);
7274
context = InstrumentationRegistry.getTargetContext();
7375

74-
Glide.init(
75-
context, new GlideBuilder().setMemoryCache(new LruResourceCache(CACHE_SIZE_BYTES)));
76+
Glide.init(context, new GlideBuilder().setMemoryCache(new LruResourceCache(CACHE_SIZE_BYTES)));
7677
}
7778

7879
@Test
@@ -164,8 +165,18 @@ public void run() {
164165
}
165166

166167
@Test
167-
public void submit_withPreviousRequestClearedFromMemory_completesFromDataDiskCache()
168-
throws InterruptedException, ExecutionException, TimeoutException {
168+
public void submit_withPreviousRequestClearedFromMemory_completesFromDataDiskCache() {
169+
// Clearing the future here can race with clearing the EngineResource held on to by EngineJob
170+
// while it's notifying callbacks. Forcing all executors to use the same thread avoids the race
171+
// by making our clear and EngineJob's clear run on the same thread.
172+
GlideExecutor mainThreadExecutor = MockGlideExecutor.newMainThreadExecutor();
173+
Glide.init(
174+
context,
175+
new GlideBuilder()
176+
.setSourceExecutor(mainThreadExecutor)
177+
.setDiskCacheExecutor(mainThreadExecutor)
178+
.setAnimationExecutor(mainThreadExecutor));
179+
169180
FutureTarget<Drawable> future = GlideApp.with(context)
170181
.load(ResourceIds.raw.canonical)
171182
.diskCacheStrategy(DiskCacheStrategy.DATA)
@@ -192,8 +203,7 @@ public void submit_withPreviousRequestClearedFromMemory_completesFromDataDiskCac
192203
}
193204

194205
@Test
195-
public void submit_withPreviousButNoLongerReferencedIdenticalRequest_completesFromMemoryCache()
196-
throws InterruptedException, TimeoutException, ExecutionException {
206+
public void submit_withPreviousButNoLongerReferencedIdenticalRequest_completesFromMemoryCache() {
197207
// We can't allow any mocks (RequestListner, Target etc) to reference this request or the test
198208
// will fail due to the transient strong reference to the request.
199209
concurrency.get(
@@ -224,8 +234,7 @@ public void submit_withPreviousButNoLongerReferencedIdenticalRequest_completesFr
224234
}
225235

226236
@Test
227-
public void submit_withPreviousButNoLongerReferencedIdenticalRequest_doesNotRecycleBitmap()
228-
throws InterruptedException, TimeoutException, ExecutionException {
237+
public void submit_withPreviousButNoLongerReferencedIdenticalRequest_doesNotRecycleBitmap() {
229238
// We can't allow any mocks (RequestListener, Target etc) to reference this request or the test
230239
// will fail due to the transient strong reference to the request.
231240
Bitmap bitmap =
@@ -255,34 +264,51 @@ public void submit_withPreviousButNoLongerReferencedIdenticalRequest_doesNotRecy
255264
}
256265

257266
@Test
258-
public void clearDiskCache_doesNotPreventFutureLoads()
259-
throws ExecutionException, InterruptedException, TimeoutException {
260-
FutureTarget<Drawable> future = GlideApp.with(context)
261-
.load(ResourceIds.raw.canonical)
262-
.diskCacheStrategy(DiskCacheStrategy.DATA)
263-
.submit(IMAGE_SIZE_PIXELS, IMAGE_SIZE_PIXELS);
267+
public void clearDiskCache_doesNotPreventFutureLoads() {
268+
// Clearing the future here can race with clearing the EngineResource held on to by EngineJob
269+
// while it's notifying callbacks. Forcing all executors to use the same thread avoids the race
270+
// by making our clear and EngineJob's clear run on the same thread.
271+
GlideExecutor mainThreadExecutor = MockGlideExecutor.newMainThreadExecutor();
272+
Glide.init(
273+
context,
274+
new GlideBuilder()
275+
.setSourceExecutor(mainThreadExecutor)
276+
.setDiskCacheExecutor(mainThreadExecutor)
277+
.setAnimationExecutor(mainThreadExecutor));
278+
279+
// Load the request once.
280+
FutureTarget<Drawable> future =
281+
GlideApp.with(context)
282+
.load(ResourceIds.raw.canonical)
283+
.diskCacheStrategy(DiskCacheStrategy.DATA)
284+
.submit(IMAGE_SIZE_PIXELS, IMAGE_SIZE_PIXELS);
264285
concurrency.get(future);
286+
// Clear the result from all of our caches.
265287
GlideApp.with(context).clear(future);
266-
267288
clearMemoryCacheOnMainThread();
268289
GlideApp.get(context).clearDiskCache();
269290

270-
future = GlideApp.with(context)
271-
.load(ResourceIds.raw.canonical)
272-
.diskCacheStrategy(DiskCacheStrategy.DATA)
273-
.submit(IMAGE_SIZE_PIXELS, IMAGE_SIZE_PIXELS);
291+
// Load the request a second time into the disk cache.
292+
future =
293+
GlideApp.with(context)
294+
.load(ResourceIds.raw.canonical)
295+
.diskCacheStrategy(DiskCacheStrategy.DATA)
296+
.submit(IMAGE_SIZE_PIXELS, IMAGE_SIZE_PIXELS);
274297
concurrency.get(future);
275-
298+
// Clear the second request from everywhere but the disk cache.
276299
GlideApp.with(context).clear(future);
277300
clearMemoryCacheOnMainThread();
278301

302+
// Load the request a third time.
279303
concurrency.get(
280304
GlideApp.with(context)
281305
.load(ResourceIds.raw.canonical)
282306
.listener(requestListener)
283307
.diskCacheStrategy(DiskCacheStrategy.DATA)
284308
.submit(IMAGE_SIZE_PIXELS, IMAGE_SIZE_PIXELS));
285309

310+
// Assert that the third request comes from the disk cache (which was populated by the second
311+
// request).
286312
verify(requestListener)
287313
.onResourceReady(
288314
anyDrawable(),
@@ -351,12 +377,14 @@ public void run() {
351377
// Verify that the request that didn't have retrieve from cache succeeds
352378
assertThat(concurrency.get(expectedFuture)).isNotNull();
353379
// The first request only from cache should fail because the item is not in cache.
354-
assertThrows(RuntimeException.class, new ThrowingRunnable() {
355-
@Override
356-
public void run() throws Throwable {
357-
concurrency.get(firstQueuedFuture);
358-
}
359-
});
380+
assertThrows(
381+
RuntimeException.class,
382+
new ThrowingRunnable() {
383+
@Override
384+
public void run() {
385+
concurrency.get(firstQueuedFuture);
386+
}
387+
});
360388
}
361389

362390
@Test
@@ -482,7 +510,7 @@ public void loadIntoView_withSkipMemoryCache_doesNotLoadFromMemoryCacheIfPresent
482510
anyBoolean());
483511
}
484512

485-
private void clearMemoryCacheOnMainThread() throws InterruptedException {
513+
private void clearMemoryCacheOnMainThread() {
486514
concurrency.runOnMainThread(new Runnable() {
487515
@Override
488516
public void run() {

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

Lines changed: 69 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,8 @@
2323
import com.bumptech.glide.load.engine.bitmap_recycle.LruBitmapPool;
2424
import com.bumptech.glide.load.engine.cache.LruResourceCache;
2525
import com.bumptech.glide.load.engine.cache.MemoryCacheAdapter;
26+
import com.bumptech.glide.load.engine.executor.GlideExecutor;
27+
import com.bumptech.glide.load.engine.executor.MockGlideExecutor;
2628
import com.bumptech.glide.request.RequestListener;
2729
import com.bumptech.glide.request.target.Target;
2830
import com.bumptech.glide.test.ConcurrencyHelper;
@@ -45,18 +47,31 @@ public class LoadBitmapTest {
4547

4648
private final ConcurrencyHelper concurrency = new ConcurrencyHelper();
4749
private Context context;
50+
private GlideBuilder glideBuilder;
4851

4952
@Before
5053
public void setUp() {
5154
MockitoAnnotations.initMocks(this);
5255
context = InstrumentationRegistry.getTargetContext();
56+
57+
// Clearing the future here can race with clearing the EngineResource held on to by EngineJob
58+
// while it's notifying callbacks. Forcing all executors to use the same thread avoids the race
59+
// by making our clear and EngineJob's clear run on the same thread.
60+
GlideExecutor mainThreadExecutor = MockGlideExecutor.newMainThreadExecutor();
61+
glideBuilder =
62+
new GlideBuilder()
63+
.setSourceExecutor(mainThreadExecutor)
64+
.setDiskCacheExecutor(mainThreadExecutor)
65+
.setAnimationExecutor(mainThreadExecutor);
5366
}
5467

5568
@Test
5669
public void clearFromRequestBuilder_asDrawable_withLoadedBitmap_doesNotRecycleBitmap() {
57-
Glide.init(context, new GlideBuilder()
58-
.setMemoryCache(new MemoryCacheAdapter())
59-
.setBitmapPool(new BitmapPoolAdapter()));
70+
Glide.init(
71+
context,
72+
new GlideBuilder()
73+
.setMemoryCache(new MemoryCacheAdapter())
74+
.setBitmapPool(new BitmapPoolAdapter()));
6075
Bitmap bitmap = BitmapFactory.decodeResource(context.getResources(), ResourceIds.raw.canonical);
6176
Target<Drawable> target =
6277
concurrency.wait(
@@ -74,9 +89,11 @@ public void clearFromRequestBuilder_asDrawable_withLoadedBitmap_doesNotRecycleBi
7489

7590
@Test
7691
public void transformFromRequestBuilder_asDrawable_withLoadedBitmap_doesNotRecycleBitmap() {
77-
Glide.init(context, new GlideBuilder()
78-
.setMemoryCache(new MemoryCacheAdapter())
79-
.setBitmapPool(new BitmapPoolAdapter()));
92+
Glide.init(
93+
context,
94+
new GlideBuilder()
95+
.setMemoryCache(new MemoryCacheAdapter())
96+
.setBitmapPool(new BitmapPoolAdapter()));
8097
Bitmap bitmap = BitmapFactory.decodeResource(context.getResources(), ResourceIds.raw.canonical);
8198
concurrency.wait(
8299
GlideApp.with(context)
@@ -90,9 +107,11 @@ public void transformFromRequestBuilder_asDrawable_withLoadedBitmap_doesNotRecyc
90107

91108
@Test
92109
public void clearFromRequestManager_withLoadedBitmap_doesNotRecycleBitmap() {
93-
Glide.init(context, new GlideBuilder()
94-
.setMemoryCache(new MemoryCacheAdapter())
95-
.setBitmapPool(new BitmapPoolAdapter()));
110+
Glide.init(
111+
context,
112+
new GlideBuilder()
113+
.setMemoryCache(new MemoryCacheAdapter())
114+
.setBitmapPool(new BitmapPoolAdapter()));
96115
Bitmap bitmap = BitmapFactory.decodeResource(context.getResources(), ResourceIds.raw.canonical);
97116
Target<Drawable> target =
98117
concurrency.wait(
@@ -109,9 +128,11 @@ public void clearFromRequestManager_withLoadedBitmap_doesNotRecycleBitmap() {
109128

110129
@Test
111130
public void transformFromRequestManager_withLoadedBitmap_doesNotRecycleBitmap() {
112-
Glide.init(context, new GlideBuilder()
113-
.setMemoryCache(new MemoryCacheAdapter())
114-
.setBitmapPool(new BitmapPoolAdapter()));
131+
Glide.init(
132+
context,
133+
new GlideBuilder()
134+
.setMemoryCache(new MemoryCacheAdapter())
135+
.setBitmapPool(new BitmapPoolAdapter()));
115136
Bitmap bitmap = BitmapFactory.decodeResource(context.getResources(), ResourceIds.raw.canonical);
116137
concurrency.wait(
117138
GlideApp.with(context)
@@ -124,9 +145,11 @@ public void transformFromRequestManager_withLoadedBitmap_doesNotRecycleBitmap()
124145

125146
@Test
126147
public void clearFromRequestBuilder_withLoadedBitmap_asBitmap_doesNotRecycleBitmap() {
127-
Glide.init(context, new GlideBuilder()
128-
.setMemoryCache(new MemoryCacheAdapter())
129-
.setBitmapPool(new BitmapPoolAdapter()));
148+
Glide.init(
149+
context,
150+
new GlideBuilder()
151+
.setMemoryCache(new MemoryCacheAdapter())
152+
.setBitmapPool(new BitmapPoolAdapter()));
130153
Bitmap bitmap = BitmapFactory.decodeResource(context.getResources(), ResourceIds.raw.canonical);
131154
Target<Bitmap> target =
132155
concurrency.wait(
@@ -144,9 +167,11 @@ public void clearFromRequestBuilder_withLoadedBitmap_asBitmap_doesNotRecycleBitm
144167

145168
@Test
146169
public void transformFromRequestBuilder_withLoadedBitmap_asBitmap_doesNotRecycleBitmap() {
147-
Glide.init(context, new GlideBuilder()
148-
.setMemoryCache(new MemoryCacheAdapter())
149-
.setBitmapPool(new BitmapPoolAdapter()));
170+
Glide.init(
171+
context,
172+
new GlideBuilder()
173+
.setMemoryCache(new MemoryCacheAdapter())
174+
.setBitmapPool(new BitmapPoolAdapter()));
150175
Bitmap bitmap = BitmapFactory.decodeResource(context.getResources(), ResourceIds.raw.canonical);
151176
concurrency.wait(
152177
GlideApp.with(context)
@@ -161,9 +186,11 @@ public void transformFromRequestBuilder_withLoadedBitmap_asBitmap_doesNotRecycle
161186
@Test
162187
public void loadFromRequestManager_withBitmap_doesNotLoadFromDiskCache() {
163188
Bitmap bitmap = BitmapFactory.decodeResource(context.getResources(), ResourceIds.raw.canonical);
164-
Glide.init(context, new GlideBuilder()
165-
.setMemoryCache(new LruResourceCache(Util.getBitmapByteSize(bitmap) * 10))
166-
.setBitmapPool(new LruBitmapPool(Util.getBitmapByteSize(bitmap) * 10)));
189+
Glide.init(
190+
context,
191+
glideBuilder
192+
.setMemoryCache(new LruResourceCache(Util.getBitmapByteSize(bitmap) * 10))
193+
.setBitmapPool(new LruBitmapPool(Util.getBitmapByteSize(bitmap) * 10)));
167194
Target<Drawable> target =
168195
concurrency.wait(
169196
GlideApp.with(context)
@@ -200,9 +227,11 @@ public void run() {
200227
@Test
201228
public void loadFromRequestBuilder_asDrawable_withBitmap_doesNotLoadFromDiskCache() {
202229
Bitmap bitmap = BitmapFactory.decodeResource(context.getResources(), ResourceIds.raw.canonical);
203-
Glide.init(context, new GlideBuilder()
204-
.setMemoryCache(new LruResourceCache(Util.getBitmapByteSize(bitmap) * 10))
205-
.setBitmapPool(new LruBitmapPool(Util.getBitmapByteSize(bitmap) * 10)));
230+
Glide.init(
231+
context,
232+
glideBuilder
233+
.setMemoryCache(new LruResourceCache(Util.getBitmapByteSize(bitmap) * 10))
234+
.setBitmapPool(new LruBitmapPool(Util.getBitmapByteSize(bitmap) * 10)));
206235
Target<Drawable> target =
207236
concurrency.wait(
208237
GlideApp.with(context)
@@ -240,9 +269,11 @@ public void run() {
240269
@Test
241270
public void loadFromRequestBuilder_asDrawable_withBitmapAndStrategyBeforeLoad_notFromCache() {
242271
Bitmap bitmap = BitmapFactory.decodeResource(context.getResources(), ResourceIds.raw.canonical);
243-
Glide.init(context, new GlideBuilder()
244-
.setMemoryCache(new LruResourceCache(Util.getBitmapByteSize(bitmap) * 10))
245-
.setBitmapPool(new LruBitmapPool(Util.getBitmapByteSize(bitmap) * 10)));
272+
Glide.init(
273+
context,
274+
glideBuilder
275+
.setMemoryCache(new LruResourceCache(Util.getBitmapByteSize(bitmap) * 10))
276+
.setBitmapPool(new LruBitmapPool(Util.getBitmapByteSize(bitmap) * 10)));
246277
Target<Drawable> target =
247278
concurrency.wait(
248279
GlideApp.with(context)
@@ -281,9 +312,11 @@ public void run() {
281312
@Test
282313
public void loadFromRequestBuilder_asBitmap_withBitmap_doesNotLoadFromDiskCache() {
283314
Bitmap bitmap = BitmapFactory.decodeResource(context.getResources(), ResourceIds.raw.canonical);
284-
Glide.init(context, new GlideBuilder()
285-
.setMemoryCache(new LruResourceCache(Util.getBitmapByteSize(bitmap) * 10))
286-
.setBitmapPool(new LruBitmapPool(Util.getBitmapByteSize(bitmap) * 10)));
315+
Glide.init(
316+
context,
317+
glideBuilder
318+
.setMemoryCache(new LruResourceCache(Util.getBitmapByteSize(bitmap) * 10))
319+
.setBitmapPool(new LruBitmapPool(Util.getBitmapByteSize(bitmap) * 10)));
287320
Target<Bitmap> target =
288321
concurrency.wait(
289322
GlideApp.with(context)
@@ -322,9 +355,12 @@ public void run() {
322355
@Test
323356
public void loadFromRequestBuilder_asBitmap_withBitmapAndStrategyBeforeLoad_notFromCache() {
324357
Bitmap bitmap = BitmapFactory.decodeResource(context.getResources(), ResourceIds.raw.canonical);
325-
Glide.init(context, new GlideBuilder()
326-
.setMemoryCache(new LruResourceCache(Util.getBitmapByteSize(bitmap) * 10))
327-
.setBitmapPool(new LruBitmapPool(Util.getBitmapByteSize(bitmap) * 10)));
358+
Glide.init(
359+
context,
360+
glideBuilder
361+
.setMemoryCache(new LruResourceCache(Util.getBitmapByteSize(bitmap) * 10))
362+
.setBitmapPool(new LruBitmapPool(Util.getBitmapByteSize(bitmap) * 10)));
363+
328364
Target<Bitmap> target =
329365
concurrency.wait(
330366
GlideApp.with(context)

0 commit comments

Comments
 (0)