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

Image Fetcher: Delete deprecated fetchImage() APIs.

Due to the introduction of ImageFetcher.Params, some old APIs can be
deleted. All plumbing for fetchImage() should use the Params now.

Bug: 1076515
Change-Id: I5c4cbda79c882a0b763fcad86b84625366c20018
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2192379Reviewed-by: default avatarDavid Trainor <dtrainor@chromium.org>
Reviewed-by: default avatarLuca Hunkeler <hluca@google.com>
Reviewed-by: default avatarFilip Gorski <fgorski@chromium.org>
Reviewed-by: default avatarBrandon Wylie <wylieb@chromium.org>
Commit-Queue: Xing Liu <xingliu@chromium.org>
Cr-Commit-Position: refs/heads/master@{#771214}
parent ed3b88d9
......@@ -175,8 +175,10 @@ class AssistantDetailsViewBinder
}
} else {
// Download image and then set it in the view.
mImageFetcher.fetchImage(details.getImageUrl(),
ImageFetcher.ASSISTANT_DETAILS_UMA_CLIENT_NAME, image -> {
ImageFetcher.Params params = ImageFetcher.Params.create(
details.getImageUrl(), ImageFetcher.ASSISTANT_DETAILS_UMA_CLIENT_NAME);
mImageFetcher.fetchImage(
params, image -> {
if (image != null) {
viewHolder.mImageView.setImageDrawable(getRoundedImage(image));
if (details.hasImageClickthroughData()
......
......@@ -103,8 +103,10 @@ public abstract class AssistantDrawable {
@Override
public void getDrawable(Context context, Callback<Drawable> callback) {
// TODO(b/143517837) Merge autofill assistant image fetcher UMA names.
ImageFetcher.Params params = ImageFetcher.Params.create(
mUrl, ImageFetcher.ASSISTANT_DETAILS_UMA_CLIENT_NAME);
mImageFetcher.fetchImage(
mUrl, ImageFetcher.ASSISTANT_DETAILS_UMA_CLIENT_NAME, result -> {
params, result -> {
if (result != null) {
callback.onResult(new BitmapDrawable(context.getResources(),
Bitmap.createScaledBitmap(
......
......@@ -75,14 +75,15 @@ class AssistantInfoBoxViewBinder
if (infoBox.getImagePath().isEmpty()) {
viewHolder.mExplanationView.setCompoundDrawablesWithIntrinsicBounds(0, 0, 0, 0);
} else {
mImageFetcher.fetchImage(infoBox.getImagePath(),
ImageFetcher.ASSISTANT_INFO_BOX_UMA_CLIENT_NAME, image -> {
if (image != null) {
Drawable d = new BitmapDrawable(mContext.getResources(), image);
viewHolder.mExplanationView.setCompoundDrawablesWithIntrinsicBounds(
null, d, null, null);
}
});
ImageFetcher.Params params = ImageFetcher.Params.create(
infoBox.getImagePath(), ImageFetcher.ASSISTANT_INFO_BOX_UMA_CLIENT_NAME);
mImageFetcher.fetchImage(params, image -> {
if (image != null) {
Drawable d = new BitmapDrawable(mContext.getResources(), image);
viewHolder.mExplanationView.setCompoundDrawablesWithIntrinsicBounds(
null, d, null, null);
}
});
}
}
}
......@@ -89,8 +89,10 @@ public class AssistantOverlayCoordinator {
if (image != null && !TextUtils.isEmpty(image.mImageUrl)) {
DisplayMetrics displayMetrics = context.getResources().getDisplayMetrics();
// TODO(b/143517837) Merge autofill assistant image fetcher UMA names.
mImageFetcher.fetchImage(image.mImageUrl,
ImageFetcher.ASSISTANT_DETAILS_UMA_CLIENT_NAME, result -> {
ImageFetcher.Params params = ImageFetcher.Params.create(
image.mImageUrl, ImageFetcher.ASSISTANT_DETAILS_UMA_CLIENT_NAME);
mImageFetcher.fetchImage(
params, result -> {
image.mImageBitmap = result != null ? Bitmap.createScaledBitmap(
result, image.mImageSizeInPixels,
image.mImageSizeInPixels, true)
......
......@@ -89,12 +89,6 @@ class AutofillAssistantUiTestUtil {
callback.onResult(mGifToFetch);
}
@Override
public void fetchImage(
String url, String clientName, int width, int height, Callback<Bitmap> callback) {
callback.onResult(mBitmapToFetch);
}
@Override
public void fetchImage(Params params, Callback<Bitmap> callback) {
callback.onResult(mBitmapToFetch);
......
......@@ -179,7 +179,9 @@ public class FeedImageLoader implements ImageLoaderApi {
@VisibleForTesting
protected void fetchImage(String url, int width, int height, Callback<Bitmap> callback) {
mImageFetcher.fetchImage(url, ImageFetcher.FEED_UMA_CLIENT_NAME, width, height, callback);
ImageFetcher.Params params =
ImageFetcher.Params.create(url, ImageFetcher.FEED_UMA_CLIENT_NAME, width, height);
mImageFetcher.fetchImage(params, callback);
}
@VisibleForTesting
......
......@@ -114,9 +114,10 @@ public class AnswerSuggestionProcessor extends BaseSuggestionViewProcessor {
List<PropertyModel> models = new ArrayList<>();
models.add(model);
mPendingAnswerRequestUrls.put(url, models);
ImageFetcher.Params params =
ImageFetcher.Params.create(url, ImageFetcher.ANSWER_SUGGESTIONS_UMA_CLIENT_NAME);
imageFetcher.fetchImage(
url, ImageFetcher.ANSWER_SUGGESTIONS_UMA_CLIENT_NAME, (Bitmap bitmap) -> {
params, (Bitmap bitmap) -> {
ThreadUtils.assertOnUiThread();
// Remove models for the URL ahead of all the checks to ensure we
// do not keep them around waiting in case image fetch failed.
......
......@@ -119,8 +119,11 @@ public class EntitySuggestionProcessor extends BaseSuggestionViewProcessor {
models.add(model);
mPendingImageRequests.put(url, models);
imageFetcher.fetchImage(url.getSpec(), ImageFetcher.ENTITY_SUGGESTIONS_UMA_CLIENT_NAME,
mEntityImageSizePx, mEntityImageSizePx, (Bitmap bitmap) -> {
ImageFetcher.Params params = ImageFetcher.Params.create(url.getSpec(),
ImageFetcher.ENTITY_SUGGESTIONS_UMA_CLIENT_NAME, mEntityImageSizePx,
mEntityImageSizePx);
imageFetcher.fetchImage(
params, (Bitmap bitmap) -> {
ThreadUtils.assertOnUiThread();
final List<PropertyModel> pendingModels = mPendingImageRequests.remove(url);
......
......@@ -50,10 +50,6 @@ public class ImageFetcherTest {
@Override
public void fetchGif(String url, String clientName, Callback<BaseGifImage> callback) {}
@Override
public void fetchImage(
String url, String clientName, int width, int height, Callback<Bitmap> callback) {}
@Override
public void fetchImage(Params params, Callback<Bitmap> callback) {}
......@@ -112,7 +108,7 @@ public class ImageFetcherTest {
@Test
public void testFetchImageNoDimensionsAlias() {
mImageFetcher.fetchImage(URL, CLIENT_NAME, result -> {});
mImageFetcher.fetchImage(ImageFetcher.Params.create(URL, CLIENT_NAME), result -> {});
// No arguments should alias to 0, 0.
verify(mImageFetcher)
......
......@@ -149,8 +149,9 @@ public class InMemoryCachedImageFetcherTest {
answerFetch(mBitmap, true);
// No exception should be thrown here when bitmap cache is null.
mInMemoryCachedImageFetcher.fetchImage(
URL, UMA_CLIENT_NAME, WIDTH_PX, HEIGHT_PX, (Bitmap bitmap) -> {});
ImageFetcher.Params params =
ImageFetcher.Params.create(URL, UMA_CLIENT_NAME, WIDTH_PX, HEIGHT_PX);
mInMemoryCachedImageFetcher.fetchImage(params, (Bitmap bitmap) -> {});
} catch (Exception e) {
Assert.fail("Destroy called in the middle of execution shouldn't throw");
}
......@@ -183,7 +184,7 @@ public class InMemoryCachedImageFetcherTest {
mExpectedException.expectMessage("fetchGif called after destroy");
mInMemoryCachedImageFetcher.fetchGif("", "", null);
mExpectedException.expectMessage("fetchImage called after destroy");
mInMemoryCachedImageFetcher.fetchImage("", "", 100, 100, null);
mInMemoryCachedImageFetcher.fetchImage(ImageFetcher.Params.create("", "", 100, 100), null);
mExpectedException.expectMessage("clear called after destroy");
mInMemoryCachedImageFetcher.clear();
}
......
......@@ -5,9 +5,8 @@
package org.chromium.chrome.browser.omnibox.suggestions.answer;
import static org.hamcrest.core.IsInstanceOf.instanceOf;
import static org.mockito.ArgumentMatchers.eq;
import static org.mockito.Mockito.any;
import static org.mockito.Mockito.anyString;
import static org.mockito.Mockito.eq;
import static org.mockito.Mockito.times;
import static org.mockito.Mockito.verify;
import static org.mockito.Mockito.when;
......@@ -191,6 +190,10 @@ public class AnswerSuggestionProcessorUnitTest {
when(mUrlStateProvider.getTextWithoutAutocomplete()).thenReturn(null);
}
ImageFetcher.Params createParams(String url) {
return ImageFetcher.Params.create(url, ImageFetcher.ANSWER_SUGGESTIONS_UMA_CLIENT_NAME);
}
@CalledByNativeJavaTest
public void regularAnswer_order() {
final SuggestionTestHelper suggHelper =
......@@ -338,15 +341,13 @@ public class AnswerSuggestionProcessorUnitTest {
processSuggestion(sugg3);
processSuggestion(sugg4);
verify(mImageFetcher, times(1)).fetchImage(eq(url1), anyString(), any());
verify(mImageFetcher, times(1)).fetchImage(eq(url2), anyString(), any());
verify(mImageFetcher, times(2)).fetchImage(anyString(), anyString(), any());
verify(mImageFetcher).fetchImage(eq(createParams(url1)), any());
verify(mImageFetcher).fetchImage(eq(createParams(url2)), any());
}
@CalledByNativeJavaTest
public void answerImage_bitmapReplacesIconForAllSuggestionsWithSameUrl() {
final String url = "http://site.com";
final ArgumentCaptor<Callback<Bitmap>> callback = ArgumentCaptor.forClass(Callback.class);
final SuggestionTestHelper sugg1 =
createAnswerSuggestion(AnswerType.WEATHER, "", 1, "", 1, url);
final SuggestionTestHelper sugg2 =
......@@ -358,7 +359,8 @@ public class AnswerSuggestionProcessorUnitTest {
processSuggestion(sugg2);
processSuggestion(sugg3);
verify(mImageFetcher).fetchImage(eq(url), anyString(), callback.capture());
final ArgumentCaptor<Callback<Bitmap>> callback = ArgumentCaptor.forClass(Callback.class);
verify(mImageFetcher).fetchImage(eq(createParams(url)), callback.capture());
final Drawable icon1 = sugg1.getIcon();
final Drawable icon2 = sugg2.getIcon();
......@@ -390,7 +392,7 @@ public class AnswerSuggestionProcessorUnitTest {
processSuggestion(suggHelper);
verify(mImageFetcher).fetchImage(eq(url), anyString(), callback.capture());
verify(mImageFetcher).fetchImage(eq(createParams(url)), callback.capture());
final Drawable icon = suggHelper.getIcon();
callback.getValue().onResult(null);
......@@ -422,7 +424,7 @@ public class AnswerSuggestionProcessorUnitTest {
processSuggestion(sugg1);
processSuggestion(sugg2);
verify(mImageFetcher, times(1)).fetchImage(eq(url), anyString(), callback.capture());
verify(mImageFetcher, times(1)).fetchImage(eq(createParams(url)), callback.capture());
final Drawable icon1 = sugg1.getIcon();
final Drawable icon2 = sugg2.getIcon();
......
......@@ -5,11 +5,8 @@
package org.chromium.chrome.browser.omnibox.suggestions.entity;
import static org.hamcrest.core.IsInstanceOf.instanceOf;
import static org.mockito.ArgumentMatchers.eq;
import static org.mockito.Mockito.any;
import static org.mockito.Mockito.anyInt;
import static org.mockito.Mockito.anyString;
import static org.mockito.Mockito.eq;
import static org.mockito.Mockito.times;
import static org.mockito.Mockito.verify;
import android.graphics.Bitmap;
......@@ -30,9 +27,9 @@ import org.chromium.base.CommandLine;
import org.chromium.base.ContextUtils;
import org.chromium.base.annotations.CalledByNative;
import org.chromium.base.annotations.CalledByNativeJavaTest;
import org.chromium.chrome.R;
import org.chromium.chrome.browser.image_fetcher.ImageFetcher;
import org.chromium.chrome.browser.omnibox.OmniboxSuggestionType;
import org.chromium.chrome.browser.omnibox.UrlBarEditingTextStateProvider;
import org.chromium.chrome.browser.omnibox.suggestions.OmniboxSuggestion;
import org.chromium.chrome.browser.omnibox.suggestions.OmniboxSuggestionBuilderForTest;
import org.chromium.chrome.browser.omnibox.suggestions.base.BaseSuggestionViewProperties;
......@@ -55,9 +52,6 @@ public class EntitySuggestionProcessorUnitTest {
@Mock
SuggestionHost mSuggestionHost;
@Mock
UrlBarEditingTextStateProvider mUrlStateProvider;
@Mock
ImageFetcher mImageFetcher;
......@@ -133,6 +127,13 @@ public class EntitySuggestionProcessorUnitTest {
CommandLine.reset();
}
ImageFetcher.Params createParams(String url) {
int size = ContextUtils.getApplicationContext().getResources().getDimensionPixelSize(
R.dimen.omnibox_suggestion_entity_icon_size);
return ImageFetcher.Params.create(
url, ImageFetcher.ENTITY_SUGGESTIONS_UMA_CLIENT_NAME, size, size);
}
@CalledByNativeJavaTest
public void contentTest_basicContent() {
SuggestionTestHelper suggHelper =
......@@ -199,13 +200,12 @@ public class EntitySuggestionProcessorUnitTest {
@CalledByNativeJavaTest
public void decorationTest_basicSuccessfulBitmapFetch() {
final GURL url = new GURL("http://site.com");
final ArgumentCaptor<Callback<Bitmap>> callback = ArgumentCaptor.forClass(Callback.class);
SuggestionTestHelper suggHelper = createSuggestion("", "", "red", url);
processSuggestion(suggHelper);
verify(mImageFetcher)
.fetchImage(eq(url.getSpec()), anyString(), anyInt(), anyInt(), callback.capture());
final ArgumentCaptor<Callback<Bitmap>> callback = ArgumentCaptor.forClass(Callback.class);
verify(mImageFetcher).fetchImage(eq(createParams(url.getSpec())), callback.capture());
suggHelper.verifyReportedType(DECORATION_TYPE_COLOR);
Assert.assertThat(suggHelper.getIcon(), instanceOf(ColorDrawable.class));
callback.getValue().onResult(mBitmap);
......@@ -228,18 +228,13 @@ public class EntitySuggestionProcessorUnitTest {
processSuggestion(sugg3);
processSuggestion(sugg4);
verify(mImageFetcher)
.fetchImage(eq(url1.getSpec()), anyString(), anyInt(), anyInt(), any());
verify(mImageFetcher)
.fetchImage(eq(url2.getSpec()), anyString(), anyInt(), anyInt(), any());
verify(mImageFetcher, times(2))
.fetchImage(anyString(), anyString(), anyInt(), anyInt(), any());
verify(mImageFetcher).fetchImage(eq(createParams(url1.getSpec())), any());
verify(mImageFetcher).fetchImage(eq(createParams(url2.getSpec())), any());
}
@CalledByNativeJavaTest
public void decorationTest_bitmapReplacesIconForAllSuggestionsWithSameUrl() {
final GURL url = new GURL("http://site.com");
final ArgumentCaptor<Callback<Bitmap>> callback = ArgumentCaptor.forClass(Callback.class);
final SuggestionTestHelper sugg1 = createSuggestion("", "", "", url);
final SuggestionTestHelper sugg2 = createSuggestion("", "", "", url);
final SuggestionTestHelper sugg3 = createSuggestion("", "", "", url);
......@@ -248,8 +243,8 @@ public class EntitySuggestionProcessorUnitTest {
processSuggestion(sugg2);
processSuggestion(sugg3);
verify(mImageFetcher)
.fetchImage(eq(url.getSpec()), anyString(), anyInt(), anyInt(), callback.capture());
final ArgumentCaptor<Callback<Bitmap>> callback = ArgumentCaptor.forClass(Callback.class);
verify(mImageFetcher).fetchImage(eq(createParams(url.getSpec())), callback.capture());
final Drawable icon1 = sugg1.getIcon();
final Drawable icon2 = sugg2.getIcon();
......@@ -282,12 +277,11 @@ public class EntitySuggestionProcessorUnitTest {
@CalledByNativeJavaTest
public void decorationTest_failedBitmapFetchDoesNotReplaceIcon() {
final GURL url = new GURL("http://site.com");
final ArgumentCaptor<Callback<Bitmap>> callback = ArgumentCaptor.forClass(Callback.class);
final SuggestionTestHelper suggHelper = createSuggestion("", "", null, url);
processSuggestion(suggHelper);
verify(mImageFetcher)
.fetchImage(eq(url.getSpec()), anyString(), anyInt(), anyInt(), callback.capture());
final ArgumentCaptor<Callback<Bitmap>> callback = ArgumentCaptor.forClass(Callback.class);
verify(mImageFetcher).fetchImage(eq(createParams(url.getSpec())), callback.capture());
final Drawable oldIcon = suggHelper.getIcon();
callback.getValue().onResult(null);
......@@ -301,12 +295,11 @@ public class EntitySuggestionProcessorUnitTest {
@CalledByNativeJavaTest
public void decorationTest_failedBitmapFetchDoesNotReplaceColor() {
final GURL url = new GURL("http://site.com");
final ArgumentCaptor<Callback<Bitmap>> callback = ArgumentCaptor.forClass(Callback.class);
final SuggestionTestHelper suggHelper = createSuggestion("", "", "red", url);
processSuggestion(suggHelper);
verify(mImageFetcher)
.fetchImage(eq(url.getSpec()), anyString(), anyInt(), anyInt(), callback.capture());
final ArgumentCaptor<Callback<Bitmap>> callback = ArgumentCaptor.forClass(Callback.class);
verify(mImageFetcher).fetchImage(eq(createParams(url.getSpec())), callback.capture());
final Drawable oldIcon = suggHelper.getIcon();
callback.getValue().onResult(null);
......@@ -319,18 +312,16 @@ public class EntitySuggestionProcessorUnitTest {
@CalledByNativeJavaTest
public void decorationTest_updatedModelsAreRemovedFromPendingRequestsList() {
ArgumentCaptor<Callback<Bitmap>> callback = ArgumentCaptor.forClass(Callback.class);
final GURL url = new GURL("http://site1.com");
final SuggestionTestHelper sugg1 = createSuggestion("", "", "", url);
final SuggestionTestHelper sugg2 = createSuggestion("", "", "", url);
processSuggestion(sugg1);
processSuggestion(sugg2);
verify(mImageFetcher)
.fetchImage(eq(url.getSpec()), anyString(), anyInt(), anyInt(), callback.capture());
verify(mImageFetcher).fetchImage(anyString(), anyString(), anyInt(), anyInt(), any());
final ArgumentCaptor<Callback<Bitmap>> callback = ArgumentCaptor.forClass(Callback.class);
verify(mImageFetcher).fetchImage(eq(createParams(url.getSpec())), callback.capture());
verify(mImageFetcher).fetchImage(any(), any());
final Drawable icon1 = sugg1.getIcon();
final Drawable icon2 = sugg2.getIcon();
......
......@@ -116,15 +116,6 @@ public class CachedImageFetcher extends ImageFetcher {
}
}
/**
* Tries to load the gif from disk, if not it falls back to the bridge.
*/
@Override
public void fetchImage(
String url, String clientName, int width, int height, Callback<Bitmap> callback) {
fetchImage(ImageFetcher.Params.create(url, clientName, width, height), callback);
}
@Override
public void fetchImage(final Params params, Callback<Bitmap> callback) {
long startTimeMillis = System.currentTimeMillis();
......
......@@ -200,33 +200,6 @@ public abstract class ImageFetcher {
*/
public abstract void fetchGif(String url, String clientName, Callback<BaseGifImage> callback);
/**
* Fetches the image at url with the desired size. Image is null if not found or fails decoding.
*
* @param url The url to fetch the image from.
* @param clientName Name of the cached image fetcher client to report UMA metrics for.
* @param width The new bitmap's desired width (in pixels). If the given value is <= 0, the
* image won't be scaled.
* @param height The new bitmap's desired height (in pixels). If the given value is <= 0, the
* image won't be scaled.
* @param callback The function which will be called when the image is ready; will be called
* with null result if fetching fails;
*/
public abstract void fetchImage(
String url, String clientName, int width, int height, Callback<Bitmap> callback);
/**
* Alias of fetchImage that ignores scaling.
*
* @param url The url to fetch the image from.
* @param clientName Name of the cached image fetcher client to report UMA metrics for.
* @param callback The function which will be called when the image is ready; will be called
* with null result if fetching fails;
*/
public void fetchImage(String url, String clientName, Callback<Bitmap> callback) {
fetchImage(ImageFetcher.Params.create(url, clientName), callback);
}
/**
* Fetches the image based on customized parameters specified.
*
......
......@@ -91,12 +91,6 @@ public class InMemoryCachedImageFetcher extends ImageFetcher {
mImageFetcher.fetchGif(url, clientName, callback);
}
@Override
public void fetchImage(
String url, String clientName, int width, int height, Callback<Bitmap> callback) {
fetchImage(ImageFetcher.Params.create(url, clientName, width, height), callback);
}
@Override
public void fetchImage(final Params params, Callback<Bitmap> callback) {
assert mBitmapCache != null && mImageFetcher != null : "fetchImage called after destroy";
......
......@@ -33,12 +33,6 @@ public class NetworkImageFetcher extends ImageFetcher {
getImageFetcherBridge().fetchGif(getConfig(), url, clientName, callback);
}
@Override
public void fetchImage(
String url, String clientName, int width, int height, Callback<Bitmap> callback) {
fetchImage(ImageFetcher.Params.create(url, clientName, width, height), callback);
}
@Override
public void fetchImage(final Params params, Callback<Bitmap> callback) {
long startTimeMillis = System.currentTimeMillis();
......
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