Bug 1254663 - Fall back to generated favicons if downloaded icon is too small r?sebastian
The only locations where we might want to use smaller icons are in:
- Tab.java: used for:
-- Doorhangers / Site ID popup: we only show a tiny icon regardless.
-- TabStrip (on tablets): also only a small icon.
-- One exception: the media notification probably requires a larger icon, this will require followup.
- SearchEnginePreference: this is used for the search engine settings screen, where icons are also smaller.
MozReview-Commit-ID: KUUVKMSmE3G
--- a/mobile/android/base/java/org/mozilla/gecko/Tab.java
+++ b/mobile/android/base/java/org/mozilla/gecko/Tab.java
@@ -135,17 +135,17 @@ public class Tab {
mTitle = title == null ? "" : title;
mSiteIdentity = new SiteIdentity();
mHistoryIndex = -1;
mContentType = "";
mZoomConstraints = new ZoomConstraints(false);
mPluginViews = new ArrayList<View>();
mState = shouldShowProgress(url) ? STATE_LOADING : STATE_SUCCESS;
mLoadProgress = LOAD_PROGRESS_INIT;
- mIconRequestBuilder = Icons.with(mAppContext).pageUrl(mUrl);
+ mIconRequestBuilder = Icons.with(mAppContext).pageUrl(mUrl).permitSmallIcons(true);
updateBookmark();
}
private ContentResolver getContentResolver() {
return mAppContext.getContentResolver();
}
@@ -617,17 +617,18 @@ public class Tab {
// We can unconditionally clear the favicon and title here: we
// already filtered both cases in which this was a (pseudo-)
// spurious location change, so we're definitely loading a new
// page.
clearFavicon();
// Start to build a new request to load a favicon.
mIconRequestBuilder = Icons.with(mAppContext)
- .pageUrl(uri);
+ .pageUrl(uri)
+ .permitSmallIcons(true);
// Load local static Favicons immediately
if (AboutPages.isBuiltinIconPage(uri)) {
loadFavicon();
}
updateTitle(null);
}
--- a/mobile/android/base/java/org/mozilla/gecko/icons/IconRequest.java
+++ b/mobile/android/base/java/org/mozilla/gecko/icons/IconRequest.java
@@ -24,30 +24,35 @@ public class IconRequest {
/* package-private */ String pageUrl;
/* package-private */ boolean privileged;
/* package-private */ TreeSet<IconDescriptor> icons;
/* package-private */ boolean skipNetwork;
/* package-private */ boolean backgroundThread;
/* package-private */ boolean skipDisk;
/* package-private */ boolean skipMemory;
/* package-private */ int targetSize;
+ /* package-private */ boolean ignoreSmallIcons;
/* package-private */ boolean prepareOnly;
+
private IconCallback callback;
/* package-private */ IconRequest(Context context) {
this.context = context.getApplicationContext();
this.icons = new TreeSet<>(new IconDescriptorComparator());
// Setting some sensible defaults.
this.privileged = false;
this.skipMemory = false;
this.skipDisk = false;
this.skipNetwork = false;
this.targetSize = context.getResources().getDimensionPixelSize(R.dimen.favicon_bg);
this.prepareOnly = false;
+ // By default we want larger favicons, as most of the UI uses large FaviconViews. This limits
+ // the number of callsites where we need to flip this parameter.
+ this.ignoreSmallIcons = true;
}
/**
* Execute this request and try to load an icon. Once an icon has been loaded successfully the
* callback will be executed.
*
* The returned Future can be used to cancel the job.
*/
--- a/mobile/android/base/java/org/mozilla/gecko/icons/IconRequestBuilder.java
+++ b/mobile/android/base/java/org/mozilla/gecko/icons/IconRequestBuilder.java
@@ -72,16 +72,25 @@ public class IconRequestBuilder {
*/
@CheckResult
public IconRequestBuilder skipDisk() {
request.skipDisk = true;
return this;
}
/**
+ * Allow returning tiny icons, that might be less than 1/2 target size.
+ */
+ @CheckResult
+ public IconRequestBuilder permitSmallIcons(boolean permitSmallIcons) {
+ request.ignoreSmallIcons = !permitSmallIcons;
+ return this;
+ }
+
+ /**
* Skip the memory cache and do not return a previously loaded icon.
*/
@CheckResult
public IconRequestBuilder skipMemory() {
request.skipMemory = true;
return this;
}
--- a/mobile/android/base/java/org/mozilla/gecko/icons/IconTask.java
+++ b/mobile/android/base/java/org/mozilla/gecko/icons/IconTask.java
@@ -118,23 +118,36 @@ import java.util.concurrent.Callable;
logPreparer(request, preparer);
}
}
private IconResponse loadIcon(IconRequest request) throws InterruptedException {
while (request.hasIconDescriptors()) {
for (IconLoader loader : loaders) {
+
ensureNotInterrupted();
IconResponse response = loader.load(request);
logLoader(request, loader, response);
if (response != null) {
+ // Ignore icons that are less than 2/3 the requested size: we fallback
+ // to generated icons in this case. We want to do this here since the smaller
+ // icon may already be in memory, in which case we can skip directly to generated
+ // icons.
+
+ // TODO: we should still store this smaller icon in memory / process it for those
+ // requests where it is useful?
+ if (request.ignoreSmallIcons &&
+ response.getBitmap().getWidth() < 2 * request.targetSize / 3) {
+ break;
+ }
+
return response;
}
}
request.moveToNextIcon();
}
return generator.load(request);
--- a/mobile/android/base/java/org/mozilla/gecko/preferences/SearchEnginePreference.java
+++ b/mobile/android/base/java/org/mozilla/gecko/preferences/SearchEnginePreference.java
@@ -158,16 +158,17 @@ public class SearchEnginePreference exte
final String iconURI = geckoEngineJSON.getString("iconURI");
// Keep a reference to the bitmap - we'll need it later in onBindView.
try {
Icons.with(getContext())
.pageUrl(mIdentifier)
.icon(IconDescriptor.createGenericIcon(iconURI))
.privileged(true)
+ .permitSmallIcons(true)
.build()
.execute(new IconCallback() {
@Override
public void onIconResponse(IconResponse response) {
mIconBitmap = response.getBitmap();
if (mFaviconView != null) {
mFaviconView.updateAndScaleImage(response);