Bug 914027: Do not attempt to decode Favicons of unsupported types. Fallback on Favicon failure. r=margaret

This commit is contained in:
Chris Kitching 2014-11-04 21:41:20 +00:00
parent 459d47ca20
commit 0df8196842
8 changed files with 255 additions and 110 deletions

View File

@ -270,9 +270,6 @@ public class BrowserApp extends GeckoApp
Log.d(LOGTAG, "BrowserApp.onTabChanged: " + tab.getId() + ": " + msg);
switch(msg) {
case LOCATION_CHANGE:
if (Tabs.getInstance().isSelectedTab(tab)) {
maybeCancelFaviconLoad(tab);
}
// fall through
case SELECTED:
if (Tabs.getInstance().isSelectedTab(tab)) {
@ -296,16 +293,7 @@ public class BrowserApp extends GeckoApp
}
break;
case PAGE_SHOW:
loadFavicon(tab);
break;
case LINK_FAVICON:
// If tab is not loading and the favicon is updated, we
// want to load the image straight away. If tab is still
// loading, we only load the favicon once the page's content
// is fully loaded.
if (tab.getState() != Tab.STATE_LOADING) {
loadFavicon(tab);
}
tab.loadFavicon();
break;
case BOOKMARK_ADDED:
showBookmarkAddedToast();
@ -1841,41 +1829,6 @@ public class BrowserApp extends GeckoApp
&& mHomePagerContainer != null && mHomePagerContainer.getVisibility() == View.VISIBLE);
}
/* Favicon stuff. */
private static OnFaviconLoadedListener sFaviconLoadedListener = new OnFaviconLoadedListener() {
@Override
public void onFaviconLoaded(String pageUrl, String faviconURL, Bitmap favicon) {
// If we failed to load a favicon, we use the default favicon instead.
Tabs.getInstance()
.updateFaviconForURL(pageUrl,
(favicon == null) ? Favicons.defaultFavicon : favicon);
}
};
private void loadFavicon(final Tab tab) {
maybeCancelFaviconLoad(tab);
final int tabFaviconSize = getResources().getDimensionPixelSize(R.dimen.browser_toolbar_favicon_size);
int flags = (tab.isPrivate() || tab.getErrorType() != Tab.ErrorType.NONE) ? 0 : LoadFaviconTask.FLAG_PERSIST;
int id = Favicons.getSizedFavicon(getContext(), tab.getURL(), tab.getFaviconURL(), tabFaviconSize, flags, sFaviconLoadedListener);
tab.setFaviconLoadId(id);
}
private void maybeCancelFaviconLoad(Tab tab) {
int faviconLoadId = tab.getFaviconLoadId();
if (Favicons.NOT_LOADING == faviconLoadId) {
return;
}
// Cancel load task and reset favicon load state if it wasn't already
// in NOT_LOADING state.
Favicons.cancelFaviconLoad(faviconLoadId);
tab.setFaviconLoadId(Favicons.NOT_LOADING);
}
/**
* Enters editing mode with the current tab's URL. There might be no
* tabs loaded by the time the user enters editing mode e.g. just after

View File

@ -9,6 +9,7 @@ import java.util.ArrayList;
import java.util.Collection;
import java.util.HashMap;
import java.util.Map;
import java.util.TreeSet;
import java.util.regex.Matcher;
import java.util.regex.Pattern;
@ -17,6 +18,10 @@ import org.json.JSONObject;
import org.mozilla.gecko.db.BrowserContract.Bookmarks;
import org.mozilla.gecko.db.BrowserDB;
import org.mozilla.gecko.db.URLMetadata;
import org.mozilla.gecko.favicons.Favicons;
import org.mozilla.gecko.favicons.LoadFaviconTask;
import org.mozilla.gecko.favicons.OnFaviconLoadedListener;
import org.mozilla.gecko.favicons.RemoteFavicon;
import org.mozilla.gecko.gfx.Layer;
import org.mozilla.gecko.util.ThreadUtils;
@ -42,7 +47,9 @@ public class Tab {
private String mTitle;
private Bitmap mFavicon;
private String mFaviconUrl;
private int mFaviconSize;
// The set of all available Favicons for this tab, sorted by attractiveness.
final TreeSet<RemoteFavicon> mAvailableFavicons = new TreeSet<>();
private boolean mHasFeeds;
private boolean mHasOpenSearch;
private final SiteIdentity mSiteIdentity;
@ -365,46 +372,81 @@ public class Tab {
return mHasTouchListeners;
}
public void setFaviconLoadId(int faviconLoadId) {
mFaviconLoadId = faviconLoadId;
}
public synchronized void addFavicon(String faviconURL, int faviconSize, String mimeType) {
RemoteFavicon favicon = new RemoteFavicon(faviconURL, faviconSize, mimeType);
public int getFaviconLoadId() {
return mFaviconLoadId;
}
/**
* Returns true if the favicon changed.
*/
public boolean updateFavicon(Bitmap favicon) {
if (mFavicon == favicon) {
return false;
// Add this Favicon to the set of available Favicons.
synchronized (mAvailableFavicons) {
mAvailableFavicons.add(favicon);
}
mFavicon = favicon;
return true;
}
public synchronized void updateFaviconURL(String faviconUrl, int size) {
// If we already have an "any" sized icon, don't update the icon.
if (mFaviconSize == -1)
return;
public void loadFavicon() {
// If we have a Favicon explicitly set, load it.
if (!mAvailableFavicons.isEmpty()) {
RemoteFavicon newFavicon = mAvailableFavicons.first();
// Only update the favicon if it's bigger than the current favicon.
// We use -1 to represent icons with sizes="any".
if (size == -1 || size >= mFaviconSize) {
mFaviconUrl = faviconUrl;
mFaviconSize = size;
// If the new Favicon is different, cancel the old load. Else, abort.
if (newFavicon.faviconUrl.equals(mFaviconUrl)) {
return;
}
Favicons.cancelFaviconLoad(mFaviconLoadId);
mFaviconUrl = newFavicon.faviconUrl;
} else {
// Otherwise, fallback to the default Favicon.
mFaviconUrl = null;
}
int flags = (isPrivate() || mErrorType != ErrorType.NONE) ? 0 : LoadFaviconTask.FLAG_PERSIST;
mFaviconLoadId = Favicons.getSizedFavicon(mAppContext, mUrl, mFaviconUrl, Favicons.browserToolbarFaviconSize, flags,
new OnFaviconLoadedListener() {
@Override
public void onFaviconLoaded(String pageUrl, String faviconURL, Bitmap favicon) {
// The tab might be pointing to another URL by the time the
// favicon is finally loaded, in which case we simply ignore it.
if (!pageUrl.equals(mUrl)) {
return;
}
// That one failed. Try the next one.
if (favicon == null) {
// If what we just tried to load originated from the set of declared icons..
if (!mAvailableFavicons.isEmpty()) {
// Discard it.
mAvailableFavicons.remove(mAvailableFavicons.first());
// Load the next best, if we have one. If not, it'll fall back to the
// default Favicon URL, before giving up.
loadFavicon();
return;
}
// Total failure: display the default favicon.
favicon = Favicons.defaultFavicon;
}
mFavicon = favicon;
mFaviconLoadId = Favicons.NOT_LOADING;
Tabs.getInstance().notifyListeners(Tab.this, Tabs.TabEvents.FAVICON);
}
}
);
}
public synchronized void clearFavicon() {
// Cancel any ongoing favicon load (if we never finished downloading the old favicon before
// we changed page).
Favicons.cancelFaviconLoad(mFaviconLoadId);
// Keep the favicon unchanged while entering reader mode
if (mEnteringReaderMode)
return;
mFavicon = null;
mFaviconUrl = null;
mFaviconSize = 0;
mAvailableFavicons.clear();
}
public void setHasFeeds(boolean hasFeeds) {

View File

@ -11,6 +11,7 @@ import java.util.List;
import java.util.concurrent.CopyOnWriteArrayList;
import java.util.concurrent.atomic.AtomicInteger;
import android.graphics.Bitmap;
import org.json.JSONException;
import org.json.JSONObject;
@ -29,7 +30,6 @@ import android.accounts.OnAccountsUpdateListener;
import android.content.ContentResolver;
import android.content.Context;
import android.database.ContentObserver;
import android.graphics.Bitmap;
import android.graphics.Color;
import android.net.Uri;
import android.os.Handler;
@ -505,8 +505,20 @@ public class Tabs implements GeckoEventListener {
} else if (event.equals("DOMTitleChanged")) {
tab.updateTitle(message.getString("title"));
} else if (event.equals("Link:Favicon")) {
tab.updateFaviconURL(message.getString("href"), message.getInt("size"));
notifyListeners(tab, TabEvents.LINK_FAVICON);
// Don't bother if the type isn't one we can decode.
if (!Favicons.canDecodeType(message.getString("mime"))) {
return;
}
// Add the favicon to the set of available icons for this tab.
tab.addFavicon(message.getString("href"), message.getInt("size"), message.getString("mime"));
// Load the favicon. If the tab is still loading, we actually do the load once the
// page has loaded, in an attempt to prevent the favicon load from having a
// detrimental effect on page load time.
if (tab.getState() != Tab.STATE_LOADING) {
tab.loadFavicon();
}
} else if (event.equals("Link:Feed")) {
tab.setHasFeeds(true);
notifyListeners(tab, TabEvents.LINK_FEED);
@ -535,24 +547,6 @@ public class Tabs implements GeckoEventListener {
}
}
/**
* Set the favicon for any tabs loaded with this page URL.
*/
public void updateFaviconForURL(String pageURL, Bitmap favicon) {
// The tab might be pointing to another URL by the time the
// favicon is finally loaded, in which case we won't find the tab.
// See also: Bug 920331.
for (Tab tab : mOrder) {
String tabURL = tab.getURL();
if (pageURL.equals(tabURL)) {
tab.setFaviconLoadId(Favicons.NOT_LOADING);
if (tab.updateFavicon(favicon)) {
notifyListeners(tab, TabEvents.FAVICON);
}
}
}
}
public void refreshThumbnails() {
final ThumbnailHelper helper = ThumbnailHelper.getInstance();
ThreadUtils.postToBackgroundThread(new Runnable() {
@ -595,7 +589,6 @@ public class Tabs implements GeckoEventListener {
LOCATION_CHANGE,
MENU_UPDATED,
PAGE_SHOW,
LINK_FAVICON,
LINK_FEED,
SECURITY_CHANGE,
READER_ENABLED,
@ -839,7 +832,8 @@ public class Tabs implements GeckoEventListener {
// TODO: surely we could just fetch *any* cached icon?
if (AboutPages.isBuiltinIconPage(url)) {
Log.d(LOGTAG, "Setting about: tab favicon inline.");
added.updateFavicon(getAboutPageFavicon(url));
added.addFavicon(url, Favicons.browserToolbarFaviconSize, "");
added.loadFavicon();
}
return added;
@ -853,17 +847,6 @@ public class Tabs implements GeckoEventListener {
return loadUrl(AboutPages.PRIVATEBROWSING, Tabs.LOADURL_NEW_TAB | Tabs.LOADURL_PRIVATE);
}
/**
* These favicons are only used for the URL bar, so
* we fetch with that size.
*
* This method completes on the calling thread.
*/
private Bitmap getAboutPageFavicon(final String url) {
int faviconSize = Math.round(mAppContext.getResources().getDimension(R.dimen.browser_toolbar_favicon_size));
return Favicons.getSizedFaviconForPageFromCache(url, faviconSize);
}
/**
* Open the url as a new tab, and mark the selected tab as its "parent".
*

View File

@ -32,6 +32,7 @@ import java.net.URI;
import java.net.URISyntaxException;
import java.util.Arrays;
import java.util.Collections;
import java.util.HashSet;
import java.util.Iterator;
import java.util.List;
import java.util.concurrent.ExecutorService;
@ -65,6 +66,9 @@ public class Favicons {
// The density-adjusted maximum Favicon dimensions.
public static int largestFaviconSize;
// The density-adjusted desired size for a browser-toolbar favicon.
public static int browserToolbarFaviconSize;
// Used to prevent multiple-initialisation.
public static final AtomicBoolean isInitialized = new AtomicBoolean(false);
@ -77,6 +81,57 @@ public class Favicons {
// doing so is not necessary.
private static final NonEvictingLruCache<String, String> pageURLMappings = new NonEvictingLruCache<>(NUM_PAGE_URL_MAPPINGS_TO_STORE);
// Mime types of things we are capable of decoding.
private static final HashSet<String> sDecodableMimeTypes = new HashSet<>();
// Mime types of things we are both capable of decoding and are container formats (May contain
// multiple different sizes of image)
private static final HashSet<String> sContainerMimeTypes = new HashSet<>();
static {
// MIME types extracted from http://filext.com - ostensibly all in-use mime types for the
// corresponding formats.
// ICO
sContainerMimeTypes.add("image/vnd.microsoft.icon");
sContainerMimeTypes.add("image/ico");
sContainerMimeTypes.add("image/icon");
sContainerMimeTypes.add("image/x-icon");
sContainerMimeTypes.add("text/ico");
sContainerMimeTypes.add("application/ico");
// Add supported container types to the set of supported types.
sDecodableMimeTypes.addAll(sContainerMimeTypes);
// PNG
sDecodableMimeTypes.add("image/png");
sDecodableMimeTypes.add("application/png");
sDecodableMimeTypes.add("application/x-png");
// GIF
sDecodableMimeTypes.add("image/gif");
// JPEG
sDecodableMimeTypes.add("image/jpeg");
sDecodableMimeTypes.add("image/jpg");
sDecodableMimeTypes.add("image/pipeg");
sDecodableMimeTypes.add("image/vnd.swiftview-jpeg");
sDecodableMimeTypes.add("application/jpg");
sDecodableMimeTypes.add("application/x-jpg");
// BMP
sDecodableMimeTypes.add("application/bmp");
sDecodableMimeTypes.add("application/x-bmp");
sDecodableMimeTypes.add("application/x-win-bitmap");
sDecodableMimeTypes.add("image/bmp");
sDecodableMimeTypes.add("image/x-bmp");
sDecodableMimeTypes.add("image/x-bitmap");
sDecodableMimeTypes.add("image/x-xbitmap");
sDecodableMimeTypes.add("image/x-win-bitmap");
sDecodableMimeTypes.add("image/x-windows-bitmap");
sDecodableMimeTypes.add("image/x-ms-bitmap");
sDecodableMimeTypes.add("image/x-ms-bmp");
sDecodableMimeTypes.add("image/ms-bmp");
}
public static String getFaviconURLForPageURLFromCache(String pageURL) {
return pageURLMappings.get(pageURL);
}
@ -416,6 +471,7 @@ public class Favicons {
// TODO: Remove this branch when old tablet is removed.
final int browserToolbarFaviconSizeDimenID = NewTabletUI.isEnabled(context) ?
R.dimen.new_tablet_tab_strip_favicon_size : R.dimen.browser_toolbar_favicon_size;
browserToolbarFaviconSize = res.getDimensionPixelSize(browserToolbarFaviconSizeDimenID);
faviconsCache = new FaviconCache(FAVICON_CACHE_SIZE_BYTES, largestFaviconSize);
@ -484,6 +540,25 @@ public class Favicons {
}
}
/**
* Helper function to determine if we can decode a particular mime type.
* @param imgType Mime type to check for decodability.
* @return false if the given mime type is certainly not decodable, true if it might be.
*/
public static boolean canDecodeType(String imgType) {
return "".equals(imgType) || sDecodableMimeTypes.contains(imgType);
}
/**
* Helper function to determine if the provided mime type is that of a format that can contain
* multiple image types. At time of writing, the only such type is ICO.
* @param mimeType Mime type to check.
* @return true if the given mime type is a container type, false otherwise.
*/
public static boolean isContainerType(String mimeType) {
return sDecodableMimeTypes.contains(mimeType);
}
public static void removeLoadTask(int taskId) {
synchronized(loadTasks) {
loadTasks.delete(taskId);

View File

@ -0,0 +1,89 @@
/* This Source Code Form is subject to the terms of the Mozilla Public
* License, v. 2.0. If a copy of the MPL was not distributed with this
* file, You can obtain one at http://mozilla.org/MPL/2.0/. */
package org.mozilla.gecko.favicons;
/**
* Class to represent a Favicon declared on a webpage which we may or may not have downloaded.
* Tab objects maintain a list of these and attempt to load them in descending order of quality
* until one works. This enables us to fallback if something fails trying to decode a Favicon.
*/
public class RemoteFavicon implements Comparable<RemoteFavicon> {
public static final int FAVICON_SIZE_ANY = -1;
// URL of the Favicon referred to by this object.
public String faviconUrl;
// The size of the Favicon, as given in the <link> tag, if any. Zero if unspecified. -1 if "any".
public int declaredSize;
// Mime type of the Favicon, as given in the link tag.
public String mimeType;
public RemoteFavicon(String faviconURL, int givenSize, String mime) {
faviconUrl = faviconURL;
declaredSize = givenSize;
mimeType = mime;
}
/**
* Determine equality of two RemoteFavicons.
* Two RemoteFavicons are considered equal if they have the same URL, size, and mime type.
* @param o The object to compare to this one.
* @return true if o is of type RemoteFavicon and is considered equal to this one, false otherwise.
*/
@Override
public boolean equals(Object o) {
if (!(o instanceof RemoteFavicon)) {
return false;
}
RemoteFavicon oCast = (RemoteFavicon) o;
return oCast.faviconUrl.equals(faviconUrl) &&
oCast.declaredSize == declaredSize &&
oCast.mimeType.equals(mimeType);
}
@Override
public int hashCode() {
return super.hashCode();
}
/**
* Establish ordering on Favicons. Lower value in the partial ordering is a "better" Favicon.
* @param obj Object to compare against.
* @return -1 if this Favicon is "better" than the other, zero if they are equivalent in quality,
* based on the information here, 1 if the other appears "better" than this one.
*/
@Override
public int compareTo(RemoteFavicon obj) {
// Size "any" trumps everything else.
if (declaredSize == FAVICON_SIZE_ANY) {
if (obj.declaredSize == FAVICON_SIZE_ANY) {
return 0;
}
return -1;
}
if (obj.declaredSize == FAVICON_SIZE_ANY) {
return 1;
}
// Unspecified sizes are "worse".
if (declaredSize > obj.declaredSize) {
return -1;
}
if (declaredSize == obj.declaredSize) {
// If there's no other way to choose, we prefer container types. They *might* contain
// an image larger than the size given in the <link> tag.
if (Favicons.isContainerType(mimeType)) {
return -1;
}
return 0;
}
return 1;
}
}

View File

@ -178,6 +178,7 @@ gbjar.sources += [
'favicons/Favicons.java',
'favicons/LoadFaviconTask.java',
'favicons/OnFaviconLoadedListener.java',
'favicons/RemoteFavicon.java',
'FilePicker.java',
'FilePickerResultHandler.java',
'FindInPageBar.java',

View File

@ -19,6 +19,7 @@ import org.mozilla.gecko.SiteIdentity.SecurityMode;
import org.mozilla.gecko.Tab;
import org.mozilla.gecko.animation.PropertyAnimator;
import org.mozilla.gecko.animation.ViewHelper;
import org.mozilla.gecko.favicons.Favicons;
import org.mozilla.gecko.toolbar.BrowserToolbarTabletBase.ForwardButtonAnimation;
import org.mozilla.gecko.util.StringUtils;
import org.mozilla.gecko.widget.ThemedLinearLayout;
@ -178,7 +179,7 @@ public class ToolbarDisplayLayout extends ThemedLinearLayout
if (Versions.feature16Plus) {
mFavicon.setImportantForAccessibility(View.IMPORTANT_FOR_ACCESSIBILITY_NO);
}
mFaviconSize = Math.round(res.getDimension(R.dimen.browser_toolbar_favicon_size));
mFaviconSize = Math.round(Favicons.browserToolbarFaviconSize);
}
mSiteSecurityVisible = (mSiteSecurity.getVisibility() == View.VISIBLE);

View File

@ -4012,7 +4012,8 @@ Tab.prototype = {
type: "Link:Favicon",
tabID: this.id,
href: resolveGeckoURI(target.href),
size: maxSize
size: maxSize,
mime: target.getAttribute("type") || ""
};
Messaging.sendRequest(json);
} else if (list.indexOf("[alternate]") != -1 && aEvent.type == "DOMLinkAdded") {