[GTK] IconDatabase is not thread-safe
authorcarlosgc@webkit.org <carlosgc@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 30 Sep 2019 12:38:44 +0000 (12:38 +0000)
committercarlosgc@webkit.org <carlosgc@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 30 Sep 2019 12:38:44 +0000 (12:38 +0000)
https://bugs.webkit.org/show_bug.cgi?id=201303

Reviewed by Žan Doberšek.

Source/WebKit:

Rewrite the IconDatabase implementation. Now that we are the only users of this code, we can simply it a lot and
design it specifically for our API, which implementation has been simplified a lot too. There's no change in
the database schema nor in behavior from the API point of view.

* UIProcess/API/glib/IconDatabase.cpp:
(WebKit::IconDatabase::IconDatabase):
(WebKit::IconDatabase::~IconDatabase):
(WebKit::IconDatabase::createTablesIfNeeded):
(WebKit::IconDatabase::populatePageURLToIconURLMap):
(WebKit::IconDatabase::clearStatements):
(WebKit::IconDatabase::pruneTimerFired):
(WebKit::IconDatabase::startPruneTimer):
(WebKit::IconDatabase::clearLoadedIconsTimerFired):
(WebKit::IconDatabase::startClearLoadedIconsTimer):
(WebKit::IconDatabase::iconIDForIconURL):
(WebKit::IconDatabase::setIconIDForPageURL):
(WebKit::IconDatabase::iconData):
(WebKit::IconDatabase::addIcon):
(WebKit::IconDatabase::updateIconTimestamp):
(WebKit::IconDatabase::deleteIcon):
(WebKit::IconDatabase::checkIconURLAndSetPageURLIfNeeded):
(WebKit::IconDatabase::loadIconForPageURL):
(WebKit::IconDatabase::iconURLForPageURL):
(WebKit::IconDatabase::setIconForPageURL):
(WebKit::IconDatabase::clear):
* UIProcess/API/glib/IconDatabase.h:
(WebKit::IconDatabase::create):
* UIProcess/API/glib/WebKitFaviconDatabase.cpp:
(webkitFaviconDatabaseCreate):
(webkitFaviconDatabaseIsOpen):
(webkitFaviconDatabaseOpen):
(webkitFaviconDatabaseClose):
(webkitFaviconDatabaseGetLoadDecisionForIcon):
(webkitFaviconDatabaseSetIconForPageURL):
(webkitFaviconDatabaseGetFaviconInternal):
(webkit_favicon_database_get_favicon):
(webkit_favicon_database_get_favicon_finish):
(webkit_favicon_database_get_favicon_uri):
(webkit_favicon_database_clear):
* UIProcess/API/glib/WebKitFaviconDatabasePrivate.h:
* UIProcess/API/glib/WebKitWebContext.cpp:
(webkitWebContextDispose):
(webkit_web_context_set_favicon_database_directory):
(webkitWebContextCreatePageForWebView):
(webkitWebContextWebViewDestroyed):
* UIProcess/API/glib/WebKitWebView.cpp:
(webkitWebViewRequestFavicon):
(webkitWebViewGetLoadDecisionForIcon):
(webkitWebViewSetIcon):

Tools:

Rewrite the WebKitFaviconDatabase tests, splitting tests cases again and making them independent to each other.

* TestWebKitAPI/Tests/WebKitGLib/TestWebKitFaviconDatabase.cpp:
(testFaviconDatabaseInitialization):
(testFaviconDatabaseGetFavicon):
(ephemeralViewFaviconChanged):
(testFaviconDatabaseEphemeral):
(testFaviconDatabaseClear):
(beforeAll):
(afterAll):
* TestWebKitAPI/glib/TestExpectations.json: TestWebKitFaviconDatabase shouls always pass now.

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@250518 268f45cc-cd09-0410-ab3c-d52691b4dbfc

12 files changed:
Source/WebKit/ChangeLog
Source/WebKit/UIProcess/API/glib/IconDatabase.cpp
Source/WebKit/UIProcess/API/glib/IconDatabase.h
Source/WebKit/UIProcess/API/glib/WebKitFaviconDatabase.cpp
Source/WebKit/UIProcess/API/glib/WebKitFaviconDatabasePrivate.h
Source/WebKit/UIProcess/API/glib/WebKitWebContext.cpp
Source/WebKit/UIProcess/API/glib/WebKitWebView.cpp
Tools/ChangeLog
Tools/TestWebKitAPI/Tests/WebKitGLib/TestWebKitFaviconDatabase.cpp
Tools/TestWebKitAPI/glib/CMakeLists.txt
Tools/TestWebKitAPI/glib/PlatformGTK.cmake
Tools/TestWebKitAPI/glib/TestExpectations.json

index e68e1cc..e2e2de4 100644 (file)
@@ -1,5 +1,62 @@
 2019-09-30  Carlos Garcia Campos  <cgarcia@igalia.com>
 
+        [GTK] IconDatabase is not thread-safe
+        https://bugs.webkit.org/show_bug.cgi?id=201303
+
+        Reviewed by Žan Doberšek.
+
+        Rewrite the IconDatabase implementation. Now that we are the only users of this code, we can simply it a lot and
+        design it specifically for our API, which implementation has been simplified a lot too. There's no change in
+        the database schema nor in behavior from the API point of view.
+
+        * UIProcess/API/glib/IconDatabase.cpp:
+        (WebKit::IconDatabase::IconDatabase):
+        (WebKit::IconDatabase::~IconDatabase):
+        (WebKit::IconDatabase::createTablesIfNeeded):
+        (WebKit::IconDatabase::populatePageURLToIconURLMap):
+        (WebKit::IconDatabase::clearStatements):
+        (WebKit::IconDatabase::pruneTimerFired):
+        (WebKit::IconDatabase::startPruneTimer):
+        (WebKit::IconDatabase::clearLoadedIconsTimerFired):
+        (WebKit::IconDatabase::startClearLoadedIconsTimer):
+        (WebKit::IconDatabase::iconIDForIconURL):
+        (WebKit::IconDatabase::setIconIDForPageURL):
+        (WebKit::IconDatabase::iconData):
+        (WebKit::IconDatabase::addIcon):
+        (WebKit::IconDatabase::updateIconTimestamp):
+        (WebKit::IconDatabase::deleteIcon):
+        (WebKit::IconDatabase::checkIconURLAndSetPageURLIfNeeded):
+        (WebKit::IconDatabase::loadIconForPageURL):
+        (WebKit::IconDatabase::iconURLForPageURL):
+        (WebKit::IconDatabase::setIconForPageURL):
+        (WebKit::IconDatabase::clear):
+        * UIProcess/API/glib/IconDatabase.h:
+        (WebKit::IconDatabase::create):
+        * UIProcess/API/glib/WebKitFaviconDatabase.cpp:
+        (webkitFaviconDatabaseCreate):
+        (webkitFaviconDatabaseIsOpen):
+        (webkitFaviconDatabaseOpen):
+        (webkitFaviconDatabaseClose):
+        (webkitFaviconDatabaseGetLoadDecisionForIcon):
+        (webkitFaviconDatabaseSetIconForPageURL):
+        (webkitFaviconDatabaseGetFaviconInternal):
+        (webkit_favicon_database_get_favicon):
+        (webkit_favicon_database_get_favicon_finish):
+        (webkit_favicon_database_get_favicon_uri):
+        (webkit_favicon_database_clear):
+        * UIProcess/API/glib/WebKitFaviconDatabasePrivate.h:
+        * UIProcess/API/glib/WebKitWebContext.cpp:
+        (webkitWebContextDispose):
+        (webkit_web_context_set_favicon_database_directory):
+        (webkitWebContextCreatePageForWebView):
+        (webkitWebContextWebViewDestroyed):
+        * UIProcess/API/glib/WebKitWebView.cpp:
+        (webkitWebViewRequestFavicon):
+        (webkitWebViewGetLoadDecisionForIcon):
+        (webkitWebViewSetIcon):
+
+2019-09-30  Carlos Garcia Campos  <cgarcia@igalia.com>
+
         [GTK][WPE] Add about:gpu
         https://bugs.webkit.org/show_bug.cgi?id=202305
 
index 06870d2..bd0de82 100644 (file)
@@ -1,27 +1,20 @@
 /*
- * Copyright (C) 2006-2017 Apple Inc. All rights reserved.
- * Copyright (C) 2007 Justin Haygood (jhaygood@reaktix.com)
+ * Copyright (C) 2019 Igalia S.L.
  *
- * Redistribution and use in source and binary forms, with or without
- * modification, are permitted provided that the following conditions
- * are met:
- * 1. Redistributions of source code must retain the above copyright
- *    notice, this list of conditions and the following disclaimer.
- * 2. Redistributions in binary form must reproduce the above copyright
- *    notice, this list of conditions and the following disclaimer in the
- *    documentation and/or other materials provided with the distribution.
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Library General Public
+ * License as published by the Free Software Foundation; either
+ * version 2 of the License, or (at your option) any later version.
  *
- * 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 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
- * PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY
- * OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
- * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
- * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Library General Public License for more details.
+ *
+ * You should have received a copy of the GNU Library General Public License
+ * along with this library; see the file COPYING.LIB.  If not, write to
+ * the Free Software Foundation, Inc., 51 Franklin Street, Fifth Floor,
+ * Boston, MA 02110-1301, USA.
  */
 
 #include "config.h"
 #include "Logging.h"
 #include <WebCore/BitmapImage.h>
 #include <WebCore/Image.h>
-#include <WebCore/SQLiteStatement.h>
 #include <WebCore/SQLiteTransaction.h>
 #include <WebCore/SharedBuffer.h>
 #include <wtf/FileSystem.h>
-#include <wtf/MainThread.h>
-#include <wtf/NeverDestroyed.h>
-#include <wtf/StdLibExtras.h>
-#include <wtf/URL.h>
-#include <wtf/text/StringConcatenateNumbers.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())
-
-// For methods that are meant to support the sync thread ONLY
-#define IS_ICON_SYNC_THREAD() (m_syncThread == &Thread::current())
-#define ASSERT_ICON_SYNC_THREAD() ASSERT(IS_ICON_SYNC_THREAD())
+#include <wtf/RunLoop.h>
+#include <wtf/glib/RunLoopSourcePriority.h>
+#include <wtf/threads/BinarySemaphore.h>
 
 namespace WebKit {
 using namespace WebCore;
 
-static int databaseCleanupCounter = 0;
-
 // This version number is in the DB and marks the current generation of the schema
-// Currently, a mismatched schema causes the DB to be wiped and reset. This isn't
-// so bad during development but in the future, we would need to write a conversion
-// function to advance older released schemas to "current"
+// Currently, a mismatched schema causes the DB to be wiped and reset.
 static const int currentDatabaseVersion = 6;
 
-// Icons expire once every 4 days
-static const int iconExpirationTime = 60*60*24*4;
-
-static const Seconds updateTimerDelay { 5_s };
-
-static bool checkIntegrityOnOpen = false;
-
-// We are not interested in icons that have been unused for more than
-// 30 days, delete them even if they have not been explicitly released.
-static const Seconds notUsedIconExpirationTime { 60*60*24*30 };
-
-#if !LOG_DISABLED || !ERROR_DISABLED
-static String urlForLogging(const String& url)
-{
-    static const unsigned urlTruncationLength = 120;
-
-    if (url.length() < urlTruncationLength)
-        return url;
-    return url.substring(0, urlTruncationLength) + "...";
-}
-#endif
-
-class DefaultIconDatabaseClient final : public IconDatabaseClient {
-    WTF_MAKE_FAST_ALLOCATED;
-public:
-    void didImportIconURLForPageURL(const String&) override { }
-    void didImportIconDataForPageURL(const String&) override { }
-    void didChangeIconForPageURL(const String&) override { }
-    void didFinishURLImport() override { }
-};
-
-static IconDatabaseClient* defaultClient()
-{
-    static IconDatabaseClient* defaultClient = new DefaultIconDatabaseClient;
-    return defaultClient;
-}
-
-IconDatabase::IconRecord::IconRecord(const String& url)
-    : m_iconURL(url)
-{
-}
+// Icons expire once every 4 days.
+static const Seconds iconExpirationTime { 60 * 60 * 24 * 4 };
 
-IconDatabase::IconRecord::~IconRecord()
-{
-    LOG(IconDatabase, "Destroying IconRecord for icon url %s", m_iconURL.ascii().data());
-}
+// We are not interested in icons that have been unused for more than 30 days.
+static const Seconds notUsedIconExpirationTime { 60 * 60 * 24 * 30 };
 
-NativeImagePtr IconDatabase::IconRecord::image(const IntSize&)
-{
-    // FIXME rdar://4680377 - For size right now, we are returning our one and only image and the Bridge
-    // is resizing it in place. We need to actually store all the original representations here and return a native
-    // one, or resize the best one to the requested size and cache that result.
-    return m_image;
-}
+// Loaded icons are cleared after 30 seconds of being requested.
+static const Seconds loadedIconExpirationTime { 30_s };
 
-void IconDatabase::IconRecord::setImageData(RefPtr<SharedBuffer>&& data)
+IconDatabase::IconDatabase(const String& path, AllowDatabaseWrite allowDatabaseWrite)
+    : m_workQueue(WorkQueue::create("org.webkit.IconDatabase"))
+    , m_allowDatabaseWrite(allowDatabaseWrite)
+    , m_clearLoadedIconsTimer(RunLoop::main(), this, &IconDatabase::clearLoadedIconsTimerFired)
 {
-    m_dataSet = true;
-    m_imageData = WTFMove(data);
-    m_image = nullptr;
-
-    if (!m_imageData || !m_imageData->size()) {
-        m_imageData = nullptr;
-        return;
-    }
-
-    auto image = BitmapImage::create();
-    if (image->setData(RefPtr<SharedBuffer> { m_imageData }, true) < EncodedDataStatus::SizeAvailable) {
-        LOG(IconDatabase, "Manual image data for iconURL '%s' FAILED - it was probably invalid image data", m_iconURL.ascii().data());
-        m_imageData = nullptr;
-        return;
-    }
-
-    m_image = image->nativeImageForCurrentFrame();
-    if (!m_image) {
-        LOG(IconDatabase, "Manual image data for iconURL '%s' FAILED - it was probably invalid image data", m_iconURL.ascii().data());
-        m_imageData = nullptr;
-    }
-}
+    m_clearLoadedIconsTimer.setPriority(RunLoopSourcePriority::ReleaseUnusedResourcesTimer);
 
-IconDatabase::IconRecord::ImageDataStatus IconDatabase::IconRecord::imageDataStatus()
-{
-    if (!m_dataSet)
-        return ImageDataStatus::Unknown;
-    if (!m_image)
-        return ImageDataStatus::Missing;
-    return ImageDataStatus::Present;
-}
+    // We initialize the database synchronously, it's hopefully fast enough because it makes
+    // the implementation a lot simpler.
+    BinarySemaphore semaphore;
+    m_workQueue->dispatch([&] {
+        if (allowDatabaseWrite == AllowDatabaseWrite::No && !FileSystem::fileExists(path)) {
+            semaphore.signal();
+            return;
+        }
 
-IconDatabase::IconSnapshot IconDatabase::IconRecord::snapshot(bool forDeletion) const
-{
-    if (forDeletion)
-        return IconSnapshot(m_iconURL, 0, nullptr);
+        auto databaseDirectory = FileSystem::directoryName(path);
+        FileSystem::makeAllDirectories(databaseDirectory);
+        if (!m_db.open(path)) {
+            LOG_ERROR("Unable to open favicon database at path %s - %s", path.utf8().data(), m_db.lastErrorMsg());
+            semaphore.signal();
+            return;
+        }
 
-    return IconSnapshot(m_iconURL, m_stamp, m_imageData ? m_imageData.get() : nullptr);
-}
+        auto databaseVersionNumber = SQLiteStatement(m_db, "SELECT value FROM IconDatabaseInfo WHERE key = 'Version';").getColumnInt(0);
+        if (databaseVersionNumber > currentDatabaseVersion) {
+            LOG(IconDatabase, "Database version number %d is greater than our current version number %d - closing the database to prevent overwriting newer versions",
+                databaseVersionNumber, currentDatabaseVersion);
+            m_db.close();
+            semaphore.signal();
+            return;
+        }
 
-IconDatabase::PageURLRecord::PageURLRecord(const String& pageURL)
-    : m_pageURL(pageURL)
-{
-}
+        if (databaseVersionNumber < currentDatabaseVersion) {
+            if (m_allowDatabaseWrite == AllowDatabaseWrite::No) {
+                m_db.close();
+                semaphore.signal();
+                return;
+            }
 
-IconDatabase::PageURLRecord::~PageURLRecord()
-{
-    if (m_iconRecord)
-        m_iconRecord->retainingPageURLs().remove(m_pageURL);
-}
+            m_db.clearAllTables();
+        }
 
-void IconDatabase::PageURLRecord::setIconRecord(RefPtr<IconRecord>&& icon)
-{
-    if (m_iconRecord)
-        m_iconRecord->retainingPageURLs().remove(m_pageURL);
+        // Reduce sqlite RAM cache size from default 2000 pages (~1.5kB per page). 3MB of cache for icon database is overkill.
+        SQLiteStatement(m_db, "PRAGMA cache_size = 200;").executeCommand();
 
-    m_iconRecord = WTFMove(icon);
+        if (allowDatabaseWrite == AllowDatabaseWrite::Yes) {
+            m_pruneTimer = makeUnique<RunLoop::Timer<IconDatabase>>(RunLoop::current(), this, &IconDatabase::pruneTimerFired);
+            m_pruneTimer->setPriority(RunLoopSourcePriority::ReleaseUnusedResourcesTimer);
+        }
 
-    if (m_iconRecord)
-        m_iconRecord->retainingPageURLs().add(m_pageURL);
-}
+        if (!createTablesIfNeeded())
+            populatePageURLToIconURLMap();
 
-IconDatabase::PageURLSnapshot IconDatabase::PageURLRecord::snapshot(bool forDeletion) const
-{
-    return { m_pageURL, (m_iconRecord && !forDeletion) ? m_iconRecord->iconURL() : String() };
+        semaphore.signal();
+    });
+    semaphore.wait();
 }
 
-// ************************
-// *** Main Thread Only ***
-// ************************
-
-void IconDatabase::setClient(std::unique_ptr<IconDatabaseClient>&& client)
+IconDatabase::~IconDatabase()
 {
-    // Don't allow a client change after the thread has already began
-    // (setting the client should occur before the database is opened)
-    ASSERT(!m_syncThreadRunning);
-    if (!client || m_syncThreadRunning)
-        return;
-
-    m_client = WTFMove(client);
+    BinarySemaphore semaphore;
+    m_workQueue->dispatch([&] {
+        if (m_db.isOpen()) {
+            m_pruneTimer = nullptr;
+            clearStatements();
+            m_db.close();
+        }
+        semaphore.signal();
+    });
+    semaphore.wait();
 }
 
-bool IconDatabase::open(const String& directory, const String& filename)
+bool IconDatabase::createTablesIfNeeded()
 {
-    ASSERT_NOT_SYNC_THREAD();
-
-    if (!m_isEnabled)
+    if (m_db.tableExists("IconInfo") && m_db.tableExists("IconData") && m_db.tableExists("PageURL") && m_db.tableExists("IconDatabaseInfo"))
         return false;
 
-    if (isOpen()) {
-        LOG_ERROR("Attempt to reopen the IconDatabase which is already open. Must close it first.");
+    if (m_allowDatabaseWrite == AllowDatabaseWrite::No) {
+        m_db.close();
         return false;
     }
 
-    m_mainThreadNotifier.setActive(true);
-
-    m_databaseDirectory = directory.isolatedCopy();
-
-    // Formulate the full path for the database file
-    m_completeDatabasePath = FileSystem::pathByAppendingComponent(m_databaseDirectory, filename);
-
-    // Lock here as well as first thing in the thread so the thread doesn't actually commence until the createThread() call
-    // completes and m_syncThreadRunning is properly set
-    m_syncLock.lock();
-    m_syncThread = Thread::create("WebCore: IconDatabase", [this] {
-        iconDatabaseSyncThread();
-    });
-    m_syncThreadRunning = true;
-    m_syncLock.unlock();
-    return true;
-}
-
-void IconDatabase::close()
-{
-    ASSERT_NOT_SYNC_THREAD();
-
-    m_mainThreadNotifier.stop();
-
-    if (m_syncThreadRunning) {
-        // Set the flag to tell the sync thread to wrap it up
-        m_threadTerminationRequested = true;
+    m_db.clearAllTables();
 
-        // Wake up the sync thread if it's waiting
-        wakeSyncThread();
-
-        // Wait for the sync thread to terminate
-        m_syncThread->waitForCompletion();
+    if (!m_db.executeCommand("CREATE TABLE PageURL (url TEXT NOT NULL ON CONFLICT FAIL UNIQUE ON CONFLICT REPLACE,iconID INTEGER NOT NULL ON CONFLICT FAIL);")) {
+        LOG_ERROR("Could not create PageURL table in database (%i) - %s", m_db.lastError(), m_db.lastErrorMsg());
+        m_db.close();
+        return false;
+    }
+    if (!m_db.executeCommand("CREATE INDEX PageURLIndex ON PageURL (url);")) {
+        LOG_ERROR("Could not create PageURL index in database (%i) - %s", m_db.lastError(), m_db.lastErrorMsg());
+        m_db.close();
+        return false;
+    }
+    if (!m_db.executeCommand("CREATE TABLE IconInfo (iconID INTEGER PRIMARY KEY AUTOINCREMENT UNIQUE ON CONFLICT REPLACE, url TEXT NOT NULL ON CONFLICT FAIL UNIQUE ON CONFLICT FAIL, stamp INTEGER);")) {
+        LOG_ERROR("Could not create IconInfo table in database (%i) - %s", m_db.lastError(), m_db.lastErrorMsg());
+        m_db.close();
+        return false;
+    }
+    if (!m_db.executeCommand("CREATE INDEX IconInfoIndex ON IconInfo (url, iconID);")) {
+        LOG_ERROR("Could not create PageURL index in database (%i) - %s", m_db.lastError(), m_db.lastErrorMsg());
+        m_db.close();
+        return false;
+    }
+    if (!m_db.executeCommand("CREATE TABLE IconData (iconID INTEGER PRIMARY KEY AUTOINCREMENT UNIQUE ON CONFLICT REPLACE, data BLOB);")) {
+        LOG_ERROR("Could not create IconData table in database (%i) - %s", m_db.lastError(), m_db.lastErrorMsg());
+        m_db.close();
+        return false;
+    }
+    if (!m_db.executeCommand("CREATE INDEX IconDataIndex ON IconData (iconID);")) {
+        LOG_ERROR("Could not create PageURL index in database (%i) - %s", m_db.lastError(), m_db.lastErrorMsg());
+        m_db.close();
+        return false;
+    }
+    if (!m_db.executeCommand("CREATE TABLE IconDatabaseInfo (key TEXT NOT NULL ON CONFLICT FAIL UNIQUE ON CONFLICT REPLACE,value TEXT NOT NULL ON CONFLICT FAIL);")) {
+        LOG_ERROR("Could not create IconDatabaseInfo table in database (%i) - %s", m_db.lastError(), m_db.lastErrorMsg());
+        m_db.close();
+        return false;
+    }
+    if (!m_db.executeCommand(String("INSERT INTO IconDatabaseInfo VALUES ('Version', ") + String::number(currentDatabaseVersion) + ");")) {
+        LOG_ERROR("Could not insert icon database version into IconDatabaseInfo table (%i) - %s", m_db.lastError(), m_db.lastErrorMsg());
+        m_db.close();
+        return false;
     }
 
-    m_syncThreadRunning = false;
-    m_threadTerminationRequested = false;
-    m_removeIconsRequested = false;
-
-    m_syncDB.close();
-
-    m_client = nullptr;
+    return true;
 }
 
-void IconDatabase::removeAllIcons()
+void IconDatabase::populatePageURLToIconURLMap()
 {
-    ASSERT_NOT_SYNC_THREAD();
-
-    if (!isOpen())
+    if (!m_db.isOpen())
         return;
 
-    LOG(IconDatabase, "Requesting background thread to remove all icons");
-
-    // Clear the in-memory record of every IconRecord, anything waiting to be read from disk, and anything waiting to be written to disk
-    {
-        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
-        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
-        {
-            LockHolder locker(m_pendingSyncLock);
-            m_pageURLsPendingSync.clear();
-            m_iconsPendingSync.clear();
-        }
+    String importQuery = makeString("SELECT PageURL.url, IconInfo.url, IconInfo.stamp FROM PageURL INNER JOIN IconInfo ON PageURL.iconID=IconInfo.iconID WHERE IconInfo.stamp > ", floor((WallTime::now() - notUsedIconExpirationTime).secondsSinceEpoch().seconds()), ';');
+    SQLiteStatement query(m_db, importQuery);
+    if (query.prepare() != SQLITE_OK) {
+        LOG_ERROR("Unable to prepare icon url import query");
+        return;
+    }
 
-        // Clear all in-memory records of things that need to be read in from disk
-        {
-            LockHolder locker(m_pendingReadingLock);
-            m_pageURLsPendingImport.clear();
-            m_pageURLsInterestedInIcons.clear();
-            m_iconsPendingReading.clear();
-        }
+    auto result = query.step();
+    while (result == SQLITE_ROW) {
+        m_pageURLToIconURLMap.set(query.getColumnText(0), query.getColumnText(1));
+        result = query.step();
     }
 
-    m_removeIconsRequested = true;
-    wakeSyncThread();
+    startPruneTimer();
 }
 
-static bool documentCanHaveIcon(const String& documentURL)
+void IconDatabase::clearStatements()
 {
-    return !documentURL.isEmpty() && !protocolIs(documentURL, "about");
+    RELEASE_ASSERT(m_db.isOpen());
+
+    m_iconIDForIconURLStatement = nullptr;
+    m_setIconIDForPageURLStatement = nullptr;
+    m_iconDataStatement = nullptr;
+    m_addIconStatement = nullptr;
+    m_addIconDataStatement = nullptr;
+    m_updateIconTimestampStatement = nullptr;
+    m_deletePageURLsForIconStatement = nullptr;
+    m_deleteIconDataStatement = nullptr;
+    m_deleteIconStatement = nullptr;
+    m_pruneIconsStatement = nullptr;
 }
 
-std::pair<NativeImagePtr, IconDatabase::IsKnownIcon> IconDatabase::synchronousIconForPageURL(const String& pageURLOriginal, const IntSize& size)
+void IconDatabase::pruneTimerFired()
 {
-    ASSERT_NOT_SYNC_THREAD();
-
-    // pageURLOriginal cannot be stored without being deep copied first.
-    // We should go our of our way to only copy it if we have to store it
+    RELEASE_ASSERT(m_db.isOpen());
 
-    if (!isOpen() || !documentCanHaveIcon(pageURLOriginal))
-        return { nullptr, IsKnownIcon::No };
-
-    LockHolder locker(m_urlAndIconLock);
-
-    performPendingRetainAndReleaseOperations();
-
-    String pageURLCopy; // Creates a null string for easy testing
-
-    PageURLRecord* pageRecord = m_pageURLToRecordMap.get(pageURLOriginal);
-    if (!pageRecord) {
-        pageURLCopy = pageURLOriginal.isolatedCopy();
-        pageRecord = getOrCreatePageURLRecord(pageURLCopy);
+    if (!m_pruneIconsStatement) {
+        m_pruneIconsStatement = makeUnique<SQLiteStatement>(m_db, "DELETE FROM IconInfo WHERE stamp <= (?);");
+        if (m_pruneIconsStatement->prepare() != SQLITE_OK) {
+            LOG_ERROR("Preparing statement pruneIcons failed");
+            m_pruneIconsStatement = nullptr;
+            return;
+        }
     }
 
-    // If pageRecord is nullptr, one of two things is true -
-    // 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) {
-        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 { nullptr, IsKnownIcon::No };
+    if (m_pruneIconsStatement->bindInt64(1, floor((WallTime::now() - notUsedIconExpirationTime).secondsSinceEpoch().seconds())) != SQLITE_OK) {
+        LOG_ERROR("FaviconDatabse::pruneTimerFired failed: %s", m_db.lastErrorMsg());
+        return;
     }
 
-    IconRecord* iconRecord = pageRecord->iconRecord();
-
-    // If the initial URL import isn't complete, it's possible to have a PageURL record without an associated icon
-    // 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 { nullptr, IsKnownIcon::No };
-
-    // 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 { nullptr, IsKnownIcon::No };
-
-    // If it's a new IconRecord object that doesn't have its imageData set yet,
-    // mark it to be read by the background thread
-    if (iconRecord->imageDataStatus() == IconRecord::ImageDataStatus::Unknown) {
-        if (pageURLCopy.isNull())
-            pageURLCopy = pageURLOriginal.isolatedCopy();
-
-        LockHolder locker(m_pendingReadingLock);
-        m_pageURLsInterestedInIcons.add(pageURLCopy);
-        m_iconsPendingReading.add(iconRecord);
-        wakeSyncThread();
-        return { nullptr, IsKnownIcon::No };
+    SQLiteTransaction transaction(m_db);
+    transaction.begin();
+    if (m_pruneIconsStatement->step() == SQLITE_DONE) {
+        m_db.executeCommand("DELETE FROM IconData WHERE iconID NOT IN (SELECT iconID FROM IconInfo);");
+        m_db.executeCommand("DELETE FROM PageURL WHERE iconID NOT IN (SELECT iconID FROM IconInfo);");
     }
+    m_pruneIconsStatement->reset();
 
-    // 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 { nullptr, IsKnownIcon::Yes };
-
-    return { iconRecord->image(size), IsKnownIcon::Yes };
-}
-
-String IconDatabase::synchronousIconURLForPageURL(const String& pageURLOriginal)
-{
-    ASSERT_NOT_SYNC_THREAD();
-
-    // Cannot do anything with pageURLOriginal that would end up storing it without deep copying first
-    // Also, in the case we have a real answer for the caller, we must deep copy that as well
-
-    if (!isOpen() || !documentCanHaveIcon(pageURLOriginal))
-        return String();
-
-    LockHolder locker(m_urlAndIconLock);
-
-    PageURLRecord* pageRecord = m_pageURLToRecordMap.get(pageURLOriginal);
-    if (!pageRecord)
-        pageRecord = getOrCreatePageURLRecord(pageURLOriginal.isolatedCopy());
-
-    // If pageRecord is nullptr, one of two things is true -
-    // 1 - The initial url import is incomplete and this pageURL has already been 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)
-        return String();
-
-    // Possible the pageRecord is around because it's a retained pageURL with no iconURL, so we have to check
-    return pageRecord->iconRecord() ? pageRecord->iconRecord()->iconURL().isolatedCopy() : String();
+    transaction.commit();
 }
 
-void IconDatabase::retainIconForPageURL(const String& pageURL)
+void IconDatabase::startPruneTimer()
 {
-    ASSERT_NOT_SYNC_THREAD();
-
-    if (!isEnabled() || !documentCanHaveIcon(pageURL))
+    if (!m_pruneTimer || !m_db.isOpen())
         return;
 
-    {
-        LockHolder locker(m_urlsToRetainOrReleaseLock);
-        m_urlsToRetain.add(pageURL.isolatedCopy());
-        m_retainOrReleaseIconRequested = true;
-    }
-
-    scheduleOrDeferSyncTimer();
+    if (m_pruneTimer->isActive())
+        m_pruneTimer->stop();
+    m_pruneTimer->startOneShot(10_s);
 }
 
-void IconDatabase::performRetainIconForPageURL(const String& pageURLOriginal, int retainCount)
+void IconDatabase::clearLoadedIconsTimerFired()
 {
-    PageURLRecord* record = m_pageURLToRecordMap.get(pageURLOriginal);
-
-    String pageURL;
-
-    if (!record) {
-        pageURL = pageURLOriginal.isolatedCopy();
-
-        record = new PageURLRecord(pageURL);
-        m_pageURLToRecordMap.set(pageURL, record);
+    auto now = MonotonicTime::now();
+    Vector<String> iconsToRemove;
+    for (auto iter : m_loadedIcons) {
+        if (now - iter.value.second >= loadedIconExpirationTime)
+            iconsToRemove.append(iter.key);
     }
 
-    if (!record->retain(retainCount)) {
-        if (pageURL.isNull())
-            pageURL = pageURLOriginal.isolatedCopy();
-
-        // This page just had its retain count bumped from 0 to 1 - Record that fact
-        m_retainedPageURLs.add(pageURL);
-
-        // If we read the iconURLs yet, we want to avoid any pageURL->iconURL lookups and the pageURLsPendingDeletion is moot,
-        // so we bail here and skip those steps
-        if (!m_iconURLImportComplete)
-            return;
+    for (auto& iconURL : iconsToRemove)
+        m_loadedIcons.remove(iconURL);
 
-        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)) {
-            LOG(IconDatabase, "Bringing %s back from the brink", pageURL.ascii().data());
-            m_pageURLsPendingSync.set(pageURL, record->snapshot());
-        }
-    }
+    if (!m_loadedIcons.isEmpty())
+        startClearLoadedIconsTimer();
 }
 
-void IconDatabase::releaseIconForPageURL(const String& pageURL)
+void IconDatabase::startClearLoadedIconsTimer()
 {
-    ASSERT_NOT_SYNC_THREAD();
-
-    // Cannot do anything with pageURLOriginal that would end up storing it without deep copying first
-
-    if (!isEnabled() || !documentCanHaveIcon(pageURL))
+    if (m_clearLoadedIconsTimer.isActive())
         return;
 
-    {
-        LockHolder locker(m_urlsToRetainOrReleaseLock);
-        m_urlsToRelease.add(pageURL.isolatedCopy());
-        m_retainOrReleaseIconRequested = true;
-    }
-    scheduleOrDeferSyncTimer();
+    m_clearLoadedIconsTimer.startOneShot(loadedIconExpirationTime);
 }
 
-void IconDatabase::performReleaseIconForPageURL(const String& pageURLOriginal, int releaseCount)
+Optional<int64_t> IconDatabase::iconIDForIconURL(const String& iconURL, bool& expired)
 {
-    // Check if this pageURL is actually retained
-    if (!m_retainedPageURLs.contains(pageURLOriginal)) {
-        LOG_ERROR("Attempting to release icon for URL %s which is not retained", urlForLogging(pageURLOriginal).ascii().data());
-        return;
-    }
+    RELEASE_ASSERT(m_db.isOpen());
 
-    // Get its retain count - if it's retained, we'd better have a PageURLRecord for it
-    PageURLRecord* pageRecord = m_pageURLToRecordMap.get(pageURLOriginal);
-    ASSERT(pageRecord);
-    LOG(IconDatabase, "Releasing pageURL %s to a retain count of %i", urlForLogging(pageURLOriginal).ascii().data(), pageRecord->retainCount() - 1);
-    ASSERT(pageRecord->retainCount() > 0);
-
-    // If it still has a positive retain count, store the new count and bail
-    if (pageRecord->release(releaseCount))
-        return;
-
-    // This pageRecord has now been fully released. Do the appropriate cleanup
-    LOG(IconDatabase, "No more retainers for PageURL %s", urlForLogging(pageURLOriginal).ascii().data());
-    m_pageURLToRecordMap.remove(pageURLOriginal);
-    m_retainedPageURLs.remove(pageURLOriginal);
-
-    // Grab the iconRecord for later use (and do a sanity check on it for kicks)
-    IconRecord* iconRecord = pageRecord->iconRecord();
-
-    ASSERT(!iconRecord || (iconRecord && m_iconURLToRecordMap.get(iconRecord->iconURL()) == iconRecord));
-
-    {
-        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)
-            m_pageURLsPendingImport.remove(pageURLOriginal);
-        m_pageURLsInterestedInIcons.remove(pageURLOriginal);
-
-        // If this icon is down to it's last retainer, we don't care about reading it in from disk anymore
-        if (iconRecord && iconRecord->hasOneRef()) {
-            m_iconURLToRecordMap.remove(iconRecord->iconURL());
-            m_iconsPendingReading.remove(iconRecord);
+    if (!m_iconIDForIconURLStatement) {
+        m_iconIDForIconURLStatement = makeUnique<SQLiteStatement>(m_db, "SELECT IconInfo.iconID, IconInfo.stamp FROM IconInfo WHERE IconInfo.url = (?);");
+        if (m_iconIDForIconURLStatement->prepare() != SQLITE_OK) {
+            LOG_ERROR("Preparing statement iconIDForIconURL failed");
+            m_iconIDForIconURLStatement = nullptr;
+            return WTF::nullopt;
         }
     }
 
-    // Mark stuff for deletion from the database only if we're not in private browsing
-    if (!m_privateBrowsingEnabled) {
-        LockHolder locker(m_pendingSyncLock);
-        m_pageURLsPendingSync.set(pageURLOriginal.isolatedCopy(), pageRecord->snapshot(true));
+    if (m_iconIDForIconURLStatement->bindText(1, iconURL) != SQLITE_OK) {
+        LOG_ERROR("FaviconDatabse::iconIDForIconURL failed: %s", m_db.lastErrorMsg());
+        return WTF::nullopt;
+    }
 
-        // If this page is the last page to refer to a particular IconRecord, that IconRecord needs to
-        // be marked for deletion
-        if (iconRecord && iconRecord->hasOneRef())
-            m_iconsPendingSync.set(iconRecord->iconURL(), iconRecord->snapshot(true));
+    Optional<int64_t> result;
+    if (m_iconIDForIconURLStatement->step() == SQLITE_ROW) {
+        result = m_iconIDForIconURLStatement->getColumnInt64(0);
+        expired = m_iconIDForIconURLStatement->getColumnInt64(1) <= floor((WallTime::now() - iconExpirationTime).secondsSinceEpoch().seconds());
     }
 
-    delete pageRecord;
+    m_iconIDForIconURLStatement->reset();
+    return result;
 }
 
-void IconDatabase::setIconDataForIconURL(RefPtr<SharedBuffer>&& data, const String& iconURLOriginal)
+bool IconDatabase::setIconIDForPageURL(int64_t iconID, const String& pageURL)
 {
-    ASSERT_NOT_SYNC_THREAD();
-
-    // Cannot do anything with iconURLOriginal that would end up storing them without deep copying first
-
-    if (!isOpen() || iconURLOriginal.isEmpty())
-        return;
+    RELEASE_ASSERT(m_db.isOpen());
+    RELEASE_ASSERT(m_allowDatabaseWrite == AllowDatabaseWrite::Yes);
 
-    String iconURL = iconURLOriginal.isolatedCopy();
-    Vector<String> pageURLs;
-    {
-        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) {
-            LockHolder locker(m_pendingReadingLock);
-            m_iconsPendingReading.remove(icon.get());
-        } else
-            icon = getOrCreateIconRecord(iconURL);
-
-        // Update the data and set the time stamp
-        icon->setImageData(WTFMove(data));
-        icon->setTimestamp((int)WallTime::now().secondsSinceEpoch().seconds());
-
-        // Copy the current retaining pageURLs - if any - to notify them of the change
-        pageURLs.appendRange(icon->retainingPageURLs().begin(), icon->retainingPageURLs().end());
-
-        // Mark the IconRecord as requiring an update to the database only if private browsing is disabled
-        if (!m_privateBrowsingEnabled) {
-            LockHolder locker(m_pendingSyncLock);
-            m_iconsPendingSync.set(iconURL, icon->snapshot());
+    if (!m_setIconIDForPageURLStatement) {
+        m_setIconIDForPageURLStatement = makeUnique<SQLiteStatement>(m_db, "INSERT INTO PageURL (url, iconID) VALUES ((?), ?);");
+        if (m_setIconIDForPageURLStatement->prepare() != SQLITE_OK) {
+            LOG_ERROR("Preparing statement setIconIDForPageURL failed");
+            m_setIconIDForPageURLStatement = nullptr;
+            return false;
         }
+    }
 
-        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());
-        }
+    if (m_setIconIDForPageURLStatement->bindText(1, pageURL) != SQLITE_OK
+        || m_setIconIDForPageURLStatement->bindInt64(2, iconID) != SQLITE_OK) {
+        LOG_ERROR("FaviconDatabse::setIconIDForPageURL failed: %s", m_db.lastErrorMsg());
+        return false;
     }
 
-    // Send notification out regarding all PageURLs that retain this icon
-    // Start the timer to commit this change - or further delay the timer if it was already started
-    scheduleOrDeferSyncTimer();
+    if (m_setIconIDForPageURLStatement->step() != SQLITE_DONE)
+        ASSERT_NOT_REACHED();
 
-    for (auto& pageURL : pageURLs) {
-        LOG(IconDatabase, "Dispatching notification that retaining pageURL %s has a new icon", urlForLogging(pageURL).ascii().data());
-        if (m_client)
-            m_client->didChangeIconForPageURL(pageURL);
-    }
+    m_setIconIDForPageURLStatement->reset();
+    return true;
 }
 
-void IconDatabase::setIconURLForPageURL(const String& iconURLOriginal, const String& pageURLOriginal)
+Vector<char> IconDatabase::iconData(int64_t iconID)
 {
-    ASSERT_NOT_SYNC_THREAD();
-
-    // Cannot do anything with iconURLOriginal or pageURLOriginal that would end up storing them without deep copying first
-
-    ASSERT(!iconURLOriginal.isEmpty());
-
-    if (!isOpen() || !documentCanHaveIcon(pageURLOriginal))
-        return;
-
-    String iconURL, pageURL;
-    {
-        LockHolder locker(m_urlAndIconLock);
-
-        PageURLRecord* pageRecord = m_pageURLToRecordMap.get(pageURLOriginal);
-
-        // If the urls already map to each other, bail.
-        // This happens surprisingly often, and seems to cream iBench performance
-        if (pageRecord && pageRecord->iconRecord() && pageRecord->iconRecord()->iconURL() == iconURLOriginal)
-            return;
-
-        pageURL = pageURLOriginal.isolatedCopy();
-        iconURL = iconURLOriginal.isolatedCopy();
-
-        if (!pageRecord) {
-            pageRecord = new PageURLRecord(pageURL);
-            m_pageURLToRecordMap.set(pageURL, pageRecord);
-        }
-
-        RefPtr<IconRecord> iconRecord = pageRecord->iconRecord();
+    RELEASE_ASSERT(m_db.isOpen());
 
-        // Otherwise, set the new icon record for this page
-        pageRecord->setIconRecord(getOrCreateIconRecord(iconURL));
-
-        // If the current icon has only a single ref left, it is about to get wiped out.
-        // Remove it from the in-memory records and don't bother reading it in from disk anymore
-        if (iconRecord && iconRecord->hasOneRef()) {
-            ASSERT(!iconRecord->retainingPageURLs().size());
-            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());
-            LockHolder locker(m_pendingReadingLock);
-            m_iconsPendingReading.remove(iconRecord.get());
-        }
-
-        // And mark this mapping to be added to the database
-        if (!m_privateBrowsingEnabled) {
-            LockHolder locker(m_pendingSyncLock);
-            m_pageURLsPendingSync.set(pageURL, pageRecord->snapshot());
-
-            // If the icon is on its last ref, mark it for deletion
-            if (iconRecord && iconRecord->hasOneRef())
-                m_iconsPendingSync.set(iconRecord->iconURL(), iconRecord->snapshot(true));
+    if (!m_iconDataStatement) {
+        m_iconDataStatement = makeUnique<SQLiteStatement>(m_db, "SELECT IconData.data FROM IconData WHERE IconData.iconID = (?);");
+        if (m_iconDataStatement->prepare() != SQLITE_OK) {
+            LOG_ERROR("Preparing statement iconData failed");
+            m_iconDataStatement = nullptr;
+            return { };
         }
     }
 
-    // Since this mapping is new, send the notification out - but not if we're on the sync thread because that implies this mapping
-    // comes from the initial import which we don't want notifications for
-    if (!IS_ICON_SYNC_THREAD()) {
-        // Start the timer to commit this change - or further delay the timer if it was already started
-        scheduleOrDeferSyncTimer();
-
-        LOG(IconDatabase, "Dispatching notification that we changed an icon mapping for url %s", urlForLogging(pageURL).ascii().data());
-        if (m_client)
-            m_client->didChangeIconForPageURL(pageURL);
-    }
-}
-
-IconDatabase::IconLoadDecision IconDatabase::synchronousLoadDecisionForIconURL(const String& iconURL)
-{
-    ASSERT_NOT_SYNC_THREAD();
-
-    if (!isOpen() || iconURL.isEmpty())
-        return IconLoadDecision::No;
-
-    // If we have a IconRecord, it should also have its timeStamp marked because there is only two times when we create the IconRecord:
-    // 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
-    {
-        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>(WallTime::now().secondsSinceEpoch().seconds()) - static_cast<int>(icon->getTimestamp()) > iconExpirationTime ? IconLoadDecision::Yes : IconLoadDecision::No;
-        }
+    if (m_iconDataStatement->bindInt64(1, iconID) != SQLITE_OK) {
+        LOG_ERROR("IconDatabase::iconData failed: %s", m_db.lastErrorMsg());
+        return { };
     }
 
-    // If we don't have a record for it, but we *have* imported all iconURLs from disk, then we should load it now
-    LockHolder readingLocker(m_pendingReadingLock);
-    if (m_iconURLImportComplete)
-        return IconLoadDecision::Yes;
-
-    // Otherwise - since we refuse to perform I/O on the main thread to find out for sure - we return the answer that says
-    // "You might be asked to load this later, so flag that"
-    return IconLoadDecision::Unknown;
-}
-
-bool IconDatabase::synchronousIconDataKnownForIconURL(const String& iconURL)
-{
-    ASSERT_NOT_SYNC_THREAD();
-
-    LockHolder locker(m_urlAndIconLock);
-    if (IconRecord* icon = m_iconURLToRecordMap.get(iconURL))
-        return icon->imageDataStatus() != IconRecord::ImageDataStatus::Unknown;
+    Vector<char> result;
+    if (m_iconDataStatement->step() == SQLITE_ROW)
+        m_iconDataStatement->getColumnBlobAsVector(0, result);
 
-    return false;
+    m_iconDataStatement->reset();
+    return result;
 }
 
-void IconDatabase::setEnabled(bool enabled)
+Optional<int64_t> IconDatabase::addIcon(const String& iconURL, const Vector<char>& iconData)
 {
-    ASSERT_NOT_SYNC_THREAD();
+    RELEASE_ASSERT(m_db.isOpen());
+    RELEASE_ASSERT(m_allowDatabaseWrite == AllowDatabaseWrite::Yes);
 
-    if (!enabled && isOpen())
-        close();
-    m_isEnabled = enabled;
-}
+    if (!m_addIconStatement) {
+        m_addIconStatement = makeUnique<SQLiteStatement>(m_db, "INSERT INTO IconInfo (url, stamp) VALUES (?, 0);");
+        if (m_addIconStatement->prepare() != SQLITE_OK) {
+            LOG_ERROR("Preparing statement addIcon failed");
+            m_addIconStatement = nullptr;
+            return WTF::nullopt;
+        }
+    }
+    if (!m_addIconDataStatement) {
+        m_addIconDataStatement = makeUnique<SQLiteStatement>(m_db, "INSERT INTO IconData (iconID, data) VALUES (?, ?);");
+        if (m_addIconDataStatement->prepare() != SQLITE_OK) {
+            LOG_ERROR("Preparing statement addIconData failed");
+            m_addIconDataStatement = nullptr;
+            return WTF::nullopt;
+        }
+    }
 
-bool IconDatabase::isEnabled() const
-{
-    ASSERT_NOT_SYNC_THREAD();
+    if (m_addIconStatement->bindText(1, iconURL) != SQLITE_OK) {
+        LOG_ERROR("IconDatabase::addIcon failed: %s", m_db.lastErrorMsg());
+        return WTF::nullopt;
+    }
 
-    return m_isEnabled;
-}
+    m_addIconStatement->step();
+    m_addIconStatement->reset();
 
-void IconDatabase::setPrivateBrowsingEnabled(bool flag)
-{
-    m_privateBrowsingEnabled = flag;
-}
+    auto iconID = m_db.lastInsertRowID();
+    if (m_addIconDataStatement->bindInt64(1, iconID) != SQLITE_OK || m_addIconDataStatement->bindBlob(2, iconData.data(), iconData.size()) != SQLITE_OK) {
+        LOG_ERROR("IconDatabase::addIcon failed: %s", m_db.lastErrorMsg());
+        return WTF::nullopt;
+    }
 
-bool IconDatabase::isPrivateBrowsingEnabled() const
-{
-    return m_privateBrowsingEnabled;
-}
+    m_addIconDataStatement->step();
+    m_addIconDataStatement->reset();
 
-void IconDatabase::delayDatabaseCleanup()
-{
-    ++databaseCleanupCounter;
-    if (databaseCleanupCounter == 1)
-        LOG(IconDatabase, "Database cleanup is now DISABLED");
+    return iconID;
 }
 
-void IconDatabase::allowDatabaseCleanup()
+void IconDatabase::updateIconTimestamp(int64_t iconID, int64_t timestamp)
 {
-    if (--databaseCleanupCounter < 0)
-        databaseCleanupCounter = 0;
-    if (!databaseCleanupCounter)
-        LOG(IconDatabase, "Database cleanup is now ENABLED");
-}
+    RELEASE_ASSERT(m_db.isOpen());
+    RELEASE_ASSERT(m_allowDatabaseWrite == AllowDatabaseWrite::Yes);
 
-void IconDatabase::checkIntegrityBeforeOpening()
-{
-    checkIntegrityOnOpen = true;
-}
+    if (!m_updateIconTimestampStatement) {
+        m_updateIconTimestampStatement = makeUnique<SQLiteStatement>(m_db, "UPDATE IconInfo SET stamp = ? WHERE iconID = ?;");
+        if (m_updateIconTimestampStatement->prepare() != SQLITE_OK) {
+            LOG_ERROR("Preparing statement updateIconTimestamp failed");
+            m_updateIconTimestampStatement = nullptr;
+            return;
+        }
+    }
 
-IconDatabase::IconDatabase()
-    : m_syncTimer(RunLoop::main(), this, &IconDatabase::syncTimerFired)
-    , m_client(defaultClient())
-{
-    LOG(IconDatabase, "Creating IconDatabase %p", this);
-    ASSERT(isMainThread());
-}
+    if (m_updateIconTimestampStatement->bindInt64(1, timestamp) != SQLITE_OK || m_updateIconTimestampStatement->bindInt64(2, iconID) != SQLITE_OK) {
+        LOG_ERROR("IconDatabase::updateIconTimestamp failed: %s", m_db.lastErrorMsg());
+        return;
+    }
 
-IconDatabase::~IconDatabase()
-{
-    ASSERT(!isOpen());
+    m_updateIconTimestampStatement->step();
+    m_updateIconTimestampStatement->reset();
 }
 
-void IconDatabase::wakeSyncThread()
+void IconDatabase::deleteIcon(int64_t iconID)
 {
-    LockHolder locker(m_syncLock);
-
-    m_syncThreadHasWorkToDo = true;
-    m_syncCondition.notifyOne();
-}
+    RELEASE_ASSERT(m_db.isOpen());
+    RELEASE_ASSERT(m_allowDatabaseWrite == AllowDatabaseWrite::Yes);
 
-void IconDatabase::scheduleOrDeferSyncTimer()
-{
-    ASSERT_NOT_SYNC_THREAD();
+    if (!m_deletePageURLsForIconStatement) {
+        m_deletePageURLsForIconStatement = makeUnique<SQLiteStatement>(m_db, "DELETE FROM PageURL WHERE PageURL.iconID = (?);");
+        if (m_deletePageURLsForIconStatement->prepare() != SQLITE_OK) {
+            LOG_ERROR("Preparing statement deletePageURLsForIcon failed");
+            m_deletePageURLsForIconStatement = nullptr;
+            return;
+        }
+    }
+    if (!m_deleteIconDataStatement) {
+        m_deleteIconDataStatement = makeUnique<SQLiteStatement>(m_db, "DELETE FROM IconData WHERE IconData.iconID = (?);");
+        if (m_deleteIconDataStatement->prepare() != SQLITE_OK) {
+            LOG_ERROR("Preparing statement deleteIcon failed");
+            m_deleteIconDataStatement = nullptr;
+            return;
+        }
+    }
+    if (!m_deleteIconStatement) {
+        m_deleteIconStatement = makeUnique<SQLiteStatement>(m_db, "DELETE FROM IconInfo WHERE IconInfo.iconID = (?);");
+        if (m_deleteIconStatement->prepare() != SQLITE_OK) {
+            LOG_ERROR("Preparing statement deleteIcon failed");
+            m_deleteIconStatement = nullptr;
+            return;
+        }
+    }
 
-    if (m_scheduleOrDeferSyncTimerRequested)
+    if (m_deletePageURLsForIconStatement->bindInt64(1, iconID) != SQLITE_OK
+        || m_deleteIconDataStatement->bindInt64(1, iconID) != SQLITE_OK
+        || m_deleteIconStatement->bindInt64(1, iconID) != SQLITE_OK) {
+        LOG_ERROR("IconDatabase::deleteIcon failed: %s", m_db.lastErrorMsg());
         return;
+    }
 
-    m_scheduleOrDeferSyncTimerRequested = true;
-    callOnMainThread([this] {
-        m_syncTimer.startOneShot(updateTimerDelay);
-        m_scheduleOrDeferSyncTimerRequested = false;
+    m_deletePageURLsForIconStatement->step();
+    m_deleteIconDataStatement->step();
+    m_deleteIconStatement->step();
+
+    m_deletePageURLsForIconStatement->reset();
+    m_deleteIconDataStatement->reset();
+    m_deleteIconStatement->reset();
+}
+
+void IconDatabase::checkIconURLAndSetPageURLIfNeeded(const String& iconURL, const String& pageURL, AllowDatabaseWrite allowDatabaseWrite, CompletionHandler<void(bool, bool)>&& completionHandler)
+{
+    m_workQueue->dispatch([this, protectedThis = makeRef(*this), iconURL = iconURL.isolatedCopy(), pageURL = pageURL.isolatedCopy(), allowDatabaseWrite, completionHandler = WTFMove(completionHandler)]() mutable {
+        bool result = false;
+        bool changed = false;
+        if (m_db.isOpen()) {
+            bool canWriteToDatabase = m_allowDatabaseWrite == AllowDatabaseWrite::Yes && allowDatabaseWrite == AllowDatabaseWrite::Yes;
+            bool expired = false;
+            if (m_pageURLToIconURLMap.get(pageURL) == iconURL)
+                result = true;
+            else if (auto iconID = iconIDForIconURL(iconURL, expired)) {
+                if (expired && canWriteToDatabase) {
+                    SQLiteTransaction transaction(m_db);
+                    transaction.begin();
+                    deleteIcon(iconID.value());
+                    transaction.commit();
+                } else {
+                    result = true;
+                    if (!canWriteToDatabase || setIconIDForPageURL(iconID.value(), pageURL)) {
+                        m_pageURLToIconURLMap.set(pageURL, iconURL);
+                        changed = true;
+                    }
+                }
+            } else if (!canWriteToDatabase && m_loadedIcons.contains(iconURL)) {
+                // Found in memory cache.
+                result = true;
+                m_pageURLToIconURLMap.set(pageURL, iconURL);
+                changed = true;
+            }
+        }
+        startPruneTimer();
+        RunLoop::main().dispatch([result, changed, completionHandler = WTFMove(completionHandler)]() mutable {
+            completionHandler(result, changed);
+        });
     });
 }
 
-void IconDatabase::syncTimerFired()
+void IconDatabase::loadIconForPageURL(const String& pageURL, AllowDatabaseWrite allowDatabaseWrite, CompletionHandler<void(NativeImagePtr&&)>&& completionHandler)
 {
-    ASSERT_NOT_SYNC_THREAD();
-    wakeSyncThread();
-}
+    m_workQueue->dispatch([this, protectedThis = makeRef(*this), pageURL = pageURL.isolatedCopy(), allowDatabaseWrite, timestamp = WallTime::now().secondsSinceEpoch(), completionHandler = WTFMove(completionHandler)]() mutable {
+        Optional<int64_t> iconID;
+        Vector<char> iconData;
+        auto iconURL = m_pageURLToIconURLMap.get(pageURL);
+        if (m_db.isOpen() && !iconURL.isEmpty()) {
+            bool expired;
+            iconID = iconIDForIconURL(iconURL, expired);
+            if (iconID && !m_loadedIcons.contains(iconURL)) {
+                iconData = this->iconData(iconID.value());
+                m_loadedIcons.set(iconURL, std::make_pair<NativeImagePtr, MonotonicTime>(nullptr, { }));
+            }
+            bool canWriteToDatabase = m_allowDatabaseWrite == AllowDatabaseWrite::Yes && allowDatabaseWrite == AllowDatabaseWrite::Yes;
+            if (iconID && canWriteToDatabase)
+                updateIconTimestamp(iconID.value(), timestamp.secondsAs<int64_t>());
+        }
+        startPruneTimer();
+        RunLoop::main().dispatch([this, protectedThis = makeRef(*this), iconURL = WTFMove(iconURL), iconData = WTFMove(iconData), completionHandler = WTFMove(completionHandler)]() mutable {
+            if (iconURL.isEmpty()) {
+                completionHandler(nullptr);
+                return;
+            }
 
-// ******************
-// *** Any Thread ***
-// ******************
+            auto it = m_loadedIcons.find(iconURL);
+            if (it != m_loadedIcons.end() && it->value.first) {
+                auto icon = it->value.first;
+                it->value.second = MonotonicTime::now();
+                startClearLoadedIconsTimer();
+                completionHandler(WTFMove(icon));
+                return;
+            }
 
-bool IconDatabase::isOpen() const
-{
-    LockHolder locker(m_syncLock);
-    return m_syncThreadRunning || m_syncDB.isOpen();
-}
+            auto addResult = m_loadedIcons.set(iconURL, std::make_pair<NativeImagePtr, MonotonicTime>(nullptr, MonotonicTime::now()));
+            if (!iconData.isEmpty()) {
+                auto image = BitmapImage::create();
+                if (image->setData(SharedBuffer::create(WTFMove(iconData)), true) < EncodedDataStatus::SizeAvailable) {
+                    completionHandler(nullptr);
+                    return;
+                }
+                addResult.iterator->value.first = image->nativeImageForCurrentFrame();
+            }
 
-String IconDatabase::databasePath() const
-{
-    LockHolder locker(m_syncLock);
-    return m_completeDatabasePath.isolatedCopy();
+            auto icon = addResult.iterator->value.first;
+            startClearLoadedIconsTimer();
+            completionHandler(WTFMove(icon));
+        });
+    });
 }
 
-String IconDatabase::defaultDatabaseFilename()
+String IconDatabase::iconURLForPageURL(const String& pageURL)
 {
-    static NeverDestroyed<String> defaultDatabaseFilename(MAKE_STATIC_STRING_IMPL("WebpageIcons.db"));
-    return defaultDatabaseFilename.get().isolatedCopy();
+    return m_pageURLToIconURLMap.get(pageURL);
 }
 
-// Unlike getOrCreatePageURLRecord(), getOrCreateIconRecord() does not mark the icon as "interested in import"
-Ref<IconDatabase::IconRecord> IconDatabase::getOrCreateIconRecord(const String& iconURL)
+void IconDatabase::setIconForPageURL(const String& iconURL, const unsigned char* iconData, size_t iconDataSize, const String& pageURL, AllowDatabaseWrite allowDatabaseWrite, CompletionHandler<void(bool)>&& completionHandler)
 {
-    // Clients of getOrCreateIconRecord() are required to acquire the m_urlAndIconLock before calling this method
-    ASSERT(!m_urlAndIconLock.tryLock());
+    // If database write is not allowed load the icon to cache it in memory only.
+    if (m_allowDatabaseWrite == AllowDatabaseWrite::No || allowDatabaseWrite == AllowDatabaseWrite::No) {
+        bool result = true;
+        auto addResult = m_loadedIcons.set(iconURL, std::make_pair<NativeImagePtr, MonotonicTime>(nullptr, { }));
+        if (iconDataSize) {
+            auto image = BitmapImage::create();
+            if (image->setData(SharedBuffer::create(iconData, iconDataSize), true) < EncodedDataStatus::SizeAvailable)
+                result = false;
+            else
+                addResult.iterator->value.first = image->nativeImageForCurrentFrame();
+        }
+        startClearLoadedIconsTimer();
+        m_workQueue->dispatch([this, protectedThis = makeRef(*this), result, iconURL = iconURL.isolatedCopy(), pageURL = pageURL.isolatedCopy(), completionHandler = WTFMove(completionHandler)]() mutable {
+            m_pageURLToIconURLMap.set(pageURL, iconURL);
+            RunLoop::main().dispatch([result, completionHandler = WTFMove(completionHandler)]() mutable {
+                completionHandler(result);
+            });
+        });
+        return;
+    }
 
-    if (auto* icon = m_iconURLToRecordMap.get(iconURL))
-        return *icon;
+    Vector<char> data;
+    data.reserveInitialCapacity(iconDataSize);
+    data.append(reinterpret_cast<const char*>(iconData), iconDataSize);
+    m_workQueue->dispatch([this, protectedThis = makeRef(*this), iconURL = iconURL.isolatedCopy(), iconData = WTFMove(data), pageURL = pageURL.isolatedCopy(), completionHandler = WTFMove(completionHandler)]() mutable {
+        bool result = false;
+        if (m_db.isOpen()) {
+            SQLiteTransaction transaction(m_db);
+            transaction.begin();
+
+            bool expired = false;
+            auto iconID = iconIDForIconURL(iconURL, expired);
+            if (!iconID)
+                iconID = addIcon(iconURL, iconData);
+
+            if (iconID) {
+                result = true;
+                if (setIconIDForPageURL(iconID.value(), pageURL))
+                    m_pageURLToIconURLMap.set(pageURL, iconURL);
+            }
 
-    auto newIcon = IconRecord::create(iconURL);
-    m_iconURLToRecordMap.set(iconURL, newIcon.ptr());
-    return newIcon;
+            transaction.commit();
+        }
+        startPruneTimer();
+        RunLoop::main().dispatch([result, completionHandler = WTFMove(completionHandler)]() mutable {
+            completionHandler(result);
+        });
+    });
 }
 
-// This method retrieves the existing PageURLRecord, or creates a new one and marks it as "interested in the import" for later notification
-IconDatabase::PageURLRecord* IconDatabase::getOrCreatePageURLRecord(const String& pageURL)
+void IconDatabase::clear(CompletionHandler<void()>&& completionHandler)
 {
-    // Clients of getOrCreatePageURLRecord() are required to acquire the m_urlAndIconLock before calling this method
-    ASSERT(!m_urlAndIconLock.tryLock());
+    m_loadedIcons.clear();
+    m_workQueue->dispatch([this, protectedThis = makeRef(*this), completionHandler = WTFMove(completionHandler)]() mutable {
+        m_pageURLToIconURLMap.clear();
 
-    if (!documentCanHaveIcon(pageURL))
-        return nullptr;
+        if (m_db.isOpen() && m_allowDatabaseWrite == AllowDatabaseWrite::Yes) {
+            m_pageURLToIconURLMap.clear();
 
-    PageURLRecord* pageRecord = m_pageURLToRecordMap.get(pageURL);
-
-    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) {
-            LOG(IconDatabase, "Creating new PageURLRecord for pageURL %s", urlForLogging(pageURL).ascii().data());
-            pageRecord = new PageURLRecord(pageURL);
-            m_pageURLToRecordMap.set(pageURL, pageRecord);
-        }
-
-        // If the pageRecord for this page does not have an iconRecord attached to it, then it is a new pageRecord still awaiting the initial import
-        // Mark the URL as "interested in the result of the import" then bail
-        if (!pageRecord->iconRecord()) {
-            m_pageURLsPendingImport.add(pageURL);
-            return nullptr;
-        }
-    }
-
-    // We've done the initial import of all URLs known in the database. If this record doesn't exist now, it never will
-    return pageRecord;
-}
-
-
-// ************************
-// *** Sync Thread Only ***
-// ************************
-
-bool IconDatabase::shouldStopThreadActivity() const
-{
-    ASSERT_ICON_SYNC_THREAD();
-
-    return m_threadTerminationRequested || m_removeIconsRequested;
-}
-
-void IconDatabase::iconDatabaseSyncThread()
-{
-    // The call to create this thread might not complete before the thread actually starts, so we might fail this ASSERT_ICON_SYNC_THREAD() because the pointer
-    // to our thread structure hasn't been filled in yet.
-    // To fix this, the main thread acquires this lock before creating us, then releases the lock after creation is complete. A quick lock/unlock cycle here will
-    // prevent us from running before that call completes
-    m_syncLock.lock();
-    m_syncLock.unlock();
-
-    ASSERT_ICON_SYNC_THREAD();
-
-    LOG(IconDatabase, "(THREAD) IconDatabase sync thread started");
-
-#if !LOG_DISABLED
-    MonotonicTime startTime = MonotonicTime::now();
-#endif
-
-    // Need to create the database path if it doesn't already exist
-    FileSystem::makeAllDirectories(m_databaseDirectory);
-
-    // Existence of a journal file is evidence of a previous crash/force quit and automatically qualifies
-    // us to do an integrity check
-    String journalFilename = m_completeDatabasePath + "-journal";
-    if (!checkIntegrityOnOpen)
-        checkIntegrityOnOpen = FileSystem::fileExists(journalFilename);
-
-    {
-        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;
-        }
-    }
-
-    if (shouldStopThreadActivity()) {
-        syncThreadMainLoop();
-        return;
-    }
-
-#if !LOG_DISABLED
-    MonotonicTime timeStamp = MonotonicTime::now();
-    Seconds delta = timeStamp - startTime;
-    LOG(IconDatabase, "(THREAD) Open took %.4f seconds", delta.value());
-#endif
-
-    performOpenInitialization();
-    if (shouldStopThreadActivity()) {
-        syncThreadMainLoop();
-        return;
-    }
-
-#if !LOG_DISABLED
-    MonotonicTime newStamp = MonotonicTime::now();
-    Seconds totalDelta = newStamp - startTime;
-    delta = newStamp - timeStamp;
-    LOG(IconDatabase, "(THREAD) performOpenInitialization() took %.4f seconds, now %.4f seconds from thread start", delta.value(), totalDelta.value());
-    timeStamp = newStamp;
-#endif
-
-    // Uncomment the following line to simulate a long lasting URL import (*HUGE* icon databases, or network home directories)
-    // while (MonotonicTime::now() - timeStamp < 10_s);
-
-    // Read in URL mappings from the database
-    LOG(IconDatabase, "(THREAD) Starting iconURL import");
-    performURLImport();
-
-    if (shouldStopThreadActivity()) {
-        syncThreadMainLoop();
-        return;
-    }
-
-#if !LOG_DISABLED
-    newStamp = MonotonicTime::now();
-    totalDelta = newStamp - startTime;
-    delta = newStamp - timeStamp;
-    LOG(IconDatabase, "(THREAD) performURLImport() took %.4f seconds. Entering main loop %.4f seconds from thread start", delta.value(), totalDelta.value());
-#endif
-
-    LOG(IconDatabase, "(THREAD) Beginning sync");
-    syncThreadMainLoop();
-}
-
-static int databaseVersionNumber(SQLiteDatabase& db)
-{
-    return SQLiteStatement(db, "SELECT value FROM IconDatabaseInfo WHERE key = 'Version';").getColumnInt(0);
-}
-
-static bool isValidDatabase(SQLiteDatabase& db)
-{
-    // These four tables should always exist in a valid db
-    if (!db.tableExists("IconInfo") || !db.tableExists("IconData") || !db.tableExists("PageURL") || !db.tableExists("IconDatabaseInfo"))
-        return false;
-
-    if (databaseVersionNumber(db) < currentDatabaseVersion) {
-        LOG(IconDatabase, "DB version is not found or below expected valid version");
-        return false;
-    }
-
-    return true;
-}
-
-static void createDatabaseTables(SQLiteDatabase& db)
-{
-    if (!db.executeCommand("CREATE TABLE PageURL (url TEXT NOT NULL ON CONFLICT FAIL UNIQUE ON CONFLICT REPLACE,iconID INTEGER NOT NULL ON CONFLICT FAIL);")) {
-        LOG_ERROR("Could not create PageURL table in database (%i) - %s", db.lastError(), db.lastErrorMsg());
-        db.close();
-        return;
-    }
-    if (!db.executeCommand("CREATE INDEX PageURLIndex ON PageURL (url);")) {
-        LOG_ERROR("Could not create PageURL index in database (%i) - %s", db.lastError(), db.lastErrorMsg());
-        db.close();
-        return;
-    }
-    if (!db.executeCommand("CREATE TABLE IconInfo (iconID INTEGER PRIMARY KEY AUTOINCREMENT UNIQUE ON CONFLICT REPLACE, url TEXT NOT NULL ON CONFLICT FAIL UNIQUE ON CONFLICT FAIL, stamp INTEGER);")) {
-        LOG_ERROR("Could not create IconInfo table in database (%i) - %s", db.lastError(), db.lastErrorMsg());
-        db.close();
-        return;
-    }
-    if (!db.executeCommand("CREATE INDEX IconInfoIndex ON IconInfo (url, iconID);")) {
-        LOG_ERROR("Could not create PageURL index in database (%i) - %s", db.lastError(), db.lastErrorMsg());
-        db.close();
-        return;
-    }
-    if (!db.executeCommand("CREATE TABLE IconData (iconID INTEGER PRIMARY KEY AUTOINCREMENT UNIQUE ON CONFLICT REPLACE, data BLOB);")) {
-        LOG_ERROR("Could not create IconData table in database (%i) - %s", db.lastError(), db.lastErrorMsg());
-        db.close();
-        return;
-    }
-    if (!db.executeCommand("CREATE INDEX IconDataIndex ON IconData (iconID);")) {
-        LOG_ERROR("Could not create PageURL index in database (%i) - %s", db.lastError(), db.lastErrorMsg());
-        db.close();
-        return;
-    }
-    if (!db.executeCommand("CREATE TABLE IconDatabaseInfo (key TEXT NOT NULL ON CONFLICT FAIL UNIQUE ON CONFLICT REPLACE,value TEXT NOT NULL ON CONFLICT FAIL);")) {
-        LOG_ERROR("Could not create IconDatabaseInfo table in database (%i) - %s", db.lastError(), db.lastErrorMsg());
-        db.close();
-        return;
-    }
-    if (!db.executeCommand(String("INSERT INTO IconDatabaseInfo VALUES ('Version', ") + String::number(currentDatabaseVersion) + ");")) {
-        LOG_ERROR("Could not insert icon database version into IconDatabaseInfo table (%i) - %s", db.lastError(), db.lastErrorMsg());
-        db.close();
-        return;
-    }
-}
-
-void IconDatabase::performOpenInitialization()
-{
-    ASSERT_ICON_SYNC_THREAD();
-
-    if (!isOpen())
-        return;
-
-    if (checkIntegrityOnOpen) {
-        checkIntegrityOnOpen = false;
-        if (!checkIntegrity()) {
-            LOG(IconDatabase, "Integrity check was bad - dumping IconDatabase");
-
-            m_syncDB.close();
-
-            {
-                LockHolder locker(m_syncLock);
-                // Should've been consumed by SQLite, delete just to make sure we don't see it again in the future;
-                FileSystem::deleteFile(m_completeDatabasePath + "-journal");
-                FileSystem::deleteFile(m_completeDatabasePath);
-            }
-
-            // Reopen the write database, creating it from scratch
-            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;
-            }
-        }
-    }
-
-    int version = databaseVersionNumber(m_syncDB);
-
-    if (version > currentDatabaseVersion) {
-        LOG(IconDatabase, "Database version number %i is greater than our current version number %i - closing the database to prevent overwriting newer versions", version, currentDatabaseVersion);
-        m_syncDB.close();
-        m_threadTerminationRequested = true;
-        return;
-    }
-
-    if (!isValidDatabase(m_syncDB)) {
-        LOG(IconDatabase, "%s is missing or in an invalid state - reconstructing", m_completeDatabasePath.ascii().data());
-        m_syncDB.clearAllTables();
-        createDatabaseTables(m_syncDB);
-    }
-
-    // Reduce sqlite RAM cache size from default 2000 pages (~1.5kB per page). 3MB of cache for icon database is overkill
-    if (!SQLiteStatement(m_syncDB, "PRAGMA cache_size = 200;").executeCommand())
-        LOG_ERROR("SQLite database could not set cache_size");
-}
-
-bool IconDatabase::checkIntegrity()
-{
-    ASSERT_ICON_SYNC_THREAD();
-
-    SQLiteStatement integrity(m_syncDB, "PRAGMA integrity_check;");
-    if (integrity.prepare() != SQLITE_OK) {
-        LOG_ERROR("checkIntegrity failed to execute");
-        return false;
-    }
-
-    int resultCode = integrity.step();
-    if (resultCode == SQLITE_OK)
-        return true;
-
-    if (resultCode != SQLITE_ROW)
-        return false;
-
-    int columns = integrity.columnCount();
-    if (columns != 1) {
-        LOG_ERROR("Received %i columns performing integrity check, should be 1", columns);
-        return false;
-    }
-
-    String resultText = integrity.getColumnText(0);
-
-    // A successful, no-error integrity check will be "ok" - all other strings imply failure
-    if (resultText == "ok")
-        return true;
-
-    LOG_ERROR("Icon database integrity check failed - \n%s", resultText.ascii().data());
-    return false;
-}
-
-void IconDatabase::performURLImport()
-{
-    ASSERT_ICON_SYNC_THREAD();
-
-    // Do not import icons not used in the last 30 days. They will be automatically pruned later if nobody retains them.
-    // Note that IconInfo.stamp is only set when the icon data is retrieved from the server (and thus is not updated whether
-    // we use it or not). This code works anyway because the IconDatabase downloads icons again if they are older than 4 days,
-    // so if the timestamp goes back in time more than those 30 days we can be sure that the icon was not used at all.
-    String importQuery = makeString("SELECT PageURL.url, IconInfo.url, IconInfo.stamp FROM PageURL INNER JOIN IconInfo ON PageURL.iconID=IconInfo.iconID WHERE IconInfo.stamp > ", floor((WallTime::now() - notUsedIconExpirationTime).secondsSinceEpoch().seconds()), ';');
-
-    SQLiteStatement query(m_syncDB, importQuery);
-
-    if (query.prepare() != SQLITE_OK) {
-        LOG_ERROR("Unable to prepare icon url import query");
-        return;
-    }
-
-    int result = query.step();
-    while (result == SQLITE_ROW) {
-        String pageURL = query.getColumnText(0);
-        String iconURL = query.getColumnText(1);
-
-        {
-            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 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)) {
-                pageRecord = new PageURLRecord(pageURL);
-                m_pageURLToRecordMap.set(pageURL, pageRecord);
-            }
-
-            if (pageRecord) {
-                IconRecord* currentIcon = pageRecord->iconRecord();
-
-                if (!currentIcon || currentIcon->iconURL() != iconURL) {
-                    pageRecord->setIconRecord(getOrCreateIconRecord(iconURL));
-                    currentIcon = pageRecord->iconRecord();
-                }
-
-                // Regardless, the time stamp from disk still takes precedence. Until we read this icon from disk, we didn't think we'd seen it before
-                // so we marked the timestamp as "now", but it's really much older
-                currentIcon->setTimestamp(query.getColumnInt(2));
-            }
-        }
-
-        // FIXME: Currently the WebKit API supports 1 type of notification that is sent whenever we get an Icon URL for a Page URL. We might want to re-purpose it to work for
-        // getting the actually icon itself also (so each pageurl would get this notification twice) or we might want to add a second type of notification -
-        // 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
-        {
-            LockHolder locker(m_pendingReadingLock);
-            if (m_pageURLsPendingImport.contains(pageURL)) {
-                dispatchDidImportIconURLForPageURLOnMainThread(pageURL);
-                m_pageURLsPendingImport.remove(pageURL);
-            }
-        }
-
-        // Stop the import at any time of the thread has been asked to shutdown
-        if (shouldStopThreadActivity()) {
-            LOG(IconDatabase, "IconDatabase asked to terminate during performURLImport()");
-            return;
-        }
-
-        result = query.step();
-    }
-
-    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;
-    {
-        LockHolder locker(m_pendingReadingLock);
-
-        urls.appendRange(m_pageURLsPendingImport.begin(), m_pageURLsPendingImport.end());
-        m_pageURLsPendingImport.clear();
-        m_iconURLImportComplete = true;
-    }
-
-    Vector<String> urlsToNotify;
-
-    // Loop through the urls pending import
-    // Remove unretained ones if database cleanup is allowed
-    // Keep a set of ones that are retained and pending notification
-    {
-        LockHolder locker(m_urlAndIconLock);
-
-        performPendingRetainAndReleaseOperations();
-
-        for (auto& url : urls) {
-            if (!m_retainedPageURLs.contains(url)) {
-                PageURLRecord* record = m_pageURLToRecordMap.get(url);
-                if (record && !databaseCleanupCounter) {
-                    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
-                    // reading anything related to it
-                    if (iconRecord && iconRecord->hasOneRef()) {
-                        m_iconURLToRecordMap.remove(iconRecord->iconURL());
-
-                        {
-                            LockHolder locker(m_pendingReadingLock);
-                            m_pageURLsInterestedInIcons.remove(url);
-                            m_iconsPendingReading.remove(iconRecord);
-                        }
-                        {
-                            LockHolder locker(m_pendingSyncLock);
-                            m_iconsPendingSync.set(iconRecord->iconURL(), iconRecord->snapshot(true));
-                        }
-                    }
-
-                    delete record;
-                }
-            } else
-                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 (auto& url : urlsToNotify) {
-        LOG(IconDatabase, "Notifying icon info known for pageURL %s", url.ascii().data());
-        dispatchDidImportIconURLForPageURLOnMainThread(url);
-        if (shouldStopThreadActivity())
-            return;
-    }
-
-    // Notify the client that the URL import is complete in case it's managing its own pending notifications.
-    dispatchDidFinishURLImportOnMainThread();
-}
-
-void IconDatabase::syncThreadMainLoop()
-{
-    ASSERT_ICON_SYNC_THREAD();
-
-    m_syncLock.lock();
-
-    // 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.
-    m_syncThreadHasWorkToDo = false;
-
-    // It's possible thread termination is requested before the main loop even starts - in that case, just skip straight to cleanup
-    while (!m_threadTerminationRequested) {
-        m_syncLock.unlock();
-
-#if !LOG_DISABLED
-        MonotonicTime timeStamp = MonotonicTime::now();
-#endif
-        LOG(IconDatabase, "(THREAD) Main work loop starting");
-
-        // If we should remove all icons, do it now. This is an uninteruptible procedure that we will always do before quitting if it is requested
-        if (m_removeIconsRequested) {
-            removeAllIconsOnThread();
-            m_removeIconsRequested = false;
-        }
-
-        // Then, if the thread should be quitting, quit now!
-        if (m_threadTerminationRequested) {
-            cleanupSyncThread();
-            return;
-        }
-
-        {
-            LockHolder locker(m_urlAndIconLock);
-            performPendingRetainAndReleaseOperations();
-        }
-
-        bool didAnyWork = true;
-        while (didAnyWork) {
-            bool didWrite = writeToDatabase();
-            if (shouldStopThreadActivity())
-                break;
-
-            didAnyWork = readFromDatabase();
-            if (shouldStopThreadActivity())
-                break;
-
-            // Prune unretained icons after the first time we sync anything out to the database
-            // This way, pruning won't be the only operation we perform to the database by itself
-            // We also don't want to bother doing this if the thread should be terminating (the user is quitting)
-            // or if private browsing is enabled
-            // We also don't want to prune if the m_databaseCleanupCounter count is non-zero - that means someone
-            // has asked to delay pruning
-            static bool prunedUnretainedIcons = false;
-            if (didWrite && !m_privateBrowsingEnabled && !prunedUnretainedIcons && !databaseCleanupCounter) {
-#if !LOG_DISABLED
-                MonotonicTime time = MonotonicTime::now();
-#endif
-                LOG(IconDatabase, "(THREAD) Starting pruneUnretainedIcons()");
-
-                pruneUnretainedIcons();
-
-#if !LOG_DISABLED
-                Seconds delta = MonotonicTime::now() - time;
-#endif
-                LOG(IconDatabase, "(THREAD) pruneUnretainedIcons() took %.4f seconds", delta.value());
-
-                // If pruneUnretainedIcons() returned early due to requested thread termination, its still okay
-                // to mark prunedUnretainedIcons true because we're about to terminate anyway
-                prunedUnretainedIcons = true;
-            }
-
-            didAnyWork = didAnyWork || didWrite;
-            if (shouldStopThreadActivity())
-                break;
-        }
-
-#if !LOG_DISABLED
-        MonotonicTime newstamp = MonotonicTime::now();
-        Seconds delta = newstamp - timeStamp;
-        LOG(IconDatabase, "(THREAD) Main work loop ran for %.4f seconds, %s requested to terminate", delta.value(), shouldStopThreadActivity() ? "was" : "was not");
-#endif
-
-        m_syncLock.lock();
-
-        // There is some condition that is asking us to stop what we're doing now and handle a special case
-        // This is either removing all icons, or shutting down the thread to quit the app
-        // We handle those at the top of this main loop so continue to jump back up there
-        if (shouldStopThreadActivity())
-            continue;
-
-        while (!m_syncThreadHasWorkToDo)
-            m_syncCondition.wait(m_syncLock);
-
-        m_syncThreadHasWorkToDo = false;
-    }
-
-    m_syncLock.unlock();
-
-    // Thread is terminating at this point
-    cleanupSyncThread();
-}
-
-void IconDatabase::performPendingRetainAndReleaseOperations()
-{
-    // NOTE: The caller is assumed to hold m_urlAndIconLock.
-    ASSERT(!m_urlAndIconLock.tryLock());
-
-    HashCountedSet<String> toRetain;
-    HashCountedSet<String> toRelease;
-
-    {
-        LockHolder pendingWorkLocker(m_urlsToRetainOrReleaseLock);
-        if (!m_retainOrReleaseIconRequested)
-            return;
-
-        // Make a copy of the URLs to retain and/or release so we can release m_urlsToRetainOrReleaseLock ASAP.
-        // Holding m_urlAndIconLock protects this function from being re-entered.
-
-        toRetain.swap(m_urlsToRetain);
-        toRelease.swap(m_urlsToRelease);
-        m_retainOrReleaseIconRequested = false;
-    }
-
-    for (auto& entry : toRetain) {
-        ASSERT(!entry.key.impl() || entry.key.impl()->hasOneRef());
-        performRetainIconForPageURL(entry.key, entry.value);
-    }
-
-    for (auto& entry : toRelease) {
-        ASSERT(!entry.key.impl() || entry.key.impl()->hasOneRef());
-        performReleaseIconForPageURL(entry.key, entry.value);
-    }
-}
-
-bool IconDatabase::readFromDatabase()
-{
-    ASSERT_ICON_SYNC_THREAD();
-
-#if !LOG_DISABLED
-    MonotonicTime timeStamp = MonotonicTime::now();
-#endif
-
-    bool didAnyWork = false;
-
-    // We'll make a copy of the sets of things that need to be read. Then we'll verify at the time of updating the record that it still wants to be updated
-    // This way we won't hold the lock for a long period of time
-    Vector<IconRecord*> icons;
-    {
-        LockHolder locker(m_pendingReadingLock);
-        icons.appendRange(m_iconsPendingReading.begin(), m_iconsPendingReading.end());
-    }
-
-    // Keep track of icons we actually read to notify them of the new icon
-    HashSet<String> urlsToNotify;
-
-    for (unsigned i = 0; i < icons.size(); ++i) {
-        didAnyWork = true;
-        auto imageData = getImageDataForIconURLFromSQLDatabase(icons[i]->iconURL());
-
-        // Verify this icon still wants to be read from disk
-        {
-            LockHolder urlLocker(m_urlAndIconLock);
-            {
-                LockHolder readLocker(m_pendingReadingLock);
-
-                if (m_iconsPendingReading.contains(icons[i])) {
-                    // Set the new data
-                    icons[i]->setImageData(WTFMove(imageData));
-
-                    // Remove this icon from the set that needs to be read
-                    m_iconsPendingReading.remove(icons[i]);
-
-                    // We have a set of all Page URLs that retain this icon as well as all PageURLs waiting for an icon
-                    // We want to find the intersection of these two sets to notify them
-                    // Check the sizes of these two sets to minimize the number of iterations
-                    const HashSet<String>* outerHash;
-                    const HashSet<String>* innerHash;
-
-                    if (icons[i]->retainingPageURLs().size() > m_pageURLsInterestedInIcons.size()) {
-                        outerHash = &m_pageURLsInterestedInIcons;
-                        innerHash = &(icons[i]->retainingPageURLs());
-                    } else {
-                        innerHash = &m_pageURLsInterestedInIcons;
-                        outerHash = &(icons[i]->retainingPageURLs());
-                    }
-
-                    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
-                        if (urlsToNotify.size() == m_pageURLsInterestedInIcons.size())
-                            break;
-                    }
-
-                    // We don't need to notify a PageURL twice, so all the ones we're about to notify can be removed from the interested set
-                    if (urlsToNotify.size() == m_pageURLsInterestedInIcons.size())
-                        m_pageURLsInterestedInIcons.clear();
-                    else {
-                        for (auto& url : urlsToNotify)
-                            m_pageURLsInterestedInIcons.remove(url);
-                    }
-                }
-            }
-        }
-
-        if (shouldStopThreadActivity())
-            return didAnyWork;
-
-        // Now that we don't hold any locks, perform the actual notifications
-        for (HashSet<String>::const_iterator it = urlsToNotify.begin(), end = urlsToNotify.end(); it != end; ++it) {
-            LOG(IconDatabase, "Notifying icon received for pageURL %s", urlForLogging(*it).ascii().data());
-            dispatchDidImportIconDataForPageURLOnMainThread(*it);
-            if (shouldStopThreadActivity())
-                return didAnyWork;
-        }
-
-        LOG(IconDatabase, "Done notifying %i pageURLs who just received their icons", urlsToNotify.size());
-        urlsToNotify.clear();
-
-        if (shouldStopThreadActivity())
-            return didAnyWork;
-    }
-
-#if !LOG_DISABLED
-    Seconds delta = MonotonicTime::now() - timeStamp;
-    LOG(IconDatabase, "Reading from database took %.4f seconds", delta.value());
-#endif
-
-    return didAnyWork;
-}
-
-bool IconDatabase::writeToDatabase()
-{
-    ASSERT_ICON_SYNC_THREAD();
-
-#if !LOG_DISABLED
-    MonotonicTime timeStamp = MonotonicTime::now();
-#endif
-
-    bool didAnyWork = false;
-
-    // We can copy the current work queue then clear it out - If any new work comes in while we're writing out,
-    // 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
-    {
-        LockHolder locker(m_urlAndIconLock);
-        Vector<IconSnapshot> iconSnapshots;
-        Vector<PageURLSnapshot> pageSnapshots;
-        {
-            LockHolder locker(m_pendingSyncLock);
-
-            iconSnapshots.appendRange(m_iconsPendingSync.begin().values(), m_iconsPendingSync.end().values());
-            m_iconsPendingSync.clear();
-
-            pageSnapshots.appendRange(m_pageURLsPendingSync.begin().values(), m_pageURLsPendingSync.end().values());
-            m_pageURLsPendingSync.clear();
-        }
-
-        if (iconSnapshots.size() || pageSnapshots.size())
-            didAnyWork = true;
-
-        SQLiteTransaction syncTransaction(m_syncDB);
-        syncTransaction.begin();
-
-        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 (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 (snapshot.iconURL().isEmpty())
-                removePageURLFromSQLDatabase(snapshot.pageURL());
-            else
-                setIconURLForPageURLInSQLDatabase(snapshot.iconURL(), snapshot.pageURL());
-            LOG(IconDatabase, "Committed IconURL for PageURL %s to database", urlForLogging(snapshot.pageURL()).ascii().data());
-        }
-
-        syncTransaction.commit();
-    }
-
-    // Check to make sure there are no dangling PageURLs - If there are, we want to output one log message but not spam the console potentially every few seconds
-    if (didAnyWork)
-        checkForDanglingPageURLs(false);
-
-#if !LOG_DISABLED
-    Seconds delta = MonotonicTime::now() - timeStamp;
-    LOG(IconDatabase, "Updating the database took %.4f seconds", delta.value());
-#endif
-
-    return didAnyWork;
-}
-
-void IconDatabase::pruneUnretainedIcons()
-{
-    ASSERT_ICON_SYNC_THREAD();
-
-    if (!isOpen())
-        return;
-
-    // This method should only be called once per run
-    ASSERT(!m_initialPruningComplete);
-
-    // This method relies on having read in all page URLs from the database earlier.
-    ASSERT(m_iconURLImportComplete);
-
-    // Get the known PageURLs from the db, and record the ID of any that are not in the retain count set.
-    Vector<int64_t> pageIDsToDelete;
-
-    SQLiteStatement pageSQL(m_syncDB, "SELECT rowid, url FROM PageURL;");
-    pageSQL.prepare();
-
-    int result;
-    while ((result = pageSQL.step()) == SQLITE_ROW) {
-        LockHolder locker(m_urlAndIconLock);
-        if (!m_pageURLToRecordMap.contains(pageSQL.getColumnText(1)))
-            pageIDsToDelete.append(pageSQL.getColumnInt64(0));
-    }
-
-    if (result != SQLITE_DONE)
-        LOG_ERROR("Error reading PageURL table from on-disk DB");
-    pageSQL.finalize();
-
-    // Delete page URLs that were in the table, but not in our retain count set.
-    size_t numToDelete = pageIDsToDelete.size();
-    if (numToDelete) {
-        SQLiteTransaction pruningTransaction(m_syncDB);
-        pruningTransaction.begin();
-
-        SQLiteStatement pageDeleteSQL(m_syncDB, "DELETE FROM PageURL WHERE rowid = (?);");
-        pageDeleteSQL.prepare();
-        for (size_t i = 0; i < numToDelete; ++i) {
-            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 != SQLITE_DONE)
-                LOG_ERROR("Unabled to delete page with id %lli from disk", static_cast<long long>(pageIDsToDelete[i]));
-            pageDeleteSQL.reset();
-
-            // If the thread was asked to terminate, we should commit what pruning we've done so far, figuring we can
-            // finish the rest later (hopefully)
-            if (shouldStopThreadActivity()) {
-                pruningTransaction.commit();
-                return;
-            }
+            clearStatements();
+            m_db.clearAllTables();
+            m_db.runVacuumCommand();
+            createTablesIfNeeded();
         }
-        pruningTransaction.commit();
-        pageDeleteSQL.finalize();
-    }
-
-    // Deleting unreferenced icons from the Icon tables has to be atomic -
-    // If the user quits while these are taking place, they might have to wait. Thankfully this will rarely be an issue
-    // A user on a network home directory with a wildly inconsistent database might see quite a pause...
-
-    SQLiteTransaction pruningTransaction(m_syncDB);
-    pruningTransaction.begin();
-
-    // Wipe Icons that aren't retained
-    if (!m_syncDB.executeCommand("DELETE FROM IconData WHERE iconID NOT IN (SELECT iconID FROM PageURL);"))
-        LOG_ERROR("Failed to execute SQL to prune unretained icons from the on-disk IconData table");
-    if (!m_syncDB.executeCommand("DELETE FROM IconInfo WHERE iconID NOT IN (SELECT iconID FROM PageURL);"))
-        LOG_ERROR("Failed to execute SQL to prune unretained icons from the on-disk IconInfo table");
-
-    pruningTransaction.commit();
-
-    checkForDanglingPageURLs(true);
-
-    m_initialPruningComplete = true;
-}
-
-void IconDatabase::checkForDanglingPageURLs(bool pruneIfFound)
-{
-    ASSERT_ICON_SYNC_THREAD();
-
-    // This check can be relatively expensive so we don't do it in a release build unless the caller has asked us to prune any dangling
-    // entries. We also don't want to keep performing this check and reporting this error if it has already found danglers before so we
-    // keep track of whether we've found any. We skip the check in the release build pretending to have already found danglers already.
-#ifndef NDEBUG
-    static bool danglersFound = true;
-#else
-    static bool danglersFound = false;
-#endif
-
-    if ((pruneIfFound || !danglersFound) && SQLiteStatement(m_syncDB, "SELECT url FROM PageURL WHERE PageURL.iconID NOT IN (SELECT iconID FROM IconInfo) LIMIT 1;").returnsAtLeastOneResult()) {
-        danglersFound = true;
-        LOG(IconDatabase, "Dangling PageURL entries found");
-        if (pruneIfFound && !m_syncDB.executeCommand("DELETE FROM PageURL WHERE iconID NOT IN (SELECT iconID FROM IconInfo);"))
-            LOG(IconDatabase, "Unable to prune dangling PageURLs");
-    }
-}
-
-void IconDatabase::removeAllIconsOnThread()
-{
-    ASSERT_ICON_SYNC_THREAD();
-
-    LOG(IconDatabase, "Removing all icons on the sync thread");
-
-    // Delete all the prepared statements so they can start over
-    deleteAllPreparedStatements();
-
-    // To reset the on-disk database, we'll wipe all its tables then vacuum it
-    // This is easier and safer than closing it, deleting the file, and recreating from scratch
-    m_syncDB.clearAllTables();
-    m_syncDB.runVacuumCommand();
-    createDatabaseTables(m_syncDB);
-}
-
-void IconDatabase::deleteAllPreparedStatements()
-{
-    ASSERT_ICON_SYNC_THREAD();
-
-    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()
-{
-    ASSERT_ICON_SYNC_THREAD();
-
-#if !LOG_DISABLED
-    MonotonicTime timeStamp = MonotonicTime::now();
-#endif
-
-    // If the removeIcons flag is set, remove all icons from the db.
-    if (m_removeIconsRequested)
-        removeAllIconsOnThread();
-
-    // Sync remaining icons out
-    LOG(IconDatabase, "(THREAD) Doing final writeout and closure of sync thread");
-    writeToDatabase();
-
-    // Close the database
-    LockHolder locker(m_syncLock);
-
-    m_databaseDirectory = String();
-    m_completeDatabasePath = String();
-    deleteAllPreparedStatements();
-    m_syncDB.close();
-
-#if !LOG_DISABLED
-    Seconds delta = MonotonicTime::now() - timeStamp;
-    LOG(IconDatabase, "(THREAD) Final closure took %.4f seconds", delta.value());
-#endif
-
-    m_syncThreadRunning = false;
-    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(std::unique_ptr<SQLiteStatement>& statement, SQLiteDatabase& db, const String& str)
-{
-    if (statement && (&statement->database() != &db || statement->isExpired())) {
-        if (statement->isExpired())
-            LOG(IconDatabase, "SQLiteStatement associated with %s is expired", str.ascii().data());
-        statement = nullptr;
-    }
-    if (!statement) {
-        statement = makeUnique<SQLiteStatement>(db, str);
-        if (statement->prepare() != SQLITE_OK)
-            LOG_ERROR("Preparing statement %s failed", str.ascii().data());
-    }
-}
-
-void IconDatabase::setIconURLForPageURLInSQLDatabase(const String& iconURL, const String& pageURL)
-{
-    ASSERT_ICON_SYNC_THREAD();
-
-    int64_t iconID = getIconIDForIconURLFromSQLDatabase(iconURL);
-
-    if (!iconID)
-        iconID = addIconURLToSQLDatabase(iconURL);
-
-    if (!iconID) {
-        LOG_ERROR("Failed to establish an ID for iconURL %s", urlForLogging(iconURL).ascii().data());
-        ASSERT_NOT_REACHED();
-        return;
-    }
-
-    setIconIDForPageURLInSQLDatabase(iconID, pageURL);
-}
-
-void IconDatabase::setIconIDForPageURLInSQLDatabase(int64_t iconID, const String& pageURL)
-{
-    ASSERT_ICON_SYNC_THREAD();
-
-    readySQLiteStatement(m_setIconIDForPageURLStatement, m_syncDB, "INSERT INTO PageURL (url, iconID) VALUES ((?), ?);");
-    m_setIconIDForPageURLStatement->bindText(1, pageURL);
-    m_setIconIDForPageURLStatement->bindInt64(2, iconID);
-
-    int result = m_setIconIDForPageURLStatement->step();
-    if (result != SQLITE_DONE) {
-        ASSERT_NOT_REACHED();
-        LOG_ERROR("setIconIDForPageURLQuery failed for url %s", urlForLogging(pageURL).ascii().data());
-    }
-
-    m_setIconIDForPageURLStatement->reset();
-}
-
-void IconDatabase::removePageURLFromSQLDatabase(const String& pageURL)
-{
-    ASSERT_ICON_SYNC_THREAD();
-
-    readySQLiteStatement(m_removePageURLStatement, m_syncDB, "DELETE FROM PageURL WHERE url = (?);");
-    m_removePageURLStatement->bindText(1, pageURL);
-
-    if (m_removePageURLStatement->step() != SQLITE_DONE)
-        LOG_ERROR("removePageURLFromSQLDatabase failed for url %s", urlForLogging(pageURL).ascii().data());
-
-    m_removePageURLStatement->reset();
-}
-
-
-int64_t IconDatabase::getIconIDForIconURLFromSQLDatabase(const String& iconURL)
-{
-    ASSERT_ICON_SYNC_THREAD();
-
-    readySQLiteStatement(m_getIconIDForIconURLStatement, m_syncDB, "SELECT IconInfo.iconID FROM IconInfo WHERE IconInfo.url = (?);");
-    m_getIconIDForIconURLStatement->bindText(1, iconURL);
-
-    int64_t result = m_getIconIDForIconURLStatement->step();
-    if (result == SQLITE_ROW)
-        result = m_getIconIDForIconURLStatement->getColumnInt64(0);
-    else {
-        if (result != SQLITE_DONE)
-            LOG_ERROR("getIconIDForIconURLFromSQLDatabase failed for url %s", urlForLogging(iconURL).ascii().data());
-        result = 0;
-    }
-
-    m_getIconIDForIconURLStatement->reset();
-    return result;
-}
-
-int64_t IconDatabase::addIconURLToSQLDatabase(const String& iconURL)
-{
-    ASSERT_ICON_SYNC_THREAD();
-
-    // There would be a transaction here to make sure these two inserts are atomic
-    // In practice the only caller of this method is always wrapped in a transaction itself so placing another
-    // here is unnecessary
-
-    readySQLiteStatement(m_addIconToIconInfoStatement, m_syncDB, "INSERT INTO IconInfo (url, stamp) VALUES (?, 0);");
-    m_addIconToIconInfoStatement->bindText(1, iconURL);
-
-    int result = m_addIconToIconInfoStatement->step();
-    m_addIconToIconInfoStatement->reset();
-    if (result != SQLITE_DONE) {
-        LOG_ERROR("addIconURLToSQLDatabase failed to insert %s into IconInfo", urlForLogging(iconURL).ascii().data());
-        return 0;
-    }
-    int64_t iconID = m_syncDB.lastInsertRowID();
-
-    readySQLiteStatement(m_addIconToIconDataStatement, m_syncDB, "INSERT INTO IconData (iconID, data) VALUES (?, ?);");
-    m_addIconToIconDataStatement->bindInt64(1, iconID);
-
-    result = m_addIconToIconDataStatement->step();
-    m_addIconToIconDataStatement->reset();
-    if (result != SQLITE_DONE) {
-        LOG_ERROR("addIconURLToSQLDatabase failed to insert %s into IconData", urlForLogging(iconURL).ascii().data());
-        return 0;
-    }
-
-    return iconID;
-}
-
-RefPtr<SharedBuffer> IconDatabase::getImageDataForIconURLFromSQLDatabase(const String& iconURL)
-{
-    ASSERT_ICON_SYNC_THREAD();
-
-    RefPtr<SharedBuffer> imageData;
-
-    readySQLiteStatement(m_getImageDataForIconURLStatement, m_syncDB, "SELECT IconData.data FROM IconData WHERE IconData.iconID IN (SELECT iconID FROM IconInfo WHERE IconInfo.url = (?));");
-    m_getImageDataForIconURLStatement->bindText(1, iconURL);
-
-    int result = m_getImageDataForIconURLStatement->step();
-    if (result == SQLITE_ROW) {
-        Vector<char> data;
-        m_getImageDataForIconURLStatement->getColumnBlobAsVector(0, data);
-        imageData = SharedBuffer::create(data.data(), data.size());
-    } else if (result != SQLITE_DONE)
-        LOG_ERROR("getImageDataForIconURLFromSQLDatabase failed for url %s", urlForLogging(iconURL).ascii().data());
-
-    m_getImageDataForIconURLStatement->reset();
-
-    return imageData;
-}
-
-void IconDatabase::removeIconFromSQLDatabase(const String& iconURL)
-{
-    ASSERT_ICON_SYNC_THREAD();
-
-    if (iconURL.isEmpty())
-        return;
-
-    // There would be a transaction here to make sure these removals are atomic
-    // In practice the only caller of this method is always wrapped in a transaction itself so placing another here is unnecessary
-
-    // It's possible this icon is not in the database because of certain rapid browsing patterns (such as a stress test) where the
-    // icon is marked to be added then marked for removal before it is ever written to disk. No big deal, early return
-    int64_t iconID = getIconIDForIconURLFromSQLDatabase(iconURL);
-    if (!iconID)
-        return;
-
-    readySQLiteStatement(m_deletePageURLsForIconURLStatement, m_syncDB, "DELETE FROM PageURL WHERE PageURL.iconID = (?);");
-    m_deletePageURLsForIconURLStatement->bindInt64(1, iconID);
-
-    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() != 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() != SQLITE_DONE)
-        LOG_ERROR("m_deleteIconFromIconDataStatement failed for url %s", urlForLogging(iconURL).ascii().data());
-
-    m_deletePageURLsForIconURLStatement->reset();
-    m_deleteIconFromIconInfoStatement->reset();
-    m_deleteIconFromIconDataStatement->reset();
-}
-
-void IconDatabase::writeIconSnapshotToSQLDatabase(const IconSnapshot& snapshot)
-{
-    ASSERT_ICON_SYNC_THREAD();
-
-    if (snapshot.iconURL().isEmpty())
-        return;
-
-    // A nulled out timestamp and data means this icon is destined to be deleted - do that instead of writing it out
-    if (!snapshot.timestamp() && !snapshot.data()) {
-        LOG(IconDatabase, "Removing %s from on-disk database", urlForLogging(snapshot.iconURL()).ascii().data());
-        removeIconFromSQLDatabase(snapshot.iconURL());
-        return;
-    }
-
-    // There would be a transaction here to make sure these removals are atomic
-    // In practice the only caller of this method is always wrapped in a transaction itself so placing another here is unnecessary
-
-    // Get the iconID for this url
-    int64_t iconID = getIconIDForIconURLFromSQLDatabase(snapshot.iconURL());
-
-    // If there is already an iconID in place, update the database.
-    // Otherwise, insert new records
-    if (iconID) {
-        readySQLiteStatement(m_updateIconInfoStatement, m_syncDB, "UPDATE IconInfo SET stamp = ?, url = ? WHERE iconID = ?;");
-        m_updateIconInfoStatement->bindInt64(1, snapshot.timestamp());
-        m_updateIconInfoStatement->bindText(2, snapshot.iconURL());
-        m_updateIconInfoStatement->bindInt64(3, iconID);
-
-        if (m_updateIconInfoStatement->step() != SQLITE_DONE)
-            LOG_ERROR("Failed to update icon info for url %s", urlForLogging(snapshot.iconURL()).ascii().data());
-
-        m_updateIconInfoStatement->reset();
-
-        readySQLiteStatement(m_updateIconDataStatement, m_syncDB, "UPDATE IconData SET data = ? WHERE iconID = ?;");
-        m_updateIconDataStatement->bindInt64(2, iconID);
-
-        // If we *have* image data, bind it to this statement - Otherwise bind "null" for the blob data,
-        // signifying that this icon doesn't have any data
-        if (snapshot.data() && snapshot.data()->size())
-            m_updateIconDataStatement->bindBlob(1, snapshot.data()->data(), snapshot.data()->size());
-        else
-            m_updateIconDataStatement->bindNull(1);
-
-        if (m_updateIconDataStatement->step() != SQLITE_DONE)
-            LOG_ERROR("Failed to update icon data for url %s", urlForLogging(snapshot.iconURL()).ascii().data());
-
-        m_updateIconDataStatement->reset();
-    } else {
-        readySQLiteStatement(m_setIconInfoStatement, m_syncDB, "INSERT INTO IconInfo (url,stamp) VALUES (?, ?);");
-        m_setIconInfoStatement->bindText(1, snapshot.iconURL());
-        m_setIconInfoStatement->bindInt64(2, snapshot.timestamp());
-
-        if (m_setIconInfoStatement->step() != SQLITE_DONE)
-            LOG_ERROR("Failed to set icon info for url %s", urlForLogging(snapshot.iconURL()).ascii().data());
-
-        m_setIconInfoStatement->reset();
-
-        int64_t iconID = m_syncDB.lastInsertRowID();
-
-        readySQLiteStatement(m_setIconDataStatement, m_syncDB, "INSERT INTO IconData (iconID, data) VALUES (?, ?);");
-        m_setIconDataStatement->bindInt64(1, iconID);
-
-        // If we *have* image data, bind it to this statement - Otherwise bind "null" for the blob data,
-        // signifying that this icon doesn't have any data
-        if (snapshot.data() && snapshot.data()->size())
-            m_setIconDataStatement->bindBlob(2, snapshot.data()->data(), snapshot.data()->size());
-        else
-            m_setIconDataStatement->bindNull(2);
-
-        if (m_setIconDataStatement->step() != SQLITE_DONE)
-            LOG_ERROR("Failed to set icon data for url %s", urlForLogging(snapshot.iconURL()).ascii().data());
-
-        m_setIconDataStatement->reset();
-    }
-}
-
-void IconDatabase::dispatchDidImportIconURLForPageURLOnMainThread(const String& pageURL)
-{
-    ASSERT_ICON_SYNC_THREAD();
-
-    m_mainThreadNotifier.notify([this, pageURL = pageURL.isolatedCopy()] {
-        if (m_client)
-            m_client->didImportIconURLForPageURL(pageURL);
-    });
-}
-
-void IconDatabase::dispatchDidImportIconDataForPageURLOnMainThread(const String& pageURL)
-{
-    ASSERT_ICON_SYNC_THREAD();
-
-    m_mainThreadNotifier.notify([this, pageURL = pageURL.isolatedCopy()] {
-        if (m_client)
-            m_client->didImportIconDataForPageURL(pageURL);
-    });
-}
-
-void IconDatabase::dispatchDidFinishURLImportOnMainThread()
-{
-    ASSERT_ICON_SYNC_THREAD();
 
-    m_mainThreadNotifier.notify([this] {
-        if (m_client)
-            m_client->didFinishURLImport();
+        RunLoop::main().dispatch([completionHandler = WTFMove(completionHandler)]() mutable {
+            completionHandler();
+        });
     });
 }
 
index 8a8cabb..0d6059e 100644 (file)
 /*
- * Copyright (C) 2006, 2007, 2008, 2009, 2014 Apple Inc. All rights reserved.
- * Copyright (C) 2007 Justin Haygood (jhaygood@reaktix.com)
+ * Copyright (C) 2019 Igalia S.L.
  *
- * Redistribution and use in source and binary forms, with or without
- * modification, are permitted provided that the following conditions
- * are met:
- * 1. Redistributions of source code must retain the above copyright
- *    notice, this list of conditions and the following disclaimer.
- * 2. Redistributions in binary form must reproduce the above copyright
- *    notice, this list of conditions and the following disclaimer in the
- *    documentation and/or other materials provided with the distribution.
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Library General Public
+ * License as published by the Free Software Foundation; either
+ * version 2 of the License, or (at your option) any later version.
  *
- * 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 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
- * PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY
- * OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
- * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
- * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Library General Public License for more details.
+ *
+ * You should have received a copy of the GNU Library General Public License
+ * along with this library; see the file COPYING.LIB.  If not, write to
+ * the Free Software Foundation, Inc., 51 Franklin Street, Fifth Floor,
+ * Boston, MA 02110-1301, USA.
  */
 
 #pragma once
 
 #include <WebCore/NativeImage.h>
 #include <WebCore/SQLiteDatabase.h>
-#include <wtf/Condition.h>
-#include <wtf/HashCountedSet.h>
+#include <WebCore/SQLiteStatement.h>
+#include <wtf/CompletionHandler.h>
 #include <wtf/HashMap.h>
-#include <wtf/HashSet.h>
-#include <wtf/RunLoop.h>
-#include <wtf/glib/RunLoopSourcePriority.h>
-#include <wtf/text/StringHash.h>
-#include <wtf/text/WTFString.h>
-
-namespace WebCore {
-class SharedBuffer;
-}
+#include <wtf/ThreadSafeRefCounted.h>
+#include <wtf/WorkQueue.h>
 
 namespace WebKit {
 
-class IconDatabaseClient {
-    WTF_MAKE_FAST_ALLOCATED;
+class IconDatabase : public ThreadSafeRefCounted<IconDatabase> {
 public:
-    virtual ~IconDatabaseClient() = default;
-
-    virtual void didImportIconURLForPageURL(const String&) { }
-    virtual void didImportIconDataForPageURL(const String&) { }
-    virtual void didChangeIconForPageURL(const String&) { }
-    virtual void didFinishURLImport() { }
-};
-
-class IconDatabase {
-    WTF_MAKE_FAST_ALLOCATED;
-
-private:
-    class IconSnapshot {
-        WTF_MAKE_FAST_ALLOCATED;
-    public:
-        IconSnapshot() = default;
-
-        IconSnapshot(const String& iconURL, int timestamp, WebCore::SharedBuffer* data)
-            : m_iconURL(iconURL)
-            , m_timestamp(timestamp)
-            , m_data(data)
-        { }
-
-        const String& iconURL() const { return m_iconURL; }
-        int timestamp() const { return m_timestamp; }
-        WebCore::SharedBuffer* data() const { return m_data.get(); }
-
-    private:
-        String m_iconURL;
-        int m_timestamp { 0 };
-        RefPtr<WebCore::SharedBuffer> m_data;
-    };
-
-    class IconRecord : public RefCounted<IconRecord> {
-    public:
-        static Ref<IconRecord> create(const String& url)
-        {
-            return adoptRef(*new IconRecord(url));
-        }
-        ~IconRecord();
-
-        time_t getTimestamp() { return m_stamp; }
-        void setTimestamp(time_t stamp) { m_stamp = stamp; }
-
-        void setImageData(RefPtr<WebCore::SharedBuffer>&&);
-        WebCore::NativeImagePtr image(const WebCore::IntSize&);
-
-        String iconURL() { return m_iconURL; }
-
-        enum class ImageDataStatus { Present, Missing, Unknown };
-        ImageDataStatus imageDataStatus();
-
-        HashSet<String>& retainingPageURLs() { return m_retainingPageURLs; }
-
-        IconSnapshot snapshot(bool forDeletion = false) const;
-
-    private:
-        IconRecord(const String& url);
-
-        String m_iconURL;
-        time_t m_stamp { 0 };
-        RefPtr<WebCore::SharedBuffer> m_imageData;
-        WebCore::NativeImagePtr m_image;
-
-        HashSet<String> m_retainingPageURLs;
-
-        // This allows us to cache whether or not a SiteIcon has had its data set yet
-        // This helps the IconDatabase know if it has to set the data on a new object or not,
-        // and also to determine if the icon is missing data or if it just hasn't been brought
-        // in from the DB yet
-        bool m_dataSet { false };
-    };
-
-    class PageURLSnapshot {
-        WTF_MAKE_FAST_ALLOCATED;
-    public:
-        PageURLSnapshot() = default;
-
-        PageURLSnapshot(const String& pageURL, const String& iconURL)
-            : m_pageURL(pageURL)
-            , m_iconURL(iconURL)
-        { }
-
-        const String& pageURL() const { return m_pageURL; }
-        const String& iconURL() const { return m_iconURL; }
-
-    private:
-        String m_pageURL;
-        String m_iconURL;
-    };
-
-    class PageURLRecord {
-        WTF_MAKE_NONCOPYABLE(PageURLRecord); WTF_MAKE_FAST_ALLOCATED;
-    public:
-        PageURLRecord(const String& pageURL);
-        ~PageURLRecord();
-
-        inline String url() const { return m_pageURL; }
-
-        void setIconRecord(RefPtr<IconRecord>&&);
-        IconRecord* iconRecord() { return m_iconRecord.get(); }
-
-        PageURLSnapshot snapshot(bool forDeletion = false) const;
-
-        // Returns false if the page wasn't retained beforehand, true if the retain count was already 1 or higher.
-        bool retain(int count)
-        {
-            bool wasRetained = m_retainCount > 0;
-            m_retainCount += count;
-            return wasRetained;
-        }
-
-        // Returns true if the page is still retained after the call. False if the retain count just dropped to 0.
-        bool release(int count)
-        {
-            ASSERT(m_retainCount >= count);
-            m_retainCount -= count;
-            return m_retainCount > 0;
-        }
-
-        int retainCount() const { return m_retainCount; }
-
-    private:
-        String m_pageURL;
-        RefPtr<IconRecord> m_iconRecord;
-        int m_retainCount { 0 };
-    };
-
-    class MainThreadNotifier {
-        WTF_MAKE_FAST_ALLOCATED;
-    public:
-        MainThreadNotifier()
-            : m_timer(RunLoop::main(), this, &MainThreadNotifier::timerFired)
-        {
-            m_timer.setPriority(RunLoopSourcePriority::MainThreadDispatcherTimer);
-        }
-
-        void setActive(bool active)
-        {
-            m_isActive.store(active);
-        }
-
-        void notify(Function<void()>&& notification)
-        {
-            if (!m_isActive.load())
-                return;
-
-            {
-                LockHolder locker(m_notificationQueueLock);
-                m_notificationQueue.append(WTFMove(notification));
-            }
-
-            if (!m_timer.isActive())
-                m_timer.startOneShot(0_s);
-        }
+    enum class AllowDatabaseWrite { No, Yes };
 
-        void stop()
-        {
-            setActive(false);
-            m_timer.stop();
-            LockHolder locker(m_notificationQueueLock);
-            m_notificationQueue.clear();
-        }
+    static Ref<IconDatabase> create(const String& path, AllowDatabaseWrite allowDatabaseWrite)
+    {
+        return adoptRef(*new IconDatabase(path, allowDatabaseWrite));
+    }
 
-    private:
-        void timerFired()
-        {
-            Deque<Function<void()>> notificationQueue;
-            {
-                LockHolder locker(m_notificationQueueLock);
-                notificationQueue = WTFMove(m_notificationQueue);
-            }
-
-            if (!m_isActive.load())
-                return;
-
-            while (!notificationQueue.isEmpty()) {
-                auto function = notificationQueue.takeFirst();
-                function();
-            }
-        }
-
-        Deque<Function<void()>> m_notificationQueue;
-        Lock m_notificationQueueLock;
-        Atomic<bool> m_isActive;
-        RunLoop::Timer<MainThreadNotifier> m_timer;
-    };
-
-// *** Main Thread Only ***
-public:
-    IconDatabase();
     ~IconDatabase();
 
-    enum class IconLoadDecision { Yes, No, Unknown };
-
-    void setClient(std::unique_ptr<IconDatabaseClient>&&);
-
-    bool open(const String& directory, const String& filename);
-    void close();
-
-    void removeAllIcons();
-
-    void retainIconForPageURL(const String&);
-    void releaseIconForPageURL(const String&);
-    void setIconDataForIconURL(RefPtr<WebCore::SharedBuffer>&&, const String& iconURL);
-    void setIconURLForPageURL(const String& iconURL, const String& pageURL);
-
-    enum class IsKnownIcon { No, Yes };
-    std::pair<WebCore::NativeImagePtr, IsKnownIcon> synchronousIconForPageURL(const String&, const WebCore::IntSize&);
-    String synchronousIconURLForPageURL(const String&);
-    bool synchronousIconDataKnownForIconURL(const String&);
-    IconLoadDecision synchronousLoadDecisionForIconURL(const String&);
+    void invalidate();
 
-    void setEnabled(bool);
-    bool isEnabled() const;
-
-    void setPrivateBrowsingEnabled(bool flag);
-    bool isPrivateBrowsingEnabled() const;
-
-    static void delayDatabaseCleanup();
-    static void allowDatabaseCleanup();
-    static void checkIntegrityBeforeOpening();
+    void checkIconURLAndSetPageURLIfNeeded(const String& iconURL, const String& pageURL, AllowDatabaseWrite, CompletionHandler<void(bool, bool)>&&);
+    void loadIconForPageURL(const String&, AllowDatabaseWrite, CompletionHandler<void(WebCore::NativeImagePtr&&)>&&);
+    String iconURLForPageURL(const String&);
+    void setIconForPageURL(const String& iconURL, const unsigned char*, size_t, const String& pageURL, AllowDatabaseWrite, CompletionHandler<void(bool)>&&);
+    void clear(CompletionHandler<void()>&&);
 
 private:
-    void wakeSyncThread();
-    void scheduleOrDeferSyncTimer();
-    void syncTimerFired();
-
-    RunLoop::Timer<IconDatabase> m_syncTimer;
-    RefPtr<Thread> m_syncThread;
-    bool m_syncThreadRunning { false };
-    bool m_scheduleOrDeferSyncTimerRequested { false };
-
-// *** Any Thread ***
-public:
-    bool isOpen() const;
-    String databasePath() const;
-    static String defaultDatabaseFilename();
-
-private:
-    Ref<IconRecord> getOrCreateIconRecord(const String& iconURL);
-    PageURLRecord* getOrCreatePageURLRecord(const String& pageURL);
-
-    bool m_isEnabled { false };
-    bool m_privateBrowsingEnabled { false };
-
-    mutable Lock m_syncLock;
-    Condition m_syncCondition;
-    String m_databaseDirectory;
-    // Holding m_syncLock is required when accessing m_completeDatabasePath
-    String m_completeDatabasePath;
-
-    bool m_threadTerminationRequested { false };
-    bool m_removeIconsRequested { false };
-    bool m_iconURLImportComplete { false };
-    bool m_syncThreadHasWorkToDo { false };
-
-    Lock m_urlAndIconLock;
-    // Holding m_urlAndIconLock is required when accessing any of the following data structures or the objects they contain
-    HashMap<String, IconRecord*> m_iconURLToRecordMap;
-    HashMap<String, PageURLRecord*> m_pageURLToRecordMap;
-    HashSet<String> m_retainedPageURLs;
-
-    Lock m_pendingSyncLock;
-    // Holding m_pendingSyncLock is required when accessing any of the following data structures
-    HashMap<String, PageURLSnapshot> m_pageURLsPendingSync;
-    HashMap<String, IconSnapshot> m_iconsPendingSync;
-
-    Lock m_pendingReadingLock;
-    // Holding m_pendingSyncLock is required when accessing any of the following data structures - when dealing with IconRecord*s, holding m_urlAndIconLock is also required
-    HashSet<String> m_pageURLsPendingImport;
-    HashSet<String> m_pageURLsInterestedInIcons;
-    HashSet<IconRecord*> m_iconsPendingReading;
-
-    Lock m_urlsToRetainOrReleaseLock;
-    // Holding m_urlsToRetainOrReleaseLock is required when accessing any of the following data structures.
-    HashCountedSet<String> m_urlsToRetain;
-    HashCountedSet<String> m_urlsToRelease;
-    bool m_retainOrReleaseIconRequested { false };
-
-// *** Sync Thread Only ***
-public:
-    bool shouldStopThreadActivity() const;
-
-private:
-    void iconDatabaseSyncThread();
-
-    // The following block of methods are called exclusively by the sync thread to manage i/o to and from the database
-    // Each method should periodically monitor m_threadTerminationRequested when it makes sense to return early on shutdown
-    void performOpenInitialization();
-    bool checkIntegrity();
-    void performURLImport();
-    void syncThreadMainLoop();
-    bool readFromDatabase();
-    bool writeToDatabase();
-    void pruneUnretainedIcons();
-    void checkForDanglingPageURLs(bool pruneIfFound);
-    void removeAllIconsOnThread();
-    void deleteAllPreparedStatements();
-    void* cleanupSyncThread();
-    void performRetainIconForPageURL(const String&, int retainCount);
-    void performReleaseIconForPageURL(const String&, int releaseCount);
-
-    bool m_initialPruningComplete { false };
-
-    void setIconURLForPageURLInSQLDatabase(const String&, const String&);
-    void setIconIDForPageURLInSQLDatabase(int64_t, const String&);
-    void removePageURLFromSQLDatabase(const String& pageURL);
-    int64_t getIconIDForIconURLFromSQLDatabase(const String& iconURL);
-    int64_t addIconURLToSQLDatabase(const String&);
-    RefPtr<WebCore::SharedBuffer> getImageDataForIconURLFromSQLDatabase(const String& iconURL);
-    void removeIconFromSQLDatabase(const String& iconURL);
-    void writeIconSnapshotToSQLDatabase(const IconSnapshot&);
-
-    void performPendingRetainAndReleaseOperations();
-
-    // Methods to dispatch client callbacks on the main thread
-    void dispatchDidImportIconURLForPageURLOnMainThread(const String&);
-    void dispatchDidImportIconDataForPageURLOnMainThread(const String&);
-    void dispatchDidFinishURLImportOnMainThread();
-
-    // The client is set by the main thread before the thread starts, and from then on is only used by the sync thread
-    std::unique_ptr<IconDatabaseClient> m_client;
-
-    WebCore::SQLiteDatabase m_syncDB;
-
+    IconDatabase(const String&, AllowDatabaseWrite);
+
+    bool createTablesIfNeeded();
+    void populatePageURLToIconURLMap();
+    void pruneTimerFired();
+    void startPruneTimer();
+    void clearStatements();
+    void clearLoadedIconsTimerFired();
+    void startClearLoadedIconsTimer();
+    Optional<int64_t> iconIDForIconURL(const String&, bool& expired);
+    bool setIconIDForPageURL(int64_t, const String&);
+    Vector<char> iconData(int64_t);
+    Optional<int64_t> addIcon(const String&, const Vector<char>&);
+    void updateIconTimestamp(int64_t iconID, int64_t timestamp);
+    void deleteIcon(int64_t);
+
+    Ref<WorkQueue> m_workQueue;
+    AllowDatabaseWrite m_allowDatabaseWrite { AllowDatabaseWrite::Yes };
+    WebCore::SQLiteDatabase m_db;
+    HashMap<String, String> m_pageURLToIconURLMap;
+    HashMap<String, std::pair<WebCore::NativeImagePtr, MonotonicTime>> m_loadedIcons;
+
+    std::unique_ptr<WebCore::SQLiteStatement> m_iconIDForIconURLStatement;
     std::unique_ptr<WebCore::SQLiteStatement> m_setIconIDForPageURLStatement;
-    std::unique_ptr<WebCore::SQLiteStatement> m_removePageURLStatement;
-    std::unique_ptr<WebCore::SQLiteStatement> m_getIconIDForIconURLStatement;
-    std::unique_ptr<WebCore::SQLiteStatement> m_getImageDataForIconURLStatement;
-    std::unique_ptr<WebCore::SQLiteStatement> m_addIconToIconInfoStatement;
-    std::unique_ptr<WebCore::SQLiteStatement> m_addIconToIconDataStatement;
-    std::unique_ptr<WebCore::SQLiteStatement> m_getImageDataStatement;
-    std::unique_ptr<WebCore::SQLiteStatement> m_deletePageURLsForIconURLStatement;
-    std::unique_ptr<WebCore::SQLiteStatement> m_deleteIconFromIconInfoStatement;
-    std::unique_ptr<WebCore::SQLiteStatement> m_deleteIconFromIconDataStatement;
-    std::unique_ptr<WebCore::SQLiteStatement> m_updateIconInfoStatement;
-    std::unique_ptr<WebCore::SQLiteStatement> m_updateIconDataStatement;
-    std::unique_ptr<WebCore::SQLiteStatement> m_setIconInfoStatement;
-    std::unique_ptr<WebCore::SQLiteStatement> m_setIconDataStatement;
-
-    MainThreadNotifier m_mainThreadNotifier;
+    std::unique_ptr<WebCore::SQLiteStatement> m_iconDataStatement;
+    std::unique_ptr<WebCore::SQLiteStatement> m_addIconStatement;
+    std::unique_ptr<WebCore::SQLiteStatement> m_addIconDataStatement;
+    std::unique_ptr<WebCore::SQLiteStatement> m_updateIconTimestampStatement;
+    std::unique_ptr<WebCore::SQLiteStatement> m_deletePageURLsForIconStatement;
+    std::unique_ptr<WebCore::SQLiteStatement> m_deleteIconDataStatement;
+    std::unique_ptr<WebCore::SQLiteStatement> m_deleteIconStatement;
+    std::unique_ptr<WebCore::SQLiteStatement> m_pruneIconsStatement;
+
+    std::unique_ptr<RunLoop::Timer<IconDatabase>> m_pruneTimer;
+    RunLoop::Timer<IconDatabase> m_clearLoadedIconsTimer;
 };
 
 } // namespace WebKit
index 28e6620..36cc282 100644 (file)
@@ -22,7 +22,6 @@
 
 #include "IconDatabase.h"
 #include "WebKitFaviconDatabasePrivate.h"
-#include "WebPreferences.h"
 #include <WebCore/Image.h>
 #include <WebCore/IntSize.h>
 #include <WebCore/RefPtrCairo.h>
 #include <glib/gi18n-lib.h>
 #include <wtf/FileSystem.h>
 #include <wtf/RunLoop.h>
-#include <wtf/SetForScope.h>
 #include <wtf/glib/GRefPtr.h>
 #include <wtf/glib/GUniquePtr.h>
 #include <wtf/glib/WTFGType.h>
 #include <wtf/text/CString.h>
-#include <wtf/text/StringHash.h>
 
 using namespace WebKit;
 using namespace WebCore;
@@ -68,35 +65,14 @@ enum {
 
 static guint signals[LAST_SIGNAL] = { 0, };
 
-typedef Vector<GRefPtr<GTask> > PendingIconRequestVector;
-typedef HashMap<String, PendingIconRequestVector*> PendingIconRequestMap;
-
 struct _WebKitFaviconDatabasePrivate {
-    std::unique_ptr<IconDatabase> iconDatabase;
-    Vector<std::pair<String, Function<void(bool)>>> pendingLoadDecisions;
-    PendingIconRequestMap pendingIconRequests;
-    HashMap<String, String> pageURLToIconURLMap;
-    bool isURLImportCompleted;
-    bool isSettingIcon;
+    RefPtr<IconDatabase> iconDatabase;
 };
 
 WEBKIT_DEFINE_TYPE(WebKitFaviconDatabase, webkit_favicon_database, G_TYPE_OBJECT)
 
-static void webkitFaviconDatabaseDispose(GObject* object)
-{
-    WebKitFaviconDatabase* database = WEBKIT_FAVICON_DATABASE(object);
-
-    if (webkitFaviconDatabaseIsOpen(database))
-        database->priv->iconDatabase->close();
-
-    G_OBJECT_CLASS(webkit_favicon_database_parent_class)->dispose(object);
-}
-
 static void webkit_favicon_database_class_init(WebKitFaviconDatabaseClass* faviconDatabaseClass)
 {
-    GObjectClass* gObjectClass = G_OBJECT_CLASS(faviconDatabaseClass);
-    gObjectClass->dispose = webkitFaviconDatabaseDispose;
-
     /**
      * WebKitFaviconDatabase::favicon-changed:
      * @database: the object on which the signal is emitted
@@ -121,234 +97,98 @@ static void webkit_favicon_database_class_init(WebKitFaviconDatabaseClass* favic
         G_TYPE_STRING);
 }
 
-#if PLATFORM(GTK)
-struct GetFaviconSurfaceAsyncData {
-    ~GetFaviconSurfaceAsyncData()
-    {
-        if (shouldReleaseIconForPageURL)
-            faviconDatabase->priv->iconDatabase->releaseIconForPageURL(pageURL);
-    }
-
-    GRefPtr<WebKitFaviconDatabase> faviconDatabase;
-    String pageURL;
-    RefPtr<cairo_surface_t> icon;
-    GRefPtr<GCancellable> cancellable;
-    bool shouldReleaseIconForPageURL;
-};
-WEBKIT_DEFINE_ASYNC_DATA_STRUCT(GetFaviconSurfaceAsyncData)
-
-static RefPtr<cairo_surface_t> getIconSurfaceSynchronously(WebKitFaviconDatabase* database, const String& pageURL, GError** error)
+WebKitFaviconDatabase* webkitFaviconDatabaseCreate()
 {
-    ASSERT(RunLoop::isMain());
-
-    // The exact size we pass is irrelevant to the iconDatabase code.
-    // We must pass something greater than 0x0 to get an icon.
-    auto iconData = database->priv->iconDatabase->synchronousIconForPageURL(pageURL, WebCore::IntSize(1, 1));
-    if (iconData.second == IconDatabase::IsKnownIcon::No) {
-        g_set_error(error, WEBKIT_FAVICON_DATABASE_ERROR, WEBKIT_FAVICON_DATABASE_ERROR_FAVICON_UNKNOWN, _("Unknown favicon for page %s"), pageURL.utf8().data());
-        return nullptr;
-    }
-
-    if (!iconData.first) {
-        g_set_error(error, WEBKIT_FAVICON_DATABASE_ERROR, WEBKIT_FAVICON_DATABASE_ERROR_FAVICON_NOT_FOUND, _("Page %s does not have a favicon"), pageURL.utf8().data());
-        return nullptr;
-    }
-
-    return iconData.first;
+    return WEBKIT_FAVICON_DATABASE(g_object_new(WEBKIT_TYPE_FAVICON_DATABASE, nullptr));
 }
 
-static void deletePendingIconRequests(WebKitFaviconDatabase* database, PendingIconRequestVector* requests, const String& pageURL)
+static bool webkitFaviconDatabaseIsOpen(WebKitFaviconDatabase* database)
 {
-    database->priv->pendingIconRequests.remove(pageURL);
-    delete requests;
+    return !!database->priv->iconDatabase;
 }
 
-static void processPendingIconsForPageURL(WebKitFaviconDatabase* database, const String& pageURL)
+void webkitFaviconDatabaseOpen(WebKitFaviconDatabase* database, const String& path, bool isEphemeral)
 {
-    PendingIconRequestVector* pendingIconRequests = database->priv->pendingIconRequests.get(pageURL);
-    if (!pendingIconRequests)
+    if (webkitFaviconDatabaseIsOpen(database))
         return;
 
-    GUniqueOutPtr<GError> error;
-    RefPtr<cairo_surface_t> icon = getIconSurfaceSynchronously(database, pageURL, &error.outPtr());
-
-    for (size_t i = 0; i < pendingIconRequests->size(); ++i) {
-        GTask* task = pendingIconRequests->at(i).get();
-        if (error)
-            g_task_return_error(task, error.release());
-        else {
-            GetFaviconSurfaceAsyncData* data = static_cast<GetFaviconSurfaceAsyncData*>(g_task_get_task_data(task));
-            data->icon = icon;
-            data->shouldReleaseIconForPageURL = false;
-            g_task_return_boolean(task, TRUE);
-        }
-    }
-    deletePendingIconRequests(database, pendingIconRequests, pageURL);
+    database->priv->iconDatabase = IconDatabase::create(path, isEphemeral ? IconDatabase::AllowDatabaseWrite::No : IconDatabase::AllowDatabaseWrite::Yes);
 }
-#endif
 
-static void webkitFaviconDatabaseSetIconURLForPageURL(WebKitFaviconDatabase* database, const String& iconURL, const String& pageURL)
+void webkitFaviconDatabaseClose(WebKitFaviconDatabase* database)
 {
-    WebKitFaviconDatabasePrivate* priv = database->priv;
-    if (!priv->isURLImportCompleted)
-        return;
-
-    if (pageURL.isEmpty())
-        return;
-
-    const String& currentIconURL = priv->pageURLToIconURLMap.get(pageURL);
-    if (iconURL == currentIconURL)
-        return;
-
-    priv->pageURLToIconURLMap.set(pageURL, iconURL);
-    if (priv->isSettingIcon)
-        return;
-
-    g_signal_emit(database, signals[FAVICON_CHANGED], 0, pageURL.utf8().data(), iconURL.utf8().data());
+    database->priv->iconDatabase = nullptr;
 }
 
-class WebKitIconDatabaseClient final : public IconDatabaseClient {
-public:
-    explicit WebKitIconDatabaseClient(WebKitFaviconDatabase* database)
-        : m_database(database)
-    {
-    }
-
-private:
-    void didImportIconURLForPageURL(const String& pageURL) override
-    {
-        String iconURL = m_database->priv->iconDatabase->synchronousIconURLForPageURL(pageURL);
-        webkitFaviconDatabaseSetIconURLForPageURL(m_database, iconURL, pageURL);
-    }
-
-    void didChangeIconForPageURL(const String& pageURL) override
-    {
-        if (m_database->priv->isSettingIcon)
-            return;
-        String iconURL = m_database->priv->iconDatabase->synchronousIconURLForPageURL(pageURL);
-        webkitFaviconDatabaseSetIconURLForPageURL(m_database, iconURL, pageURL);
-    }
-
-    void didImportIconDataForPageURL(const String& pageURL) override
-    {
 #if PLATFORM(GTK)
-        processPendingIconsForPageURL(m_database, pageURL);
-#endif
-        String iconURL = m_database->priv->iconDatabase->synchronousIconURLForPageURL(pageURL);
-        webkitFaviconDatabaseSetIconURLForPageURL(m_database, iconURL, pageURL);
-    }
-
-    void didFinishURLImport() override
-    {
-        WebKitFaviconDatabasePrivate* priv = m_database->priv;
-
-        while (!priv->pendingLoadDecisions.isEmpty()) {
-            auto iconURLAndCallback = priv->pendingLoadDecisions.takeLast();
-            auto decision = priv->iconDatabase->synchronousLoadDecisionForIconURL(iconURLAndCallback.first);
-            // Decisions should never be unknown after the inital import is complete.
-            ASSERT(decision != IconDatabase::IconLoadDecision::Unknown);
-            iconURLAndCallback.second(decision == IconDatabase::IconLoadDecision::Yes);
-        }
-
-        priv->isURLImportCompleted = true;
+void webkitFaviconDatabaseGetLoadDecisionForIcon(WebKitFaviconDatabase* database, const LinkIcon& icon, const String& pageURL, bool isEphemeral, Function<void(bool)>&& completionHandler)
+{
+    if (!webkitFaviconDatabaseIsOpen(database)) {
+        completionHandler(false);
+        return;
     }
 
-    WebKitFaviconDatabase* m_database;
-};
+    database->priv->iconDatabase->checkIconURLAndSetPageURLIfNeeded(icon.url.string(), pageURL,
+        isEphemeral ? IconDatabase::AllowDatabaseWrite::No : IconDatabase::AllowDatabaseWrite::Yes,
+            [database = GRefPtr<WebKitFaviconDatabase>(database), url = icon.url.string().isolatedCopy(), pageURL = pageURL.isolatedCopy(), completionHandler = WTFMove(completionHandler)](bool found, bool changed) {
+            if (!webkitFaviconDatabaseIsOpen(database.get()))
+                return;
 
-WebKitFaviconDatabase* webkitFaviconDatabaseCreate()
-{
-    return WEBKIT_FAVICON_DATABASE(g_object_new(WEBKIT_TYPE_FAVICON_DATABASE, nullptr));
+            if (found && changed)
+                g_signal_emit(database.get(), signals[FAVICON_CHANGED], 0, pageURL.utf8().data(), url.utf8().data());
+            completionHandler(!found);
+        });
 }
 
-void webkitFaviconDatabaseOpen(WebKitFaviconDatabase* database, const String& path)
+void webkitFaviconDatabaseSetIconForPageURL(WebKitFaviconDatabase* database, const LinkIcon& icon, API::Data& iconData, const String& pageURL, bool isEphemeral)
 {
-    if (webkitFaviconDatabaseIsOpen(database))
+    if (!webkitFaviconDatabaseIsOpen(database))
         return;
 
-    WebKitFaviconDatabasePrivate* priv = database->priv;
-    priv->iconDatabase = makeUnique<IconDatabase>();
-    priv->iconDatabase->setClient(makeUnique<WebKitIconDatabaseClient>(database));
-    IconDatabase::delayDatabaseCleanup();
-    priv->iconDatabase->setEnabled(true);
-
-    if (!priv->iconDatabase->open(FileSystem::directoryName(path), FileSystem::pathGetFileName(path))) {
-        priv->iconDatabase = nullptr;
-        IconDatabase::allowDatabaseCleanup();
-    }
-}
+    database->priv->iconDatabase->setIconForPageURL(icon.url.string(), iconData.bytes(), iconData.size(), pageURL,
+        isEphemeral ? IconDatabase::AllowDatabaseWrite::No : IconDatabase::AllowDatabaseWrite::Yes,
+        [database = GRefPtr<WebKitFaviconDatabase>(database), url = icon.url.string().isolatedCopy(), pageURL = pageURL.isolatedCopy()](bool success) {
+            if (!webkitFaviconDatabaseIsOpen(database.get()) || !success)
+                return;
 
-bool webkitFaviconDatabaseIsOpen(WebKitFaviconDatabase* database)
-{
-    return database->priv->iconDatabase && database->priv->iconDatabase->isOpen();
+            g_signal_emit(database.get(), signals[FAVICON_CHANGED], 0, pageURL.utf8().data(), url.utf8().data());
+        });
 }
+#endif
 
-void webkitFaviconDatabaseSetPrivateBrowsingEnabled(WebKitFaviconDatabase* database, bool enabled)
+GQuark webkit_favicon_database_error_quark(void)
 {
-    if (database->priv->iconDatabase)
-        database->priv->iconDatabase->setPrivateBrowsingEnabled(enabled);
+    return g_quark_from_static_string("WebKitFaviconDatabaseError");
 }
 
 #if PLATFORM(GTK)
-void webkitFaviconDatabaseGetLoadDecisionForIcon(WebKitFaviconDatabase* database, const LinkIcon& icon, const String& pageURL, Function<void(bool)>&& completionHandler)
+void webkitFaviconDatabaseGetFaviconInternal(WebKitFaviconDatabase* database, const gchar* pageURI, bool isEphemeral, GCancellable* cancellable, GAsyncReadyCallback callback, gpointer userData)
 {
     if (!webkitFaviconDatabaseIsOpen(database)) {
-        completionHandler(false);
+        g_task_report_new_error(database, callback, userData, 0,
+            WEBKIT_FAVICON_DATABASE_ERROR, WEBKIT_FAVICON_DATABASE_ERROR_NOT_INITIALIZED, _("Favicons database not initialized yet"));
         return;
     }
 
-    WebKitFaviconDatabasePrivate* priv = database->priv;
-    auto decision = priv->iconDatabase->synchronousLoadDecisionForIconURL(icon.url.string());
-    switch (decision) {
-    case IconDatabase::IconLoadDecision::Unknown:
-        priv->pendingLoadDecisions.append(std::make_pair(icon.url.string(), WTFMove(completionHandler)));
-        priv->iconDatabase->setIconURLForPageURL(icon.url.string(), pageURL);
-        break;
-    case IconDatabase::IconLoadDecision::No:
-        priv->iconDatabase->setIconURLForPageURL(icon.url.string(), pageURL);
-        completionHandler(false);
-        break;
-    case IconDatabase::IconLoadDecision::Yes:
-        completionHandler(true);
-        break;
-    }
-}
-
-void webkitFaviconDatabaseSetIconForPageURL(WebKitFaviconDatabase* database, const LinkIcon& icon, API::Data& iconData, const String& pageURL)
-{
-    if (!webkitFaviconDatabaseIsOpen(database))
-        return;
-
-    if (pageURL.isEmpty())
+    if (g_str_has_prefix(pageURI, "about:")) {
+        g_task_report_new_error(database, callback, userData, 0,
+            WEBKIT_FAVICON_DATABASE_ERROR, WEBKIT_FAVICON_DATABASE_ERROR_FAVICON_NOT_FOUND, _("Page %s does not have a favicon"), pageURI);
         return;
-
-    WebKitFaviconDatabasePrivate* priv = database->priv;
-    SetForScope<bool> change(priv->isSettingIcon, true);
-    priv->iconDatabase->setIconURLForPageURL(icon.url.string(), pageURL);
-    priv->iconDatabase->setIconDataForIconURL(SharedBuffer::create(iconData.bytes(), iconData.size()), icon.url.string());
-    webkitFaviconDatabaseSetIconURLForPageURL(database, icon.url.string(), pageURL);
-    g_signal_emit(database, signals[FAVICON_CHANGED], 0, pageURL.utf8().data(), icon.url.string().utf8().data());
-    processPendingIconsForPageURL(database, pageURL);
-}
-
-static PendingIconRequestVector* getOrCreatePendingIconRequests(WebKitFaviconDatabase* database, const String& pageURL)
-{
-    PendingIconRequestVector* icons = database->priv->pendingIconRequests.get(pageURL);
-    if (!icons) {
-        icons = new PendingIconRequestVector;
-        database->priv->pendingIconRequests.set(pageURL, icons);
     }
 
-    return icons;
-}
-#endif
-
-GQuark webkit_favicon_database_error_quark(void)
-{
-    return g_quark_from_static_string("WebKitFaviconDatabaseError");
+    GRefPtr<GTask> task = adoptGRef(g_task_new(database, cancellable, callback, userData));
+    WebKitFaviconDatabasePrivate* priv = database->priv;
+    priv->iconDatabase->loadIconForPageURL(String::fromUTF8(pageURI), isEphemeral ? IconDatabase::AllowDatabaseWrite::No : IconDatabase::AllowDatabaseWrite::Yes,
+        [task = WTFMove(task), pageURI = CString(pageURI)](RefPtr<cairo_surface_t>&& icon) {
+            if (!icon) {
+                g_task_return_new_error(task.get(), WEBKIT_FAVICON_DATABASE_ERROR, WEBKIT_FAVICON_DATABASE_ERROR_FAVICON_UNKNOWN,
+                    _("Unknown favicon for page %s"), pageURI.data());
+                return;
+            }
+            g_task_return_pointer(task.get(), icon.leakRef(), reinterpret_cast<GDestroyNotify>(cairo_surface_destroy));
+        });
 }
 
-#if PLATFORM(GTK)
 /**
  * webkit_favicon_database_get_favicon:
  * @database: a #WebKitFaviconDatabase
@@ -377,59 +217,7 @@ void webkit_favicon_database_get_favicon(WebKitFaviconDatabase* database, const
     g_return_if_fail(WEBKIT_IS_FAVICON_DATABASE(database));
     g_return_if_fail(pageURI);
 
-    if (!webkitFaviconDatabaseIsOpen(database)) {
-        g_task_report_new_error(database, callback, userData, 0,
-            WEBKIT_FAVICON_DATABASE_ERROR, WEBKIT_FAVICON_DATABASE_ERROR_NOT_INITIALIZED, _("Favicons database not initialized yet"));
-        return;
-    }
-
-    if (g_str_has_prefix(pageURI, "about:")) {
-        g_task_report_new_error(database, callback, userData, 0,
-            WEBKIT_FAVICON_DATABASE_ERROR, WEBKIT_FAVICON_DATABASE_ERROR_FAVICON_NOT_FOUND, _("Page %s does not have a favicon"), pageURI);
-        return;
-    }
-
-    GRefPtr<GTask> task = adoptGRef(g_task_new(database, cancellable, callback, userData));
-
-    GetFaviconSurfaceAsyncData* data = createGetFaviconSurfaceAsyncData();
-    data->faviconDatabase = database;
-    data->pageURL = String::fromUTF8(pageURI);
-    g_task_set_task_data(task.get(), data, reinterpret_cast<GDestroyNotify>(destroyGetFaviconSurfaceAsyncData));
-
-    WebKitFaviconDatabasePrivate* priv = database->priv;
-    priv->iconDatabase->retainIconForPageURL(data->pageURL);
-
-    // We ask for the icon directly. If we don't get the icon data now,
-    // we'll be notified later (even if the database is still importing icons).
-    GUniqueOutPtr<GError> error;
-    data->icon = getIconSurfaceSynchronously(database, data->pageURL, &error.outPtr());
-    if (data->icon) {
-        g_task_return_boolean(task.get(), TRUE);
-        return;
-    }
-
-    // At this point we still don't know whether we will get a valid icon for pageURL.
-    data->shouldReleaseIconForPageURL = true;
-
-    if (g_error_matches(error.get(), WEBKIT_FAVICON_DATABASE_ERROR, WEBKIT_FAVICON_DATABASE_ERROR_FAVICON_NOT_FOUND)) {
-        g_task_return_error(task.get(), error.release());
-        return;
-    }
-
-    // If there's not a valid icon, but there's an iconURL registered,
-    // or it's still not registered but the import process hasn't
-    // finished yet, we need to wait for iconDataReadyForPage to be
-    // called before making and informed decision.
-    String iconURLForPageURL = priv->iconDatabase->synchronousIconURLForPageURL(data->pageURL);
-    if (!iconURLForPageURL.isEmpty() || !priv->isURLImportCompleted) {
-        PendingIconRequestVector* iconRequests = getOrCreatePendingIconRequests(database, data->pageURL);
-        ASSERT(iconRequests);
-        iconRequests->append(task);
-        return;
-    }
-
-    g_task_return_new_error(task.get(), WEBKIT_FAVICON_DATABASE_ERROR, WEBKIT_FAVICON_DATABASE_ERROR_FAVICON_UNKNOWN,
-        _("Unknown favicon for page %s"), pageURI);
+    webkitFaviconDatabaseGetFaviconInternal(database, pageURI, false, cancellable, callback, userData);
 }
 
 /**
@@ -449,11 +237,10 @@ cairo_surface_t* webkit_favicon_database_get_favicon_finish(WebKitFaviconDatabas
     g_return_val_if_fail(g_task_is_valid(result, database), 0);
 
     GTask* task = G_TASK(result);
-    if (!g_task_propagate_boolean(task, error))
-        return 0;
+    if (auto* icon = g_task_propagate_pointer(task, error))
+        return static_cast<cairo_surface_t*>(icon);
 
-    GetFaviconSurfaceAsyncData* data = static_cast<GetFaviconSurfaceAsyncData*>(g_task_get_task_data(task));
-    return cairo_surface_reference(data->icon.get());
+    return nullptr;
 }
 #endif
 
@@ -476,7 +263,7 @@ gchar* webkit_favicon_database_get_favicon_uri(WebKitFaviconDatabase* database,
     if (!webkitFaviconDatabaseIsOpen(database))
         return nullptr;
 
-    String iconURLForPageURL = database->priv->iconDatabase->synchronousIconURLForPageURL(String::fromUTF8(pageURL));
+    String iconURLForPageURL = database->priv->iconDatabase->iconURLForPageURL(String::fromUTF8(pageURL));
     if (iconURLForPageURL.isEmpty())
         return nullptr;
 
@@ -496,5 +283,5 @@ void webkit_favicon_database_clear(WebKitFaviconDatabase* database)
     if (!webkitFaviconDatabaseIsOpen(database))
         return;
 
-    database->priv->iconDatabase->removeAllIcons();
+    database->priv->iconDatabase->clear([] { });
 }
index 9ab4d5b..94144a1 100644 (file)
 #include <WebCore/LinkIcon.h>
 
 WebKitFaviconDatabase* webkitFaviconDatabaseCreate();
-void webkitFaviconDatabaseOpen(WebKitFaviconDatabase*, const String& path);
-bool webkitFaviconDatabaseIsOpen(WebKitFaviconDatabase*);
-void webkitFaviconDatabaseSetPrivateBrowsingEnabled(WebKitFaviconDatabase*, bool);
+void webkitFaviconDatabaseOpen(WebKitFaviconDatabase*, const String& path, bool isEphemeral);
+void webkitFaviconDatabaseClose(WebKitFaviconDatabase*);
 #if PLATFORM(GTK)
-void webkitFaviconDatabaseGetLoadDecisionForIcon(WebKitFaviconDatabase*, const WebCore::LinkIcon&, const String&, Function<void(bool)>&&);
-void webkitFaviconDatabaseSetIconForPageURL(WebKitFaviconDatabase*, const WebCore::LinkIcon&, API::Data&, const String&);
+void webkitFaviconDatabaseGetLoadDecisionForIcon(WebKitFaviconDatabase*, const WebCore::LinkIcon&, const String&, bool isEphemeral, Function<void(bool)>&&);
+void webkitFaviconDatabaseSetIconForPageURL(WebKitFaviconDatabase*, const WebCore::LinkIcon&, API::Data&, const String&, bool isEphemeral);
+void webkitFaviconDatabaseGetFaviconInternal(WebKitFaviconDatabase*, const gchar* pageURI, bool isEphemeral, GCancellable*, GAsyncReadyCallback, gpointer);
 #endif
index 27d8b4f..4b79caa 100644 (file)
@@ -180,7 +180,6 @@ struct _WebKitWebContextPrivate {
     WebKitProcessModel processModel;
 
     HashMap<uint64_t, WebKitWebView*> webViews;
-    unsigned ephemeralPageCount;
 
     CString webExtensionsDirectory;
     GRefPtr<GVariant> webExtensionsInitializationUserData;
@@ -373,6 +372,11 @@ static void webkitWebContextDispose(GObject* object)
         priv->websiteDataManager = nullptr;
     }
 
+    if (priv->faviconDatabase) {
+        webkitFaviconDatabaseClose(priv->faviconDatabase.get());
+        priv->faviconDatabase = nullptr;
+    }
+
     G_OBJECT_CLASS(webkit_web_context_parent_class)->dispose(object);
 }
 
@@ -873,31 +877,6 @@ static void ensureFaviconDatabase(WebKitWebContext* context)
     priv->faviconDatabase = adoptGRef(webkitFaviconDatabaseCreate());
 }
 
-static void webkitWebContextEnableIconDatabasePrivateBrowsingIfNeeded(WebKitWebContext* context, WebKitWebView* webView)
-{
-    if (webkit_web_context_is_ephemeral(context))
-        return;
-    if (!webkit_web_view_is_ephemeral(webView))
-        return;
-
-    if (!context->priv->ephemeralPageCount && context->priv->faviconDatabase)
-        webkitFaviconDatabaseSetPrivateBrowsingEnabled(context->priv->faviconDatabase.get(), true);
-    context->priv->ephemeralPageCount++;
-}
-
-static void webkitWebContextDisableIconDatabasePrivateBrowsingIfNeeded(WebKitWebContext* context, WebKitWebView* webView)
-{
-    if (webkit_web_context_is_ephemeral(context))
-        return;
-    if (!webkit_web_view_is_ephemeral(webView))
-        return;
-
-    ASSERT(context->priv->ephemeralPageCount);
-    context->priv->ephemeralPageCount--;
-    if (!context->priv->ephemeralPageCount && context->priv->faviconDatabase)
-        webkitFaviconDatabaseSetPrivateBrowsingEnabled(context->priv->faviconDatabase.get(), false);
-}
-
 /**
  * webkit_web_context_set_favicon_database_directory:
  * @context: a #WebKitWebContext
@@ -938,10 +917,7 @@ void webkit_web_context_set_favicon_database_directory(WebKitWebContext* context
         "WebpageIcons.db", nullptr));
 
     // Setting the path will cause the icon database to be opened.
-    webkitFaviconDatabaseOpen(priv->faviconDatabase.get(), FileSystem::stringFromFileSystemRepresentation(faviconDatabasePath.get()));
-
-    if (webkit_web_context_is_ephemeral(context))
-        webkitFaviconDatabaseSetPrivateBrowsingEnabled(priv->faviconDatabase.get(), true);
+    webkitFaviconDatabaseOpen(priv->faviconDatabase.get(), FileSystem::stringFromFileSystemRepresentation(faviconDatabasePath.get()), webkit_web_context_is_ephemeral(context));
 }
 
 /**
@@ -1729,10 +1705,6 @@ bool webkitWebContextIsLoadingCustomProtocol(WebKitWebContext* context, uint64_t
 
 void webkitWebContextCreatePageForWebView(WebKitWebContext* context, WebKitWebView* webView, WebKitUserContentManager* userContentManager, WebKitWebView* relatedView)
 {
-    // FIXME: icon database private mode is global, not per page, so while there are
-    // pages in private mode we need to enable the private mode in the icon database.
-    webkitWebContextEnableIconDatabasePrivateBrowsingIfNeeded(context, webView);
-
     auto pageConfiguration = API::PageConfiguration::create();
     pageConfiguration->setProcessPool(context->priv->processPool.get());
     pageConfiguration->setPreferences(webkitSettingsGetPreferences(webkit_web_view_get_settings(webView)));
@@ -1751,7 +1723,6 @@ void webkitWebContextCreatePageForWebView(WebKitWebContext* context, WebKitWebVi
 
 void webkitWebContextWebViewDestroyed(WebKitWebContext* context, WebKitWebView* webView)
 {
-    webkitWebContextDisableIconDatabasePrivateBrowsingIfNeeded(context, webView);
     context->priv->webViews.remove(webkit_web_view_get_page_id(webView));
 }
 
index 1e5ebe7..9a01494 100644 (file)
@@ -559,7 +559,7 @@ static void webkitWebViewRequestFavicon(WebKitWebView* webView)
     WebKitWebViewPrivate* priv = webView->priv;
     priv->faviconCancellable = adoptGRef(g_cancellable_new());
     WebKitFaviconDatabase* database = webkit_web_context_get_favicon_database(priv->context.get());
-    webkit_favicon_database_get_favicon(database, priv->activeURI.data(), priv->faviconCancellable.get(), gotFaviconCallback, webView);
+    webkitFaviconDatabaseGetFaviconInternal(database, priv->activeURI.data(), priv->isEphemeral, priv->faviconCancellable.get(), gotFaviconCallback, webView);
 }
 
 static void webkitWebViewUpdateFaviconURI(WebKitWebView* webView, const char* faviconURI)
@@ -2169,14 +2169,19 @@ void webkitWebViewLoadFailedWithTLSErrors(WebKitWebView* webView, const char* fa
 #if PLATFORM(GTK)
 void webkitWebViewGetLoadDecisionForIcon(WebKitWebView* webView, const LinkIcon& icon, Function<void(bool)>&& completionHandler)
 {
+    // We only support favicons for now.
+    if (icon.type != LinkIconType::Favicon) {
+        completionHandler(false);
+        return;
+    }
     WebKitFaviconDatabase* database = webkit_web_context_get_favicon_database(webView->priv->context.get());
-    webkitFaviconDatabaseGetLoadDecisionForIcon(database, icon, getPage(webView).pageLoadState().activeURL(), WTFMove(completionHandler));
+    webkitFaviconDatabaseGetLoadDecisionForIcon(database, icon, getPage(webView).pageLoadState().activeURL(), webView->priv->isEphemeral, WTFMove(completionHandler));
 }
 
 void webkitWebViewSetIcon(WebKitWebView* webView, const LinkIcon& icon, API::Data& iconData)
 {
     WebKitFaviconDatabase* database = webkit_web_context_get_favicon_database(webView->priv->context.get());
-    webkitFaviconDatabaseSetIconForPageURL(database, icon, iconData, getPage(webView).pageLoadState().activeURL());
+    webkitFaviconDatabaseSetIconForPageURL(database, icon, iconData, getPage(webView).pageLoadState().activeURL(), webView->priv->isEphemeral);
 }
 #endif
 
index 0c161ea..5ea2b51 100644 (file)
@@ -1,5 +1,24 @@
 2019-09-30  Carlos Garcia Campos  <cgarcia@igalia.com>
 
+        [GTK] IconDatabase is not thread-safe
+        https://bugs.webkit.org/show_bug.cgi?id=201303
+
+        Reviewed by Žan Doberšek.
+
+        Rewrite the WebKitFaviconDatabase tests, splitting tests cases again and making them independent to each other.
+
+        * TestWebKitAPI/Tests/WebKitGLib/TestWebKitFaviconDatabase.cpp:
+        (testFaviconDatabaseInitialization):
+        (testFaviconDatabaseGetFavicon):
+        (ephemeralViewFaviconChanged):
+        (testFaviconDatabaseEphemeral):
+        (testFaviconDatabaseClear):
+        (beforeAll):
+        (afterAll):
+        * TestWebKitAPI/glib/TestExpectations.json: TestWebKitFaviconDatabase shouls always pass now.
+
+2019-09-30  Carlos Garcia Campos  <cgarcia@igalia.com>
+
         [GTK][WPE] Add about:gpu
         https://bugs.webkit.org/show_bug.cgi?id=202305
 
index 63cac05..f63c80f 100644 (file)
 
 #include "config.h"
 
+#if PLATFORM(GTK)
+
 #include "WebKitTestServer.h"
 #include "WebViewTest.h"
 #include <glib/gstdio.h>
 #include <libsoup/soup.h>
 #include <wtf/glib/GUniquePtr.h>
 
-#if PLATFORM(GTK)
 static WebKitTestServer* kServer;
-#endif
 
 class FaviconDatabaseTest: public WebViewTest {
 public:
     MAKE_GLIB_TEST_FIXTURE(FaviconDatabaseTest);
 
     FaviconDatabaseTest()
+        : m_database(webkit_web_context_get_favicon_database(m_webContext.get()))
     {
-#if PLATFORM(GTK)
-        WebKitFaviconDatabase* database = webkit_web_context_get_favicon_database(m_webContext.get());
-        g_signal_connect(database, "favicon-changed", G_CALLBACK(faviconChangedCallback), this);
-#endif
+        g_assert_true(WEBKIT_IS_FAVICON_DATABASE(m_database));
+        assertObjectIsDeletedWhenTestFinishes(G_OBJECT(m_database));
+        g_signal_connect(m_database, "favicon-changed", G_CALLBACK(faviconChangedCallback), this);
     }
 
     ~FaviconDatabaseTest()
     {
-#if PLATFORM(GTK)
         if (m_favicon)
             cairo_surface_destroy(m_favicon);
 
-        WebKitFaviconDatabase* database = webkit_web_context_get_favicon_database(m_webContext.get());
-        g_signal_handlers_disconnect_matched(database, G_SIGNAL_MATCH_DATA, 0, 0, 0, 0, this);
-#endif
+        g_signal_handlers_disconnect_matched(m_database, G_SIGNAL_MATCH_DATA, 0, 0, 0, 0, this);
+    }
+
+    void open(const char* directory)
+    {
+        GUniquePtr<char> databaseDirectory(g_build_filename(dataDirectory(), directory, nullptr));
+        webkit_web_context_set_favicon_database_directory(m_webContext.get(), databaseDirectory.get());
+        g_assert_cmpstr(databaseDirectory.get(), ==, webkit_web_context_get_favicon_database_directory(m_webContext.get()));
     }
 
-#if PLATFORM(GTK)
     static void faviconChangedCallback(WebKitFaviconDatabase* database, const char* pageURI, const char* faviconURI, FaviconDatabaseTest* test)
     {
         if (!g_strcmp0(webkit_web_view_get_uri(test->m_webView), pageURI)) {
@@ -62,19 +65,29 @@ public:
         }
     }
 
-    static void viewFaviconChangedCallback(WebKitWebView* webView, GParamSpec* pspec, gpointer data)
+    static void viewFaviconChangedCallback(WebKitWebView* webView, GParamSpec* pspec, FaviconDatabaseTest* test)
     {
-        FaviconDatabaseTest* test = static_cast<FaviconDatabaseTest*>(data);
         g_assert_true(test->m_webView == webView);
         test->m_faviconNotificationReceived = true;
-        test->quitMainLoop();
+        if (test->m_loadFinished)
+            test->quitMainLoop();
+    }
+
+    static void viewLoadChangedCallback(WebKitWebView* webView, WebKitLoadEvent loadEvent, FaviconDatabaseTest* test)
+    {
+        g_assert_true(test->m_webView == webView);
+        if (loadEvent != WEBKIT_LOAD_FINISHED)
+            return;
+
+        test->m_loadFinished = true;
+        if (test->m_faviconNotificationReceived)
+            test->quitMainLoop();
     }
 
     static void getFaviconCallback(GObject* sourceObject, GAsyncResult* result, void* data)
     {
         FaviconDatabaseTest* test = static_cast<FaviconDatabaseTest*>(data);
-        WebKitFaviconDatabase* database = webkit_web_context_get_favicon_database(test->m_webContext.get());
-        test->m_favicon = webkit_favicon_database_get_favicon_finish(database, result, &test->m_error.outPtr());
+        test->m_favicon = webkit_favicon_database_get_favicon_finish(test->m_database, result, &test->m_error.outPtr());
         test->quitMainLoop();
     }
 
@@ -86,16 +99,25 @@ public:
         g_signal_handler_disconnect(m_webView, handlerID);
     }
 
+    void waitUntilLoadFinishedAndFaviconChanged()
+    {
+        m_faviconNotificationReceived = false;
+        m_loadFinished = false;
+        unsigned long faviconChangedID = g_signal_connect(m_webView, "notify::favicon", G_CALLBACK(viewFaviconChangedCallback), this);
+        unsigned long loadChangedID = g_signal_connect(m_webView, "load-changed", G_CALLBACK(viewLoadChangedCallback), this);
+        g_main_loop_run(m_mainLoop);
+        g_signal_handler_disconnect(m_webView, faviconChangedID);
+        g_signal_handler_disconnect(m_webView, loadChangedID);
+    }
+
     void getFaviconForPageURIAndWaitUntilReady(const char* pageURI)
     {
         if (m_favicon) {
             cairo_surface_destroy(m_favicon);
-            m_favicon = 0;
+            m_favicon = nullptr;
         }
 
-        WebKitFaviconDatabase* database = webkit_web_context_get_favicon_database(m_webContext.get());
-        webkit_favicon_database_get_favicon(database, pageURI, 0, getFaviconCallback, this);
-
+        webkit_favicon_database_get_favicon(m_database, pageURI, 0, getFaviconCallback, this);
         g_main_loop_run(m_mainLoop);
     }
 
@@ -108,16 +130,15 @@ public:
         m_waitingForFaviconURI = false;
     }
 
+    WebKitFaviconDatabase* m_database { nullptr };
     cairo_surface_t* m_favicon { nullptr };
-
     CString m_faviconURI;
     GUniqueOutPtr<GError> m_error;
     bool m_faviconNotificationReceived { false };
+    bool m_loadFinished { false };
     bool m_waitingForFaviconURI { false };
-#endif
 };
 
-#if PLATFORM(GTK)
 static void
 serverCallback(SoupServer* server, SoupMessage* message, const char* path, GHashTable* query, SoupClientContext* context, void* data)
 {
@@ -150,163 +171,139 @@ serverCallback(SoupServer* server, SoupMessage* message, const char* path, GHash
     soup_message_body_complete(message->response_body);
 }
 
-static void testNotInitialized(FaviconDatabaseTest* test)
+static void testFaviconDatabaseInitialization(FaviconDatabaseTest* test, gconstpointer)
 {
-    // Try to retrieve a valid favicon from a not initialized database.
     test->getFaviconForPageURIAndWaitUntilReady(kServer->getURIForPath("/foo").data());
     g_assert_null(test->m_favicon);
-    g_assert_nonnull(test->m_error);
-    g_assert_cmpint(test->m_error->code, ==, WEBKIT_FAVICON_DATABASE_ERROR_NOT_INITIALIZED);
-}
-#endif
-
-static void testSetDirectory(FaviconDatabaseTest* test)
-{
-    webkit_web_context_set_favicon_database_directory(test->m_webContext.get(), Test::dataDirectory());
-    g_assert_cmpstr(Test::dataDirectory(), ==, webkit_web_context_get_favicon_database_directory(test->m_webContext.get()));
-}
-
-#if PLATFORM(GTK)
-static void testClearDatabase(FaviconDatabaseTest* test)
-{
-    WebKitFaviconDatabase* database = webkit_web_context_get_favicon_database(test->m_webContext.get());
-    webkit_favicon_database_clear(database);
+    g_assert_error(test->m_error.get(), WEBKIT_FAVICON_DATABASE_ERROR, WEBKIT_FAVICON_DATABASE_ERROR_NOT_INITIALIZED);
 
-    GUniquePtr<char> iconURI(webkit_favicon_database_get_favicon_uri(database, kServer->getURIForPath("/foo").data()));
-    g_assert_null(iconURI);
+    test->open("testFaviconDatabaseInitialization");
+    GUniquePtr<char> databaseFile(g_build_filename(webkit_web_context_get_favicon_database_directory(test->m_webContext.get()), "WebpageIcons.db", nullptr));
+    g_assert_true(g_file_test(databaseFile.get(), G_FILE_TEST_IS_REGULAR));
 }
 
-static void ephemeralViewLoadChanged(WebKitWebView* webView, WebKitLoadEvent loadEvent, WebViewTest* test)
+static void testFaviconDatabaseGetFavicon(FaviconDatabaseTest* test, gconstpointer)
 {
-    if (loadEvent != WEBKIT_LOAD_FINISHED)
-        return;
-    g_signal_handlers_disconnect_by_func(webView, reinterpret_cast<void*>(ephemeralViewLoadChanged), test);
-    test->quitMainLoop();
-}
-
-static void testPrivateBrowsing(FaviconDatabaseTest* test)
-{
-    auto webView = Test::adoptView(g_object_new(WEBKIT_TYPE_WEB_VIEW,
-        "web-context", test->m_webContext.get(),
-        "is-ephemeral", TRUE,
-        nullptr));
-    g_signal_connect(webView.get(), "load-changed", G_CALLBACK(ephemeralViewLoadChanged), test);
-    webkit_web_view_load_uri(webView.get(), kServer->getURIForPath("/foo").data());
-    g_main_loop_run(test->m_mainLoop);
-
-    // An ephemeral web view should not write to the database.
-    test->getFaviconForPageURIAndWaitUntilReady(kServer->getURIForPath("/foo").data());
-    g_assert_null(test->m_favicon);
-    g_assert_nonnull(test->m_error);
-}
+    test->open("testFaviconDatabaseGetFavicon");
 
-static void testGetFavicon(FaviconDatabaseTest* test)
-{
-    // We need to load the page first to ensure the icon data will be
-    // in the database in case there's an associated favicon.
     test->loadURI(kServer->getURIForPath("/foo").data());
-    test->waitUntilFaviconChanged();
+    test->waitUntilLoadFinishedAndFaviconChanged();
     CString faviconURI = kServer->getURIForPath("/icon/favicon.ico");
 
-    // Check the API retrieving a valid favicon.
     test->getFaviconForPageURIAndWaitUntilReady(kServer->getURIForPath("/foo").data());
     g_assert_nonnull(test->m_favicon);
-    g_assert_cmpstr(test->m_faviconURI.data(), ==, faviconURI.data());
-    g_assert_no_error(test->m_error.get());
-
-    // Check that width and height match those from blank.ico (16x16 favicon).
     g_assert_cmpint(cairo_image_surface_get_width(test->m_favicon), ==, 16);
     g_assert_cmpint(cairo_image_surface_get_height(test->m_favicon), ==, 16);
+    g_assert_cmpstr(test->m_faviconURI.data(), ==, faviconURI.data());
+    g_assert_no_error(test->m_error.get());
 
-    // Check that another page with the same favicon return the same icon.
+    // Check that another page with the same favicon returns the same icon.
     cairo_surface_t* favicon = cairo_surface_reference(test->m_favicon);
     test->loadURI(kServer->getURIForPath("/bar").data());
-    // It's a new page in the database, so favicon will change twice, first to reset it
-    // and then when the icon is loaded.
-    test->waitUntilFaviconChanged();
+    test->waitUntilLoadFinishedAndFaviconChanged();
+    // favicon changes twice, first to reset it and then when the new icon is loaded.
     test->waitUntilFaviconChanged();
     test->getFaviconForPageURIAndWaitUntilReady(kServer->getURIForPath("/bar").data());
     g_assert_nonnull(test->m_favicon);
-    g_assert_cmpstr(test->m_faviconURI.data(), ==, faviconURI.data());
     g_assert_true(test->m_favicon == favicon);
+    g_assert_cmpstr(test->m_faviconURI.data(), ==, faviconURI.data());
     g_assert_no_error(test->m_error.get());
     cairo_surface_destroy(favicon);
 
-    // Check the API retrieving an invalid favicon. Favicon changes only once to reset it, then
-    // the database is updated with the favicon URI, but not with favicon image data.
+    faviconURI = kServer->getURIForPath("/favicon.ico");
     test->loadURI(kServer->getURIForPath("/nofavicon").data());
-    test->waitUntilFaviconChanged();
+    test->waitUntilLoadFinishedAndFaviconChanged();
     test->waitUntilFaviconURIChanged();
-
     test->getFaviconForPageURIAndWaitUntilReady(kServer->getURIForPath("/nofavicon").data());
     g_assert_null(test->m_favicon);
-    g_assert_nonnull(test->m_error);
+    g_assert_cmpstr(test->m_faviconURI.data(), ==, faviconURI.data());
+    g_assert_error(test->m_error.get(), WEBKIT_FAVICON_DATABASE_ERROR, WEBKIT_FAVICON_DATABASE_ERROR_FAVICON_UNKNOWN);
+
+    // Loading an icon that is already in the database should emit
+    // WebKitWebView::notify::favicon, but not WebKitFaviconDatabase::icon-changed.
+    g_assert_null(webkit_web_view_get_favicon(test->m_webView));
+    test->m_faviconURI = { };
+    test->loadURI(kServer->getURIForPath("/foo").data());
+    test->waitUntilFaviconChanged();
+    g_assert_true(test->m_faviconURI.isNull());
+    g_assert_nonnull(webkit_web_view_get_favicon(test->m_webView));
 }
 
-static void testGetFaviconURI(FaviconDatabaseTest* test)
+static void ephemeralViewFaviconChanged(WebKitWebView* webView, GParamSpec*, WebViewTest* test)
 {
-    WebKitFaviconDatabase* database = webkit_web_context_get_favicon_database(test->m_webContext.get());
-
-    CString baseURI = kServer->getURIForPath("/foo");
-    GUniquePtr<char> iconURI(webkit_favicon_database_get_favicon_uri(database, baseURI.data()));
-    ASSERT_CMP_CSTRING(iconURI.get(), ==, kServer->getURIForPath("/icon/favicon.ico"));
+    g_signal_handlers_disconnect_by_func(webView, reinterpret_cast<void*>(ephemeralViewFaviconChanged), test);
+    test->quitMainLoop();
 }
 
-static void testWebViewFavicon(FaviconDatabaseTest* test)
+static void testFaviconDatabaseEphemeral(FaviconDatabaseTest* test, gconstpointer)
 {
-    test->m_faviconURI = CString();
-
-    cairo_surface_t* iconFromWebView = webkit_web_view_get_favicon(test->m_webView);
-    g_assert_null(iconFromWebView);
+    // If the context is ephemeral, the database is not created if it doesn't exist.
+    GRefPtr<WebKitWebContext> ephemeralContext = adoptGRef(webkit_web_context_new_ephemeral());
+    GUniquePtr<char> databaseDirectory(g_build_filename(Test::dataDirectory(), "testFaviconDatabaseEphemeral", nullptr));
+    webkit_web_context_set_favicon_database_directory(ephemeralContext.get(), databaseDirectory.get());
+    g_assert_cmpstr(databaseDirectory.get(), ==, webkit_web_context_get_favicon_database_directory(ephemeralContext.get()));
+    GUniquePtr<char> databaseFile(g_build_filename(databaseDirectory.get(), "WebpageIcons.db", nullptr));
+    g_assert_false(g_file_test(databaseFile.get(), G_FILE_TEST_EXISTS));
+    ephemeralContext = nullptr;
+
+    test->open("testFaviconDatabaseEphemeral");
+    g_assert_true(g_file_test(databaseFile.get(), G_FILE_TEST_EXISTS));
 
     test->loadURI(kServer->getURIForPath("/foo").data());
-    test->waitUntilFaviconChanged();
-    g_assert_true(test->m_faviconNotificationReceived);
-    // The icon is known and hasn't changed in the database, so notify::favicon is emitted
-    // but WebKitFaviconDatabase::icon-changed isn't.
-    g_assert_true(test->m_faviconURI.isNull());
+    test->waitUntilLoadFinishedAndFaviconChanged();
+
+    // An ephemeral web view doesn't write to the database.
+    auto webView = Test::adoptView(g_object_new(WEBKIT_TYPE_WEB_VIEW,
+        "web-context", test->m_webContext.get(), "is-ephemeral", TRUE, nullptr));
+    g_signal_connect(webView.get(), "notify::favicon", G_CALLBACK(ephemeralViewFaviconChanged), test);
+    webkit_web_view_load_uri(webView.get(), kServer->getURIForPath("/bar").data());
+    g_main_loop_run(test->m_mainLoop);
+
+    // We get a favicon, but it's only in database memory cache.
+    g_assert_nonnull(webkit_favicon_database_get_favicon_uri(test->m_database, kServer->getURIForPath("/bar").data()));
+
+    // If the database exists, it's used in read only mode for epeheral contexts.
+    ephemeralContext = adoptGRef(webkit_web_context_new_ephemeral());
+    webkit_web_context_set_favicon_database_directory(ephemeralContext.get(), databaseDirectory.get());
+    auto* ephemeralDatabase = webkit_web_context_get_favicon_database(ephemeralContext.get());
+    g_assert_nonnull(webkit_favicon_database_get_favicon_uri(ephemeralDatabase, kServer->getURIForPath("/foo").data()));
 
-    iconFromWebView = webkit_web_view_get_favicon(test->m_webView);
-    g_assert_nonnull(iconFromWebView);
-    g_assert_cmpuint(cairo_image_surface_get_width(iconFromWebView), ==, 16);
-    g_assert_cmpuint(cairo_image_surface_get_height(iconFromWebView), ==, 16);
+    // Page URL loaded in ephemeral web view is not in the database.
+    g_assert_null(webkit_favicon_database_get_favicon_uri(ephemeralDatabase, kServer->getURIForPath("/bar").data()));
+    ephemeralContext = nullptr;
 }
-#endif
 
-static void testFaviconDatabase(FaviconDatabaseTest* test, gconstpointer)
+void testFaviconDatabaseClear(FaviconDatabaseTest* test, gconstpointer)
 {
-    // These tests depend on this order to run properly so we declare them in a single one.
-    // See https://bugs.webkit.org/show_bug.cgi?id=111434.
-#if PLATFORM(GTK)
-    testNotInitialized(test);
-#endif
+    test->open("testFaviconDatabaseClear");
+    test->loadURI(kServer->getURIForPath("/foo").data());
+    test->waitUntilLoadFinishedAndFaviconChanged();
+    test->getFaviconForPageURIAndWaitUntilReady(kServer->getURIForPath("/foo").data());
+    g_assert_nonnull(test->m_favicon);
+    g_assert_nonnull(webkit_favicon_database_get_favicon_uri(test->m_database, kServer->getURIForPath("/foo").data()));
 
-    testSetDirectory(test);
+    webkit_favicon_database_clear(test->m_database);
 
-#if PLATFORM(GTK)
-    testPrivateBrowsing(test);
-    testGetFavicon(test);
-    testWebViewFavicon(test);
-    testGetFaviconURI(test);
-    testClearDatabase(test);
-#endif
+    test->getFaviconForPageURIAndWaitUntilReady(kServer->getURIForPath("/foo").data());
+    g_assert_null(test->m_favicon);
+    g_assert_null(webkit_favicon_database_get_favicon_uri(test->m_database, kServer->getURIForPath("/foo").data()));
 }
 
 void beforeAll()
 {
-#if PLATFORM(GTK)
     // Start a soup server for testing.
     kServer = new WebKitTestServer();
     kServer->run(serverCallback);
-#endif
 
-    // Add tests to the suite.
-    FaviconDatabaseTest::add("WebKitFaviconDatabase", "favicon-database-test", testFaviconDatabase);
+    FaviconDatabaseTest::add("WebKitFaviconDatabase", "initialization", testFaviconDatabaseInitialization);
+    FaviconDatabaseTest::add("WebKitFaviconDatabase", "get-favicon", testFaviconDatabaseGetFavicon);
+    FaviconDatabaseTest::add("WebKitFaviconDatabase", "ephemeral", testFaviconDatabaseEphemeral);
+    FaviconDatabaseTest::add("WebKitFaviconDatabase", "clear", testFaviconDatabaseClear);
 }
 
 void afterAll()
 {
-#if PLATFORM(GTK)
     delete kServer;
-#endif
 }
+
+#endif
index 4eb4bfe..3f49f85 100644 (file)
@@ -122,7 +122,6 @@ ADD_WK2_TEST(TestAuthentication ${TOOLS_DIR}/TestWebKitAPI/Tests/WebKitGLib/Test
 ADD_WK2_TEST(TestAutomationSession ${TOOLS_DIR}/TestWebKitAPI/Tests/WebKitGLib/TestAutomationSession.cpp)
 ADD_WK2_TEST(TestBackForwardList ${TOOLS_DIR}/TestWebKitAPI/Tests/WebKitGLib/TestBackForwardList.cpp)
 ADD_WK2_TEST(TestDownloads ${TOOLS_DIR}/TestWebKitAPI/Tests/WebKitGLib/TestDownloads.cpp)
-ADD_WK2_TEST(TestWebKitFaviconDatabase ${TOOLS_DIR}/TestWebKitAPI/Tests/WebKitGLib/TestWebKitFaviconDatabase.cpp)
 ADD_WK2_TEST(TestWebKitFindController ${TOOLS_DIR}/TestWebKitAPI/Tests/WebKitGLib/TestWebKitFindController.cpp)
 ADD_WK2_TEST(TestEditor ${TOOLS_DIR}/TestWebKitAPI/Tests/WebKitGLib/TestEditor.cpp)
 ADD_WK2_TEST(TestFrame ${TOOLS_DIR}/TestWebKitAPI/Tests/WebKitGLib/TestFrame.cpp)
index 5b29b85..9b23e74 100644 (file)
@@ -43,6 +43,7 @@ ADD_WK2_TEST(TestInspector ${TOOLS_DIR}/TestWebKitAPI/Tests/WebKitGtk/TestInspec
 ADD_WK2_TEST(TestInspectorServer ${TOOLS_DIR}/TestWebKitAPI/Tests/WebKitGtk/TestInspectorServer.cpp)
 ADD_WK2_TEST(TestOptionMenu ${TOOLS_DIR}/TestWebKitAPI/Tests/WebKitGtk/TestOptionMenu.cpp)
 ADD_WK2_TEST(TestPrinting ${TOOLS_DIR}/TestWebKitAPI/Tests/WebKitGtk/TestPrinting.cpp)
+ADD_WK2_TEST(TestWebKitFaviconDatabase ${TOOLS_DIR}/TestWebKitAPI/Tests/WebKitGLib/TestWebKitFaviconDatabase.cpp)
 ADD_WK2_TEST(TestWebKitVersion ${TOOLS_DIR}/TestWebKitAPI/Tests/WebKitGtk/TestWebKitVersion.cpp)
 ADD_WK2_TEST(TestWebViewEditor ${TOOLS_DIR}/TestWebKitAPI/Tests/WebKitGtk/TestWebViewEditor.cpp)
 
index 0755ea6..4c2503b 100644 (file)
                 "expected": {"all@Debug": {"slow": true}}
             }
         }
-    },
-    "TestWebKitFaviconDatabase": {
-        "subtests": {
-            "/webkit/WebKitFaviconDatabase/favicon-database-test": {
-                "expected": {"gtk": {"status": ["PASS", "FAIL"], "bug": "webkit.org/b/188110"}}
-            }
-        }
     }
 }