Commit c04cd957 authored by Ted Choc's avatar Ted Choc Committed by Commit Bot

Move Omnibox Answers image cache to java.

The C++ image cache requires making a new java bitmap
for every cache hit.  Since the suggestions are rendered
in java widgets, it makes sense to cache purely in java.

BUG=883410

Change-Id: I66d5f1df458e1122e5a0d191ce67c2bf3a7160ab
Reviewed-on: https://chromium-review.googlesource.com/c/1300753
Commit-Queue: Ted Choc <tedchoc@chromium.org>
Reviewed-by: default avatarMatthew Jones <mdjones@chromium.org>
Reviewed-by: default avatarRachel Blum <groby@chromium.org>
Cr-Commit-Position: refs/heads/master@{#603129}
parent cbcfb3be
...@@ -5,14 +5,20 @@ ...@@ -5,14 +5,20 @@
package org.chromium.chrome.browser.omnibox.suggestions; package org.chromium.chrome.browser.omnibox.suggestions;
import android.graphics.Bitmap; import android.graphics.Bitmap;
import android.support.v4.util.LruCache;
import org.chromium.base.annotations.CalledByNative; import org.chromium.base.annotations.CalledByNative;
import org.chromium.chrome.browser.profiles.Profile; import org.chromium.chrome.browser.profiles.Profile;
import org.chromium.chrome.browser.util.ConversionUtils;
/** /**
* Provides access to images used by Answers in Suggest. * Provides access to images used by Answers in Suggest.
*/ */
public class AnswersImage { public class AnswersImageFetcher {
// Matches the value in BitmapFetcherService.
private static final int INVALID_IMAGE_REQUEST_ID = 0;
private static final int MAX_CACHE_SIZE = 500 * ConversionUtils.BYTES_PER_KILOBYTE;
/** /**
* Observer for updating an image when it is available. * Observer for updating an image when it is available.
*/ */
...@@ -23,7 +29,25 @@ public class AnswersImage { ...@@ -23,7 +29,25 @@ public class AnswersImage {
* @param bitmap the image * @param bitmap the image
*/ */
@CalledByNative("AnswersImageObserver") @CalledByNative("AnswersImageObserver")
public void onAnswersImageChanged(Bitmap bitmap); void onAnswersImageChanged(Bitmap bitmap);
}
// Intentionally not using BitmapCache as that does not cache for low end devices (it ensures
// the bitmaps are de-dups across instances, but discards them if there is not an active
// reference to one).
private final LruCache<String, Bitmap> mBitmapCache =
new LruCache<String, Bitmap>(MAX_CACHE_SIZE) {
@Override
protected int sizeOf(String key, Bitmap value) {
return value.getByteCount();
}
};
/**
* Clears the cached answer images.
*/
public void clearCache() {
mBitmapCache.evictAll();
} }
/** /**
...@@ -34,9 +58,27 @@ public class AnswersImage { ...@@ -34,9 +58,27 @@ public class AnswersImage {
* strong reference to this. * strong reference to this.
* @return A request_id. * @return A request_id.
*/ */
public static int requestAnswersImage( public int requestAnswersImage(
Profile profile, String imageUrl, AnswersImageObserver observer) { Profile profile, String imageUrl, AnswersImageObserver observer) {
return nativeRequestAnswersImage(profile, imageUrl, observer); if (!profile.isOffTheRecord()) {
Bitmap bitmap = mBitmapCache.get(imageUrl);
if (bitmap != null) {
observer.onAnswersImageChanged(bitmap);
return INVALID_IMAGE_REQUEST_ID;
}
}
AnswersImageObserver cacheObserver = observer;
if (!profile.isOffTheRecord()) {
cacheObserver = new AnswersImageObserver() {
@Override
public void onAnswersImageChanged(Bitmap bitmap) {
if (bitmap == null) return;
mBitmapCache.put(imageUrl, bitmap);
observer.onAnswersImageChanged(bitmap);
}
};
}
return nativeRequestAnswersImage(profile, imageUrl, cacheObserver);
} }
/** /**
...@@ -44,7 +86,7 @@ public class AnswersImage { ...@@ -44,7 +86,7 @@ public class AnswersImage {
* @param profile Profile the request was issued for. * @param profile Profile the request was issued for.
* @param requestId The ID of the request to be cancelled. * @param requestId The ID of the request to be cancelled.
*/ */
public static void cancelAnswersImageRequest(Profile profile, int requestId) { public void cancelAnswersImageRequest(Profile profile, int requestId) {
nativeCancelAnswersImageRequest(profile, requestId); nativeCancelAnswersImageRequest(profile, requestId);
} }
......
...@@ -60,6 +60,7 @@ public class AutocompleteCoordinator ...@@ -60,6 +60,7 @@ public class AutocompleteCoordinator
private final UrlBarEditingTextStateProvider mUrlBarEditingTextProvider; private final UrlBarEditingTextStateProvider mUrlBarEditingTextProvider;
private final OmniboxResultsAdapter mSuggestionListAdapter; private final OmniboxResultsAdapter mSuggestionListAdapter;
private final AnswersImageFetcher mAnswersImageFetcher;
private final List<OmniboxResultItem> mSuggestionItems; private final List<OmniboxResultItem> mSuggestionItems;
private final List<Runnable> mDeferredNativeRunnables = new ArrayList<Runnable>(); private final List<Runnable> mDeferredNativeRunnables = new ArrayList<Runnable>();
...@@ -171,7 +172,9 @@ public class AutocompleteCoordinator ...@@ -171,7 +172,9 @@ public class AutocompleteCoordinator
mUrlBarEditingTextProvider = urlBarEditingTextProvider; mUrlBarEditingTextProvider = urlBarEditingTextProvider;
mSuggestionItems = new ArrayList<OmniboxResultItem>(); mSuggestionItems = new ArrayList<OmniboxResultItem>();
mSuggestionListAdapter = new OmniboxResultsAdapter(mContext, mSuggestionItems); mAnswersImageFetcher = new AnswersImageFetcher();
mSuggestionListAdapter =
new OmniboxResultsAdapter(mContext, mSuggestionItems, mAnswersImageFetcher);
mAutocomplete = new AutocompleteController(this); mAutocomplete = new AutocompleteController(this);
} }
...@@ -970,6 +973,7 @@ public class AutocompleteCoordinator ...@@ -970,6 +973,7 @@ public class AutocompleteCoordinator
mHasStartedNewOmniboxEditSession = false; mHasStartedNewOmniboxEditSession = false;
mNewOmniboxEditSessionTimestamp = -1; mNewOmniboxEditSessionTimestamp = -1;
hideSuggestions(); hideSuggestions();
mAnswersImageFetcher.clearCache();
} }
if (mNativeInitialized) { if (mNativeInitialized) {
......
...@@ -44,15 +44,19 @@ import java.util.Set; ...@@ -44,15 +44,19 @@ import java.util.Set;
public class OmniboxResultsAdapter extends BaseAdapter { public class OmniboxResultsAdapter extends BaseAdapter {
private final List<OmniboxResultItem> mSuggestionItems; private final List<OmniboxResultItem> mSuggestionItems;
private final Context mContext; private final Context mContext;
private final AnswersImageFetcher mImageFetcher;
private ToolbarDataProvider mDataProvider; private ToolbarDataProvider mDataProvider;
private OmniboxSuggestionDelegate mSuggestionDelegate; private OmniboxSuggestionDelegate mSuggestionDelegate;
private boolean mUseDarkColors = true; private boolean mUseDarkColors = true;
private Set<String> mPendingAnswerRequestUrls = new HashSet<>(); private Set<String> mPendingAnswerRequestUrls = new HashSet<>();
private int mLayoutDirection; private int mLayoutDirection;
public OmniboxResultsAdapter(Context context, List<OmniboxResultItem> suggestionItems) { public OmniboxResultsAdapter(Context context, List<OmniboxResultItem> suggestionItems,
AnswersImageFetcher answersImageFetcher) {
mContext = context; mContext = context;
mSuggestionItems = suggestionItems; mSuggestionItems = suggestionItems;
mImageFetcher = answersImageFetcher;
} }
public void notifySuggestionsChanged() { public void notifySuggestionsChanged() {
...@@ -367,8 +371,8 @@ public class OmniboxResultsAdapter extends BaseAdapter { ...@@ -367,8 +371,8 @@ public class OmniboxResultsAdapter extends BaseAdapter {
if (mPendingAnswerRequestUrls.contains(url)) return; if (mPendingAnswerRequestUrls.contains(url)) return;
mPendingAnswerRequestUrls.add(url); mPendingAnswerRequestUrls.add(url);
AnswersImage.requestAnswersImage( mImageFetcher.requestAnswersImage(
mDataProvider.getProfile(), url, new AnswersImage.AnswersImageObserver() { mDataProvider.getProfile(), url, new AnswersImageFetcher.AnswersImageObserver() {
@Override @Override
public void onAnswersImageChanged(Bitmap bitmap) { public void onAnswersImageChanged(Bitmap bitmap) {
ThreadUtils.assertOnUiThread(); ThreadUtils.assertOnUiThread();
......
...@@ -1093,7 +1093,7 @@ chrome_java_sources = [ ...@@ -1093,7 +1093,7 @@ chrome_java_sources = [
"java/src/org/chromium/chrome/browser/omnibox/geo/VisibleNetworksTracker.java", "java/src/org/chromium/chrome/browser/omnibox/geo/VisibleNetworksTracker.java",
"java/src/org/chromium/chrome/browser/omnibox/status/StatusViewCoordinator.java", "java/src/org/chromium/chrome/browser/omnibox/status/StatusViewCoordinator.java",
"java/src/org/chromium/chrome/browser/omnibox/suggestions/AnswerTextBuilder.java", "java/src/org/chromium/chrome/browser/omnibox/suggestions/AnswerTextBuilder.java",
"java/src/org/chromium/chrome/browser/omnibox/suggestions/AnswersImage.java", "java/src/org/chromium/chrome/browser/omnibox/suggestions/AnswersImageFetcher.java",
"java/src/org/chromium/chrome/browser/omnibox/suggestions/AutocompleteController.java", "java/src/org/chromium/chrome/browser/omnibox/suggestions/AutocompleteController.java",
"java/src/org/chromium/chrome/browser/omnibox/suggestions/AutocompleteCoordinator.java", "java/src/org/chromium/chrome/browser/omnibox/suggestions/AutocompleteCoordinator.java",
"java/src/org/chromium/chrome/browser/omnibox/suggestions/OmniboxResultsAdapter.java", "java/src/org/chromium/chrome/browser/omnibox/suggestions/OmniboxResultsAdapter.java",
......
...@@ -4765,7 +4765,7 @@ if (is_android) { ...@@ -4765,7 +4765,7 @@ if (is_android) {
"../android/java/src/org/chromium/chrome/browser/omnibox/OmniboxViewUtil.java", "../android/java/src/org/chromium/chrome/browser/omnibox/OmniboxViewUtil.java",
"../android/java/src/org/chromium/chrome/browser/omnibox/QueryInOmnibox.java", "../android/java/src/org/chromium/chrome/browser/omnibox/QueryInOmnibox.java",
"../android/java/src/org/chromium/chrome/browser/omnibox/geo/GeolocationHeader.java", "../android/java/src/org/chromium/chrome/browser/omnibox/geo/GeolocationHeader.java",
"../android/java/src/org/chromium/chrome/browser/omnibox/suggestions/AnswersImage.java", "../android/java/src/org/chromium/chrome/browser/omnibox/suggestions/AnswersImageFetcher.java",
"../android/java/src/org/chromium/chrome/browser/omnibox/suggestions/AutocompleteController.java", "../android/java/src/org/chromium/chrome/browser/omnibox/suggestions/AutocompleteController.java",
"../android/java/src/org/chromium/chrome/browser/page_info/CertificateChainHelper.java", "../android/java/src/org/chromium/chrome/browser/page_info/CertificateChainHelper.java",
"../android/java/src/org/chromium/chrome/browser/page_info/CertificateViewer.java", "../android/java/src/org/chromium/chrome/browser/page_info/CertificateViewer.java",
......
...@@ -10,7 +10,7 @@ ...@@ -10,7 +10,7 @@
#include "chrome/browser/bitmap_fetcher/bitmap_fetcher_service_factory.h" #include "chrome/browser/bitmap_fetcher/bitmap_fetcher_service_factory.h"
#include "chrome/browser/profiles/profile.h" #include "chrome/browser/profiles/profile.h"
#include "chrome/browser/profiles/profile_android.h" #include "chrome/browser/profiles/profile_android.h"
#include "jni/AnswersImage_jni.h" #include "jni/AnswersImageFetcher_jni.h"
#include "net/traffic_annotation/network_traffic_annotation.h" #include "net/traffic_annotation/network_traffic_annotation.h"
#include "third_party/skia/include/core/SkBitmap.h" #include "third_party/skia/include/core/SkBitmap.h"
#include "ui/gfx/android/java_bitmap.h" #include "ui/gfx/android/java_bitmap.h"
...@@ -49,7 +49,7 @@ class AnswersImageObserverAndroid : public BitmapFetcherService::Observer { ...@@ -49,7 +49,7 @@ class AnswersImageObserverAndroid : public BitmapFetcherService::Observer {
} // namespace } // namespace
static void JNI_AnswersImage_CancelAnswersImageRequest( static void JNI_AnswersImageFetcher_CancelAnswersImageRequest(
JNIEnv* env, JNIEnv* env,
const JavaParamRef<jclass>&, const JavaParamRef<jclass>&,
const JavaParamRef<jobject>& java_profile, const JavaParamRef<jobject>& java_profile,
...@@ -61,7 +61,7 @@ static void JNI_AnswersImage_CancelAnswersImageRequest( ...@@ -61,7 +61,7 @@ static void JNI_AnswersImage_CancelAnswersImageRequest(
bitmap_fetcher_service->CancelRequest(java_request_id); bitmap_fetcher_service->CancelRequest(java_request_id);
} }
static int JNI_AnswersImage_RequestAnswersImage( static int JNI_AnswersImageFetcher_RequestAnswersImage(
JNIEnv* env, JNIEnv* env,
const JavaParamRef<jclass>&, const JavaParamRef<jclass>&,
const JavaParamRef<jobject>& java_profile, const JavaParamRef<jobject>& java_profile,
......
...@@ -28,7 +28,8 @@ const size_t kMaxRequests = 25; // Maximum number of inflight requests allowed. ...@@ -28,7 +28,8 @@ const size_t kMaxRequests = 25; // Maximum number of inflight requests allowed.
// 12 is double the default number of maximum suggestions so this can // 12 is double the default number of maximum suggestions so this can
// accommodate one match image plus one answer image for each result. // accommodate one match image plus one answer image for each result.
#if defined(OS_ANDROID) #if defined(OS_ANDROID)
const int kMaxCacheEntries = 5; // Android caches the images in the java layer.
const int kMaxCacheEntries = 0;
#else #else
const int kMaxCacheEntries = 12; const int kMaxCacheEntries = 12;
#endif #endif
......
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