Commit b57f41d7 authored by David Trainor's avatar David Trainor Committed by Commit Bot

Fix ThumbnailProviderImpl destroy chain.

Add safety checks to the following classes to make sure they work better
during destroy:
- ThumbnailProviderImpl - Cancel all requests after destroy and do not
use any results given.
- BitmapCache - Do not add bitmaps to the cache after destroy (real
exception).
- ThumbnailDiskStorage - Do not take requests after destroy and do not
return results after destroy.

Bug: 934515
Change-Id: Id2bbac832fd4b9c81ae81ee806f66494eda59e16
Reviewed-on: https://chromium-review.googlesource.com/c/1482590
Commit-Queue: David Trainor <dtrainor@chromium.org>
Reviewed-by: default avatarTheresa <twellington@chromium.org>
Cr-Commit-Position: refs/heads/master@{#634956}
parent 63c8fae6
...@@ -104,6 +104,8 @@ public class BitmapCache { ...@@ -104,6 +104,8 @@ public class BitmapCache {
public Bitmap getBitmap(String key) { public Bitmap getBitmap(String key) {
ThreadUtils.assertOnUiThread(); ThreadUtils.assertOnUiThread();
if (mBitmapCache == null) return null;
Bitmap cachedBitmap = getBitmapCache().get(key); Bitmap cachedBitmap = getBitmapCache().get(key);
assert cachedBitmap == null || !cachedBitmap.isRecycled(); assert cachedBitmap == null || !cachedBitmap.isRecycled();
maybeScheduleDeduplicationCache(); maybeScheduleDeduplicationCache();
...@@ -112,7 +114,8 @@ public class BitmapCache { ...@@ -112,7 +114,8 @@ public class BitmapCache {
public void putBitmap(@NonNull String key, @Nullable Bitmap bitmap) { public void putBitmap(@NonNull String key, @Nullable Bitmap bitmap) {
ThreadUtils.assertOnUiThread(); ThreadUtils.assertOnUiThread();
if (bitmap == null) return; if (bitmap == null || mBitmapCache == null) return;
if (!SysUtils.isLowEndDevice()) getBitmapCache().put(key, bitmap); if (!SysUtils.isLowEndDevice()) getBitmapCache().put(key, bitmap);
maybeScheduleDeduplicationCache(); maybeScheduleDeduplicationCache();
sDeduplicationCache.put(key, new WeakReference<>(bitmap)); sDeduplicationCache.put(key, new WeakReference<>(bitmap));
......
...@@ -83,6 +83,9 @@ public class ThumbnailDiskStorage implements ThumbnailGeneratorCallback { ...@@ -83,6 +83,9 @@ public class ThumbnailDiskStorage implements ThumbnailGeneratorCallback {
@VisibleForTesting @VisibleForTesting
long mSizeBytes; long mSizeBytes;
// Whether or not this class has been destroyed and should not be used.
private boolean mDestroyed;
private class InitTask extends AsyncTask<Void> { private class InitTask extends AsyncTask<Void> {
@Override @Override
protected Void doInBackground() { protected Void doInBackground() {
...@@ -201,6 +204,7 @@ public class ThumbnailDiskStorage implements ThumbnailGeneratorCallback { ...@@ -201,6 +204,7 @@ public class ThumbnailDiskStorage implements ThumbnailGeneratorCallback {
*/ */
public void destroy() { public void destroy() {
mThumbnailGenerator.destroy(); mThumbnailGenerator.destroy();
mDestroyed = true;
} }
/** /**
...@@ -217,7 +221,7 @@ public class ThumbnailDiskStorage implements ThumbnailGeneratorCallback { ...@@ -217,7 +221,7 @@ public class ThumbnailDiskStorage implements ThumbnailGeneratorCallback {
*/ */
public void retrieveThumbnail(ThumbnailProvider.ThumbnailRequest request) { public void retrieveThumbnail(ThumbnailProvider.ThumbnailRequest request) {
ThreadUtils.assertOnUiThread(); ThreadUtils.assertOnUiThread();
if (TextUtils.isEmpty(request.getContentId())) return; if (mDestroyed || TextUtils.isEmpty(request.getContentId())) return;
new GetThumbnailTask(request).executeOnExecutor(AsyncTask.SERIAL_EXECUTOR); new GetThumbnailTask(request).executeOnExecutor(AsyncTask.SERIAL_EXECUTOR);
} }
...@@ -233,6 +237,9 @@ public class ThumbnailDiskStorage implements ThumbnailGeneratorCallback { ...@@ -233,6 +237,9 @@ public class ThumbnailDiskStorage implements ThumbnailGeneratorCallback {
@Override @Override
public void onThumbnailRetrieved( public void onThumbnailRetrieved(
@NonNull String contentId, @Nullable Bitmap bitmap, int iconSizePx) { @NonNull String contentId, @Nullable Bitmap bitmap, int iconSizePx) {
// If we've been destroyed, drop any responses coming back from retrieval tasks.
if (mDestroyed) return;
ThreadUtils.assertOnUiThread(); ThreadUtils.assertOnUiThread();
if (bitmap != null && !TextUtils.isEmpty(contentId)) { if (bitmap != null && !TextUtils.isEmpty(contentId)) {
new CacheThumbnailTask(contentId, bitmap, iconSizePx) new CacheThumbnailTask(contentId, bitmap, iconSizePx)
......
...@@ -84,6 +84,10 @@ public class ThumbnailProviderImpl implements ThumbnailProvider, ThumbnailStorag ...@@ -84,6 +84,10 @@ public class ThumbnailProviderImpl implements ThumbnailProvider, ThumbnailStorag
@Override @Override
public void destroy() { public void destroy() {
// Drop any references to any current requests.
mCurrentRequest = null;
mRequestQueue.clear();
ThreadUtils.assertOnUiThread(); ThreadUtils.assertOnUiThread();
mStorage.destroy(); mStorage.destroy();
mBitmapCache.destroy(); mBitmapCache.destroy();
...@@ -195,6 +199,9 @@ public class ThumbnailProviderImpl implements ThumbnailProvider, ThumbnailStorag ...@@ -195,6 +199,9 @@ public class ThumbnailProviderImpl implements ThumbnailProvider, ThumbnailStorag
*/ */
@Override @Override
public void onThumbnailRetrieved(@NonNull String contentId, @Nullable Bitmap bitmap) { public void onThumbnailRetrieved(@NonNull String contentId, @Nullable Bitmap bitmap) {
// Early-out if we have no actual current request.
if (mCurrentRequest == null) return;
if (bitmap != null) { if (bitmap != null) {
// The bitmap returned here is retrieved from the native side. The image decoder there // The bitmap returned here is retrieved from the native side. The image decoder there
// scales down the image (if it is too big) so that one of its sides is smaller than or // scales down the image (if it is too big) so that one of its sides is smaller than or
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment