Commit a823ccb2 authored by Xing Liu's avatar Xing Liu Committed by Commit Bot

Reland "Download home v2: Increase thumbnail cache size."

This is a reland of b859fdbb

Reason for reland: Link to a wrong crbug.

Original change's description:
> Download home v2: Increase thumbnail cache size.
>
> On Android we use an in-memory thumbnail cache to improve view recycling
> performance and use a disk cache to avoid costly backend operations like
> decoding images, videos etc.
>
> For download home v2, the thumbnail size is much larger than the old
> download home, thus we need to tweak the size of cache. 90% percentile
> of users have around 15 media thumbnails. The size of bitmap varies from
> under 1MB to around 5MB. The disk cache uses PNG to compress, the
> compression rate is around 1/3.
>
> This CL does the following:
>
> 1. The size of the thumbnail for high dpi devices are scaled down to
> mdpi, which can save memory/disk.
>
> 2. For in-memory cache size, download home v2 uses 15MB for high end
> devices. In-memory cache size is now passed in from owner of
> ThumbnailProviderImpl.
>
> 3. Disk cache size is increased. This should be shared by multiple
> owners of ThumbnailProviderImpl, or the owner with smaller disk cache
> size may remove files unexpectedly used by other owners.
>
> Bug: 87292
> Change-Id: If7cb700a60a8d119bca136b1a7d1fe7feb787d8b
> Reviewed-on: https://chromium-review.googlesource.com/c/1296546
> Commit-Queue: Xing Liu <xingliu@chromium.org>
> Reviewed-by: David Trainor <dtrainor@chromium.org>
> Reviewed-by: Shakti Sahu <shaktisahu@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#602957}

TBR=dtrainor@chromium.org,qinmin@chromium.org

Bug: 872925
Change-Id: Ida007e9255b033a48e259a214bf89431b25daafd
Reviewed-on: https://chromium-review.googlesource.com/c/1300684
Commit-Queue: Xing Liu <xingliu@chromium.org>
Reviewed-by: default avatarXing Liu <xingliu@chromium.org>
Cr-Commit-Position: refs/heads/master@{#603140}
parent 577cf26c
...@@ -4,6 +4,8 @@ ...@@ -4,6 +4,8 @@
package org.chromium.chrome.browser.download.home; package org.chromium.chrome.browser.download.home;
import static org.chromium.chrome.browser.util.ConversionUtils.BYTES_PER_MEGABYTE;
import org.chromium.base.ContextUtils; import org.chromium.base.ContextUtils;
import org.chromium.base.SysUtils; import org.chromium.base.SysUtils;
import org.chromium.chrome.browser.ChromeFeatureList; import org.chromium.chrome.browser.ChromeFeatureList;
...@@ -32,6 +34,17 @@ public class DownloadManagerUiConfig { ...@@ -32,6 +34,17 @@ public class DownloadManagerUiConfig {
*/ */
public final boolean useNewDownloadPathThumbnails; public final boolean useNewDownloadPathThumbnails;
/**
* The in-memory thumbnail size in bytes.
*/
public final int inMemoryThumbnailCacheSizeBytes;
/**
* The maximum thumbnail scale factor, thumbnail on higher dpi devices will downscale the
* quality to this level.
*/
public final float maxThumbnailScaleFactor;
/** /**
* The time interval during which a download update is considered recent enough to show * The time interval during which a download update is considered recent enough to show
* in Just Now section. * in Just Now section.
...@@ -46,6 +59,8 @@ public class DownloadManagerUiConfig { ...@@ -46,6 +59,8 @@ public class DownloadManagerUiConfig {
supportFullWidthImages = builder.mSupportFullWidthImages; supportFullWidthImages = builder.mSupportFullWidthImages;
useNewDownloadPath = builder.mUseNewDownloadPath; useNewDownloadPath = builder.mUseNewDownloadPath;
useNewDownloadPathThumbnails = builder.mUseNewDownloadPathThumbnails; useNewDownloadPathThumbnails = builder.mUseNewDownloadPathThumbnails;
inMemoryThumbnailCacheSizeBytes = builder.mInMemoryThumbnailCacheSizeBytes;
maxThumbnailScaleFactor = builder.mMaxThumbnailScaleFactor;
justNowThresholdSeconds = builder.mJustNowThresholdSeconds; justNowThresholdSeconds = builder.mJustNowThresholdSeconds;
} }
...@@ -56,12 +71,16 @@ public class DownloadManagerUiConfig { ...@@ -56,12 +71,16 @@ public class DownloadManagerUiConfig {
/** Default value for threshold time interval to show up in Just Now section. */ /** Default value for threshold time interval to show up in Just Now section. */
private static final int JUST_NOW_THRESHOLD_SECONDS_DEFAULT = 30 * 60; private static final int JUST_NOW_THRESHOLD_SECONDS_DEFAULT = 30 * 60;
private static final int IN_MEMORY_THUMBNAIL_CACHE_SIZE_BYTES = 15 * BYTES_PER_MEGABYTE;
private boolean mIsOffTheRecord; private boolean mIsOffTheRecord;
private boolean mIsSeparateActivity; private boolean mIsSeparateActivity;
private boolean mUseGenericViewTypes; private boolean mUseGenericViewTypes;
private boolean mSupportFullWidthImages; private boolean mSupportFullWidthImages;
private boolean mUseNewDownloadPath; private boolean mUseNewDownloadPath;
private boolean mUseNewDownloadPathThumbnails; private boolean mUseNewDownloadPathThumbnails;
private int mInMemoryThumbnailCacheSizeBytes = IN_MEMORY_THUMBNAIL_CACHE_SIZE_BYTES;
private float mMaxThumbnailScaleFactor = 1.f; /* mdpi scale factor. */
private long mJustNowThresholdSeconds; private long mJustNowThresholdSeconds;
public Builder() { public Builder() {
...@@ -101,6 +120,16 @@ public class DownloadManagerUiConfig { ...@@ -101,6 +120,16 @@ public class DownloadManagerUiConfig {
return this; return this;
} }
public Builder setInMemoryThumbnailCacheSizeBytes(int inMemoryThumbnailCacheSizeBytes) {
mInMemoryThumbnailCacheSizeBytes = inMemoryThumbnailCacheSizeBytes;
return this;
}
public Builder setMaxThumbnailScaleFactor(float maxThumbnailScaleFactor) {
mMaxThumbnailScaleFactor = maxThumbnailScaleFactor;
return this;
}
public DownloadManagerUiConfig build() { public DownloadManagerUiConfig build() {
return new DownloadManagerUiConfig(this); return new DownloadManagerUiConfig(this);
} }
......
...@@ -7,6 +7,7 @@ package org.chromium.chrome.browser.download.home.glue; ...@@ -7,6 +7,7 @@ package org.chromium.chrome.browser.download.home.glue;
import android.graphics.Bitmap; import android.graphics.Bitmap;
import org.chromium.base.Callback; import org.chromium.base.Callback;
import org.chromium.base.ContextUtils;
import org.chromium.chrome.browser.widget.ThumbnailProvider; import org.chromium.chrome.browser.widget.ThumbnailProvider;
import org.chromium.chrome.browser.widget.ThumbnailProvider.ThumbnailRequest; import org.chromium.chrome.browser.widget.ThumbnailProvider.ThumbnailRequest;
import org.chromium.chrome.browser.widget.ThumbnailProviderImpl; import org.chromium.chrome.browser.widget.ThumbnailProviderImpl;
...@@ -14,6 +15,7 @@ import org.chromium.components.offline_items_collection.OfflineContentProvider; ...@@ -14,6 +15,7 @@ import org.chromium.components.offline_items_collection.OfflineContentProvider;
import org.chromium.components.offline_items_collection.OfflineItem; import org.chromium.components.offline_items_collection.OfflineItem;
import org.chromium.components.offline_items_collection.OfflineItemVisuals; import org.chromium.components.offline_items_collection.OfflineItemVisuals;
import org.chromium.components.offline_items_collection.VisualsCallback; import org.chromium.components.offline_items_collection.VisualsCallback;
import org.chromium.ui.display.DisplayAndroid;
/** /**
* Glue class responsible for connecting the current downloads and {@link OfflineContentProvider} * Glue class responsible for connecting the current downloads and {@link OfflineContentProvider}
...@@ -28,11 +30,15 @@ public class ThumbnailRequestGlue implements ThumbnailRequest { ...@@ -28,11 +30,15 @@ public class ThumbnailRequestGlue implements ThumbnailRequest {
/** Creates a {@link ThumbnailRequestGlue} instance. */ /** Creates a {@link ThumbnailRequestGlue} instance. */
public ThumbnailRequestGlue(OfflineContentProviderGlue provider, OfflineItem item, public ThumbnailRequestGlue(OfflineContentProviderGlue provider, OfflineItem item,
int iconWidthPx, int iconHeightPx, VisualsCallback callback) { int iconWidthPx, int iconHeightPx, float maxThumbnailScaleFactor,
VisualsCallback callback) {
mProvider = provider; mProvider = provider;
mItem = item; mItem = item;
mIconWidthPx = iconWidthPx;
mIconHeightPx = iconHeightPx; // Scale the thumbnail quality to mdpi for high dpi devices.
mIconWidthPx = downscaleThumbnailSize(iconWidthPx, maxThumbnailScaleFactor);
mIconHeightPx = downscaleThumbnailSize(iconHeightPx, maxThumbnailScaleFactor);
mCallback = callback; mCallback = callback;
} }
...@@ -90,4 +96,18 @@ public class ThumbnailRequestGlue implements ThumbnailRequest { ...@@ -90,4 +96,18 @@ public class ThumbnailRequestGlue implements ThumbnailRequest {
} }
}); });
} }
/**
* Returns size in pixel used by the thumbnail request, considering dip scale factor.
* @param currentSize The current size before considering the dip scale factor.
* @param maxScaleFactor The maximum scale factor we expected to show as the thumbnail. Device
* with higher scale factor will be downscaled to this level.
*/
private int downscaleThumbnailSize(int currentSize, float maxScaleFactor) {
DisplayAndroid display =
DisplayAndroid.getNonMultiDisplay(ContextUtils.getApplicationContext());
float scale = display.getDipScale();
if (scale <= maxScaleFactor) return currentSize;
return (int) (maxScaleFactor * currentSize / scale);
}
} }
...@@ -74,6 +74,7 @@ class DateOrderedListMediator { ...@@ -74,6 +74,7 @@ class DateOrderedListMediator {
private final ThumbnailProvider mThumbnailProvider; private final ThumbnailProvider mThumbnailProvider;
private final MediatorSelectionObserver mSelectionObserver; private final MediatorSelectionObserver mSelectionObserver;
private final SelectionDelegate<ListItem> mSelectionDelegate; private final SelectionDelegate<ListItem> mSelectionDelegate;
private final DownloadManagerUiConfig mUiConfig;
private final OffTheRecordOfflineItemFilter mOffTheRecordFilter; private final OffTheRecordOfflineItemFilter mOffTheRecordFilter;
private final InvalidStateOfflineItemFilter mInvalidStateFilter; private final InvalidStateOfflineItemFilter mInvalidStateFilter;
...@@ -140,6 +141,7 @@ class DateOrderedListMediator { ...@@ -140,6 +141,7 @@ class DateOrderedListMediator {
mModel = model; mModel = model;
mDeleteController = deleteController; mDeleteController = deleteController;
mSelectionDelegate = selectionDelegate; mSelectionDelegate = selectionDelegate;
mUiConfig = config;
mSource = new OfflineItemSource(mProvider); mSource = new OfflineItemSource(mProvider);
mOffTheRecordFilter = new OffTheRecordOfflineItemFilter(config.isOffTheRecord, mSource); mOffTheRecordFilter = new OffTheRecordOfflineItemFilter(config.isOffTheRecord, mSource);
...@@ -154,7 +156,8 @@ class DateOrderedListMediator { ...@@ -154,7 +156,8 @@ class DateOrderedListMediator {
mSearchFilter.addObserver(new EmptyStateObserver(mSearchFilter, dateOrderedListObserver)); mSearchFilter.addObserver(new EmptyStateObserver(mSearchFilter, dateOrderedListObserver));
mThumbnailProvider = new ThumbnailProviderImpl( mThumbnailProvider = new ThumbnailProviderImpl(
((ChromeApplication) ContextUtils.getApplicationContext()).getReferencePool()); ((ChromeApplication) ContextUtils.getApplicationContext()).getReferencePool(),
config.inMemoryThumbnailCacheSizeBytes);
mSelectionObserver = new MediatorSelectionObserver(selectionDelegate); mSelectionObserver = new MediatorSelectionObserver(selectionDelegate);
mModel.getProperties().set(ListProperties.ENABLE_ITEM_ANIMATIONS, true); mModel.getProperties().set(ListProperties.ENABLE_ITEM_ANIMATIONS, true);
...@@ -358,8 +361,8 @@ class DateOrderedListMediator { ...@@ -358,8 +361,8 @@ class DateOrderedListMediator {
return () -> {}; return () -> {};
} }
ThumbnailRequest request = ThumbnailRequest request = new ThumbnailRequestGlue(mProvider, item, iconWidthPx,
new ThumbnailRequestGlue(mProvider, item, iconWidthPx, iconHeightPx, callback); iconHeightPx, mUiConfig.maxThumbnailScaleFactor, callback);
mThumbnailProvider.getThumbnail(request); mThumbnailProvider.getThumbnail(request);
return () -> mThumbnailProvider.cancelRetrieval(request); return () -> mThumbnailProvider.cancelRetrieval(request);
} }
......
...@@ -51,7 +51,7 @@ import java.util.LinkedHashSet; ...@@ -51,7 +51,7 @@ import java.util.LinkedHashSet;
public class ThumbnailDiskStorage implements ThumbnailGeneratorCallback { public class ThumbnailDiskStorage implements ThumbnailGeneratorCallback {
private static final String TAG = "ThumbnailStorage"; private static final String TAG = "ThumbnailStorage";
private static final int MAX_CACHE_BYTES = private static final int MAX_CACHE_BYTES =
ConversionUtils.BYTES_PER_MEGABYTE; // Max disk cache size is 1MB. 5 * ConversionUtils.BYTES_PER_MEGABYTE; // Max disk cache size is 5MB.
// LRU cache of a pair of thumbnail's contentID and size. The order is based on the sequence of // LRU cache of a pair of thumbnail's contentID and size. The order is based on the sequence of
// add and get with the most recent at the end. The order at initialization (i.e. browser // add and get with the most recent at the end. The order at initialization (i.e. browser
...@@ -76,6 +76,9 @@ public class ThumbnailDiskStorage implements ThumbnailGeneratorCallback { ...@@ -76,6 +76,9 @@ public class ThumbnailDiskStorage implements ThumbnailGeneratorCallback {
private ThumbnailStorageDelegate mDelegate; private ThumbnailStorageDelegate mDelegate;
// Maximum size in bytes for the disk cache.
private final int mMaxCacheBytes;
// Number of bytes used in disk for cache. // Number of bytes used in disk for cache.
@VisibleForTesting @VisibleForTesting
long mSizeBytes; long mSizeBytes;
...@@ -174,10 +177,12 @@ public class ThumbnailDiskStorage implements ThumbnailGeneratorCallback { ...@@ -174,10 +177,12 @@ public class ThumbnailDiskStorage implements ThumbnailGeneratorCallback {
} }
@VisibleForTesting @VisibleForTesting
ThumbnailDiskStorage(ThumbnailStorageDelegate delegate, ThumbnailGenerator thumbnailGenerator) { ThumbnailDiskStorage(ThumbnailStorageDelegate delegate, ThumbnailGenerator thumbnailGenerator,
int maxCacheSizeBytes) {
ThreadUtils.assertOnUiThread(); ThreadUtils.assertOnUiThread();
mDelegate = delegate; mDelegate = delegate;
mThumbnailGenerator = thumbnailGenerator; mThumbnailGenerator = thumbnailGenerator;
mMaxCacheBytes = maxCacheSizeBytes;
new InitTask().executeOnExecutor(AsyncTask.SERIAL_EXECUTOR); new InitTask().executeOnExecutor(AsyncTask.SERIAL_EXECUTOR);
} }
...@@ -188,7 +193,7 @@ public class ThumbnailDiskStorage implements ThumbnailGeneratorCallback { ...@@ -188,7 +193,7 @@ public class ThumbnailDiskStorage implements ThumbnailGeneratorCallback {
* @return An instance of {@link ThumbnailDiskStorage}. * @return An instance of {@link ThumbnailDiskStorage}.
*/ */
public static ThumbnailDiskStorage create(ThumbnailStorageDelegate delegate) { public static ThumbnailDiskStorage create(ThumbnailStorageDelegate delegate) {
return new ThumbnailDiskStorage(delegate, new ThumbnailGenerator()); return new ThumbnailDiskStorage(delegate, new ThumbnailGenerator(), MAX_CACHE_BYTES);
} }
/** /**
...@@ -391,12 +396,12 @@ public class ThumbnailDiskStorage implements ThumbnailGeneratorCallback { ...@@ -391,12 +396,12 @@ public class ThumbnailDiskStorage implements ThumbnailGeneratorCallback {
} }
/** /**
* Trim the cache to stay under the MAX_CACHE_BYTES limit by removing the oldest entries. * Trim the cache to stay under the max cache size by removing the oldest entries.
*/ */
@VisibleForTesting @VisibleForTesting
void trim() { void trim() {
ThreadUtils.assertOnBackgroundThread(); ThreadUtils.assertOnBackgroundThread();
while (mSizeBytes > MAX_CACHE_BYTES) { while (mSizeBytes > mMaxCacheBytes) {
removeFromDiskHelper(sDiskLruCache.iterator().next()); removeFromDiskHelper(sDiskLruCache.iterator().next());
} }
} }
...@@ -476,10 +481,9 @@ public class ThumbnailDiskStorage implements ThumbnailGeneratorCallback { ...@@ -476,10 +481,9 @@ public class ThumbnailDiskStorage implements ThumbnailGeneratorCallback {
* Get directory for thumbnail entries in the designated app (internal) cache directory. * Get directory for thumbnail entries in the designated app (internal) cache directory.
* The directory's name must be unique. * The directory's name must be unique.
* @param context The application's context. * @param context The application's context.
* @param uniqueName The name of the thumbnail directory. Must be unique.
* @return The path to the thumbnail cache directory. * @return The path to the thumbnail cache directory.
*/ */
private static File getDiskCacheDir(Context context, String thumbnailDirName) { private static File getDiskCacheDir(Context context, String thumbnailDirName) {
return new File(context.getCacheDir().getPath() + File.separator + thumbnailDirName); return new File(context.getCacheDir().getPath() + File.separator + thumbnailDirName);
} }
} }
\ No newline at end of file
...@@ -31,14 +31,18 @@ import java.util.Locale; ...@@ -31,14 +31,18 @@ import java.util.Locale;
* duplicating work to decode the same image for two different requests. * duplicating work to decode the same image for two different requests.
*/ */
public class ThumbnailProviderImpl implements ThumbnailProvider, ThumbnailStorageDelegate { public class ThumbnailProviderImpl implements ThumbnailProvider, ThumbnailStorageDelegate {
/** 5 MB of thumbnails should be enough for everyone. */ /** Default in-memory thumbnail cache size. */
private static final int MAX_CACHE_BYTES = 5 * ConversionUtils.BYTES_PER_MEGABYTE; private static final int DEFAULT_MAX_CACHE_BYTES = 5 * ConversionUtils.BYTES_PER_MEGABYTE;
/** /**
* Helper object to store in the LruCache when we don't really need a value but can't use null. * Helper object to store in the LruCache when we don't really need a value but can't use null.
*/ */
private static final Object NO_BITMAP_PLACEHOLDER = new Object(); private static final Object NO_BITMAP_PLACEHOLDER = new Object();
/**
* An in-memory LRU cache used to cache bitmaps, mostly improve performance for scrolling, when
* the view is recycled and needs a new thumbnail.
*/
private BitmapCache mBitmapCache; private BitmapCache mBitmapCache;
/** /**
...@@ -57,9 +61,22 @@ public class ThumbnailProviderImpl implements ThumbnailProvider, ThumbnailStorag ...@@ -57,9 +61,22 @@ public class ThumbnailProviderImpl implements ThumbnailProvider, ThumbnailStorag
private ThumbnailDiskStorage mStorage; private ThumbnailDiskStorage mStorage;
/**
* Constructor to build the thumbnail provider with default thumbnail cache size.
* @param referencePool The application's reference pool.
*/
public ThumbnailProviderImpl(DiscardableReferencePool referencePool) { public ThumbnailProviderImpl(DiscardableReferencePool referencePool) {
this(referencePool, DEFAULT_MAX_CACHE_BYTES);
}
/**
* Constructor to build the thumbnail provider.
* @param referencePool The application's reference pool.
* @param bitmapCacheSizeByte The size in bytes of the in-memory LRU bitmap cache.
*/
public ThumbnailProviderImpl(DiscardableReferencePool referencePool, int bitmapCacheSizeByte) {
ThreadUtils.assertOnUiThread(); ThreadUtils.assertOnUiThread();
mBitmapCache = new BitmapCache(referencePool, MAX_CACHE_BYTES); mBitmapCache = new BitmapCache(referencePool, bitmapCacheSizeByte);
mStorage = ThumbnailDiskStorage.create(this); mStorage = ThumbnailDiskStorage.create(this);
} }
......
...@@ -18,6 +18,7 @@ import org.junit.runner.RunWith; ...@@ -18,6 +18,7 @@ import org.junit.runner.RunWith;
import org.chromium.base.ThreadUtils; import org.chromium.base.ThreadUtils;
import org.chromium.base.test.util.UrlUtils; import org.chromium.base.test.util.UrlUtils;
import org.chromium.chrome.browser.util.ConversionUtils;
import org.chromium.chrome.test.ChromeJUnit4ClassRunner; import org.chromium.chrome.test.ChromeJUnit4ClassRunner;
import org.chromium.content_public.browser.test.util.Criteria; import org.chromium.content_public.browser.test.util.Criteria;
import org.chromium.content_public.browser.test.util.CriteriaHelper; import org.chromium.content_public.browser.test.util.CriteriaHelper;
...@@ -42,6 +43,7 @@ public class ThumbnailDiskStorageTest { ...@@ -42,6 +43,7 @@ public class ThumbnailDiskStorageTest {
private static final Bitmap BITMAP2 = BitmapFactory.decodeFile(FILE_PATH2); private static final Bitmap BITMAP2 = BitmapFactory.decodeFile(FILE_PATH2);
private static final int ICON_WIDTH1 = 50; private static final int ICON_WIDTH1 = 50;
private static final int ICON_WIDTH2 = 70; private static final int ICON_WIDTH2 = 70;
private static final int TEST_MAX_CACHE_BYTES = 1 * ConversionUtils.BYTES_PER_MEGABYTE;
private static final long TIMEOUT_MS = 10000; private static final long TIMEOUT_MS = 10000;
private static final long INTERVAL_MS = 500; private static final long INTERVAL_MS = 500;
...@@ -98,9 +100,9 @@ public class ThumbnailDiskStorageTest { ...@@ -98,9 +100,9 @@ public class ThumbnailDiskStorageTest {
// Accessed by test and UI threads. // Accessed by test and UI threads.
public final AtomicBoolean initialized = new AtomicBoolean(); public final AtomicBoolean initialized = new AtomicBoolean();
public TestThumbnailDiskStorage( public TestThumbnailDiskStorage(TestThumbnailStorageDelegate delegate,
TestThumbnailStorageDelegate delegate, TestThumbnailGenerator thumbnailGenerator) { TestThumbnailGenerator thumbnailGenerator, int maxCacheSizeBytes) {
super(delegate, thumbnailGenerator); super(delegate, thumbnailGenerator, maxCacheSizeBytes);
} }
@Override @Override
...@@ -159,8 +161,9 @@ public class ThumbnailDiskStorageTest { ...@@ -159,8 +161,9 @@ public class ThumbnailDiskStorageTest {
ThreadUtils.runOnUiThreadBlocking(new Runnable() { ThreadUtils.runOnUiThreadBlocking(new Runnable() {
@Override @Override
public void run() { public void run() {
mTestThumbnailDiskStorage = new TestThumbnailDiskStorage( mTestThumbnailDiskStorage =
mTestThumbnailStorageDelegate, mTestThumbnailGenerator); new TestThumbnailDiskStorage(mTestThumbnailStorageDelegate,
mTestThumbnailGenerator, TEST_MAX_CACHE_BYTES);
// Clear the disk cache so that cached entries from previous runs won't show up. // Clear the disk cache so that cached entries from previous runs won't show up.
mTestThumbnailDiskStorage.clear(); mTestThumbnailDiskStorage.clear();
} }
......
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