Changeset 20357 in webkit

Timestamp:

Mar 20, 2007 10:39:57 PM (17 years ago)

Author:

beidson

Message:

Reviewed by Anders

<rdar://problem/5073391> and ​http://bugs.webkit.org/show_bug.cgi?id=13137

Crash in IconDatabase when private browsing is enabled.

The problem was caused by ​http://trac.webkit.org/projects/webkit/changeset/20182
which changed many uses of char[] and Vector<char> to SharedBuffer. The patch
tended to literally replace a Vector<char> with RefPtr<SharedBuffers> but forgot
to enforce the concept that Vector<char>'s always exist, whereas RefPtr<SharedBuffers>
can be null. This led to derefs.

I took the opportunity to rework the iconDB functions to live in a SharedBuffer
world, as that didn't exist when they were originally written - now they just return
SharedBuffers instead of taking a Vector<char>& as a parameter

• loader/icon/IconDatabase.cpp: (WebCore::IconDatabase::imageDataForIconURL): Return a SharedBuffer (WebCore::IconDatabase::iconForPageURL): Null check the SharedBuffer before asking it if it's empty (WebCore::IconDatabase::imageDataForIconURLQuery): Return a new SharedBuffer

• loader/icon/IconDatabase.h: Return SharedBuffer's instead of taking Vector<char>&'s

Location:

trunk/WebCore

Files:

• ChangeLog (modified) (1 diff)
• loader/icon/IconDatabase.cpp (modified) (4 diffs)
• loader/icon/IconDatabase.h (modified) (2 diffs)

trunk/WebCore/ChangeLog

@@ +1 @@
+2007-03-20  Brady Eidson  <beidson@apple.com>
+
+        Reviewed by Anders
+
+        <rdar://problem/5073391> and http://bugs.webkit.org/show_bug.cgi?id=13137
+
+        Crash in IconDatabase when private browsing is enabled.
+
+        The problem was caused by http://trac.webkit.org/projects/webkit/changeset/20182 
+        which changed many uses of char[] and Vector<char> to SharedBuffer.  The patch
+        tended to literally replace a Vector<char> with RefPtr<SharedBuffers> but forgot
+        to enforce the concept that Vector<char>'s always exist, whereas RefPtr<SharedBuffers>
+        can be null.  This led to derefs.
+
+        I took the opportunity to rework the iconDB functions to live in a SharedBuffer 
+        world, as that didn't exist when they were originally written - now they just return
+        SharedBuffers instead of taking a Vector<char>& as a parameter
+
+        * loader/icon/IconDatabase.cpp:
+        (WebCore::IconDatabase::imageDataForIconURL): Return a SharedBuffer
+        (WebCore::IconDatabase::iconForPageURL): Null check the SharedBuffer before asking
+          it if it's empty
+        (WebCore::IconDatabase::imageDataForIconURLQuery): Return a new SharedBuffer
+
+        * loader/icon/IconDatabase.h: Return SharedBuffer's instead of taking Vector<char>&'s
+
 2007-03-20  Adam Roben  <aroben@apple.com>
 

trunk/WebCore/loader/icon/IconDatabase.cpp

@@ -325 +325 @@
 }    
 
-void IconDatabase::imageDataForIconURL(const String& iconURL, PassRefPtr<SharedBuffer> result)
+PassRefPtr<SharedBuffer> IconDatabase::imageDataForIconURL(const String& iconURL)
 {      
     // If private browsing is enabled, we'll check there first as the most up-to-date data for an icon will be there
     if (m_privateBrowsingEnabled) {    
-        imageDataForIconURLQuery(m_privateBrowsingDB, iconURL, result);
-        if (!result->isEmpty())
-            return;
+        RefPtr<SharedBuffer> result = imageDataForIconURLQuery(m_privateBrowsingDB, iconURL);
+        if (result && !result->isEmpty())
+            return result.release();
     } 
     
     // It wasn't found there, so lets check the main tables
-    imageDataForIconURLQuery(m_mainDB, iconURL, result);
+    return imageDataForIconURLQuery(m_mainDB, iconURL);
 }
 
@@ -375 +375 @@
     // we'll read in that image data now
     if (icon->imageDataStatus() == ImageDataStatusUnknown) {
-        RefPtr<SharedBuffer> data = new SharedBuffer();
-        imageDataForIconURL(iconURL, data.get());
+        RefPtr<SharedBuffer> data = imageDataForIconURL(iconURL);
         icon->setImageData(data.get());
     }
@@ -954 +953 @@
 }
 
-void IconDatabase::imageDataForIconURLQuery(SQLDatabase& db, const String& iconURL, PassRefPtr<SharedBuffer> imageData)
-{
+PassRefPtr<SharedBuffer> IconDatabase::imageDataForIconURLQuery(SQLDatabase& db, const String& iconURL)
+{
+    RefPtr<SharedBuffer> imageData;
+    
     readySQLStatement(m_imageDataForIconURLStatement, db, "SELECT Icon.data FROM Icon WHERE Icon.url = (?);");
     m_imageDataForIconURLStatement->bindText16(1, iconURL, false);
     
     int result = m_imageDataForIconURLStatement->step();
-    imageData->clear();
     if (result == SQLResultRow) {
         Vector<char> data;
         m_imageDataForIconURLStatement->getColumnBlobAsVector(0, data);
+        imageData = new SharedBuffer;
         imageData->append(data.data(), data.size());
     } else if (result != SQLResultDone)
@@ -969 +970 @@
 
     m_imageDataForIconURLStatement->reset();
+    
+    return imageData.release();
 }
 

trunk/WebCore/loader/icon/IconDatabase.h

@@ -123 +123 @@
     
     // Returns the image data for the given IconURL, checking both databases if necessary
-    void imageDataForIconURL(const String& iconURL, PassRefPtr<SharedBuffer> result);
+    PassRefPtr<SharedBuffer> imageDataForIconURL(const String& iconURL);
     
     // Retains an iconURL, bringing it back from the brink if it was pending deletion
@@ -163 +163 @@
     
     // Query - Returns the image data from the given database for the given IconURL
-    void imageDataForIconURLQuery(SQLDatabase& db, const String& iconURL, PassRefPtr<SharedBuffer> result);
+    PassRefPtr<SharedBuffer> imageDataForIconURLQuery(SQLDatabase& db, const String& iconURL);
     SQLStatement* m_imageDataForIconURLStatement;