diff --git a/WebCore/ChangeLog b/WebCore/ChangeLog index 6370df34e83d682d6f9d2c83954a4c1492bd30a0..692346ef182d340266b0514a00fd72f5fc02c0dd 100644 --- a/WebCore/ChangeLog +++ b/WebCore/ChangeLog @@ -1,3 +1,20 @@ +2008-06-15 Darin Adler + + Reviewed by Mitz. + + - fix https://bugs.webkit.org/show_bug.cgi?id=19556 + REGRESSION (r34544): Crash while visiting bigglook.com + + This fix eliminates the crash, but the logic remaining seems a little strange. + We create an IconRecord and then immediately destroy it. Worth taking another + look at this later. + + * loader/icon/IconDatabase.cpp: + (WebCore::IconDatabase::setIconDataForIconURL): Added code to remove the icon + just as in the other cases where we might be holding the single reference to it. + (WebCore::IconDatabase::setIconURLForPageURL): Fixed comment typo. + (WebCore::IconDatabase::writeToDatabase): Removed unused local variable. + 2008-06-15 Darin Adler Reviewed and tweaked by Sam Weinig. diff --git a/WebCore/loader/icon/IconDatabase.cpp b/WebCore/loader/icon/IconDatabase.cpp index 34b466e4e4c1d6deb6398c892e5bbc49faa3ef82..cc1739c023f2da713a76851af4ffe0dc97f9690f 100644 --- a/WebCore/loader/icon/IconDatabase.cpp +++ b/WebCore/loader/icon/IconDatabase.cpp @@ -532,6 +532,12 @@ void IconDatabase::setIconDataForIconURL(PassRefPtr dataOriginal, MutexLocker locker(m_pendingSyncLock); m_iconsPendingSync.set(iconURL, icon->snapshot()); } + + if (icon->hasOneRef()) { + ASSERT(icon->retainingPageURLs().isEmpty()); + LOG(IconDatabase, "Icon for icon url %s is about to be destroyed - removing mapping for it", urlForLogging(icon->iconURL()).ascii().data()); + m_iconURLToRecordMap.remove(icon->iconURL()); + } } // Send notification out regarding all PageURLs that retain this icon @@ -605,7 +611,7 @@ void IconDatabase::setIconURLForPageURL(const String& iconURLOriginal, const Str MutexLocker locker(m_pendingSyncLock); m_pageURLsPendingSync.set(pageURL, pageRecord->snapshot()); - // If the icon is on it's last ref, mark it for deletion + // If the icon is on its last ref, mark it for deletion if (iconRecord && iconRecord->hasOneRef()) m_iconsPendingSync.set(iconRecord->iconURL(), iconRecord->snapshot(true)); } @@ -1530,15 +1536,12 @@ bool IconDatabase::writeToDatabase() } for (unsigned i = 0; i < pageSnapshots.size(); ++i) { - String iconURL = pageSnapshots[i].iconURL; - // If the icon URL is empty, this page is meant to be deleted // ASSERTs are sanity checks to make sure the mappings exist if they should and don't if they shouldn't if (pageSnapshots[i].iconURL.isEmpty()) removePageURLFromSQLDatabase(pageSnapshots[i].pageURL); - else { + else setIconURLForPageURLInSQLDatabase(pageSnapshots[i].iconURL, pageSnapshots[i].pageURL); - } LOG(IconDatabase, "Committed IconURL for PageURL %s to database", urlForLogging(pageSnapshots[i].pageURL).ascii().data()); }