Commit 6fd4ae8a authored by Xing Liu's avatar Xing Liu Committed by Commit Bot

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

This reverts commit b859fdbb.

Reason for revert: <INSERT REASONING HERE>

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,shaktisahu@chromium.org,xingliu@chromium.org

Change-Id: I1c63b7c1d6aa4af322080f4d27cd84b1ccc244bc
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: 87292
Reviewed-on: https://chromium-review.googlesource.com/c/1300795Reviewed-by: default avatarXing Liu <xingliu@chromium.org>
Commit-Queue: Xing Liu <xingliu@chromium.org>
Cr-Commit-Position: refs/heads/master@{#602962}
parent c8ac2e2a
......@@ -4,8 +4,6 @@
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.SysUtils;
import org.chromium.chrome.browser.ChromeFeatureList;
......@@ -34,17 +32,6 @@ public class DownloadManagerUiConfig {
*/
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
* in Just Now section.
......@@ -59,8 +46,6 @@ public class DownloadManagerUiConfig {
supportFullWidthImages = builder.mSupportFullWidthImages;
useNewDownloadPath = builder.mUseNewDownloadPath;
useNewDownloadPathThumbnails = builder.mUseNewDownloadPathThumbnails;
inMemoryThumbnailCacheSizeBytes = builder.mInMemoryThumbnailCacheSizeBytes;
maxThumbnailScaleFactor = builder.mMaxThumbnailScaleFactor;
justNowThresholdSeconds = builder.mJustNowThresholdSeconds;
}
......@@ -71,16 +56,12 @@ public class DownloadManagerUiConfig {
/** 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 IN_MEMORY_THUMBNAIL_CACHE_SIZE_BYTES = 15 * BYTES_PER_MEGABYTE;
private boolean mIsOffTheRecord;
private boolean mIsSeparateActivity;
private boolean mUseGenericViewTypes;
private boolean mSupportFullWidthImages;
private boolean mUseNewDownloadPath;
private boolean mUseNewDownloadPathThumbnails;
private int mInMemoryThumbnailCacheSizeBytes = IN_MEMORY_THUMBNAIL_CACHE_SIZE_BYTES;
private float mMaxThumbnailScaleFactor = 1.f; /* mdpi scale factor. */
private long mJustNowThresholdSeconds;
public Builder() {
......@@ -120,16 +101,6 @@ public class DownloadManagerUiConfig {
return this;
}
public Builder setInMemoryThumbnailCacheSizeBytes(int inMemoryThumbnailCacheSizeBytes) {
mInMemoryThumbnailCacheSizeBytes = inMemoryThumbnailCacheSizeBytes;
return this;
}
public Builder setMaxThumbnailScaleFactor(float maxThumbnailScaleFactor) {
mMaxThumbnailScaleFactor = maxThumbnailScaleFactor;
return this;
}
public DownloadManagerUiConfig build() {
return new DownloadManagerUiConfig(this);
}
......
......@@ -7,7 +7,6 @@ package org.chromium.chrome.browser.download.home.glue;
import android.graphics.Bitmap;
import org.chromium.base.Callback;
import org.chromium.base.ContextUtils;
import org.chromium.chrome.browser.widget.ThumbnailProvider;
import org.chromium.chrome.browser.widget.ThumbnailProvider.ThumbnailRequest;
import org.chromium.chrome.browser.widget.ThumbnailProviderImpl;
......@@ -15,7 +14,6 @@ import org.chromium.components.offline_items_collection.OfflineContentProvider;
import org.chromium.components.offline_items_collection.OfflineItem;
import org.chromium.components.offline_items_collection.OfflineItemVisuals;
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}
......@@ -30,15 +28,11 @@ public class ThumbnailRequestGlue implements ThumbnailRequest {
/** Creates a {@link ThumbnailRequestGlue} instance. */
public ThumbnailRequestGlue(OfflineContentProviderGlue provider, OfflineItem item,
int iconWidthPx, int iconHeightPx, float maxThumbnailScaleFactor,
VisualsCallback callback) {
int iconWidthPx, int iconHeightPx, VisualsCallback callback) {
mProvider = provider;
mItem = item;
// Scale the thumbnail quality to mdpi for high dpi devices.
mIconWidthPx = downscaleThumbnailSize(iconWidthPx, maxThumbnailScaleFactor);
mIconHeightPx = downscaleThumbnailSize(iconHeightPx, maxThumbnailScaleFactor);
mIconWidthPx = iconWidthPx;
mIconHeightPx = iconHeightPx;
mCallback = callback;
}
......@@ -96,18 +90,4 @@ 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,7 +74,6 @@ class DateOrderedListMediator {
private final ThumbnailProvider mThumbnailProvider;
private final MediatorSelectionObserver mSelectionObserver;
private final SelectionDelegate<ListItem> mSelectionDelegate;
private final DownloadManagerUiConfig mUiConfig;
private final OffTheRecordOfflineItemFilter mOffTheRecordFilter;
private final InvalidStateOfflineItemFilter mInvalidStateFilter;
......@@ -141,7 +140,6 @@ class DateOrderedListMediator {
mModel = model;
mDeleteController = deleteController;
mSelectionDelegate = selectionDelegate;
mUiConfig = config;
mSource = new OfflineItemSource(mProvider);
mOffTheRecordFilter = new OffTheRecordOfflineItemFilter(config.isOffTheRecord, mSource);
......@@ -156,8 +154,7 @@ class DateOrderedListMediator {
mSearchFilter.addObserver(new EmptyStateObserver(mSearchFilter, dateOrderedListObserver));
mThumbnailProvider = new ThumbnailProviderImpl(
((ChromeApplication) ContextUtils.getApplicationContext()).getReferencePool(),
config.inMemoryThumbnailCacheSizeBytes);
((ChromeApplication) ContextUtils.getApplicationContext()).getReferencePool());
mSelectionObserver = new MediatorSelectionObserver(selectionDelegate);
mModel.getProperties().set(ListProperties.ENABLE_ITEM_ANIMATIONS, true);
......@@ -361,8 +358,8 @@ class DateOrderedListMediator {
return () -> {};
}
ThumbnailRequest request = new ThumbnailRequestGlue(mProvider, item, iconWidthPx,
iconHeightPx, mUiConfig.maxThumbnailScaleFactor, callback);
ThumbnailRequest request =
new ThumbnailRequestGlue(mProvider, item, iconWidthPx, iconHeightPx, callback);
mThumbnailProvider.getThumbnail(request);
return () -> mThumbnailProvider.cancelRetrieval(request);
}
......
......@@ -51,7 +51,7 @@ import java.util.LinkedHashSet;
public class ThumbnailDiskStorage implements ThumbnailGeneratorCallback {
private static final String TAG = "ThumbnailStorage";
private static final int MAX_CACHE_BYTES =
5 * ConversionUtils.BYTES_PER_MEGABYTE; // Max disk cache size is 5MB.
ConversionUtils.BYTES_PER_MEGABYTE; // Max disk cache size is 1MB.
// 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
......@@ -76,9 +76,6 @@ public class ThumbnailDiskStorage implements ThumbnailGeneratorCallback {
private ThumbnailStorageDelegate mDelegate;
// Maximum size in bytes for the disk cache.
private final int mMaxCacheBytes;
// Number of bytes used in disk for cache.
@VisibleForTesting
long mSizeBytes;
......@@ -177,12 +174,10 @@ public class ThumbnailDiskStorage implements ThumbnailGeneratorCallback {
}
@VisibleForTesting
ThumbnailDiskStorage(ThumbnailStorageDelegate delegate, ThumbnailGenerator thumbnailGenerator,
int maxCacheSizeBytes) {
ThumbnailDiskStorage(ThumbnailStorageDelegate delegate, ThumbnailGenerator thumbnailGenerator) {
ThreadUtils.assertOnUiThread();
mDelegate = delegate;
mThumbnailGenerator = thumbnailGenerator;
mMaxCacheBytes = maxCacheSizeBytes;
new InitTask().executeOnExecutor(AsyncTask.SERIAL_EXECUTOR);
}
......@@ -193,7 +188,7 @@ public class ThumbnailDiskStorage implements ThumbnailGeneratorCallback {
* @return An instance of {@link ThumbnailDiskStorage}.
*/
public static ThumbnailDiskStorage create(ThumbnailStorageDelegate delegate) {
return new ThumbnailDiskStorage(delegate, new ThumbnailGenerator(), MAX_CACHE_BYTES);
return new ThumbnailDiskStorage(delegate, new ThumbnailGenerator());
}
/**
......@@ -396,12 +391,12 @@ public class ThumbnailDiskStorage implements ThumbnailGeneratorCallback {
}
/**
* Trim the cache to stay under the max cache size by removing the oldest entries.
* Trim the cache to stay under the MAX_CACHE_BYTES limit by removing the oldest entries.
*/
@VisibleForTesting
void trim() {
ThreadUtils.assertOnBackgroundThread();
while (mSizeBytes > mMaxCacheBytes) {
while (mSizeBytes > MAX_CACHE_BYTES) {
removeFromDiskHelper(sDiskLruCache.iterator().next());
}
}
......@@ -481,6 +476,7 @@ public class ThumbnailDiskStorage implements ThumbnailGeneratorCallback {
* Get directory for thumbnail entries in the designated app (internal) cache directory.
* The directory's name must be unique.
* @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.
*/
private static File getDiskCacheDir(Context context, String thumbnailDirName) {
......
......@@ -31,18 +31,14 @@ import java.util.Locale;
* duplicating work to decode the same image for two different requests.
*/
public class ThumbnailProviderImpl implements ThumbnailProvider, ThumbnailStorageDelegate {
/** Default in-memory thumbnail cache size. */
private static final int DEFAULT_MAX_CACHE_BYTES = 5 * ConversionUtils.BYTES_PER_MEGABYTE;
/** 5 MB of thumbnails should be enough for everyone. */
private static final int 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.
*/
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;
/**
......@@ -61,22 +57,9 @@ public class ThumbnailProviderImpl implements ThumbnailProvider, ThumbnailStorag
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) {
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();
mBitmapCache = new BitmapCache(referencePool, bitmapCacheSizeByte);
mBitmapCache = new BitmapCache(referencePool, MAX_CACHE_BYTES);
mStorage = ThumbnailDiskStorage.create(this);
}
......
......@@ -18,7 +18,6 @@ import org.junit.runner.RunWith;
import org.chromium.base.ThreadUtils;
import org.chromium.base.test.util.UrlUtils;
import org.chromium.chrome.browser.util.ConversionUtils;
import org.chromium.chrome.test.ChromeJUnit4ClassRunner;
import org.chromium.content_public.browser.test.util.Criteria;
import org.chromium.content_public.browser.test.util.CriteriaHelper;
......@@ -43,7 +42,6 @@ public class ThumbnailDiskStorageTest {
private static final Bitmap BITMAP2 = BitmapFactory.decodeFile(FILE_PATH2);
private static final int ICON_WIDTH1 = 50;
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 INTERVAL_MS = 500;
......@@ -100,9 +98,9 @@ public class ThumbnailDiskStorageTest {
// Accessed by test and UI threads.
public final AtomicBoolean initialized = new AtomicBoolean();
public TestThumbnailDiskStorage(TestThumbnailStorageDelegate delegate,
TestThumbnailGenerator thumbnailGenerator, int maxCacheSizeBytes) {
super(delegate, thumbnailGenerator, maxCacheSizeBytes);
public TestThumbnailDiskStorage(
TestThumbnailStorageDelegate delegate, TestThumbnailGenerator thumbnailGenerator) {
super(delegate, thumbnailGenerator);
}
@Override
......@@ -161,9 +159,8 @@ public class ThumbnailDiskStorageTest {
ThreadUtils.runOnUiThreadBlocking(new Runnable() {
@Override
public void run() {
mTestThumbnailDiskStorage =
new TestThumbnailDiskStorage(mTestThumbnailStorageDelegate,
mTestThumbnailGenerator, TEST_MAX_CACHE_BYTES);
mTestThumbnailDiskStorage = new TestThumbnailDiskStorage(
mTestThumbnailStorageDelegate, mTestThumbnailGenerator);
// Clear the disk cache so that cached entries from previous runs won't show up.
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