Skip to content

GitLab

T
tangled
Commit d9031fa1 authored by zqzhang's avatar zqzhang Committed by Commit bot
Browse files
Options

[MediaNotification] Avoid fetching image twice when artwork doesn't change 

When switching src or modifying metadata, the whole metadata will be
sent to MediaSessionTabHelper causing it to download the same image src
twice even if the artwork stays the same. This CL make MediaImageManager
to remember the src of the last downloaded image so that it can avoid
fetch the same src.

BUG=672023

Review-Url: https://codereview.chromium.org/2627103002
Cr-Commit-Position: refs/heads/master@{#443256}
parent e17314a0
Hide whitespace changes
Inline Side-by-side
Showing with 105 additions and 19 deletions
chrome/android/java/src/org/chromium/chrome/browser/media/ui/MediaImageManager.java
View file @ d9031fa1
...@@ -77,6 +77,12 @@ public class MediaImageManager implements ImageDownloadCallback { ...@@ -77,6 +77,12 @@ public class MediaImageManager implements ImageDownloadCallback {
// The callback to be called when the pending download image request completes. // The callback to be called when the pending download image request completes.
private MediaImageCallback mCallback; private MediaImageCallback mCallback;
// The last image src for download, used for avoiding fetching the same src when artwork is set
// multiple times but the same src is chosen.
//
// Will be reset when initiating a new download request, and set to |null| when download failed.
private String mLastImageSrc;
/** /**
* MediaImageManager constructor. * MediaImageManager constructor.
* @param minimumSize The minimum size of images to download. * @param minimumSize The minimum size of images to download.
...@@ -128,11 +134,14 @@ public class MediaImageManager implements ImageDownloadCallback { ...@@ -128,11 +134,14 @@ public class MediaImageManager implements ImageDownloadCallback {
mCallback = callback; mCallback = callback;
MediaImage image = selectImage(images); MediaImage image = selectImage(images);
if (image == null) { if (image == null) {
mCallback.onImageDownloaded(null); onDownloadFailed();
clearRequests();
return; return;
} }
// Avoid fetching the same image twice.
if (TextUtils.equals(image.getSrc(), mLastImageSrc)) return;
mLastImageSrc = image.getSrc();
// Limit |maxBitmapSize| to |MAX_BITMAP_SIZE_FOR_DOWNLOAD| to avoid passing huge bitmaps // Limit |maxBitmapSize| to |MAX_BITMAP_SIZE_FOR_DOWNLOAD| to avoid passing huge bitmaps
// through JNI. |maxBitmapSize| does not prevent huge images to be downloaded. It is used to // through JNI. |maxBitmapSize| does not prevent huge images to be downloaded. It is used to
// filter/rescale the download images. See documentation of // filter/rescale the download images. See documentation of
...@@ -169,8 +178,12 @@ public class MediaImageManager implements ImageDownloadCallback { ...@@ -169,8 +178,12 @@ public class MediaImageManager implements ImageDownloadCallback {
bestScore = newScore; bestScore = newScore;
} }
} }
mCallback.onImageDownloaded(bestBitmap); if (bestBitmap != null) {
clearRequests(); mCallback.onImageDownloaded(bestBitmap);
clearRequests();
} else {
onDownloadFailed();
}
} }
/** /**
...@@ -197,6 +210,12 @@ public class MediaImageManager implements ImageDownloadCallback { ...@@ -197,6 +210,12 @@ public class MediaImageManager implements ImageDownloadCallback {
mCallback = null; mCallback = null;
} }
private void onDownloadFailed() {
mLastImageSrc = null;
mCallback.onImageDownloaded(null);
clearRequests();
}
private double getImageScore(MediaImage image) { private double getImageScore(MediaImage image) {
if (image == null) return 0; if (image == null) return 0;
if (image.getSizes().isEmpty()) return DEFAULT_IMAGE_SIZE_SCORE; if (image.getSizes().isEmpty()) return DEFAULT_IMAGE_SIZE_SCORE;
......
chrome/android/junit/src/org/chromium/chrome/browser/media/ui/MediaImageManagerTest.java
View file @ d9031fa1
...@@ -44,7 +44,8 @@ public class MediaImageManagerTest { ...@@ -44,7 +44,8 @@ public class MediaImageManagerTest {
private static final int IDEAL_IMAGE_SIZE_PX = 200; private static final int IDEAL_IMAGE_SIZE_PX = 200;
private static final int REQUEST_ID_1 = 1; private static final int REQUEST_ID_1 = 1;
private static final int REQUEST_ID_2 = 2; private static final int REQUEST_ID_2 = 2;
private static final String IMAGE_URL = "http://example.com/foo.png"; private static final String IMAGE_URL_1 = "http://example.com/foo.png";
private static final String IMAGE_URL_2 = "http://example.com/bar.png";
@Mock @Mock
private WebContents mWebContents; private WebContents mWebContents;
...@@ -70,7 +71,7 @@ public class MediaImageManagerTest { ...@@ -70,7 +71,7 @@ public class MediaImageManagerTest {
mMediaImageManager.setWebContents(mWebContents); mMediaImageManager.setWebContents(mWebContents);
mImages = new ArrayList<MediaImage>(); mImages = new ArrayList<MediaImage>();
mImages.add(new MediaImage(IMAGE_URL, "", new ArrayList<Rect>())); mImages.add(new MediaImage(IMAGE_URL_1, "", new ArrayList<Rect>()));
mBitmaps = new ArrayList<Bitmap>(); mBitmaps = new ArrayList<Bitmap>();
mBitmaps.add(Bitmap.createBitmap( mBitmaps.add(Bitmap.createBitmap(
...@@ -84,22 +85,22 @@ public class MediaImageManagerTest { ...@@ -84,22 +85,22 @@ public class MediaImageManagerTest {
public void testDownloadImage() { public void testDownloadImage() {
mMediaImageManager.downloadImage(mImages, mCallback); mMediaImageManager.downloadImage(mImages, mCallback);
verify(mWebContents) verify(mWebContents)
.downloadImage(eq(IMAGE_URL), eq(false), .downloadImage(eq(IMAGE_URL_1), eq(false),
eq(MediaImageManager.MAX_BITMAP_SIZE_FOR_DOWNLOAD), eq(false), eq(MediaImageManager.MAX_BITMAP_SIZE_FOR_DOWNLOAD), eq(false),
eq(mMediaImageManager)); eq(mMediaImageManager));
mMediaImageManager.onFinishDownloadImage( mMediaImageManager.onFinishDownloadImage(
REQUEST_ID_1, 200, IMAGE_URL, mBitmaps, mOriginalImageSizes); REQUEST_ID_1, 200, IMAGE_URL_1, mBitmaps, mOriginalImageSizes);
verify(mCallback).onImageDownloaded(isNotNull(Bitmap.class)); verify(mCallback).onImageDownloaded(isNotNull(Bitmap.class));
verify(mCallback, times(0)).onImageDownloaded(isNull(Bitmap.class)); verify(mCallback, times(0)).onImageDownloaded(isNull(Bitmap.class));
} }
@Test @Test
public void testDownloadImageTwice() { public void testDownloadSameImageTwice() {
// First download. // First download.
mMediaImageManager.downloadImage(mImages, mCallback); mMediaImageManager.downloadImage(mImages, mCallback);
mMediaImageManager.onFinishDownloadImage( mMediaImageManager.onFinishDownloadImage(
REQUEST_ID_1, 200, IMAGE_URL, mBitmaps, mOriginalImageSizes); REQUEST_ID_1, 200, IMAGE_URL_1, mBitmaps, mOriginalImageSizes);
// Second download. // Second download.
doReturn(REQUEST_ID_2) doReturn(REQUEST_ID_2)
...@@ -108,23 +109,89 @@ public class MediaImageManagerTest { ...@@ -108,23 +109,89 @@ public class MediaImageManagerTest {
any(MediaImageManager.class)); any(MediaImageManager.class));
mMediaImageManager.downloadImage(mImages, mCallback); mMediaImageManager.downloadImage(mImages, mCallback);
mMediaImageManager.onFinishDownloadImage( mMediaImageManager.onFinishDownloadImage(
REQUEST_ID_2, 200, IMAGE_URL, mBitmaps, mOriginalImageSizes); REQUEST_ID_2, 200, IMAGE_URL_1, mBitmaps, mOriginalImageSizes);
verify(mWebContents, times(2)) verify(mWebContents, times(1))
.downloadImage(eq(IMAGE_URL), eq(false), .downloadImage(eq(IMAGE_URL_1), eq(false),
eq(MediaImageManager.MAX_BITMAP_SIZE_FOR_DOWNLOAD), eq(false),
eq(mMediaImageManager));
verify(mCallback, times(1)).onImageDownloaded(isNotNull(Bitmap.class));
verify(mCallback, times(0)).onImageDownloaded(isNull(Bitmap.class));
}
@Test
public void testDownloadDifferentImagesTwice() {
// First download.
mMediaImageManager.downloadImage(mImages, mCallback);
mMediaImageManager.onFinishDownloadImage(
REQUEST_ID_1, 200, IMAGE_URL_1, mBitmaps, mOriginalImageSizes);
// Second download.
doReturn(REQUEST_ID_2)
.when(mWebContents)
.downloadImage(anyString(), anyBoolean(), anyInt(), anyBoolean(),
any(MediaImageManager.class));
mImages.clear();
mImages.add(new MediaImage(IMAGE_URL_2, "", new ArrayList<Rect>()));
mMediaImageManager.downloadImage(mImages, mCallback);
mMediaImageManager.onFinishDownloadImage(
REQUEST_ID_2, 200, IMAGE_URL_2, mBitmaps, mOriginalImageSizes);
verify(mWebContents, times(1))
.downloadImage(eq(IMAGE_URL_1), eq(false),
eq(MediaImageManager.MAX_BITMAP_SIZE_FOR_DOWNLOAD), eq(false),
eq(mMediaImageManager));
verify(mWebContents, times(1))
.downloadImage(eq(IMAGE_URL_2), eq(false),
eq(MediaImageManager.MAX_BITMAP_SIZE_FOR_DOWNLOAD), eq(false), eq(MediaImageManager.MAX_BITMAP_SIZE_FOR_DOWNLOAD), eq(false),
eq(mMediaImageManager)); eq(mMediaImageManager));
verify(mCallback, times(2)).onImageDownloaded(isNotNull(Bitmap.class)); verify(mCallback, times(2)).onImageDownloaded(isNotNull(Bitmap.class));
verify(mCallback, times(0)).onImageDownloaded(isNull(Bitmap.class)); verify(mCallback, times(0)).onImageDownloaded(isNull(Bitmap.class));
} }
@Test
public void testDownloadAnotherImageBeforeResponse() {
// First download.
mMediaImageManager.downloadImage(mImages, mCallback);
// Second download.
doReturn(REQUEST_ID_2)
.when(mWebContents)
.downloadImage(anyString(), anyBoolean(), anyInt(), anyBoolean(),
any(MediaImageManager.class));
mImages.clear();
mImages.add(new MediaImage(IMAGE_URL_2, "", new ArrayList<Rect>()));
mMediaImageManager.downloadImage(mImages, mCallback);
mMediaImageManager.onFinishDownloadImage(
REQUEST_ID_2, 200, IMAGE_URL_2, mBitmaps, mOriginalImageSizes);
// This reply should not be sent to the client.
mMediaImageManager.onFinishDownloadImage(
REQUEST_ID_1, 200, IMAGE_URL_1, mBitmaps, mOriginalImageSizes);
verify(mWebContents, times(1))
.downloadImage(eq(IMAGE_URL_1), eq(false),
eq(MediaImageManager.MAX_BITMAP_SIZE_FOR_DOWNLOAD), eq(false),
eq(mMediaImageManager));
verify(mWebContents, times(1))
.downloadImage(eq(IMAGE_URL_2), eq(false),
eq(MediaImageManager.MAX_BITMAP_SIZE_FOR_DOWNLOAD), eq(false),
eq(mMediaImageManager));
verify(mCallback, times(1)).onImageDownloaded(isNotNull(Bitmap.class));
verify(mCallback, times(0)).onImageDownloaded(isNull(Bitmap.class));
}
@Test @Test
public void testDuplicateResponce() { public void testDuplicateResponce() {
mMediaImageManager.downloadImage(mImages, mCallback); mMediaImageManager.downloadImage(mImages, mCallback);
mMediaImageManager.onFinishDownloadImage( mMediaImageManager.onFinishDownloadImage(
REQUEST_ID_1, 200, IMAGE_URL, mBitmaps, mOriginalImageSizes); REQUEST_ID_1, 200, IMAGE_URL_1, mBitmaps, mOriginalImageSizes);
mMediaImageManager.onFinishDownloadImage( mMediaImageManager.onFinishDownloadImage(
REQUEST_ID_1, 200, IMAGE_URL, mBitmaps, mOriginalImageSizes); REQUEST_ID_1, 200, IMAGE_URL_1, mBitmaps, mOriginalImageSizes);
verify(mCallback, times(1)).onImageDownloaded(isNotNull(Bitmap.class)); verify(mCallback, times(1)).onImageDownloaded(isNotNull(Bitmap.class));
verify(mCallback, times(0)).onImageDownloaded(isNull(Bitmap.class)); verify(mCallback, times(0)).onImageDownloaded(isNull(Bitmap.class));
...@@ -134,7 +201,7 @@ public class MediaImageManagerTest { ...@@ -134,7 +201,7 @@ public class MediaImageManagerTest {
public void testWrongResponceId() { public void testWrongResponceId() {
mMediaImageManager.downloadImage(mImages, mCallback); mMediaImageManager.downloadImage(mImages, mCallback);
mMediaImageManager.onFinishDownloadImage( mMediaImageManager.onFinishDownloadImage(
REQUEST_ID_2, 200, IMAGE_URL, mBitmaps, mOriginalImageSizes); REQUEST_ID_2, 200, IMAGE_URL_1, mBitmaps, mOriginalImageSizes);
verify(mCallback, times(0)).onImageDownloaded(isNotNull(Bitmap.class)); verify(mCallback, times(0)).onImageDownloaded(isNotNull(Bitmap.class));
verify(mCallback, times(0)).onImageDownloaded(isNull(Bitmap.class)); verify(mCallback, times(0)).onImageDownloaded(isNull(Bitmap.class));
...@@ -145,7 +212,7 @@ public class MediaImageManagerTest { ...@@ -145,7 +212,7 @@ public class MediaImageManagerTest {
mImages.clear(); mImages.clear();
ArrayList<Rect> sizes = new ArrayList<Rect>(); ArrayList<Rect> sizes = new ArrayList<Rect>();
sizes.add(new Rect(0, 0, TINY_IMAGE_SIZE_PX, TINY_IMAGE_SIZE_PX)); sizes.add(new Rect(0, 0, TINY_IMAGE_SIZE_PX, TINY_IMAGE_SIZE_PX));
mImages.add(new MediaImage(IMAGE_URL, "", sizes)); mImages.add(new MediaImage(IMAGE_URL_1, "", sizes));
mMediaImageManager.downloadImage(mImages, mCallback); mMediaImageManager.downloadImage(mImages, mCallback);
verify(mWebContents, times(0)) verify(mWebContents, times(0))
...@@ -167,7 +234,7 @@ public class MediaImageManagerTest { ...@@ -167,7 +234,7 @@ public class MediaImageManagerTest {
mOriginalImageSizes.add(new Rect(0, 0, TINY_IMAGE_SIZE_PX, TINY_IMAGE_SIZE_PX)); mOriginalImageSizes.add(new Rect(0, 0, TINY_IMAGE_SIZE_PX, TINY_IMAGE_SIZE_PX));
mMediaImageManager.onFinishDownloadImage( mMediaImageManager.onFinishDownloadImage(
REQUEST_ID_1, 200, IMAGE_URL, mBitmaps, mOriginalImageSizes); REQUEST_ID_1, 200, IMAGE_URL_1, mBitmaps, mOriginalImageSizes);
verify(mCallback).onImageDownloaded(isNull(Bitmap.class)); verify(mCallback).onImageDownloaded(isNull(Bitmap.class));
verify(mCallback, times(0)).onImageDownloaded(isNotNull(Bitmap.class)); verify(mCallback, times(0)).onImageDownloaded(isNotNull(Bitmap.class));
...@@ -177,7 +244,7 @@ public class MediaImageManagerTest { ...@@ -177,7 +244,7 @@ public class MediaImageManagerTest {
public void testDownloadImageFails() { public void testDownloadImageFails() {
mMediaImageManager.downloadImage(mImages, mCallback); mMediaImageManager.downloadImage(mImages, mCallback);
mMediaImageManager.onFinishDownloadImage( mMediaImageManager.onFinishDownloadImage(
REQUEST_ID_1, 404, IMAGE_URL, new ArrayList<Bitmap>(), new ArrayList<Rect>()); REQUEST_ID_1, 404, IMAGE_URL_1, new ArrayList<Bitmap>(), new ArrayList<Rect>());
verify(mCallback).onImageDownloaded(isNull(Bitmap.class)); verify(mCallback).onImageDownloaded(isNull(Bitmap.class));
verify(mCallback, times(0)).onImageDownloaded(isNotNull(Bitmap.class)); verify(mCallback, times(0)).onImageDownloaded(isNotNull(Bitmap.class));
......
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