diff options
Diffstat (limited to 'Source/WebCore/loader/icon/IconDatabase.cpp')
| -rw-r--r-- | Source/WebCore/loader/icon/IconDatabase.cpp | 559 |
1 files changed, 242 insertions, 317 deletions
diff --git a/Source/WebCore/loader/icon/IconDatabase.cpp b/Source/WebCore/loader/icon/IconDatabase.cpp index 39272e4d6..4cdcfaad6 100644 --- a/Source/WebCore/loader/icon/IconDatabase.cpp +++ b/Source/WebCore/loader/icon/IconDatabase.cpp @@ -11,10 +11,10 @@ * notice, this list of conditions and the following disclaimer in the * documentation and/or other materials provided with the distribution. * - * THIS SOFTWARE IS PROVIDED BY APPLE COMPUTER, INC. ``AS IS'' AND ANY + * THIS SOFTWARE IS PROVIDED BY APPLE INC. ``AS IS'' AND ANY * EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE * IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR - * PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL APPLE COMPUTER, INC. OR + * PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL APPLE INC. OR * CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, * EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, * PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR @@ -34,16 +34,14 @@ #include "IconDatabaseClient.h" #include "IconRecord.h" #include "Image.h" -#include "IntSize.h" #include "Logging.h" #include "SQLiteStatement.h" #include "SQLiteTransaction.h" #include "SuddenTermination.h" #include <wtf/AutodrainedPool.h> -#include <wtf/CurrentTime.h> #include <wtf/MainThread.h> +#include <wtf/NeverDestroyed.h> #include <wtf/StdLibExtras.h> -#include <wtf/text/CString.h> // For methods that are meant to support API from the main thread - should not be called internally #define ASSERT_NOT_SYNC_THREAD() ASSERT(!m_syncThreadRunning || !IS_ICON_SYNC_THREAD()) @@ -82,7 +80,7 @@ static const int notUsedIconExpirationTime = 60*60*24*30; #if !LOG_DISABLED || !ERROR_DISABLED static String urlForLogging(const String& url) { - static unsigned urlTruncationLength = 120; + static const unsigned urlTruncationLength = 120; if (url.length() < urlTruncationLength) return url; @@ -90,19 +88,19 @@ static String urlForLogging(const String& url) } #endif -class DefaultIconDatabaseClient : public IconDatabaseClient { +class DefaultIconDatabaseClient final : public IconDatabaseClient { WTF_MAKE_FAST_ALLOCATED; public: - virtual void didImportIconURLForPageURL(const String&) { } - virtual void didImportIconDataForPageURL(const String&) { } - virtual void didChangeIconForPageURL(const String&) { } - virtual void didRemoveAllIcons() { } - virtual void didFinishURLImport() { } + void didImportIconURLForPageURL(const String&) override { } + void didImportIconDataForPageURL(const String&) override { } + void didChangeIconForPageURL(const String&) override { } + void didRemoveAllIcons() override { } + void didFinishURLImport() override { } }; static IconDatabaseClient* defaultClient() { - static IconDatabaseClient* defaultClient = new DefaultIconDatabaseClient(); + static IconDatabaseClient* defaultClient = new DefaultIconDatabaseClient; return defaultClient; } @@ -171,7 +169,11 @@ void IconDatabase::close() m_removeIconsRequested = false; m_syncDB.close(); - ASSERT(!isOpen()); + + // If there are still main thread callbacks in flight then the database might not actually be closed yet. + // But if it is closed, notify the client now. + if (!isOpen() && m_client) + m_client->didClose(); } void IconDatabase::removeAllIcons() @@ -185,28 +187,26 @@ void IconDatabase::removeAllIcons() // Clear the in-memory record of every IconRecord, anything waiting to be read from disk, and anything waiting to be written to disk { - MutexLocker locker(m_urlAndIconLock); + LockHolder locker(m_urlAndIconLock); // Clear the IconRecords for every page URL - RefCounting will cause the IconRecords themselves to be deleted // We don't delete the actual PageRecords because we have the "retain icon for url" count to keep track of - HashMap<String, PageURLRecord*>::iterator iter = m_pageURLToRecordMap.begin(); - HashMap<String, PageURLRecord*>::iterator end = m_pageURLToRecordMap.end(); - for (; iter != end; ++iter) - (*iter).value->setIconRecord(0); - + for (auto& pageURL : m_pageURLToRecordMap.values()) + pageURL->setIconRecord(nullptr); + // Clear the iconURL -> IconRecord map m_iconURLToRecordMap.clear(); // Clear all in-memory records of things that need to be synced out to disk { - MutexLocker locker(m_pendingSyncLock); + LockHolder locker(m_pendingSyncLock); m_pageURLsPendingSync.clear(); m_iconsPendingSync.clear(); } // Clear all in-memory records of things that need to be read in from disk { - MutexLocker locker(m_pendingReadingLock); + LockHolder locker(m_pendingReadingLock); m_pageURLsPendingImport.clear(); m_pageURLsInterestedInIcons.clear(); m_iconsPendingReading.clear(); @@ -226,9 +226,9 @@ Image* IconDatabase::synchronousIconForPageURL(const String& pageURLOriginal, co // We should go our of our way to only copy it if we have to store it if (!isOpen() || !documentCanHaveIcon(pageURLOriginal)) - return 0; + return nullptr; - MutexLocker locker(m_urlAndIconLock); + LockHolder locker(m_urlAndIconLock); performPendingRetainAndReleaseOperations(); @@ -244,14 +244,14 @@ Image* IconDatabase::synchronousIconForPageURL(const String& pageURLOriginal, co // 1 - The initial url import is incomplete and this pageURL was marked to be notified once it is complete if an iconURL exists // 2 - The initial url import IS complete and this pageURL has no icon if (!pageRecord) { - MutexLocker locker(m_pendingReadingLock); + LockHolder locker(m_pendingReadingLock); // Import is ongoing, there might be an icon. In this case, register to be notified when the icon comes in // If we ever reach this condition, we know we've already made the pageURL copy if (!m_iconURLImportComplete) m_pageURLsInterestedInIcons.add(pageURLCopy); - return 0; + return nullptr; } IconRecord* iconRecord = pageRecord->iconRecord(); @@ -260,14 +260,14 @@ Image* IconDatabase::synchronousIconForPageURL(const String& pageURLOriginal, co // In this case, the pageURL is already in the set to alert the client when the iconURL mapping is complete so // we can just bail now if (!m_iconURLImportComplete && !iconRecord) - return 0; + return nullptr; // Assuming we're done initializing and cleanup is allowed, // the only way we should *not* have an icon record is if this pageURL is retained but has no icon yet. ASSERT(iconRecord || databaseCleanupCounter || m_retainedPageURLs.contains(pageURLOriginal)); if (!iconRecord) - return 0; + return nullptr; // If it's a new IconRecord object that doesn't have its imageData set yet, // mark it to be read by the background thread @@ -275,17 +275,17 @@ Image* IconDatabase::synchronousIconForPageURL(const String& pageURLOriginal, co if (pageURLCopy.isNull()) pageURLCopy = pageURLOriginal.isolatedCopy(); - MutexLocker locker(m_pendingReadingLock); + LockHolder locker(m_pendingReadingLock); m_pageURLsInterestedInIcons.add(pageURLCopy); m_iconsPendingReading.add(iconRecord); wakeSyncThread(); - return 0; + return nullptr; } // If the size parameter was (0, 0) that means the caller of this method just wanted the read from disk to be kicked off // and isn't actually interested in the image return value if (size == IntSize(0, 0)) - return 0; + return nullptr; // PARANOID DISCUSSION: This method makes some assumptions. It returns a WebCore::image which the icon database might dispose of at anytime in the future, // and Images aren't ref counted. So there is no way for the client to guarantee continued existence of the image. @@ -300,13 +300,13 @@ Image* IconDatabase::synchronousIconForPageURL(const String& pageURLOriginal, co return iconRecord->image(size); } -PassNativeImagePtr IconDatabase::synchronousNativeIconForPageURL(const String& pageURLOriginal, const IntSize& size) +NativeImagePtr IconDatabase::synchronousNativeIconForPageURL(const String& pageURLOriginal, const IntSize& size) { Image* icon = synchronousIconForPageURL(pageURLOriginal, size); if (!icon) - return 0; + return nullptr; - MutexLocker locker(m_urlAndIconLock); + LockHolder locker(m_urlAndIconLock); return icon->nativeImageForCurrentFrame(); } @@ -328,7 +328,7 @@ String IconDatabase::synchronousIconURLForPageURL(const String& pageURLOriginal) if (!isOpen() || !documentCanHaveIcon(pageURLOriginal)) return String(); - MutexLocker locker(m_urlAndIconLock); + LockHolder locker(m_urlAndIconLock); PageURLRecord* pageRecord = m_pageURLToRecordMap.get(pageURLOriginal); if (!pageRecord) @@ -390,8 +390,8 @@ static inline void loadDefaultIconRecord(IconRecord* defaultIconRecord) 0x00, 0x00, 0x01, 0x52, 0x00, 0x03, 0x00, 0x00, 0x00, 0x01, 0x00, 0x01, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x08, 0x00, 0x08, 0x00, 0x08, 0x00, 0x08, 0x00, 0x0A, 0xFC, 0x80, 0x00, 0x00, 0x27, 0x10, 0x00, 0x0A, 0xFC, 0x80, 0x00, 0x00, 0x27, 0x10 }; - static SharedBuffer* defaultIconBuffer = SharedBuffer::create(defaultIconData, sizeof(defaultIconData)).leakRef(); - defaultIconRecord->setImageData(defaultIconBuffer); + static auto& defaultIconBuffer = SharedBuffer::create(defaultIconData, sizeof(defaultIconData)).leakRef(); + defaultIconRecord->setImageData(&defaultIconBuffer); } #endif @@ -416,7 +416,7 @@ void IconDatabase::retainIconForPageURL(const String& pageURL) return; { - MutexLocker locker(m_urlsToRetainOrReleaseLock); + LockHolder locker(m_urlsToRetainOrReleaseLock); m_urlsToRetain.add(pageURL.isolatedCopy()); m_retainOrReleaseIconRequested = true; } @@ -449,7 +449,7 @@ void IconDatabase::performRetainIconForPageURL(const String& pageURLOriginal, in if (!m_iconURLImportComplete) return; - MutexLocker locker(m_pendingSyncLock); + LockHolder locker(m_pendingSyncLock); // If this pageURL waiting to be sync'ed, update the sync record // This saves us in the case where a page was ready to be deleted from the database but was just retained - so theres no need to delete it! if (!m_privateBrowsingEnabled && m_pageURLsPendingSync.contains(pageURL)) { @@ -469,7 +469,7 @@ void IconDatabase::releaseIconForPageURL(const String& pageURL) return; { - MutexLocker locker(m_urlsToRetainOrReleaseLock); + LockHolder locker(m_urlsToRetainOrReleaseLock); m_urlsToRelease.add(pageURL.isolatedCopy()); m_retainOrReleaseIconRequested = true; } @@ -505,7 +505,7 @@ void IconDatabase::performReleaseIconForPageURL(const String& pageURLOriginal, i ASSERT(!iconRecord || (iconRecord && m_iconURLToRecordMap.get(iconRecord->iconURL()) == iconRecord)); { - MutexLocker locker(m_pendingReadingLock); + LockHolder locker(m_pendingReadingLock); // Since this pageURL is going away, there's no reason anyone would ever be interested in its read results if (!m_iconURLImportComplete) @@ -521,7 +521,7 @@ void IconDatabase::performReleaseIconForPageURL(const String& pageURLOriginal, i // Mark stuff for deletion from the database only if we're not in private browsing if (!m_privateBrowsingEnabled) { - MutexLocker locker(m_pendingSyncLock); + LockHolder locker(m_pendingSyncLock); m_pageURLsPendingSync.set(pageURLOriginal.isolatedCopy(), pageRecord->snapshot(true)); // If this page is the last page to refer to a particular IconRecord, that IconRecord needs to @@ -533,7 +533,7 @@ void IconDatabase::performReleaseIconForPageURL(const String& pageURLOriginal, i delete pageRecord; } -void IconDatabase::setIconDataForIconURL(PassRefPtr<SharedBuffer> dataOriginal, const String& iconURLOriginal) +void IconDatabase::setIconDataForIconURL(SharedBuffer* dataOriginal, const String& iconURLOriginal) { ASSERT_NOT_SYNC_THREAD(); @@ -542,23 +542,23 @@ void IconDatabase::setIconDataForIconURL(PassRefPtr<SharedBuffer> dataOriginal, if (!isOpen() || iconURLOriginal.isEmpty()) return; - RefPtr<SharedBuffer> data = dataOriginal ? dataOriginal->copy() : PassRefPtr<SharedBuffer>(0); + auto data = dataOriginal ? RefPtr<SharedBuffer> { dataOriginal->copy() } : nullptr; String iconURL = iconURLOriginal.isolatedCopy(); Vector<String> pageURLs; { - MutexLocker locker(m_urlAndIconLock); + LockHolder locker(m_urlAndIconLock); // If this icon was pending a read, remove it from that set because this new data should override what is on disk RefPtr<IconRecord> icon = m_iconURLToRecordMap.get(iconURL); if (icon) { - MutexLocker locker(m_pendingReadingLock); + LockHolder locker(m_pendingReadingLock); m_iconsPendingReading.remove(icon.get()); } else icon = getOrCreateIconRecord(iconURL); // Update the data and set the time stamp - icon->setImageData(data.release()); + icon->setImageData(WTFMove(data)); icon->setTimestamp((int)currentTime()); // Copy the current retaining pageURLs - if any - to notify them of the change @@ -566,7 +566,7 @@ void IconDatabase::setIconDataForIconURL(PassRefPtr<SharedBuffer> dataOriginal, // Mark the IconRecord as requiring an update to the database only if private browsing is disabled if (!m_privateBrowsingEnabled) { - MutexLocker locker(m_pendingSyncLock); + LockHolder locker(m_pendingSyncLock); m_iconsPendingSync.set(iconURL, icon->snapshot()); } @@ -584,11 +584,11 @@ void IconDatabase::setIconDataForIconURL(PassRefPtr<SharedBuffer> dataOriginal, // Start the timer to commit this change - or further delay the timer if it was already started scheduleOrDeferSyncTimer(); - for (unsigned i = 0; i < pageURLs.size(); ++i) { + for (auto& pageURL : pageURLs) { AutodrainedPool pool; - LOG(IconDatabase, "Dispatching notification that retaining pageURL %s has a new icon", urlForLogging(pageURLs[i]).ascii().data()); - m_client->didChangeIconForPageURL(pageURLs[i]); + LOG(IconDatabase, "Dispatching notification that retaining pageURL %s has a new icon", urlForLogging(pageURL).ascii().data()); + m_client->didChangeIconForPageURL(pageURL); } } } @@ -607,7 +607,7 @@ void IconDatabase::setIconURLForPageURL(const String& iconURLOriginal, const Str String iconURL, pageURL; { - MutexLocker locker(m_urlAndIconLock); + LockHolder locker(m_urlAndIconLock); PageURLRecord* pageRecord = m_pageURLToRecordMap.get(pageURLOriginal); @@ -635,13 +635,13 @@ void IconDatabase::setIconURLForPageURL(const String& iconURLOriginal, const Str ASSERT(iconRecord->retainingPageURLs().size() == 0); LOG(IconDatabase, "Icon for icon url %s is about to be destroyed - removing mapping for it", urlForLogging(iconRecord->iconURL()).ascii().data()); m_iconURLToRecordMap.remove(iconRecord->iconURL()); - MutexLocker locker(m_pendingReadingLock); + LockHolder locker(m_pendingReadingLock); m_iconsPendingReading.remove(iconRecord.get()); } // And mark this mapping to be added to the database if (!m_privateBrowsingEnabled) { - MutexLocker locker(m_pendingSyncLock); + LockHolder locker(m_pendingSyncLock); m_pageURLsPendingSync.set(pageURL, pageRecord->snapshot()); // If the icon is on its last ref, mark it for deletion @@ -673,7 +673,7 @@ IconLoadDecision IconDatabase::synchronousLoadDecisionForIconURL(const String& i // 1 - When we read the icon urls from disk, getting the timeStamp at the same time // 2 - When we get a new icon from the loader, in which case the timestamp is set at that time { - MutexLocker locker(m_urlAndIconLock); + LockHolder locker(m_urlAndIconLock); if (IconRecord* icon = m_iconURLToRecordMap.get(iconURL)) { LOG(IconDatabase, "Found expiration time on a present icon based on existing IconRecord"); return static_cast<int>(currentTime()) - static_cast<int>(icon->getTimestamp()) > iconExpirationTime ? IconLoadYes : IconLoadNo; @@ -681,7 +681,7 @@ IconLoadDecision IconDatabase::synchronousLoadDecisionForIconURL(const String& i } // If we don't have a record for it, but we *have* imported all iconURLs from disk, then we should load it now - MutexLocker readingLocker(m_pendingReadingLock); + LockHolder readingLocker(m_pendingReadingLock); if (m_iconURLImportComplete) return IconLoadYes; @@ -698,7 +698,7 @@ bool IconDatabase::synchronousIconDataKnownForIconURL(const String& iconURL) { ASSERT_NOT_SYNC_THREAD(); - MutexLocker locker(m_urlAndIconLock); + LockHolder locker(m_urlAndIconLock); if (IconRecord* icon = m_iconURLToRecordMap.get(iconURL)) return icon->imageDataStatus() != ImageDataStatusUnknown; @@ -753,39 +753,36 @@ void IconDatabase::checkIntegrityBeforeOpening() size_t IconDatabase::pageURLMappingCount() { - MutexLocker locker(m_urlAndIconLock); + LockHolder locker(m_urlAndIconLock); return m_pageURLToRecordMap.size(); } size_t IconDatabase::retainedPageURLCount() { - MutexLocker locker(m_urlAndIconLock); + LockHolder locker(m_urlAndIconLock); performPendingRetainAndReleaseOperations(); return m_retainedPageURLs.size(); } size_t IconDatabase::iconRecordCount() { - MutexLocker locker(m_urlAndIconLock); + LockHolder locker(m_urlAndIconLock); return m_iconURLToRecordMap.size(); } size_t IconDatabase::iconRecordCountWithData() { - MutexLocker locker(m_urlAndIconLock); + LockHolder locker(m_urlAndIconLock); size_t result = 0; - - HashMap<String, IconRecord*>::iterator i = m_iconURLToRecordMap.begin(); - HashMap<String, IconRecord*>::iterator end = m_iconURLToRecordMap.end(); - - for (; i != end; ++i) - result += ((*i).value->imageDataStatus() == ImageDataStatusPresent); - + + for (auto& iconRecord : m_iconURLToRecordMap.values()) + result += (iconRecord->imageDataStatus() == ImageDataStatusPresent); + return result; } IconDatabase::IconDatabase() - : m_syncTimer(this, &IconDatabase::syncTimerFired) + : m_syncTimer(*this, &IconDatabase::syncTimerFired) , m_syncThreadRunning(false) , m_scheduleOrDeferSyncTimerRequested(false) , m_isEnabled(false) @@ -796,6 +793,7 @@ IconDatabase::IconDatabase() , m_syncThreadHasWorkToDo(false) , m_retainOrReleaseIconRequested(false) , m_initialPruningComplete(false) + , m_mainThreadCallbackCount(0) , m_client(defaultClient()) { LOG(IconDatabase, "Creating IconDatabase %p", this); @@ -807,11 +805,6 @@ IconDatabase::~IconDatabase() ASSERT(!isOpen()); } -void IconDatabase::notifyPendingLoadDecisionsOnMainThread(void* context) -{ - static_cast<IconDatabase*>(context)->notifyPendingLoadDecisions(); -} - void IconDatabase::notifyPendingLoadDecisions() { ASSERT_NOT_SYNC_THREAD(); @@ -819,37 +812,24 @@ void IconDatabase::notifyPendingLoadDecisions() // This method should only be called upon completion of the initial url import from the database ASSERT(m_iconURLImportComplete); LOG(IconDatabase, "Notifying all DocumentLoaders that were waiting on a load decision for their icons"); - - HashSet<RefPtr<DocumentLoader>>::iterator i = m_loadersPendingDecision.begin(); - HashSet<RefPtr<DocumentLoader>>::iterator end = m_loadersPendingDecision.end(); - - for (; i != end; ++i) - if ((*i)->refCount() > 1) - (*i)->iconLoadDecisionAvailable(); - + + for (auto& loader : m_loadersPendingDecision) { + if (loader->refCount() > 1) + loader->iconLoadDecisionAvailable(); + } + m_loadersPendingDecision.clear(); } void IconDatabase::wakeSyncThread() { - MutexLocker locker(m_syncLock); + LockHolder locker(m_syncLock); if (!m_disableSuddenTerminationWhileSyncThreadHasWorkToDo) m_disableSuddenTerminationWhileSyncThreadHasWorkToDo = std::make_unique<SuddenTerminationDisabler>(); m_syncThreadHasWorkToDo = true; - m_syncCondition.signal(); -} - -void IconDatabase::performScheduleOrDeferSyncTimer() -{ - m_syncTimer.startOneShot(updateTimerDelay); - m_scheduleOrDeferSyncTimerRequested = false; -} - -void IconDatabase::performScheduleOrDeferSyncTimerOnMainThread(void* context) -{ - static_cast<IconDatabase*>(context)->performScheduleOrDeferSyncTimer(); + m_syncCondition.notifyOne(); } void IconDatabase::scheduleOrDeferSyncTimer() @@ -863,10 +843,13 @@ void IconDatabase::scheduleOrDeferSyncTimer() m_disableSuddenTerminationWhileSyncTimerScheduled = std::make_unique<SuddenTerminationDisabler>(); m_scheduleOrDeferSyncTimerRequested = true; - callOnMainThread(performScheduleOrDeferSyncTimerOnMainThread, this); + callOnMainThread([this] { + m_syncTimer.startOneShot(updateTimerDelay); + m_scheduleOrDeferSyncTimerRequested = false; + }); } -void IconDatabase::syncTimerFired(Timer<IconDatabase>&) +void IconDatabase::syncTimerFired() { ASSERT_NOT_SYNC_THREAD(); wakeSyncThread(); @@ -880,35 +863,39 @@ void IconDatabase::syncTimerFired(Timer<IconDatabase>&) bool IconDatabase::isOpen() const { - MutexLocker locker(m_syncLock); - return m_syncDB.isOpen(); + return isOpenBesidesMainThreadCallbacks() || m_mainThreadCallbackCount; +} + +bool IconDatabase::isOpenBesidesMainThreadCallbacks() const +{ + LockHolder locker(m_syncLock); + return m_syncThreadRunning || m_syncDB.isOpen(); } String IconDatabase::databasePath() const { - MutexLocker locker(m_syncLock); + LockHolder locker(m_syncLock); return m_completeDatabasePath.isolatedCopy(); } String IconDatabase::defaultDatabaseFilename() { - DEFINE_STATIC_LOCAL(String, defaultDatabaseFilename, (ASCIILiteral("WebpageIcons.db"))); - return defaultDatabaseFilename.isolatedCopy(); + static NeverDestroyed<String> defaultDatabaseFilename(ASCIILiteral("WebpageIcons.db")); + return defaultDatabaseFilename.get().isolatedCopy(); } // Unlike getOrCreatePageURLRecord(), getOrCreateIconRecord() does not mark the icon as "interested in import" -PassRefPtr<IconRecord> IconDatabase::getOrCreateIconRecord(const String& iconURL) +Ref<IconRecord> IconDatabase::getOrCreateIconRecord(const String& iconURL) { // Clients of getOrCreateIconRecord() are required to acquire the m_urlAndIconLock before calling this method ASSERT(!m_urlAndIconLock.tryLock()); - if (IconRecord* icon = m_iconURLToRecordMap.get(iconURL)) - return icon; + if (auto* icon = m_iconURLToRecordMap.get(iconURL)) + return *icon; - RefPtr<IconRecord> newIcon = IconRecord::create(iconURL); - m_iconURLToRecordMap.set(iconURL, newIcon.get()); - - return newIcon.release(); + auto newIcon = IconRecord::create(iconURL); + m_iconURLToRecordMap.set(iconURL, newIcon.ptr()); + return newIcon; } // This method retrieves the existing PageURLRecord, or creates a new one and marks it as "interested in the import" for later notification @@ -918,11 +905,11 @@ PageURLRecord* IconDatabase::getOrCreatePageURLRecord(const String& pageURL) ASSERT(!m_urlAndIconLock.tryLock()); if (!documentCanHaveIcon(pageURL)) - return 0; + return nullptr; PageURLRecord* pageRecord = m_pageURLToRecordMap.get(pageURL); - MutexLocker locker(m_pendingReadingLock); + LockHolder locker(m_pendingReadingLock); if (!m_iconURLImportComplete) { // If the initial import of all URLs hasn't completed and we have no page record, we assume we *might* know about this later and create a record for it if (!pageRecord) { @@ -935,7 +922,7 @@ PageURLRecord* IconDatabase::getOrCreatePageURLRecord(const String& pageURL) // Mark the URL as "interested in the result of the import" then bail if (!pageRecord->iconRecord()) { m_pageURLsPendingImport.add(pageURL); - return 0; + return nullptr; } } @@ -991,7 +978,7 @@ void IconDatabase::iconDatabaseSyncThread() } { - MutexLocker locker(m_syncLock); + LockHolder locker(m_syncLock); if (!m_syncDB.open(m_completeDatabasePath)) { LOG_ERROR("Unable to open icon database at path %s - %s", m_completeDatabasePath.ascii().data(), m_syncDB.lastErrorMsg()); return; @@ -1119,7 +1106,7 @@ void IconDatabase::performOpenInitialization() m_syncDB.close(); { - MutexLocker locker(m_syncLock); + LockHolder locker(m_syncLock); // Should've been consumed by SQLite, delete just to make sure we don't see it again in the future; deleteFile(m_completeDatabasePath + "-journal"); deleteFile(m_completeDatabasePath); @@ -1165,16 +1152,16 @@ bool IconDatabase::checkIntegrity() ASSERT_ICON_SYNC_THREAD(); SQLiteStatement integrity(m_syncDB, "PRAGMA integrity_check;"); - if (integrity.prepare() != SQLResultOk) { + if (integrity.prepare() != SQLITE_OK) { LOG_ERROR("checkIntegrity failed to execute"); return false; } int resultCode = integrity.step(); - if (resultCode == SQLResultOk) + if (resultCode == SQLITE_OK) return true; - - if (resultCode != SQLResultRow) + + if (resultCode != SQLITE_ROW) return false; int columns = integrity.columnCount(); @@ -1209,25 +1196,25 @@ void IconDatabase::performURLImport() SQLiteStatement query(m_syncDB, importQuery); - if (query.prepare() != SQLResultOk) { + if (query.prepare() != SQLITE_OK) { LOG_ERROR("Unable to prepare icon url import query"); return; } int result = query.step(); - while (result == SQLResultRow) { + while (result == SQLITE_ROW) { AutodrainedPool pool; String pageURL = query.getColumnText(0); String iconURL = query.getColumnText(1); { - MutexLocker locker(m_urlAndIconLock); + LockHolder locker(m_urlAndIconLock); PageURLRecord* pageRecord = m_pageURLToRecordMap.get(pageURL); // If the pageRecord doesn't exist in this map, then no one has retained this pageURL // If the s_databaseCleanupCounter count is non-zero, then we're not supposed to be pruning the database in any manner, - // so go ahead and actually create a pageURLRecord for this url even though it's not retained. + // so actually create a pageURLRecord for this url even though it's not retained. // If database cleanup *is* allowed, we don't want to bother pulling in a page url from disk that noone is actually interested // in - we'll prune it later instead! if (!pageRecord && databaseCleanupCounter && documentCanHaveIcon(pageURL)) { @@ -1254,7 +1241,7 @@ void IconDatabase::performURLImport() // one for the URL and one for the Image itself // Note that WebIconDatabase is not neccessarily API so we might be able to make this change { - MutexLocker locker(m_pendingReadingLock); + LockHolder locker(m_pendingReadingLock); if (m_pageURLsPendingImport.contains(pageURL)) { dispatchDidImportIconURLForPageURLOnMainThread(pageURL); m_pageURLsPendingImport.remove(pageURL); @@ -1270,14 +1257,14 @@ void IconDatabase::performURLImport() result = query.step(); } - if (result != SQLResultDone) + if (result != SQLITE_DONE) LOG(IconDatabase, "Error reading page->icon url mappings from database"); // Clear the m_pageURLsPendingImport set - either the page URLs ended up with an iconURL (that we'll notify about) or not, // but after m_iconURLImportComplete is set to true, we don't care about this set anymore Vector<String> urls; { - MutexLocker locker(m_pendingReadingLock); + LockHolder locker(m_pendingReadingLock); urls.appendRange(m_pageURLsPendingImport.begin(), m_pageURLsPendingImport.end()); m_pageURLsPendingImport.clear(); @@ -1290,15 +1277,15 @@ void IconDatabase::performURLImport() // Remove unretained ones if database cleanup is allowed // Keep a set of ones that are retained and pending notification { - MutexLocker locker(m_urlAndIconLock); + LockHolder locker(m_urlAndIconLock); performPendingRetainAndReleaseOperations(); - for (unsigned i = 0; i < urls.size(); ++i) { - if (!m_retainedPageURLs.contains(urls[i])) { - PageURLRecord* record = m_pageURLToRecordMap.get(urls[i]); + for (auto& url : urls) { + if (!m_retainedPageURLs.contains(url)) { + PageURLRecord* record = m_pageURLToRecordMap.get(url); if (record && !databaseCleanupCounter) { - m_pageURLToRecordMap.remove(urls[i]); + m_pageURLToRecordMap.remove(url); IconRecord* iconRecord = record->iconRecord(); // If this page is the only remaining retainer of its icon, mark that icon for deletion and don't bother @@ -1307,12 +1294,12 @@ void IconDatabase::performURLImport() m_iconURLToRecordMap.remove(iconRecord->iconURL()); { - MutexLocker locker(m_pendingReadingLock); - m_pageURLsInterestedInIcons.remove(urls[i]); + LockHolder locker(m_pendingReadingLock); + m_pageURLsInterestedInIcons.remove(url); m_iconsPendingReading.remove(iconRecord); } { - MutexLocker locker(m_pendingSyncLock); + LockHolder locker(m_pendingSyncLock); m_iconsPendingSync.set(iconRecord->iconURL(), iconRecord->snapshot(true)); } } @@ -1320,18 +1307,18 @@ void IconDatabase::performURLImport() delete record; } } else { - urlsToNotify.append(urls[i]); + urlsToNotify.append(url); } } } LOG(IconDatabase, "Notifying %lu interested page URLs that their icon URL is known due to the import", static_cast<unsigned long>(urlsToNotify.size())); // Now that we don't hold any locks, perform the actual notifications - for (unsigned i = 0; i < urlsToNotify.size(); ++i) { + for (auto& url : urlsToNotify) { AutodrainedPool pool; - LOG(IconDatabase, "Notifying icon info known for pageURL %s", urlsToNotify[i].ascii().data()); - dispatchDidImportIconURLForPageURLOnMainThread(urlsToNotify[i]); + LOG(IconDatabase, "Notifying icon info known for pageURL %s", url.ascii().data()); + dispatchDidImportIconURLForPageURLOnMainThread(url); if (shouldStopThreadActivity()) return; } @@ -1340,7 +1327,9 @@ void IconDatabase::performURLImport() dispatchDidFinishURLImportOnMainThread(); // Notify all DocumentLoaders that were waiting for an icon load decision on the main thread - callOnMainThread(notifyPendingLoadDecisionsOnMainThread, this); + callOnMainThread([this] { + notifyPendingLoadDecisions(); + }); } void IconDatabase::syncThreadMainLoop() @@ -1349,7 +1338,7 @@ void IconDatabase::syncThreadMainLoop() m_syncLock.lock(); - std::unique_ptr<SuddenTerminationDisabler> disableSuddenTermination = std::move(m_disableSuddenTerminationWhileSyncThreadHasWorkToDo); + std::unique_ptr<SuddenTerminationDisabler> disableSuddenTermination = WTFMove(m_disableSuddenTerminationWhileSyncThreadHasWorkToDo); // We'll either do any pending work on our first pass through the loop, or we'll terminate // without doing any work. Either way we're dealing with any currently-pending work. @@ -1371,11 +1360,13 @@ void IconDatabase::syncThreadMainLoop() } // Then, if the thread should be quitting, quit now! - if (m_threadTerminationRequested) - break; + if (m_threadTerminationRequested) { + cleanupSyncThread(); + return; + } { - MutexLocker locker(m_urlAndIconLock); + LockHolder locker(m_urlAndIconLock); performPendingRetainAndReleaseOperations(); } @@ -1437,7 +1428,7 @@ void IconDatabase::syncThreadMainLoop() m_syncThreadHasWorkToDo = false; ASSERT(m_disableSuddenTerminationWhileSyncThreadHasWorkToDo); - disableSuddenTermination = std::move(m_disableSuddenTerminationWhileSyncThreadHasWorkToDo); + disableSuddenTermination = WTFMove(m_disableSuddenTerminationWhileSyncThreadHasWorkToDo); } m_syncLock.unlock(); @@ -1455,7 +1446,7 @@ void IconDatabase::performPendingRetainAndReleaseOperations() HashCountedSet<String> toRelease; { - MutexLocker pendingWorkLocker(m_urlsToRetainOrReleaseLock); + LockHolder pendingWorkLocker(m_urlsToRetainOrReleaseLock); if (!m_retainOrReleaseIconRequested) return; @@ -1467,14 +1458,14 @@ void IconDatabase::performPendingRetainAndReleaseOperations() m_retainOrReleaseIconRequested = false; } - for (HashCountedSet<String>::const_iterator it = toRetain.begin(), end = toRetain.end(); it != end; ++it) { - ASSERT(!it->key.impl() || it->key.impl()->hasOneRef()); - performRetainIconForPageURL(it->key, it->value); + for (auto& entry : toRetain) { + ASSERT(!entry.key.impl() || entry.key.impl()->hasOneRef()); + performRetainIconForPageURL(entry.key, entry.value); } - for (HashCountedSet<String>::const_iterator it = toRelease.begin(), end = toRelease.end(); it != end; ++it) { - ASSERT(!it->key.impl() || it->key.impl()->hasOneRef()); - performReleaseIconForPageURL(it->key, it->value); + for (auto& entry : toRelease) { + ASSERT(!entry.key.impl() || entry.key.impl()->hasOneRef()); + performReleaseIconForPageURL(entry.key, entry.value); } } @@ -1492,7 +1483,7 @@ bool IconDatabase::readFromDatabase() // This way we won't hold the lock for a long period of time Vector<IconRecord*> icons; { - MutexLocker locker(m_pendingReadingLock); + LockHolder locker(m_pendingReadingLock); icons.appendRange(m_iconsPendingReading.begin(), m_iconsPendingReading.end()); } @@ -1501,17 +1492,17 @@ bool IconDatabase::readFromDatabase() for (unsigned i = 0; i < icons.size(); ++i) { didAnyWork = true; - RefPtr<SharedBuffer> imageData = getImageDataForIconURLFromSQLDatabase(icons[i]->iconURL()); + auto imageData = getImageDataForIconURLFromSQLDatabase(icons[i]->iconURL()); // Verify this icon still wants to be read from disk { - MutexLocker urlLocker(m_urlAndIconLock); + LockHolder urlLocker(m_urlAndIconLock); { - MutexLocker readLocker(m_pendingReadingLock); + LockHolder readLocker(m_pendingReadingLock); if (m_iconsPendingReading.contains(icons[i])) { // Set the new data - icons[i]->setImageData(imageData.release()); + icons[i]->setImageData(WTFMove(imageData)); // Remove this icon from the set that needs to be read m_iconsPendingReading.remove(icons[i]); @@ -1530,12 +1521,10 @@ bool IconDatabase::readFromDatabase() outerHash = &(icons[i]->retainingPageURLs()); } - HashSet<String>::const_iterator iter = outerHash->begin(); - HashSet<String>::const_iterator end = outerHash->end(); - for (; iter != end; ++iter) { - if (innerHash->contains(*iter)) { - LOG(IconDatabase, "%s is interested in the icon we just read. Adding it to the notification list and removing it from the interested set", urlForLogging(*iter).ascii().data()); - urlsToNotify.add(*iter); + for (auto& outer : *outerHash) { + if (innerHash->contains(outer)) { + LOG(IconDatabase, "%s is interested in the icon we just read. Adding it to the notification list and removing it from the interested set", urlForLogging(outer).ascii().data()); + urlsToNotify.add(outer); } // If we ever get to the point were we've seen every url interested in this icon, break early @@ -1547,10 +1536,8 @@ bool IconDatabase::readFromDatabase() if (urlsToNotify.size() == m_pageURLsInterestedInIcons.size()) m_pageURLsInterestedInIcons.clear(); else { - iter = urlsToNotify.begin(); - end = urlsToNotify.end(); - for (; iter != end; ++iter) - m_pageURLsInterestedInIcons.remove(*iter); + for (auto& url : urlsToNotify) + m_pageURLsInterestedInIcons.remove(url); } } } @@ -1595,11 +1582,11 @@ bool IconDatabase::writeToDatabase() // we'll pick it up on the next pass. This greatly simplifies the locking strategy for this method and remains cohesive with changes // asked for by the database on the main thread { - MutexLocker locker(m_urlAndIconLock); + LockHolder locker(m_urlAndIconLock); Vector<IconSnapshot> iconSnapshots; Vector<PageURLSnapshot> pageSnapshots; { - MutexLocker locker(m_pendingSyncLock); + LockHolder locker(m_pendingSyncLock); iconSnapshots.appendRange(m_iconsPendingSync.begin().values(), m_iconsPendingSync.end().values()); m_iconsPendingSync.clear(); @@ -1614,19 +1601,19 @@ bool IconDatabase::writeToDatabase() SQLiteTransaction syncTransaction(m_syncDB); syncTransaction.begin(); - for (unsigned i = 0; i < iconSnapshots.size(); ++i) { - writeIconSnapshotToSQLDatabase(iconSnapshots[i]); - LOG(IconDatabase, "Wrote IconRecord for IconURL %s with timeStamp of %i to the DB", urlForLogging(iconSnapshots[i].iconURL()).ascii().data(), iconSnapshots[i].timestamp()); + for (auto& snapshot : iconSnapshots) { + writeIconSnapshotToSQLDatabase(snapshot); + LOG(IconDatabase, "Wrote IconRecord for IconURL %s with timeStamp of %i to the DB", urlForLogging(snapshot.iconURL()).ascii().data(), snapshot.timestamp()); } - for (unsigned i = 0; i < pageSnapshots.size(); ++i) { + for (auto& snapshot : pageSnapshots) { // 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()); + if (snapshot.iconURL().isEmpty()) + removePageURLFromSQLDatabase(snapshot.pageURL()); else - setIconURLForPageURLInSQLDatabase(pageSnapshots[i].iconURL(), pageSnapshots[i].pageURL()); - LOG(IconDatabase, "Committed IconURL for PageURL %s to database", urlForLogging(pageSnapshots[i].pageURL()).ascii().data()); + setIconURLForPageURLInSQLDatabase(snapshot.iconURL(), snapshot.pageURL()); + LOG(IconDatabase, "Committed IconURL for PageURL %s to database", urlForLogging(snapshot.pageURL()).ascii().data()); } syncTransaction.commit(); @@ -1661,13 +1648,13 @@ void IconDatabase::pruneUnretainedIcons() pageSQL.prepare(); int result; - while ((result = pageSQL.step()) == SQLResultRow) { - MutexLocker locker(m_urlAndIconLock); + while ((result = pageSQL.step()) == SQLITE_ROW) { + LockHolder locker(m_urlAndIconLock); if (!m_pageURLToRecordMap.contains(pageSQL.getColumnText(1))) pageIDsToDelete.append(pageSQL.getColumnInt64(0)); } - if (result != SQLResultDone) + if (result != SQLITE_DONE) LOG_ERROR("Error reading PageURL table from on-disk DB"); pageSQL.finalize(); @@ -1683,7 +1670,7 @@ void IconDatabase::pruneUnretainedIcons() LOG(IconDatabase, "Pruning page with rowid %lli from disk", static_cast<long long>(pageIDsToDelete[i])); pageDeleteSQL.bindInt64(1, pageIDsToDelete[i]); int result = pageDeleteSQL.step(); - if (result != SQLResultDone) + if (result != SQLITE_DONE) LOG_ERROR("Unabled to delete page with id %lli from disk", static_cast<long long>(pageIDsToDelete[i])); pageDeleteSQL.reset(); @@ -1762,20 +1749,20 @@ void IconDatabase::deleteAllPreparedStatements() { ASSERT_ICON_SYNC_THREAD(); - m_setIconIDForPageURLStatement.clear(); - m_removePageURLStatement.clear(); - m_getIconIDForIconURLStatement.clear(); - m_getImageDataForIconURLStatement.clear(); - m_addIconToIconInfoStatement.clear(); - m_addIconToIconDataStatement.clear(); - m_getImageDataStatement.clear(); - m_deletePageURLsForIconURLStatement.clear(); - m_deleteIconFromIconInfoStatement.clear(); - m_deleteIconFromIconDataStatement.clear(); - m_updateIconInfoStatement.clear(); - m_updateIconDataStatement.clear(); - m_setIconInfoStatement.clear(); - m_setIconDataStatement.clear(); + m_setIconIDForPageURLStatement = nullptr; + m_removePageURLStatement = nullptr; + m_getIconIDForIconURLStatement = nullptr; + m_getImageDataForIconURLStatement = nullptr; + m_addIconToIconInfoStatement = nullptr; + m_addIconToIconDataStatement = nullptr; + m_getImageDataStatement = nullptr; + m_deletePageURLsForIconURLStatement = nullptr; + m_deleteIconFromIconInfoStatement = nullptr; + m_deleteIconFromIconDataStatement = nullptr; + m_updateIconInfoStatement = nullptr; + m_updateIconDataStatement = nullptr; + m_setIconInfoStatement = nullptr; + m_setIconDataStatement = nullptr; } void* IconDatabase::cleanupSyncThread() @@ -1795,7 +1782,7 @@ void* IconDatabase::cleanupSyncThread() writeToDatabase(); // Close the database - MutexLocker locker(m_syncLock); + LockHolder locker(m_syncLock); m_databaseDirectory = String(); m_completeDatabasePath = String(); @@ -1807,23 +1794,23 @@ void* IconDatabase::cleanupSyncThread() #endif m_syncThreadRunning = false; - return 0; + return nullptr; } // readySQLiteStatement() handles two things // 1 - If the SQLDatabase& argument is different, the statement must be destroyed and remade. This happens when the user // switches to and from private browsing // 2 - Lazy construction of the Statement in the first place, in case we've never made this query before -inline void readySQLiteStatement(OwnPtr<SQLiteStatement>& statement, SQLiteDatabase& db, const String& str) +inline void readySQLiteStatement(std::unique_ptr<SQLiteStatement>& statement, SQLiteDatabase& db, const String& str) { - if (statement && (statement->database() != &db || statement->isExpired())) { + if (statement && (&statement->database() != &db || statement->isExpired())) { if (statement->isExpired()) LOG(IconDatabase, "SQLiteStatement associated with %s is expired", str.ascii().data()); - statement.clear(); + statement = nullptr; } if (!statement) { - statement = adoptPtr(new SQLiteStatement(db, str)); - if (statement->prepare() != SQLResultOk) + statement = std::make_unique<SQLiteStatement>(db, str); + if (statement->prepare() != SQLITE_OK) LOG_ERROR("Preparing statement %s failed", str.ascii().data()); } } @@ -1855,7 +1842,7 @@ void IconDatabase::setIconIDForPageURLInSQLDatabase(int64_t iconID, const String m_setIconIDForPageURLStatement->bindInt64(2, iconID); int result = m_setIconIDForPageURLStatement->step(); - if (result != SQLResultDone) { + if (result != SQLITE_DONE) { ASSERT_NOT_REACHED(); LOG_ERROR("setIconIDForPageURLQuery failed for url %s", urlForLogging(pageURL).ascii().data()); } @@ -1870,7 +1857,7 @@ void IconDatabase::removePageURLFromSQLDatabase(const String& pageURL) readySQLiteStatement(m_removePageURLStatement, m_syncDB, "DELETE FROM PageURL WHERE url = (?);"); m_removePageURLStatement->bindText(1, pageURL); - if (m_removePageURLStatement->step() != SQLResultDone) + if (m_removePageURLStatement->step() != SQLITE_DONE) LOG_ERROR("removePageURLFromSQLDatabase failed for url %s", urlForLogging(pageURL).ascii().data()); m_removePageURLStatement->reset(); @@ -1885,10 +1872,10 @@ int64_t IconDatabase::getIconIDForIconURLFromSQLDatabase(const String& iconURL) m_getIconIDForIconURLStatement->bindText(1, iconURL); int64_t result = m_getIconIDForIconURLStatement->step(); - if (result == SQLResultRow) + if (result == SQLITE_ROW) result = m_getIconIDForIconURLStatement->getColumnInt64(0); else { - if (result != SQLResultDone) + if (result != SQLITE_DONE) LOG_ERROR("getIconIDForIconURLFromSQLDatabase failed for url %s", urlForLogging(iconURL).ascii().data()); result = 0; } @@ -1910,7 +1897,7 @@ int64_t IconDatabase::addIconURLToSQLDatabase(const String& iconURL) int result = m_addIconToIconInfoStatement->step(); m_addIconToIconInfoStatement->reset(); - if (result != SQLResultDone) { + if (result != SQLITE_DONE) { LOG_ERROR("addIconURLToSQLDatabase failed to insert %s into IconInfo", urlForLogging(iconURL).ascii().data()); return 0; } @@ -1921,7 +1908,7 @@ int64_t IconDatabase::addIconURLToSQLDatabase(const String& iconURL) result = m_addIconToIconDataStatement->step(); m_addIconToIconDataStatement->reset(); - if (result != SQLResultDone) { + if (result != SQLITE_DONE) { LOG_ERROR("addIconURLToSQLDatabase failed to insert %s into IconData", urlForLogging(iconURL).ascii().data()); return 0; } @@ -1929,7 +1916,7 @@ int64_t IconDatabase::addIconURLToSQLDatabase(const String& iconURL) return iconID; } -PassRefPtr<SharedBuffer> IconDatabase::getImageDataForIconURLFromSQLDatabase(const String& iconURL) +RefPtr<SharedBuffer> IconDatabase::getImageDataForIconURLFromSQLDatabase(const String& iconURL) { ASSERT_ICON_SYNC_THREAD(); @@ -1939,16 +1926,16 @@ PassRefPtr<SharedBuffer> IconDatabase::getImageDataForIconURLFromSQLDatabase(con m_getImageDataForIconURLStatement->bindText(1, iconURL); int result = m_getImageDataForIconURLStatement->step(); - if (result == SQLResultRow) { + if (result == SQLITE_ROW) { Vector<char> data; m_getImageDataForIconURLStatement->getColumnBlobAsVector(0, data); imageData = SharedBuffer::create(data.data(), data.size()); - } else if (result != SQLResultDone) + } else if (result != SQLITE_DONE) LOG_ERROR("getImageDataForIconURLFromSQLDatabase failed for url %s", urlForLogging(iconURL).ascii().data()); m_getImageDataForIconURLStatement->reset(); - return imageData.release(); + return imageData; } void IconDatabase::removeIconFromSQLDatabase(const String& iconURL) @@ -1970,21 +1957,21 @@ void IconDatabase::removeIconFromSQLDatabase(const String& iconURL) readySQLiteStatement(m_deletePageURLsForIconURLStatement, m_syncDB, "DELETE FROM PageURL WHERE PageURL.iconID = (?);"); m_deletePageURLsForIconURLStatement->bindInt64(1, iconID); - if (m_deletePageURLsForIconURLStatement->step() != SQLResultDone) + if (m_deletePageURLsForIconURLStatement->step() != SQLITE_DONE) LOG_ERROR("m_deletePageURLsForIconURLStatement failed for url %s", urlForLogging(iconURL).ascii().data()); readySQLiteStatement(m_deleteIconFromIconInfoStatement, m_syncDB, "DELETE FROM IconInfo WHERE IconInfo.iconID = (?);"); m_deleteIconFromIconInfoStatement->bindInt64(1, iconID); - if (m_deleteIconFromIconInfoStatement->step() != SQLResultDone) + if (m_deleteIconFromIconInfoStatement->step() != SQLITE_DONE) LOG_ERROR("m_deleteIconFromIconInfoStatement failed for url %s", urlForLogging(iconURL).ascii().data()); - + readySQLiteStatement(m_deleteIconFromIconDataStatement, m_syncDB, "DELETE FROM IconData WHERE IconData.iconID = (?);"); m_deleteIconFromIconDataStatement->bindInt64(1, iconID); - if (m_deleteIconFromIconDataStatement->step() != SQLResultDone) + if (m_deleteIconFromIconDataStatement->step() != SQLITE_DONE) LOG_ERROR("m_deleteIconFromIconDataStatement failed for url %s", urlForLogging(iconURL).ascii().data()); - + m_deletePageURLsForIconURLStatement->reset(); m_deleteIconFromIconInfoStatement->reset(); m_deleteIconFromIconDataStatement->reset(); @@ -2018,7 +2005,7 @@ void IconDatabase::writeIconSnapshotToSQLDatabase(const IconSnapshot& snapshot) m_updateIconInfoStatement->bindText(2, snapshot.iconURL()); m_updateIconInfoStatement->bindInt64(3, iconID); - if (m_updateIconInfoStatement->step() != SQLResultDone) + if (m_updateIconInfoStatement->step() != SQLITE_DONE) LOG_ERROR("Failed to update icon info for url %s", urlForLogging(snapshot.iconURL()).ascii().data()); m_updateIconInfoStatement->reset(); @@ -2033,7 +2020,7 @@ void IconDatabase::writeIconSnapshotToSQLDatabase(const IconSnapshot& snapshot) else m_updateIconDataStatement->bindNull(1); - if (m_updateIconDataStatement->step() != SQLResultDone) + if (m_updateIconDataStatement->step() != SQLITE_DONE) LOG_ERROR("Failed to update icon data for url %s", urlForLogging(snapshot.iconURL()).ascii().data()); m_updateIconDataStatement->reset(); @@ -2042,7 +2029,7 @@ void IconDatabase::writeIconSnapshotToSQLDatabase(const IconSnapshot& snapshot) m_setIconInfoStatement->bindText(1, snapshot.iconURL()); m_setIconInfoStatement->bindInt64(2, snapshot.timestamp()); - if (m_setIconInfoStatement->step() != SQLResultDone) + if (m_setIconInfoStatement->step() != SQLITE_DONE) LOG_ERROR("Failed to set icon info for url %s", urlForLogging(snapshot.iconURL()).ascii().data()); m_setIconInfoStatement->reset(); @@ -2059,7 +2046,7 @@ void IconDatabase::writeIconSnapshotToSQLDatabase(const IconSnapshot& snapshot) else m_setIconDataStatement->bindNull(2); - if (m_setIconDataStatement->step() != SQLResultDone) + if (m_setIconDataStatement->step() != SQLITE_DONE) LOG_ERROR("Failed to set icon data for url %s", urlForLogging(snapshot.iconURL()).ascii().data()); m_setIconDataStatement->reset(); @@ -2080,133 +2067,71 @@ void IconDatabase::setWasExcludedFromBackup() SQLiteStatement(m_syncDB, "INSERT INTO IconDatabaseInfo (key, value) VALUES ('ExcludedFromBackup', 1)").executeCommand(); } -class ClientWorkItem { - WTF_MAKE_FAST_ALLOCATED; -public: - ClientWorkItem(IconDatabaseClient* client) - : m_client(client) - { } - virtual void performWork() = 0; - virtual ~ClientWorkItem() { } - -protected: - IconDatabaseClient* m_client; -}; - -class ImportedIconURLForPageURLWorkItem : public ClientWorkItem { -public: - ImportedIconURLForPageURLWorkItem(IconDatabaseClient* client, const String& pageURL) - : ClientWorkItem(client) - , m_pageURL(new String(pageURL.isolatedCopy())) - { } - - virtual ~ImportedIconURLForPageURLWorkItem() - { - delete m_pageURL; - } - - virtual void performWork() - { - ASSERT(m_client); - m_client->didImportIconURLForPageURL(*m_pageURL); - m_client = 0; - } - -private: - String* m_pageURL; -}; - -class ImportedIconDataForPageURLWorkItem : public ClientWorkItem { -public: - ImportedIconDataForPageURLWorkItem(IconDatabaseClient* client, const String& pageURL) - : ClientWorkItem(client) - , m_pageURL(new String(pageURL.isolatedCopy())) - { } - - virtual ~ImportedIconDataForPageURLWorkItem() - { - delete m_pageURL; - } - - virtual void performWork() - { - ASSERT(m_client); - m_client->didImportIconDataForPageURL(*m_pageURL); - m_client = 0; - } - -private: - String* m_pageURL; -}; - -class RemovedAllIconsWorkItem : public ClientWorkItem { -public: - RemovedAllIconsWorkItem(IconDatabaseClient* client) - : ClientWorkItem(client) - { } - - virtual void performWork() - { - ASSERT(m_client); - m_client->didRemoveAllIcons(); - m_client = 0; - } -}; +void IconDatabase::checkClosedAfterMainThreadCallback() +{ + ASSERT_NOT_SYNC_THREAD(); -class FinishedURLImport : public ClientWorkItem { -public: - FinishedURLImport(IconDatabaseClient* client) - : ClientWorkItem(client) - { } + // If there are still callbacks in flight from the sync thread we cannot possibly be closed. + if (--m_mainThreadCallbackCount) + return; - virtual void performWork() - { - ASSERT(m_client); - m_client->didFinishURLImport(); - m_client = 0; - } -}; + // Even if there's no more pending callbacks the database might otherwise still be open. + if (isOpenBesidesMainThreadCallbacks()) + return; -static void performWorkItem(void* context) -{ - ClientWorkItem* item = static_cast<ClientWorkItem*>(context); - item->performWork(); - delete item; + // This database is now actually closed! But first notify the client. + if (m_client) + m_client->didClose(); } void IconDatabase::dispatchDidImportIconURLForPageURLOnMainThread(const String& pageURL) { ASSERT_ICON_SYNC_THREAD(); + ++m_mainThreadCallbackCount; - ImportedIconURLForPageURLWorkItem* work = new ImportedIconURLForPageURLWorkItem(m_client, pageURL); - callOnMainThread(performWorkItem, work); + callOnMainThread([this, pageURL = pageURL.isolatedCopy()] { + if (m_client) + m_client->didImportIconURLForPageURL(pageURL); + checkClosedAfterMainThreadCallback(); + }); } void IconDatabase::dispatchDidImportIconDataForPageURLOnMainThread(const String& pageURL) { ASSERT_ICON_SYNC_THREAD(); + ++m_mainThreadCallbackCount; - ImportedIconDataForPageURLWorkItem* work = new ImportedIconDataForPageURLWorkItem(m_client, pageURL); - callOnMainThread(performWorkItem, work); + callOnMainThread([this, pageURL = pageURL.isolatedCopy()] { + if (m_client) + m_client->didImportIconDataForPageURL(pageURL); + checkClosedAfterMainThreadCallback(); + }); } void IconDatabase::dispatchDidRemoveAllIconsOnMainThread() { ASSERT_ICON_SYNC_THREAD(); + ++m_mainThreadCallbackCount; - RemovedAllIconsWorkItem* work = new RemovedAllIconsWorkItem(m_client); - callOnMainThread(performWorkItem, work); + callOnMainThread([this] { + if (m_client) + m_client->didRemoveAllIcons(); + checkClosedAfterMainThreadCallback(); + }); } void IconDatabase::dispatchDidFinishURLImportOnMainThread() { ASSERT_ICON_SYNC_THREAD(); + ++m_mainThreadCallbackCount; - FinishedURLImport* work = new FinishedURLImport(m_client); - callOnMainThread(performWorkItem, work); + callOnMainThread([this] { + if (m_client) + m_client->didFinishURLImport(); + checkClosedAfterMainThreadCallback(); + }); } - } // namespace WebCore #endif // ENABLE(ICONDATABASE) |