Commit fb32d42d authored by darin@apple.com's avatar darin@apple.com

2008-06-15 Darin Adler <darin@apple.com>

        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.



git-svn-id: http://svn.webkit.org/repository/webkit/trunk@34575 268f45cc-cd09-0410-ab3c-d52691b4dbfc
parent 3feb4c33
2008-06-15 Darin Adler <darin@apple.com>
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 <darin@apple.com> 2008-06-15 Darin Adler <darin@apple.com>
Reviewed and tweaked by Sam Weinig. Reviewed and tweaked by Sam Weinig.
...@@ -532,6 +532,12 @@ void IconDatabase::setIconDataForIconURL(PassRefPtr<SharedBuffer> dataOriginal, ...@@ -532,6 +532,12 @@ void IconDatabase::setIconDataForIconURL(PassRefPtr<SharedBuffer> dataOriginal,
MutexLocker locker(m_pendingSyncLock); MutexLocker locker(m_pendingSyncLock);
m_iconsPendingSync.set(iconURL, icon->snapshot()); 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 // Send notification out regarding all PageURLs that retain this icon
...@@ -605,7 +611,7 @@ void IconDatabase::setIconURLForPageURL(const String& iconURLOriginal, const Str ...@@ -605,7 +611,7 @@ void IconDatabase::setIconURLForPageURL(const String& iconURLOriginal, const Str
MutexLocker locker(m_pendingSyncLock); MutexLocker locker(m_pendingSyncLock);
m_pageURLsPendingSync.set(pageURL, pageRecord->snapshot()); 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()) if (iconRecord && iconRecord->hasOneRef())
m_iconsPendingSync.set(iconRecord->iconURL(), iconRecord->snapshot(true)); m_iconsPendingSync.set(iconRecord->iconURL(), iconRecord->snapshot(true));
} }
...@@ -1530,15 +1536,12 @@ bool IconDatabase::writeToDatabase() ...@@ -1530,15 +1536,12 @@ bool IconDatabase::writeToDatabase()
} }
for (unsigned i = 0; i < pageSnapshots.size(); ++i) { 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 // 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 // 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()) if (pageSnapshots[i].iconURL.isEmpty())
removePageURLFromSQLDatabase(pageSnapshots[i].pageURL); removePageURLFromSQLDatabase(pageSnapshots[i].pageURL);
else { else
setIconURLForPageURLInSQLDatabase(pageSnapshots[i].iconURL, pageSnapshots[i].pageURL); setIconURLForPageURLInSQLDatabase(pageSnapshots[i].iconURL, pageSnapshots[i].pageURL);
}
LOG(IconDatabase, "Committed IconURL for PageURL %s to database", urlForLogging(pageSnapshots[i].pageURL).ascii().data()); LOG(IconDatabase, "Committed IconURL for PageURL %s to database", urlForLogging(pageSnapshots[i].pageURL).ascii().data());
} }
......
Markdown is supported
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment