diff --git a/browser/modules/DirectoryLinksProvider.jsm b/browser/modules/DirectoryLinksProvider.jsm index 47e8a6ab9a66..9385c0921c9e 100644 --- a/browser/modules/DirectoryLinksProvider.jsm +++ b/browser/modules/DirectoryLinksProvider.jsm @@ -47,6 +47,12 @@ const PREF_DIRECTORY_PING = "browser.newtabpage.directory.ping"; // The preference that tells if newtab is enhanced const PREF_NEWTAB_ENHANCED = "browser.newtabpage.enhanced"; +// Only allow link urls that are http(s) +const ALLOWED_LINK_SCHEMES = new Set(["http", "https"]); + +// Only allow link image urls that are https or data +const ALLOWED_IMAGE_SCHEMES = new Set(["https", "data"]); + // The frecency of a directory link const DIRECTORY_FRECENCY = 1000; @@ -359,6 +365,24 @@ let DirectoryLinksProvider = { this._enhancedLinks.get(NewTabUtils.extractSite(link.url)); }, + /** + * Check if a url's scheme is in a Set of allowed schemes + */ + isURLAllowed: function DirectoryLinksProvider_isURLAllowed(url, allowed) { + // Assume no url is an allowed url + if (!url) { + return true; + } + + let scheme = ""; + try { + // A malformed url will not be allowed + scheme = Services.io.newURI(url, null, null).scheme; + } + catch(ex) {} + return allowed.has(scheme); + }, + /** * Gets the current set of directory links. * @param aCallback The function that the array of links is passed to. @@ -368,8 +392,12 @@ let DirectoryLinksProvider = { // Reset the cache of enhanced images for this new set of links this._enhancedLinks.clear(); - // all directory links have a frecency of DIRECTORY_FRECENCY - return rawLinks.map((link, position) => { + return rawLinks.filter(link => { + // Make sure the link url is allowed and images too if they exist + return this.isURLAllowed(link.url, ALLOWED_LINK_SCHEMES) && + this.isURLAllowed(link.imageURI, ALLOWED_IMAGE_SCHEMES) && + this.isURLAllowed(link.enhancedImageURI, ALLOWED_IMAGE_SCHEMES); + }).map((link, position) => { // Stash the enhanced image for the site if (link.enhancedImageURI) { this._enhancedLinks.set(NewTabUtils.extractSite(link.url), link); diff --git a/browser/modules/test/xpcshell/test_DirectoryLinksProvider.js b/browser/modules/test/xpcshell/test_DirectoryLinksProvider.js index 52e68628271b..72c28953fd43 100644 --- a/browser/modules/test/xpcshell/test_DirectoryLinksProvider.js +++ b/browser/modules/test/xpcshell/test_DirectoryLinksProvider.js @@ -556,11 +556,74 @@ add_task(function test_DirectoryLinksProvider_getLinksFromCorruptedFile() { yield promiseCleanDirectoryLinksProvider(); }); +add_task(function test_DirectoryLinksProvider_getAllowedLinks() { + let data = {"en-US": [ + {url: "ftp://example.com"}, + {url: "http://example.net"}, + {url: "javascript:5"}, + {url: "https://example.com"}, + {url: "httpJUNKjavascript:42"}, + {url: "data:text/plain,hi"}, + {url: "http/bork:eh"}, + ]}; + let dataURI = 'data:application/json,' + JSON.stringify(data); + yield promiseSetupDirectoryLinksProvider({linksURL: dataURI}); + + let links = yield fetchData(); + do_check_eq(links.length, 2); + + // The only remaining url should be http and https + do_check_eq(links[0].url, data["en-US"][1].url); + do_check_eq(links[1].url, data["en-US"][3].url); +}); + +add_task(function test_DirectoryLinksProvider_getAllowedImages() { + let data = {"en-US": [ + {url: "http://example.com", imageURI: "ftp://example.com"}, + {url: "http://example.com", imageURI: "http://example.net"}, + {url: "http://example.com", imageURI: "javascript:5"}, + {url: "http://example.com", imageURI: "https://example.com"}, + {url: "http://example.com", imageURI: "httpJUNKjavascript:42"}, + {url: "http://example.com", imageURI: "data:text/plain,hi"}, + {url: "http://example.com", imageURI: "http/bork:eh"}, + ]}; + let dataURI = 'data:application/json,' + JSON.stringify(data); + yield promiseSetupDirectoryLinksProvider({linksURL: dataURI}); + + let links = yield fetchData(); + do_check_eq(links.length, 2); + + // The only remaining images should be https and data + do_check_eq(links[0].imageURI, data["en-US"][3].imageURI); + do_check_eq(links[1].imageURI, data["en-US"][5].imageURI); +}); + +add_task(function test_DirectoryLinksProvider_getAllowedEnhancedImages() { + let data = {"en-US": [ + {url: "http://example.com", enhancedImageURI: "ftp://example.com"}, + {url: "http://example.com", enhancedImageURI: "http://example.net"}, + {url: "http://example.com", enhancedImageURI: "javascript:5"}, + {url: "http://example.com", enhancedImageURI: "https://example.com"}, + {url: "http://example.com", enhancedImageURI: "httpJUNKjavascript:42"}, + {url: "http://example.com", enhancedImageURI: "data:text/plain,hi"}, + {url: "http://example.com", enhancedImageURI: "http/bork:eh"}, + ]}; + let dataURI = 'data:application/json,' + JSON.stringify(data); + yield promiseSetupDirectoryLinksProvider({linksURL: dataURI}); + + let links = yield fetchData(); + do_check_eq(links.length, 2); + + // The only remaining enhancedImages should be http and https and data + do_check_eq(links[0].enhancedImageURI, data["en-US"][3].enhancedImageURI); + do_check_eq(links[1].enhancedImageURI, data["en-US"][5].enhancedImageURI); +}); + add_task(function test_DirectoryLinksProvider_getEnhancedLink() { let data = {"en-US": [ - {url: "http://example.net", enhancedImageURI: "net1"}, - {url: "http://example.com", enhancedImageURI: "com1"}, - {url: "http://example.com", enhancedImageURI: "com2"}, + {url: "http://example.net", enhancedImageURI: "data:,net1"}, + {url: "http://example.com", enhancedImageURI: "data:,com1"}, + {url: "http://example.com", enhancedImageURI: "data:,com2"}, ]}; let dataURI = 'data:application/json,' + JSON.stringify(data); yield promiseSetupDirectoryLinksProvider({linksURL: dataURI}); @@ -574,20 +637,20 @@ add_task(function test_DirectoryLinksProvider_getEnhancedLink() { } // Get the expected image for the same site - checkEnhanced("http://example.net/", "net1"); - checkEnhanced("http://example.net/path", "net1"); - checkEnhanced("https://www.example.net/", "net1"); - checkEnhanced("https://www3.example.net/", "net1"); + checkEnhanced("http://example.net/", "data:,net1"); + checkEnhanced("http://example.net/path", "data:,net1"); + checkEnhanced("https://www.example.net/", "data:,net1"); + checkEnhanced("https://www3.example.net/", "data:,net1"); // Get the image of the last entry - checkEnhanced("http://example.com", "com2"); + checkEnhanced("http://example.com", "data:,com2"); // Get the inline enhanced image let inline = DirectoryLinksProvider.getEnhancedLink({ url: "http://example.com/echo", - enhancedImageURI: "echo", + enhancedImageURI: "data:,echo", }); - do_check_eq(inline.enhancedImageURI, "echo"); + do_check_eq(inline.enhancedImageURI, "data:,echo"); do_check_eq(inline.url, "http://example.com/echo"); // Undefined for not enhanced @@ -598,14 +661,14 @@ add_task(function test_DirectoryLinksProvider_getEnhancedLink() { // Make sure old data is not cached data = {"en-US": [ - {url: "http://example.com", enhancedImageURI: "fresh"}, + {url: "http://example.com", enhancedImageURI: "data:,fresh"}, ]}; dataURI = 'data:application/json,' + JSON.stringify(data); yield promiseSetupDirectoryLinksProvider({linksURL: dataURI}); links = yield fetchData(); do_check_eq(links.length, 1); checkEnhanced("http://example.net", undefined); - checkEnhanced("http://example.com", "fresh"); + checkEnhanced("http://example.com", "data:,fresh"); }); add_task(function test_DirectoryLinksProvider_setDefaultEnhanced() { diff --git a/mobile/android/base/BrowserApp.java b/mobile/android/base/BrowserApp.java index c34a5f065158..2da84a9df711 100644 --- a/mobile/android/base/BrowserApp.java +++ b/mobile/android/base/BrowserApp.java @@ -267,6 +267,9 @@ 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)) { @@ -290,7 +293,16 @@ public class BrowserApp extends GeckoApp } break; case PAGE_SHOW: - tab.loadFavicon(); + 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); + } break; case BOOKMARK_ADDED: showBookmarkAddedToast(); @@ -1811,6 +1823,41 @@ 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 diff --git a/mobile/android/base/GeckoAppShell.java b/mobile/android/base/GeckoAppShell.java index a99b616b0dce..d015bb3dea92 100644 --- a/mobile/android/base/GeckoAppShell.java +++ b/mobile/android/base/GeckoAppShell.java @@ -34,8 +34,8 @@ import java.util.concurrent.ConcurrentLinkedQueue; import org.json.JSONException; import org.json.JSONObject; import org.mozilla.gecko.AppConstants.Versions; -import org.mozilla.gecko.favicons.Favicons; import org.mozilla.gecko.favicons.OnFaviconLoadedListener; +import org.mozilla.gecko.favicons.decoders.FaviconDecoder; import org.mozilla.gecko.gfx.BitmapUtils; import org.mozilla.gecko.gfx.LayerView; import org.mozilla.gecko.gfx.PanZoomController; @@ -810,24 +810,21 @@ public class GeckoAppShell // This is the entry point from nsIShellService. @WrapElementForJNI static void createShortcut(final String aTitle, final String aURI, final String aIconData) { - // We have the favicon data (base64) decoded on the background thread, callback here, then - // call the other createShortcut method with the decoded favicon. - // This is slightly contrived, but makes the images available to the favicon cache. - Favicons.getSizedFavicon(getContext(), aURI, aIconData, Integer.MAX_VALUE, 0, - new OnFaviconLoadedListener() { - @Override - public void onFaviconLoaded(String url, String faviconURL, Bitmap favicon) { - createShortcut(aTitle, url, favicon); - } + ThreadUtils.postToBackgroundThread(new Runnable() { + @Override + public void run() { + // TODO: use the cache. Bug 961600. + Bitmap icon = FaviconDecoder.getMostSuitableBitmapFromDataURI(aIconData, getPreferredIconSize()); + GeckoAppShell.doCreateShortcut(aTitle, aURI, icon); } - ); + }); } public static void createShortcut(final String aTitle, final String aURI, final Bitmap aBitmap) { ThreadUtils.postToBackgroundThread(new Runnable() { @Override public void run() { - doCreateShortcut(aTitle, aURI, aBitmap); + GeckoAppShell.doCreateShortcut(aTitle, aURI, aBitmap); } }); } diff --git a/mobile/android/base/Tab.java b/mobile/android/base/Tab.java index fd862122e07c..a126bdf4bad5 100644 --- a/mobile/android/base/Tab.java +++ b/mobile/android/base/Tab.java @@ -9,7 +9,6 @@ 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; @@ -18,10 +17,6 @@ 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; @@ -47,9 +42,7 @@ public class Tab { private String mTitle; private Bitmap mFavicon; private String mFaviconUrl; - - // The set of all available Favicons for this tab, sorted by attractiveness. - final TreeSet mAvailableFavicons = new TreeSet<>(); + private int mFaviconSize; private boolean mHasFeeds; private boolean mHasOpenSearch; private final SiteIdentity mSiteIdentity; @@ -372,81 +365,46 @@ public class Tab { return mHasTouchListeners; } - public synchronized void addFavicon(String faviconURL, int faviconSize, String mimeType) { - RemoteFavicon favicon = new RemoteFavicon(faviconURL, faviconSize, mimeType); - - // Add this Favicon to the set of available Favicons. - synchronized (mAvailableFavicons) { - mAvailableFavicons.add(favicon); - } + public void setFaviconLoadId(int faviconLoadId) { + mFaviconLoadId = faviconLoadId; } - public void loadFavicon() { - // If we have a Favicon explicitly set, load it. - if (!mAvailableFavicons.isEmpty()) { - RemoteFavicon newFavicon = mAvailableFavicons.first(); + public int getFaviconLoadId() { + return mFaviconLoadId; + } - // 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; + /** + * Returns true if the favicon changed. + */ + public boolean updateFavicon(Bitmap favicon) { + if (mFavicon == favicon) { + return false; } + mFavicon = favicon; + return true; + } - 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; - } + 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; - // 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); - } - } - ); + // 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; + } } 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; - mAvailableFavicons.clear(); + mFaviconSize = 0; } public void setHasFeeds(boolean hasFeeds) { diff --git a/mobile/android/base/Tabs.java b/mobile/android/base/Tabs.java index fbac0b21200e..2e15a122ac11 100644 --- a/mobile/android/base/Tabs.java +++ b/mobile/android/base/Tabs.java @@ -11,7 +11,6 @@ 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; @@ -30,6 +29,7 @@ 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,20 +505,8 @@ public class Tabs implements GeckoEventListener { } else if (event.equals("DOMTitleChanged")) { tab.updateTitle(message.getString("title")); } else if (event.equals("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(); - } + tab.updateFaviconURL(message.getString("href"), message.getInt("size")); + notifyListeners(tab, TabEvents.LINK_FAVICON); } else if (event.equals("Link:Feed")) { tab.setHasFeeds(true); notifyListeners(tab, TabEvents.LINK_FEED); @@ -547,6 +535,24 @@ 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() { @@ -589,6 +595,7 @@ public class Tabs implements GeckoEventListener { LOCATION_CHANGE, MENU_UPDATED, PAGE_SHOW, + LINK_FAVICON, LINK_FEED, SECURITY_CHANGE, READER_ENABLED, @@ -832,8 +839,7 @@ 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.addFavicon(url, Favicons.browserToolbarFaviconSize, ""); - added.loadFavicon(); + added.updateFavicon(getAboutPageFavicon(url)); } return added; @@ -847,6 +853,17 @@ 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". * diff --git a/mobile/android/base/favicons/Favicons.java b/mobile/android/base/favicons/Favicons.java index 6d83d871819a..2d3bf1b2013d 100644 --- a/mobile/android/base/favicons/Favicons.java +++ b/mobile/android/base/favicons/Favicons.java @@ -32,7 +32,6 @@ 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; @@ -66,9 +65,6 @@ 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); @@ -81,57 +77,6 @@ public class Favicons { // doing so is not necessary. private static final NonEvictingLruCache pageURLMappings = new NonEvictingLruCache<>(NUM_PAGE_URL_MAPPINGS_TO_STORE); - // Mime types of things we are capable of decoding. - private static final HashSet 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 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); } @@ -304,14 +249,9 @@ public class Favicons { * or a somewhat educated guess. */ public static String getFaviconURLForPageURL(Context context, String pageURL) { - // Query the URL cache. - String targetURL = getFaviconURLForPageURLFromCache(pageURL); - if (targetURL != null) { - return targetURL; - } - // Attempt to determine the Favicon URL from the Tabs datastructure. Can dodge having to use // the database sometimes by doing this. + String targetURL; Tab theTab = Tabs.getInstance().getFirstTabForUrl(pageURL); if (theTab != null) { targetURL = theTab.getFaviconURL(); @@ -324,7 +264,6 @@ public class Favicons { final ContentResolver resolver = context.getContentResolver(); targetURL = BrowserDB.getFaviconURLFromPageURL(resolver, pageURL); if (targetURL != null) { - putFaviconURLForPageURLInCache(pageURL, targetURL); return targetURL; } @@ -477,7 +416,6 @@ 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); @@ -546,25 +484,6 @@ 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); diff --git a/mobile/android/base/favicons/LoadFaviconTask.java b/mobile/android/base/favicons/LoadFaviconTask.java index caef1af24a75..596a1ddcb1f3 100644 --- a/mobile/android/base/favicons/LoadFaviconTask.java +++ b/mobile/android/base/favicons/LoadFaviconTask.java @@ -343,14 +343,36 @@ public class LoadFaviconTask { return pushToCacheAndGetResult(uriBitmaps); } - // Attempt to get a favicon URL from somewhere if we weren't given one... - if (TextUtils.isEmpty(faviconURL)) { - faviconURL = Favicons.getFaviconURLForPageURL(context, pageUrl); - } + String storedFaviconUrl; + boolean isUsingDefaultURL = false; - // We failed. Can't continue without a favicon URL! + // Handle the case of malformed favicon URL. + // If favicon is empty, fall back to the stored one. if (TextUtils.isEmpty(faviconURL)) { - return null; + // Try to get the favicon URL from the memory cache. + storedFaviconUrl = Favicons.getFaviconURLForPageURLFromCache(pageUrl); + + // If that failed, try to get the URL from the database. + if (storedFaviconUrl == null) { + storedFaviconUrl = Favicons.getFaviconURLForPageURL(context, pageUrl); + if (storedFaviconUrl != null) { + // If that succeeded, cache the URL loaded from the database in memory. + Favicons.putFaviconURLForPageURLInCache(pageUrl, storedFaviconUrl); + } + } + + // If we found a faviconURL - use it. + if (storedFaviconUrl != null) { + faviconURL = storedFaviconUrl; + } else { + // If we don't have a stored one, fall back to the default. + faviconURL = Favicons.guessDefaultFaviconURL(pageUrl); + + if (TextUtils.isEmpty(faviconURL)) { + return null; + } + isUsingDefaultURL = true; + } } // Check if favicon has failed - if so, give up. We need this check because, sometimes, we @@ -422,13 +444,18 @@ public class LoadFaviconTask { return pushToCacheAndGetResult(loadedBitmaps); } + if (isUsingDefaultURL) { + Favicons.putFaviconInFailedCache(faviconURL); + return null; + } + if (isCancelled()) { return null; } // If we're not already trying the default URL, try it now. final String guessed = Favicons.guessDefaultFaviconURL(pageUrl); - if (guessed == null || guessed.equals(faviconURL)) { + if (guessed == null) { Favicons.putFaviconInFailedCache(faviconURL); return null; } diff --git a/mobile/android/base/favicons/RemoteFavicon.java b/mobile/android/base/favicons/RemoteFavicon.java deleted file mode 100644 index 978a2a5862ce..000000000000 --- a/mobile/android/base/favicons/RemoteFavicon.java +++ /dev/null @@ -1,89 +0,0 @@ -/* 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 { - 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 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 tag. - if (Favicons.isContainerType(mimeType)) { - return -1; - } - return 0; - } - return 1; - } -} diff --git a/mobile/android/base/favicons/decoders/FaviconDecoder.java b/mobile/android/base/favicons/decoders/FaviconDecoder.java index a1a8e4c354ae..4bd0da212af0 100644 --- a/mobile/android/base/favicons/decoders/FaviconDecoder.java +++ b/mobile/android/base/favicons/decoders/FaviconDecoder.java @@ -151,6 +151,50 @@ public class FaviconDecoder { return decodeFavicon(buffer, 0, buffer.length); } + /** + * Returns the smallest bitmap in the icon represented by the provided + * data: URI that's larger than the desired width, or the largest if + * there is no larger icon. + * + * Returns null if no bitmap could be extracted. + * + * Bug 961600: we shouldn't be doing all of this work. The favicon cache + * should be used, and will give us the right size icon. + */ + public static Bitmap getMostSuitableBitmapFromDataURI(String iconURI, int desiredWidth) { + LoadFaviconResult result = FaviconDecoder.decodeDataURI(iconURI); + if (result == null) { + // Nothing we can do. + Log.w(LOG_TAG, "Unable to decode icon URI."); + return null; + } + + final Iterator bitmaps = result.getBitmaps(); + if (!bitmaps.hasNext()) { + Log.w(LOG_TAG, "No bitmaps in decoded icon."); + return null; + } + + Bitmap bitmap = bitmaps.next(); + if (!bitmaps.hasNext()) { + // We're done! There was only one, so this is as big as it gets. + return bitmap; + } + + // Find a bitmap of the most suitable size. + int currentWidth = bitmap.getWidth(); + while ((currentWidth < desiredWidth) && + bitmaps.hasNext()) { + final Bitmap b = bitmaps.next(); + if (b.getWidth() > currentWidth) { + currentWidth = b.getWidth(); + bitmap = b; + } + } + + return bitmap; + } + /** * Iterator to hold a single bitmap. */ diff --git a/mobile/android/base/moz.build b/mobile/android/base/moz.build index 3784b54128fe..702f37ad8327 100644 --- a/mobile/android/base/moz.build +++ b/mobile/android/base/moz.build @@ -177,7 +177,6 @@ gbjar.sources += [ 'favicons/Favicons.java', 'favicons/LoadFaviconTask.java', 'favicons/OnFaviconLoadedListener.java', - 'favicons/RemoteFavicon.java', 'FilePicker.java', 'FilePickerResultHandler.java', 'FindInPageBar.java', diff --git a/mobile/android/base/preferences/SearchEnginePreference.java b/mobile/android/base/preferences/SearchEnginePreference.java index bf0946a79f26..ce0fab15fd13 100644 --- a/mobile/android/base/preferences/SearchEnginePreference.java +++ b/mobile/android/base/preferences/SearchEnginePreference.java @@ -8,7 +8,6 @@ import org.json.JSONException; import org.json.JSONObject; import org.mozilla.gecko.R; import org.mozilla.gecko.favicons.Favicons; -import org.mozilla.gecko.favicons.OnFaviconLoadedListener; import org.mozilla.gecko.favicons.decoders.FaviconDecoder; import org.mozilla.gecko.util.ThreadUtils; import org.mozilla.gecko.widget.FaviconView; @@ -35,7 +34,6 @@ public class SearchEnginePreference extends CustomListPreference { // The bitmap backing the drawable above - needed separately for the FaviconView. private Bitmap mIconBitmap; - private final Object bitmapLock = new Object(); private FaviconView mFaviconView; @@ -57,16 +55,9 @@ public class SearchEnginePreference extends CustomListPreference { protected void onBindView(View view) { super.onBindView(view); - // We synchronise to avoid a race condition between this and the favicon loading callback in - // setSearchEngineFromJSON. - synchronized (bitmapLock) { - // Set the icon in the FaviconView. - mFaviconView = ((FaviconView) view.findViewById(R.id.search_engine_icon)); - - if (mIconBitmap != null) { - mFaviconView.updateAndScaleImage(mIconBitmap, getTitle().toString()); - } - } + // Set the icon in the FaviconView. + mFaviconView = ((FaviconView) view.findViewById(R.id.search_engine_icon)); + mFaviconView.updateAndScaleImage(mIconBitmap, getTitle().toString()); } @Override @@ -170,20 +161,9 @@ public class SearchEnginePreference extends CustomListPreference { } } - Favicons.getSizedFavicon(getContext(), mIdentifier, iconURI, desiredWidth, 0, - new OnFaviconLoadedListener() { - @Override - public void onFaviconLoaded(String url, String faviconURL, Bitmap favicon) { - synchronized (bitmapLock) { - mIconBitmap = favicon; + // TODO: use the cache. Bug 961600. + mIconBitmap = FaviconDecoder.getMostSuitableBitmapFromDataURI(iconURI, desiredWidth); - if (mFaviconView != null) { - mFaviconView.updateAndScaleImage(mIconBitmap, getTitle().toString()); - } - } - } - } - ); } catch (IllegalArgumentException e) { Log.e(LOGTAG, "IllegalArgumentException creating Bitmap. Most likely a zero-length bitmap.", e); } catch (NullPointerException e) { diff --git a/mobile/android/base/toolbar/ToolbarDisplayLayout.java b/mobile/android/base/toolbar/ToolbarDisplayLayout.java index f1e2286c408a..c189e9d2ff7f 100644 --- a/mobile/android/base/toolbar/ToolbarDisplayLayout.java +++ b/mobile/android/base/toolbar/ToolbarDisplayLayout.java @@ -19,7 +19,6 @@ 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; @@ -179,7 +178,7 @@ public class ToolbarDisplayLayout extends ThemedLinearLayout if (Versions.feature16Plus) { mFavicon.setImportantForAccessibility(View.IMPORTANT_FOR_ACCESSIBILITY_NO); } - mFaviconSize = Math.round(Favicons.browserToolbarFaviconSize); + mFaviconSize = Math.round(res.getDimension(R.dimen.browser_toolbar_favicon_size)); } mSiteSecurityVisible = (mSiteSecurity.getVisibility() == View.VISIBLE); diff --git a/mobile/android/chrome/content/browser.js b/mobile/android/chrome/content/browser.js index 379aa4a5ad74..ccb0404fba02 100644 --- a/mobile/android/chrome/content/browser.js +++ b/mobile/android/chrome/content/browser.js @@ -3960,8 +3960,7 @@ Tab.prototype = { type: "Link:Favicon", tabID: this.id, href: resolveGeckoURI(target.href), - size: maxSize, - mime: target.getAttribute("type") || "" + size: maxSize }; Messaging.sendRequest(json); } else if (list.indexOf("[alternate]") != -1 && aEvent.type == "DOMLinkAdded") {