Handle more favicon decoding error cases

 Bug: 6517530
 Switch to having webcoreImageToSkBitmap return a pointer instead
 as it makes dealing with error cases easier. Also add a check to
 see if SkImageDecoder::DecodeMemory returned true or false, as if
 it returns false we may have already allocated pixels (so isNull = false),
 however it might not have valid data or any config

 Also aggressively check that we have a valid bitmap with a valid config

Change-Id: I2a2e856a389d73cdda319eaeaee5d1279a9e2eb6

4 files changed, 25 insertions, 15 deletions

diff --git a/Source/WebKit/android/jni/WebHistory.cpp b/Source/WebKit/android/jni/WebHistory.cpp
index 0c4652a..b6972e4 100644
--- a/Source/WebKit/android/jni/WebHistory.cpp
+++ b/Source/WebKit/android/jni/WebHistory.cpp
@@ -263,10 +263,11 @@ static jobject WebHistoryGetFavicon(JNIEnv* env, jobject obj, jint ptr)
         return 0;
     WebHistoryItem* item = reinterpret_cast<WebHistoryItem*>(ptr);
     MutexLocker locker(item->m_lock);
-    if (!item->m_faviconCached && !item->m_favicon.isNull()) {
+    if (!item->m_faviconCached && item->m_favicon) {
         jobject favicon = GraphicsJNI::createBitmap(env,
-                                                    new SkBitmap(item->m_favicon),
+                                                    item->m_favicon,
                                                     false, NULL);
+        item->m_favicon = 0; // Framework now owns the pointer
         item->m_faviconCached = env->NewGlobalRef(favicon);
         env->DeleteLocalRef(favicon);
     }
@@ -345,6 +346,7 @@ void WebHistoryItem::updateHistoryItem(WebCore::HistoryItem* item) {
     // FIXME: This method should not be used from outside WebCore and will be removed.
     // http://trac.webkit.org/changeset/81484
     WebCore::Image* icon = WebCore::iconDatabase().synchronousIconForPageURL(url, WebCore::IntSize(16, 16));
+    delete webItem->m_favicon;
     webItem->m_favicon = webcoreImageToSkBitmap(icon);
     if (webItem->m_faviconCached) {
         env->DeleteGlobalRef(webItem->m_faviconCached);
@@ -361,6 +363,7 @@ void WebHistoryItem::updateHistoryItem(WebCore::HistoryItem* item) {
 
 WebHistoryItem::~WebHistoryItem()
 {
+    delete m_favicon;
     JNIEnv* env = JSC::Bindings::getJNIEnv();
     if (!env) {
         ALOGW("Failed to get JNIEnv*! Potential memory leak!");
diff --git a/Source/WebKit/android/jni/WebHistory.h b/Source/WebKit/android/jni/WebHistory.h
index c628f7c..e89959c 100644
--- a/Source/WebKit/android/jni/WebHistory.h
+++ b/Source/WebKit/android/jni/WebHistory.h
@@ -55,12 +55,14 @@ class WebHistoryItem : public WebCore::AndroidWebHistoryBridge {
 public:
     WebHistoryItem(WebHistoryItem* parent)
         : WebCore::AndroidWebHistoryBridge(0)
+        , m_favicon(0)
         , m_faviconCached(0)
         , m_dataCached(0)
         , m_parent(parent)
     {}
     WebHistoryItem(WebCore::HistoryItem* item)
         : WebCore::AndroidWebHistoryBridge(item)
+        , m_favicon(0)
         , m_faviconCached(0)
         , m_dataCached(0)
         , m_parent(0)
@@ -76,7 +78,7 @@ public:
     String m_url;
     String m_originalUrl;
     String m_title;
-    SkBitmap m_favicon;
+    SkBitmap* m_favicon;
     WTF::Vector<char> m_data;
     jobject m_faviconCached;
     jobject m_dataCached;
diff --git a/Source/WebKit/android/jni/WebIconDatabase.cpp b/Source/WebKit/android/jni/WebIconDatabase.cpp
index da8ce63..e9219df 100644
--- a/Source/WebKit/android/jni/WebIconDatabase.cpp
+++ b/Source/WebKit/android/jni/WebIconDatabase.cpp
@@ -50,26 +50,31 @@
 
 namespace android {
 
-SkBitmap webcoreImageToSkBitmap(WebCore::Image* icon)
+SkBitmap* webcoreImageToSkBitmap(WebCore::Image* icon)
 {
-    SkBitmap bm;
     if (!icon)
-        return bm;
+        return 0;
     WebCore::SharedBuffer* buffer = icon->data();
     if (!buffer)
-        return bm;
-    SkImageDecoder::DecodeMemory(buffer->data(), buffer->size(), &bm,
-                                 SkBitmap::kNo_Config,
-                                 SkImageDecoder::kDecodePixels_Mode);
+        return 0;
+    SkBitmap* bm = new SkBitmap;
+    if (!SkImageDecoder::DecodeMemory(buffer->data(), buffer->size(), bm,
+                                      SkBitmap::kNo_Config,
+                                      SkImageDecoder::kDecodePixels_Mode)
+            || bm->isNull() || !bm->width() || !bm->height()
+            || bm->config() == SkBitmap::kNo_Config) {
+        delete bm;
+        return 0;
+    }
     return bm;
 }
 
 jobject webcoreImageToJavaBitmap(JNIEnv* env, WebCore::Image* icon)
 {
-    SkBitmap bm = webcoreImageToSkBitmap(icon);
-    if (bm.isNull())
-        return NULL;
-    return GraphicsJNI::createBitmap(env, new SkBitmap(bm), false, NULL);
+    SkBitmap* bm = webcoreImageToSkBitmap(icon);
+    if (!bm)
+        return 0;
+    return GraphicsJNI::createBitmap(env, bm, false, NULL);
 }
 
 static WebIconDatabase* gIconDatabaseClient = new WebIconDatabase();
diff --git a/Source/WebKit/android/jni/WebIconDatabase.h b/Source/WebKit/android/jni/WebIconDatabase.h
index c3e7947..7b7c937 100644
--- a/Source/WebKit/android/jni/WebIconDatabase.h
+++ b/Source/WebKit/android/jni/WebIconDatabase.h
@@ -74,7 +74,7 @@ namespace android {
         bool                    mDeliveryRequested;
     };
 
-    SkBitmap webcoreImageToSkBitmap(WebCore::Image* icon);
+    SkBitmap* webcoreImageToSkBitmap(WebCore::Image* icon);
     jobject webcoreImageToJavaBitmap(JNIEnv* env, WebCore::Image* icon);
 
 };