Attachment 349914 Details for Bug 189669 – PATCH

[patch] PATCH

189669.diff (text/plain), 8.71 KB, created by Basuke Suzuki on 2018-09-17 11:28:13 PDT

(hide)

Description:

Filename:

MIME Type:

Creator: Basuke Suzuki

Created: 2018-09-17 11:28:13 PDT

Size: 8.71 KB

patch

obsolete

>diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
>index 0f047efc3e2..46294302b17 100644
>--- a/Source/WebCore/ChangeLog
>+++ b/Source/WebCore/ChangeLog
>@@ -1,3 +1,29 @@
>+2018-09-17  Christopher Reid  <chris.reid@sony.com>
>+
>+        [Curl] Add schema version and enable auto vacuum for cookie database.
>+        https://bugs.webkit.org/show_bug.cgi?id=189669
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        Turning on auto incremental vacuuming and adding versioning to the database. Right now we
>+        reset tables if there's an unknown schema or if the none is set. There is placeholder logic
>+        in place to upgrade databases as the schema changes in the future.
>+
>+        Tested by checking the database manually after launching MiniBrowser.
>+
>+        * platform/network/curl/CookieJarDB.cpp:
>+        (WebCore::CookieJarDB::openDatabase):
>+        (WebCore::CookieJarDB::verifySchemaVersion):
>+        (WebCore::CookieJarDB::deleteAllTables):
>+        (WebCore::CookieJarDB::createPrepareStatement):
>+        (WebCore::CookieJarDB::getPrepareStatement):
>+        (WebCore::CookieJarDB::executeSimpleSql):
>+        * platform/network/curl/CookieJarDB.h:
>+        * platform/network/curl/NetworkStorageSessionCurl.cpp:
>+        (WebCore::defaultSession):
>+        * platform/sql/SQLiteDatabase.cpp:
>+        (WebCore::SQLiteDatabase::turnOnIncrementalAutoVacuum): fixed a bug.
>+
> 2018-09-17  Youenn Fablet  <youenn@apple.com>
> 
>         Enable VCP for iOS and reenable it for MacOS
>diff --git a/Source/WebCore/platform/network/curl/CookieJarDB.cpp b/Source/WebCore/platform/network/curl/CookieJarDB.cpp
>index 204e3105cfd..3c060927366 100644
>--- a/Source/WebCore/platform/network/curl/CookieJarDB.cpp
>+++ b/Source/WebCore/platform/network/curl/CookieJarDB.cpp
>@@ -71,6 +71,13 @@ namespace WebCore {
> #define DELETE_ALL_COOKIE_SQL \
>     "DELETE FROM Cookie;"
> 
>+
>+// If the database schema is updated:
>+// - Increment schemaVersion
>+// - Add upgrade logic in verifySchemaVersion to migrate databases from the previous schema version
>+static constexpr int schemaVersion = 1;
>+
>+
> void CookieJarDB::setEnabled(bool enable)
> {
>     m_isEnabled = enable;
>@@ -125,22 +132,31 @@ bool CookieJarDB::openDatabase()
>         if (!FileSystem::makeAllDirectories(FileSystem::directoryName(m_databasePath)))
>             LOG_ERROR("Unable to create the Cookie Database path %s", m_databasePath.utf8().data());
> 
>-        if (m_database.open(m_databasePath, false)) {
>-            bool databaseValidity = true;
>-            databaseValidity &= (executeSimpleSql(CREATE_COOKIE_TABLE_SQL) == SQLITE_DONE);
>-            databaseValidity &= (executeSimpleSql(CREATE_DOMAIN_INDEX_SQL) == SQLITE_DONE);
>-            databaseValidity &= (executeSimpleSql(CREATE_PATH_INDEX_SQL) == SQLITE_DONE);
>-            if (!databaseValidity) {
>-                // give up create database at this time (all cookies on request/response are ignored)
>-                m_database.close();
>-                deleteAllDatabaseFiles();
>-            }
>-        }
>+        m_database.open(m_databasePath, false);
>     }
> 
>     if (!m_database.isOpen())
>         return false;
> 
>+    if (!isOnMemory() && !m_database.turnOnIncrementalAutoVacuum())
>+        LOG_ERROR("Unable to turn on incremental auto-vacuum (%d %s)", m_database.lastError(), m_database.lastErrorMsg());
>+
>+    verifySchemaVersion();
>+
>+    bool databaseValidity = true;
>+    if (!existsDatabaseFile || !m_database.tableExists("Cookie")) {
>+        databaseValidity &= (executeSimpleSql(CREATE_COOKIE_TABLE_SQL) == SQLITE_DONE);
>+        databaseValidity &= (executeSimpleSql(CREATE_DOMAIN_INDEX_SQL) == SQLITE_DONE);
>+        databaseValidity &= (executeSimpleSql(CREATE_PATH_INDEX_SQL) == SQLITE_DONE);
>+    }
>+
>+    if (!databaseValidity) {
>+        // give up create database at this time (all cookies on request/response are ignored)
>+        m_database.close();
>+        deleteAllDatabaseFiles();
>+        return false;
>+    }
>+
>     m_database.setSynchronous(SQLiteDatabase::SyncNormal);
> 
>     // create prepared statements
>@@ -162,6 +178,41 @@ void CookieJarDB::closeDatabase()
>     }
> }
> 
>+void CookieJarDB::verifySchemaVersion()
>+{
>+    if (isOnMemory())
>+        return;
>+
>+    int version = SQLiteStatement(m_database, "PRAGMA user_version").getColumnInt(0);
>+    if (version == schemaVersion)
>+        return;
>+
>+    switch (version) {
>+        // Placeholder for schema version upgrade logic
>+        // Ensure cases fall through to the next version's upgrade logic
>+
>+    case 0:
>+        deleteAllTables();
>+        break;
>+    default:
>+        // This case can be reached when downgrading versions
>+        LOG_ERROR("Unknown cookie database version: %d", version);
>+        deleteAllTables();
>+        break;
>+    }
>+
>+    // Update version
>+    executeSimpleSql(String::format("PRAGMA user_version=%d", schemaVersion));
>+}
>+
>+void CookieJarDB::deleteAllTables()
>+{
>+    if (!m_database.isOpen())
>+        return;
>+
>+    m_database.clearAllTables();
>+}
>+
> String CookieJarDB::getCorruptionMarkerPath() const
> {
>     ASSERT(!isOnMemory());
>@@ -484,23 +535,23 @@ int CookieJarDB::deleteAllCookies()
>     return executeSimpleSql(DELETE_ALL_COOKIE_SQL);
> }
> 
>-void CookieJarDB::createPrepareStatement(const char* sql)
>+void CookieJarDB::createPrepareStatement(const String& sql)
> {
>     auto statement = std::make_unique<SQLiteStatement>(m_database, sql);
>     int ret = statement->prepare();
>     ASSERT(ret == SQLITE_OK);
>-    m_statements.add(String(sql), WTFMove(statement));
>+    m_statements.add(sql, WTFMove(statement));
> }
> 
>-SQLiteStatement* CookieJarDB::getPrepareStatement(const char* sql)
>+SQLiteStatement* CookieJarDB::getPrepareStatement(const String& sql)
> {
>-    const auto& statement = m_statements.get(String(sql));
>+    const auto& statement = m_statements.get(sql);
>     ASSERT(statement);
>     statement->reset();
>     return statement;
> }
> 
>-int CookieJarDB::executeSimpleSql(const char* sql, bool ignoreError)
>+int CookieJarDB::executeSimpleSql(const String& sql, bool ignoreError)
> {
>     SQLiteStatement statement(m_database, sql);
>     int ret = statement.prepareAndStep();
>@@ -508,7 +559,7 @@ int CookieJarDB::executeSimpleSql(const char* sql, bool ignoreError)
> 
>     checkSQLiteReturnCode(ret);
>     if (ret != SQLITE_OK && ret != SQLITE_DONE && ret != SQLITE_ROW && !ignoreError)
>-        LOG_ERROR("Failed to execute %s error: %s", sql, m_database.lastErrorMsg());
>+        LOG_ERROR("Failed to execute %s error: %s", sql.ascii().data(), m_database.lastErrorMsg());
> 
>     return ret;
> }
>diff --git a/Source/WebCore/platform/network/curl/CookieJarDB.h b/Source/WebCore/platform/network/curl/CookieJarDB.h
>index 32bead55e02..695b967a122 100644
>--- a/Source/WebCore/platform/network/curl/CookieJarDB.h
>+++ b/Source/WebCore/platform/network/curl/CookieJarDB.h
>@@ -75,9 +75,12 @@ private:
>     bool checkDatabaseValidity();
>     void deleteAllDatabaseFiles();
> 
>-    void createPrepareStatement(const char* sql);
>-    SQLiteStatement* getPrepareStatement(const char* sql);
>-    int executeSimpleSql(const char* sql, bool ignoreError = false);
>+    void verifySchemaVersion();
>+    void deleteAllTables();
>+
>+    void createPrepareStatement(const String& sql);
>+    SQLiteStatement* getPrepareStatement(const String& sql);
>+    int executeSimpleSql(const String& sql, bool ignoreError = false);
> 
>     int deleteCookieInternal(const String& name, const String& domain, const String& path);
>     bool hasHttpOnlyCookie(const String& name, const String& domain, const String& path);
>diff --git a/Source/WebCore/platform/network/curl/NetworkStorageSessionCurl.cpp b/Source/WebCore/platform/network/curl/NetworkStorageSessionCurl.cpp
>index 86a41df732c..e110ad16491 100644
>--- a/Source/WebCore/platform/network/curl/NetworkStorageSessionCurl.cpp
>+++ b/Source/WebCore/platform/network/curl/NetworkStorageSessionCurl.cpp
>@@ -85,7 +85,7 @@ CookieJarDB& NetworkStorageSession::cookieDatabase() const
> static std::unique_ptr<NetworkStorageSession>& defaultSession()
> {
>     ASSERT(isMainThread());
>-    static NeverDestroyed<std::unique_ptr<NetworkStorageSession>> session;
>+    static std::unique_ptr<NetworkStorageSession> session;
>     return session;
> }
> 
>diff --git a/Source/WebCore/platform/sql/SQLiteDatabase.cpp b/Source/WebCore/platform/sql/SQLiteDatabase.cpp
>index a16050acea5..0d436be6672 100644
>--- a/Source/WebCore/platform/sql/SQLiteDatabase.cpp
>+++ b/Source/WebCore/platform/sql/SQLiteDatabase.cpp
>@@ -488,6 +488,7 @@ bool SQLiteDatabase::turnOnIncrementalAutoVacuum()
>     SQLiteStatement statement(*this, "PRAGMA auto_vacuum"_s);
>     int autoVacuumMode = statement.getColumnInt(0);
>     int error = lastError();
>+    statement.finalize();
> 
>     // Check if we got an error while trying to get the value of the auto_vacuum flag.
>     // If we got a SQLITE_BUSY error, then there's probably another transaction in

Actions: View | Formatted Diff | Diff

Attachments on bug 189669: 349914