Fix a rare crash when dispatching icon notifications.

I believe this happened when a WebIconDatabaseClient registered twice for
notifications. When it unregistered, it only removed the first entry. This could
lead to a crash since the second entry would be pointing to possibly deleted
memory.

Remove the mutex protection since all operations on mClients happen in the same
thread.

Add a missing include file in WebHistory found while debugging this crash.

Bug: 2279055

diff --git a/WebKit/android/jni/WebHistory.cpp b/WebKit/android/jni/WebHistory.cpp
index f722db7..76a310b 100644 (file)
--- a/WebKit/android/jni/WebHistory.cpp
+++ b/WebKit/android/jni/WebHistory.cpp
@@ -44,6 +44,7 @@
 #include "TextEncoding.h"
 #include "WebCoreFrameBridge.h"
 #include "WebCoreJni.h"
+#include "WebIconDatabase.h"
 #include "jni_utility.h"
 
 #include <JNIHelp.h>

diff --git a/WebKit/android/jni/WebIconDatabase.cpp b/WebKit/android/jni/WebIconDatabase.cpp
index e15d179..20258a4 100644 (file)
--- a/WebKit/android/jni/WebIconDatabase.cpp
+++ b/WebKit/android/jni/WebIconDatabase.cpp
@@ -67,10 +67,6 @@ static WebIconDatabase* gIconDatabaseClient = new WebIconDatabase();
 // XXX: Called by the IconDatabase thread
 void WebIconDatabase::dispatchDidAddIconForPageURL(const WebCore::String& pageURL)
 {
-    // If there are no clients currently, drop this message.
-    if (mClients.size() == 0)
-        return;
-
     mNotificationsMutex.lock();
     mNotifications.append(pageURL);
     if (!mDeliveryRequested) {
@@ -83,23 +79,25 @@ void WebIconDatabase::dispatchDidAddIconForPageURL(const WebCore::String& pageUR
 // Called in the WebCore thread
 void WebIconDatabase::RegisterForIconNotification(WebIconDatabaseClient* client)
 {
-    gIconDatabaseClient->mClientsMutex.lock();
+    WebIconDatabase* db = gIconDatabaseClient;
+    for (unsigned i = 0; i < db->mClients.size(); ++i) {
+        // Do not add the same client twice.
+        if (db->mClients[i] == client)
+            return;
+    }
     gIconDatabaseClient->mClients.append(client);
-    gIconDatabaseClient->mClientsMutex.unlock();
 }
 
 // Called in the WebCore thread
 void WebIconDatabase::UnregisterForIconNotification(WebIconDatabaseClient* client)
 {
     WebIconDatabase* db = gIconDatabaseClient;
-    db->mClientsMutex.lock();
     for (unsigned i = 0; i < db->mClients.size(); ++i) {
         if (db->mClients[i] == client) {
             db->mClients.remove(i);
             break;
         }
     }
-    db->mClientsMutex.unlock();
 }
 
 // Called in the WebCore thread
@@ -123,9 +121,7 @@ void WebIconDatabase::deliverNotifications()
 
     // Swap the clients queue
     Vector<WebIconDatabaseClient*> clients;
-    mClientsMutex.lock();
     clients.swap(mClients);
-    mClientsMutex.unlock();
 
     for (unsigned i = 0; i < queue.size(); ++i) {
         for (unsigned j = 0; j < clients.size(); ++j) {

diff --git a/WebKit/android/jni/WebIconDatabase.h b/WebKit/android/jni/WebIconDatabase.h
index 743c5eb..c91c4ae 100644 (file)
--- a/WebKit/android/jni/WebIconDatabase.h
+++ b/WebKit/android/jni/WebIconDatabase.h
@@ -59,9 +59,8 @@ namespace android {
         // Deliver all the icon notifications
         void deliverNotifications();
 
-        // List of clients and a mutex to protect it.
+        // List of clients.
         Vector<WebIconDatabaseClient*> mClients;
-        android::Mutex                 mClientsMutex;
 
         // Queue of page urls that have received an icon.
         Vector<WebCore::String> mNotifications;