Bug 1065485: Don't decode-to-encode default favicons. r=rnewman
--- a/mobile/android/base/db/LocalBrowserDB.java
+++ b/mobile/android/base/db/LocalBrowserDB.java
@@ -2,16 +2,19 @@
* 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.db;
import java.io.ByteArrayOutputStream;
import java.io.File;
+import java.io.InputStream;
+import java.lang.IllegalAccessException;
+import java.lang.NoSuchFieldException;
import java.lang.reflect.Field;
import java.util.ArrayList;
import java.util.Collection;
import java.util.EnumSet;
import java.util.HashMap;
import java.util.List;
import java.util.Locale;
import java.util.regex.Matcher;
@@ -48,22 +51,28 @@ import android.database.ContentObserver;
import android.database.Cursor;
import android.database.CursorWrapper;
import android.graphics.Bitmap;
import android.graphics.drawable.BitmapDrawable;
import android.net.Uri;
import android.provider.Browser;
import android.text.TextUtils;
import android.util.Log;
+import org.mozilla.gecko.util.IOUtils;
+
+import static org.mozilla.gecko.util.IOUtils.ConsumedInputStream;
+import static org.mozilla.gecko.favicons.LoadFaviconTask.DEFAULT_FAVICON_BUFFER_SIZE;
public class LocalBrowserDB {
// Calculate these once, at initialization. isLoggable is too expensive to
// have in-line in each log call.
private static final String LOGTAG = "GeckoLocalBrowserDB";
- private static final Integer FAVICON_ID_NOT_FOUND = Integer.MIN_VALUE;
+
+ // Sentinel value used to indicate a failure to locate an ID for a default favicon.
+ private static final int FAVICON_ID_NOT_FOUND = Integer.MIN_VALUE;
private static final boolean logDebug = Log.isLoggable(LOGTAG, Log.DEBUG);
protected static void debug(String message) {
if (logDebug) {
Log.d(LOGTAG, message);
}
}
@@ -180,40 +189,45 @@ public class LocalBrowserDB {
final Field urlField = stringsClass.getField(name.replace("_title_", "_url_"));
final int urlID = urlField.getInt(null);
final String url = context.getString(urlID);
final ContentValues bookmarkValue = createBookmark(now, title, url, pos++, folderID);
bookmarkValues.add(bookmarkValue);
- Bitmap icon = getDefaultFaviconFromPath(context, name);
- if (icon == null) {
- icon = getDefaultFaviconFromDrawable(context, name);
+ ConsumedInputStream faviconStream = getDefaultFaviconFromDrawable(context, name);
+ if (faviconStream == null) {
+ faviconStream = getDefaultFaviconFromPath(context, name);
}
- if (icon == null) {
+
+ if (faviconStream == null) {
+ continue;
+ }
+
+ // In the event that truncating the buffer fails, give up and move on.
+ byte[] icon;
+ try {
+ icon = faviconStream.getTruncatedData();
+ } catch (OutOfMemoryError e) {
continue;
}
final ContentValues iconValue = createFavicon(url, icon);
// Assign a reserved negative _id to each new favicon.
// For now, each name is expected to be unique, and duplicate
// icons will be duplicated in the DB. See Bug 1040806 Comment 8.
if (iconValue != null) {
final int faviconID = faviconIDs.get(name);
iconValue.put("_id", faviconID);
bookmarkValue.put(Bookmarks.FAVICON_ID, faviconID);
faviconValues.add(iconValue);
}
- } catch (IllegalAccessException e) {
- Log.wtf(LOGTAG, "Reflection failure.", e);
- } catch (IllegalArgumentException e) {
- Log.wtf(LOGTAG, "Reflection failure.", e);
- } catch (NoSuchFieldException e) {
+ } catch (IllegalAccessException | IllegalArgumentException | NoSuchFieldException e) {
Log.wtf(LOGTAG, "Reflection failure.", e);
}
}
if (!faviconValues.isEmpty()) {
try {
cr.bulkInsert(mFaviconsUriWithProfile, faviconValues.toArray(new ContentValues[faviconValues.size()]));
} catch (Exception e) {
@@ -287,23 +301,23 @@ public class LocalBrowserDB {
// Return early if there is no icon for this bookmark.
if (!bookmark.has("icon")) {
continue;
}
try {
final String iconData = bookmark.getString("icon");
- final Bitmap icon = BitmapUtils.getBitmapFromDataURI(iconData);
+
+ byte[] icon = BitmapUtils.getBytesFromDataURI(iconData);
if (icon == null) {
continue;
}
final ContentValues iconValue = createFavicon(url, icon);
-
if (iconValue == null) {
continue;
}
/*
* Find out if this icon is a duplicate. If it is, don't try
* to insert it again, but reuse the shared ID.
* Otherwise, assign a new reserved negative _id.
@@ -356,31 +370,21 @@ public class LocalBrowserDB {
v.put(Bookmarks.PARENT, parent);
v.put(Bookmarks.POSITION, pos);
v.put(Bookmarks.TITLE, title);
v.put(Bookmarks.URL, url);
return v;
}
- private static ContentValues createFavicon(final String url, final Bitmap icon) {
- ByteArrayOutputStream stream = new ByteArrayOutputStream();
-
+ private static ContentValues createFavicon(final String url, final byte[] icon) {
ContentValues iconValues = new ContentValues();
iconValues.put(Favicons.PAGE_URL, url);
+ iconValues.put(Favicons.DATA, icon);
- byte[] data = null;
- if (icon.compress(Bitmap.CompressFormat.PNG, 100, stream)) {
- data = stream.toByteArray();
- } else {
- Log.w(LOGTAG, "Favicon compression failed.");
- return null;
- }
-
- iconValues.put(Favicons.DATA, data);
return iconValues;
}
private static String getLocalizedProperty(final JSONObject bookmark, final String property, final Locale locale) throws JSONException {
// Try the full locale.
final String fullLocale = property + "." + locale.toString();
if (bookmark.has(fullLocale)) {
return bookmark.getString(fullLocale);
@@ -399,55 +403,63 @@ public class LocalBrowserDB {
if (bookmark.has(lang)) {
return bookmark.getString(lang);
}
// Default to the non-localized property name.
return bookmark.getString(property);
}
- private static Bitmap getDefaultFaviconFromPath(Context context, String name) {
- Class<?> stringClass = R.string.class;
+ private static int getFaviconId(String name) {
try {
- // Look for a drawable with the id R.drawable.bookmarkdefaults_favicon_*
- Field faviconField = stringClass.getField(name.replace("_title_", "_favicon_"));
- if (faviconField == null) {
- return null;
- }
- int faviconId = faviconField.getInt(null);
- String path = context.getString(faviconId);
+ Class<?> drawablesClass = R.raw.class;
+
+ // Look for a favicon with the id R.raw.bookmarkdefaults_favicon_*.
+ Field faviconField = drawablesClass.getField(name.replace("_title_", "_favicon_"));
+ faviconField.setAccessible(true);
- String apkPath = context.getPackageResourcePath();
- File apkFile = new File(apkPath);
- String bitmapPath = "jar:jar:" + apkFile.toURI() + "!/" + AppConstants.OMNIJAR_NAME + "!/" + path;
- return GeckoJarReader.getBitmap(context.getResources(), bitmapPath);
- } catch (java.lang.IllegalAccessException ex) {
- Log.e(LOGTAG, "[Path] Can't create favicon " + name, ex);
- } catch (java.lang.NoSuchFieldException ex) {
- // If the field does not exist, that means we intend to load via a drawable.
+ return faviconField.getInt(null);
+ } catch (IllegalAccessException | NoSuchFieldException ex) {
+ Log.wtf(LOGTAG, "Reflection error fetching favicon: " + name, ex);
}
- return null;
+
+ Log.e(LOGTAG, "Failed to find favicon resource ID for " + name);
+ return FAVICON_ID_NOT_FOUND;
}
- private static Bitmap getDefaultFaviconFromDrawable(Context context, String name) {
- Class<?> drawablesClass = R.drawable.class;
- try {
- // Look for a drawable with the id R.drawable.bookmarkdefaults_favicon_*
- Field faviconField = drawablesClass.getField(name.replace("_title_", "_favicon_"));
- if (faviconField == null) {
- return null;
- }
- int faviconId = faviconField.getInt(null);
- return BitmapUtils.decodeResource(context, faviconId);
- } catch (java.lang.IllegalAccessException ex) {
- Log.e(LOGTAG, "[Drawable] Can't create favicon " + name, ex);
- } catch (java.lang.NoSuchFieldException ex) {
- Log.wtf(LOGTAG, "No field, and presumably no drawable, for " + name);
+ /**
+ * Load a favicon from the omnijar.
+ * @return A ConsumedInputStream containing the bytes loaded from omnijar. This must be a format
+ * compatible with the favicon decoder (most probably a PNG or ICO file).
+ */
+ private static ConsumedInputStream getDefaultFaviconFromPath(Context context, String name) {
+ int faviconId = getFaviconId(name);
+ if (faviconId == FAVICON_ID_NOT_FOUND) {
+ return null;
}
- return null;
+
+ String path = context.getString(faviconId);
+
+ String apkPath = context.getPackageResourcePath();
+ File apkFile = new File(apkPath);
+ String bitmapPath = "jar:jar:" + apkFile.toURI() + "!/" + AppConstants.OMNIJAR_NAME + "!/" + path;
+
+ InputStream iStream = GeckoJarReader.getStream(bitmapPath);
+
+ return IOUtils.readFully(iStream, DEFAULT_FAVICON_BUFFER_SIZE);
+ }
+
+ private static ConsumedInputStream getDefaultFaviconFromDrawable(Context context, String name) {
+ int faviconId = getFaviconId(name);
+ if (faviconId == FAVICON_ID_NOT_FOUND) {
+ return null;
+ }
+
+ InputStream iStream = context.getResources().openRawResource(faviconId);
+ return IOUtils.readFully(iStream, DEFAULT_FAVICON_BUFFER_SIZE);
}
// Invalidate cached data
public void invalidateCachedState() {
mDesktopBookmarksExist = null;
}
private Uri bookmarksUriWithLimit(int limit) {
--- a/mobile/android/base/favicons/LoadFaviconTask.java
+++ b/mobile/android/base/favicons/LoadFaviconTask.java
@@ -15,28 +15,31 @@ import org.apache.http.Header;
import org.apache.http.HttpEntity;
import org.apache.http.HttpResponse;
import org.apache.http.client.methods.HttpGet;
import org.mozilla.gecko.GeckoAppShell;
import org.mozilla.gecko.db.BrowserDB;
import org.mozilla.gecko.favicons.decoders.FaviconDecoder;
import org.mozilla.gecko.favicons.decoders.LoadFaviconResult;
import org.mozilla.gecko.util.GeckoJarReader;
+import org.mozilla.gecko.util.IOUtils;
import org.mozilla.gecko.util.ThreadUtils;
import java.io.IOException;
import java.io.InputStream;
import java.net.URI;
import java.net.URISyntaxException;
import java.util.HashMap;
import java.util.HashSet;
import java.util.LinkedList;
import java.util.concurrent.RejectedExecutionException;
import java.util.concurrent.atomic.AtomicInteger;
+import static org.mozilla.gecko.util.IOUtils.ConsumedInputStream;
+
/**
* Class representing the asynchronous task to load a Favicon which is not currently in the in-memory
* cache.
* The implementation initially tries to get the Favicon from the database. Upon failure, the icon
* is loaded from the internet.
*/
public class LoadFaviconTask {
private static final String LOGTAG = "LoadFaviconTask";
@@ -44,17 +47,17 @@ public class LoadFaviconTask {
// Access to this map needs to be synchronized prevent multiple jobs loading the same favicon
// from executing concurrently.
private static final HashMap<String, LoadFaviconTask> loadsInFlight = new HashMap<>();
public static final int FLAG_PERSIST = 1;
private static final int MAX_REDIRECTS_TO_FOLLOW = 5;
// The default size of the buffer to use for downloading Favicons in the event no size is given
// by the server.
- private static final int DEFAULT_FAVICON_BUFFER_SIZE = 25000;
+ public static final int DEFAULT_FAVICON_BUFFER_SIZE = 25000;
private static final AtomicInteger nextFaviconLoadId = new AtomicInteger(0);
private final Context context;
private final int id;
private final String pageUrl;
private String faviconURL;
private final OnFaviconLoadedListener listener;
private final int flags;
@@ -276,52 +279,27 @@ public class LoadFaviconTask {
// The size was reported and sane, so let's use that.
// Integer overflow should not be a problem for Favicon sizes...
bufferSize = (int) entityReportedLength + 1;
} else {
// No declared size, so guess and reallocate later if it turns out to be too small.
bufferSize = DEFAULT_FAVICON_BUFFER_SIZE;
}
- // Allocate a buffer to hold the raw favicon data downloaded.
- byte[] buffer = new byte[bufferSize];
-
- // The offset of the start of the buffer's free space.
- int bPointer = 0;
-
- // The quantity of bytes the last call to read yielded.
- int lastRead = 0;
- InputStream contentStream = entity.getContent();
- try {
- // Fully read the entity into the buffer - decoding of streams is not supported
- // (and questionably pointful - what would one do with a half-decoded Favicon?)
- while (lastRead != -1) {
- // Read as many bytes as are currently available into the buffer.
- lastRead = contentStream.read(buffer, bPointer, buffer.length - bPointer);
- bPointer += lastRead;
-
- // If buffer has overflowed, double its size and carry on.
- if (bPointer == buffer.length) {
- bufferSize *= 2;
- byte[] newBuffer = new byte[bufferSize];
-
- // Copy the contents of the old buffer into the new buffer.
- System.arraycopy(buffer, 0, newBuffer, 0, buffer.length);
- buffer = newBuffer;
- }
- }
- } finally {
- contentStream.close();
+ // Read the InputStream into a byte[].
+ ConsumedInputStream result = IOUtils.readFully(entity.getContent(), bufferSize);
+ if (result == null) {
+ return null;
}
// Having downloaded the image, decode it.
- return FaviconDecoder.decodeFavicon(buffer, 0, bPointer + 1);
+ return FaviconDecoder.decodeFavicon(result.getData(), 0, result.consumedLength);
}
- // LoadFavicon tasks are performed on a unique background executor thread
+ // LoadFaviconTasks are performed on a unique background executor thread
// to avoid network blocking.
public final void execute() {
try {
Favicons.longRunningExecutor.execute(new Runnable() {
@Override
public void run() {
final Bitmap result = doInBackground();
--- a/mobile/android/base/gfx/BitmapUtils.java
+++ b/mobile/android/base/gfx/BitmapUtils.java
@@ -336,26 +336,43 @@ public final class BitmapUtils {
* @param dataURI a Base64-encoded data URI string
* @return the decoded bitmap, or null if the data URI is invalid
*/
public static Bitmap getBitmapFromDataURI(String dataURI) {
if (dataURI == null) {
return null;
}
- final String base64 = dataURI.substring(dataURI.indexOf(',') + 1);
+ byte[] raw = getBytesFromDataURI(dataURI);
+ if (raw == null || raw.length == 0) {
+ return null;
+ }
+
+ return decodeByteArray(raw);
+ }
+
+ /**
+ * Return a byte[] containing the bytes in a given base64 string, or null if this is not a valid
+ * base64 string.
+ */
+ public static byte[] getBytesFromBase64(String base64) {
try {
- byte[] raw = Base64.decode(base64, Base64.DEFAULT);
- return BitmapUtils.decodeByteArray(raw);
+ return Base64.decode(base64, Base64.DEFAULT);
} catch (Exception e) {
- Log.e(LOGTAG, "exception decoding bitmap from data URI: " + dataURI, e);
+ Log.e(LOGTAG, "exception decoding bitmap from data URI: " + base64, e);
}
+
return null;
}
+ public static byte[] getBytesFromDataURI(String dataURI) {
+ final String base64 = dataURI.substring(dataURI.indexOf(',') + 1);
+ return getBytesFromBase64(base64);
+ }
+
public static Bitmap getBitmapFromDrawable(Drawable drawable) {
if (drawable instanceof BitmapDrawable) {
return ((BitmapDrawable) drawable).getBitmap();
}
int width = drawable.getIntrinsicWidth();
width = width > 0 ? width : 1;
int height = drawable.getIntrinsicHeight();
--- a/mobile/android/base/moz.build
+++ b/mobile/android/base/moz.build
@@ -55,16 +55,17 @@ gujar.sources += [
'util/GamepadUtils.java',
'util/GeckoBackgroundThread.java',
'util/GeckoEventListener.java',
'util/GeckoJarReader.java',
'util/GeckoRequest.java',
'util/HardwareUtils.java',
'util/INIParser.java',
'util/INISection.java',
+ 'util/IOUtils.java',
'util/JSONUtils.java',
'util/MenuUtils.java',
'util/NativeEventListener.java',
'util/NativeJSContainer.java',
'util/NativeJSObject.java',
'util/NonEvictingLruCache.java',
'util/PrefUtils.java',
'util/ProxySelector.java',
rename from mobile/android/base/resources/drawable-mdpi/bookmarkdefaults_favicon_addons.png
rename to mobile/android/base/resources/raw/bookmarkdefaults_favicon_addons.png
rename from mobile/android/base/resources/drawable-mdpi/bookmarkdefaults_favicon_marketplace.png
rename to mobile/android/base/resources/raw/bookmarkdefaults_favicon_marketplace.png
rename from mobile/android/base/resources/drawable-mdpi/bookmarkdefaults_favicon_support.png
rename to mobile/android/base/resources/raw/bookmarkdefaults_favicon_support.png
new file mode 100644
--- /dev/null
+++ b/mobile/android/base/util/IOUtils.java
@@ -0,0 +1,111 @@
+/* -*- Mode: Java; c-basic-offset: 4; tab-width: 4; indent-tabs-mode: nil; -*-
+ * 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.util;
+
+import android.util.Log;
+
+import java.io.IOException;
+import java.io.InputStream;
+
+/**
+ * Static helper class containing useful methods for manipulating IO objects.
+ */
+public class IOUtils {
+ private static final String LOGTAG = "GeckoIOUtils";
+
+ /**
+ * Represents the result of consuming an input stream, holding the returned data as well
+ * as the length of the data returned.
+ * The byte[] is not guaranteed to be trimmed to the size of the data acquired from the stream:
+ * hence the need for the length field. This strategy avoids the need to copy the data into a
+ * trimmed buffer after consumption.
+ */
+ public static class ConsumedInputStream {
+ public final int consumedLength;
+ // Only reassigned in getTruncatedData.
+ private byte[] consumedData;
+
+ public ConsumedInputStream(int consumedLength, byte[] consumedData) {
+ this.consumedLength = consumedLength;
+ this.consumedData = consumedData;
+ }
+
+ /**
+ * Get the data trimmed to the length of the actual payload read, caching the result.
+ */
+ public byte[] getTruncatedData() {
+ if (consumedData.length == consumedLength) {
+ return consumedData;
+ }
+
+ consumedData = truncateBytes(consumedData, consumedLength);
+ return consumedData;
+ }
+
+ public byte[] getData() {
+ return consumedData;
+ }
+ }
+
+ /**
+ * Fully read an InputStream into a byte array.
+ * @param iStream the InputStream to consume.
+ * @param bufferSize The initial size of the buffer to allocate. It will be grown as
+ * needed, but if the caller knows something about the InputStream then
+ * passing a good value here can improve performance.
+ */
+ public static ConsumedInputStream readFully(InputStream iStream, int bufferSize) {
+ // Allocate a buffer to hold the raw data downloaded.
+ byte[] buffer = new byte[bufferSize];
+
+ // The offset of the start of the buffer's free space.
+ int bPointer = 0;
+
+ // The quantity of bytes the last call to read yielded.
+ int lastRead = 0;
+ try {
+ // Fully read the data into the buffer.
+ while (lastRead != -1) {
+ // Read as many bytes as are currently available into the buffer.
+ lastRead = iStream.read(buffer, bPointer, buffer.length - bPointer);
+ bPointer += lastRead;
+
+ // If buffer has overflowed, double its size and carry on.
+ if (bPointer == buffer.length) {
+ bufferSize *= 2;
+ byte[] newBuffer = new byte[bufferSize];
+
+ // Copy the contents of the old buffer into the new buffer.
+ System.arraycopy(buffer, 0, newBuffer, 0, buffer.length);
+ buffer = newBuffer;
+ }
+ }
+
+ return new ConsumedInputStream(bPointer + 1, buffer);
+ } catch (IOException e) {
+ Log.e(LOGTAG, "Error consuming input stream.", e);
+ } finally {
+ try {
+ iStream.close();
+ } catch (IOException e) {
+ Log.e(LOGTAG, "Error closing input stream.", e);
+ }
+ }
+
+ return null;
+ }
+
+ /**
+ * Truncate a given byte[] to a given length. Returns a new byte[] with the first length many
+ * bytes of the input.
+ */
+ public static byte[] truncateBytes(byte[] bytes, int length) {
+ byte[] newBytes = new byte[length];
+ System.arraycopy(bytes, 0, newBytes, 0, length);
+
+ return newBytes;
+ }
+}