Reviewed by Brady Eidson.
authortimothy@apple.com <timothy@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Sat, 9 Feb 2008 04:15:27 +0000 (04:15 +0000)
committertimothy@apple.com <timothy@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Sat, 9 Feb 2008 04:15:27 +0000 (04:15 +0000)
        <rdar://problem/5640896> Removing database then trying
        to recreate it causes trouble

        Added open Database support to DatabaseTracker. So any Database that
        is deleted will be marked as deleted and will fail to open any transaction
        or execute any new SQL queries.

        * storage/Database.cpp:
        (WebCore::Database::Database): Call DatabaseTracker::addOpenDatabase.
        (WebCore::Database::~Database): Call DatabaseTracker::removeOpenDatabase.
        (WebCore::Database::markAsDeleted): Set the m_deleted flag.
        (WebCore::Database::version): Return a null String if m_deleted is true.
        * storage/Database.h:
        (WebCore::Database::deleted): Return m_deleted.
        * storage/DatabaseTracker.cpp:
        (WebCore::DatabaseTracker::addOpenDatabase): Add the Database to a map of origins and names.
        (WebCore::DatabaseTracker::removeOpenDatabase): Remove the Database from the map.
        (WebCore::DatabaseTracker::deleteDatabaseFile): Call markAsDeleted on all the open Databases
          matching the origin/name.
        * storage/DatabaseTracker.h:
        * storage/SQLStatement.cpp:
        (WebCore::SQLStatement::setDatabaseDeletedError): Set the error about the user deleting the database.
        * storage/SQLStatement.h:
        * storage/SQLTransaction.cpp:
        (WebCore::SQLTransaction::executeSQL): If the Database is deleted, call setDatabaseDeletedError.
        (WebCore::SQLTransaction::openTransactionAndPreflight): Set the error about the user deleting the database
          if the Database was marked as deleted.

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

WebCore/ChangeLog
WebCore/storage/Database.cpp
WebCore/storage/Database.h
WebCore/storage/DatabaseTracker.cpp
WebCore/storage/DatabaseTracker.h
WebCore/storage/SQLStatement.cpp
WebCore/storage/SQLStatement.h
WebCore/storage/SQLTransaction.cpp

index bdfcfe5..0b04991 100644 (file)
@@ -1,3 +1,35 @@
+2008-02-08  Timothy Hatcher  <timothy@apple.com>
+
+        Reviewed by Brady Eidson.
+
+        <rdar://problem/5640896> Removing database then trying
+        to recreate it causes trouble
+
+        Added open Database support to DatabaseTracker. So any Database that 
+        is deleted will be marked as deleted and will fail to open any transaction
+        or execute any new SQL queries.
+
+        * storage/Database.cpp:
+        (WebCore::Database::Database): Call DatabaseTracker::addOpenDatabase.
+        (WebCore::Database::~Database): Call DatabaseTracker::removeOpenDatabase.
+        (WebCore::Database::markAsDeleted): Set the m_deleted flag.
+        (WebCore::Database::version): Return a null String if m_deleted is true.
+        * storage/Database.h:
+        (WebCore::Database::deleted): Return m_deleted.
+        * storage/DatabaseTracker.cpp:
+        (WebCore::DatabaseTracker::addOpenDatabase): Add the Database to a map of origins and names.
+        (WebCore::DatabaseTracker::removeOpenDatabase): Remove the Database from the map.
+        (WebCore::DatabaseTracker::deleteDatabaseFile): Call markAsDeleted on all the open Databases
+          matching the origin/name.
+        * storage/DatabaseTracker.h:
+        * storage/SQLStatement.cpp:
+        (WebCore::SQLStatement::setDatabaseDeletedError): Set the error about the user deleting the database.
+        * storage/SQLStatement.h:
+        * storage/SQLTransaction.cpp:
+        (WebCore::SQLTransaction::executeSQL): If the Database is deleted, call setDatabaseDeletedError.
+        (WebCore::SQLTransaction::openTransactionAndPreflight): Set the error about the user deleting the database
+          if the Database was marked as deleted.
+
 2008-02-08  Darin Adler  <darin@apple.com>
 
         Reviewed by Eric.
index 1ec531c..abe0c96 100644 (file)
@@ -145,6 +145,7 @@ Database::Database(Document* document, const String& name, const String& expecte
     , m_name(name.copy())
     , m_guid(0)
     , m_expectedVersion(expectedVersion)
+    , m_deleted(0)
     , m_databaseThread(0)
 #ifndef NDEBUG
     , m_transactionStepThread(0)
@@ -176,6 +177,8 @@ Database::Database(Document* document, const String& name, const String& expecte
     ASSERT(m_databaseThread);
 
     m_filename = DatabaseTracker::tracker().fullPathForDatabase(m_securityOrigin.get(), m_name);
+
+    DatabaseTracker::tracker().addOpenDatabase(this);
 }
 
 Database::~Database()
@@ -191,6 +194,8 @@ Database::~Database()
         delete hashSet;
         guidToVersionMap().remove(m_guid);
     }
+
+    DatabaseTracker::tracker().removeOpenDatabase(this);
 }
 
 bool Database::openAndVerifyVersion(ExceptionCode& e)
@@ -293,6 +298,14 @@ bool Database::versionMatchesExpected() const
     return true;
 }
 
+void Database::markAsDeleted()
+{
+    if (m_deleted)
+        return;
+    LOG(StorageAPI, "Marking %s (%p) as deleted", stringIdentifier().ascii().data(), this);
+    m_deleted = true;
+}
+
 unsigned long long Database::databaseSize() const
 {
     long long size;
@@ -556,6 +569,8 @@ Vector<String> Database::performGetTableNames()
 
 String Database::version() const
 {
+    if (m_deleted)
+        return String();
     MutexLocker locker(guidMutex());
     return guidToVersionMap().get(m_guid).copy();
 }
index 726a6f3..fb74c83 100644 (file)
@@ -89,7 +89,10 @@ public:
     bool setVersionInDatabase(const String&);
     void setExpectedVersion(const String&);
     bool versionMatchesExpected() const;
-    
+
+    void markAsDeleted();
+    bool deleted() const { return m_deleted; }
+
     unsigned long long databaseSize() const;
     unsigned long long maximumSize() const;
 
@@ -126,6 +129,8 @@ private:
     String m_expectedVersion;
     String m_filename;
 
+    bool m_deleted;
+
     SQLiteDatabase m_sqliteDatabase;
     RefPtr<DatabaseAuthorizer> m_databaseAuthorizer;
 
index 3b451b9..04e985a 100644 (file)
@@ -34,6 +34,7 @@
 #include "DatabaseTrackerClient.h"
 #include "Document.h"
 #include "FileSystem.h"
+#include "Logging.h"
 #include "OriginQuotaManager.h"
 #include "Page.h"
 #include "SecurityOrigin.h"
@@ -420,6 +421,80 @@ unsigned long long DatabaseTracker::usageForDatabase(const String& name, Securit
     return getFileSize(path, size) ? size : 0;
 }
 
+void DatabaseTracker::addOpenDatabase(Database* database)
+{
+    if (!database)
+        return;
+
+    MutexLocker openDatabaseMapLock(m_openDatabaseMapGuard);
+
+    if (!m_openDatabaseMap)
+        m_openDatabaseMap.set(new DatabaseOriginMap);
+
+    RefPtr<SecurityOrigin> origin(database->securityOriginCopy());
+    String name(database->stringIdentifier());
+
+    DatabaseNameMap* nameMap = m_openDatabaseMap->get(origin);
+    if (!nameMap) {
+        nameMap = new DatabaseNameMap;
+        m_openDatabaseMap->set(origin, nameMap);
+    }
+
+    DatabaseSet* databaseSet = nameMap->get(name);
+    if (!databaseSet) {
+        databaseSet = new DatabaseSet;
+        nameMap->set(name, databaseSet);
+    }
+
+    databaseSet->add(database);
+
+    LOG(StorageAPI, "Added open Database %s (%p)\n", database->stringIdentifier().ascii().data(), database);
+}
+
+void DatabaseTracker::removeOpenDatabase(Database* database)
+{
+    if (!database)
+        return;
+
+    MutexLocker openDatabaseMapLock(m_openDatabaseMapGuard);
+
+    if (!m_openDatabaseMap) {
+        ASSERT_NOT_REACHED();
+        return;
+    }
+
+    RefPtr<SecurityOrigin> origin(database->securityOriginCopy());
+    String name(database->stringIdentifier());
+
+    DatabaseNameMap* nameMap = m_openDatabaseMap->get(origin);
+    if (!nameMap) {
+        ASSERT_NOT_REACHED();
+        return;
+    }
+
+    DatabaseSet* databaseSet = nameMap->get(name);
+    if (!databaseSet) {
+        ASSERT_NOT_REACHED();
+        return;
+    }
+
+    databaseSet->remove(database);
+
+    LOG(StorageAPI, "Removed open Database %s (%p)\n", database->stringIdentifier().ascii().data(), database);
+
+    if (!databaseSet->isEmpty())
+        return;
+
+    nameMap->remove(name);
+    delete databaseSet;
+
+    if (!nameMap->isEmpty())
+        return;
+
+    m_openDatabaseMap->remove(origin);
+    delete nameMap;
+}
+
 unsigned long long DatabaseTracker::usageForOrigin(SecurityOrigin* origin)
 {
     ASSERT(currentThread() == m_thread);
@@ -649,7 +724,26 @@ bool DatabaseTracker::deleteDatabaseFile(SecurityOrigin* origin, const String& n
     String fullPath = fullPathForDatabase(origin, name, false);
     if (fullPath.isEmpty())
         return true;
-        
+
+    {
+        MutexLocker openDatabaseMapLock(m_openDatabaseMapGuard);
+        if (m_openDatabaseMap) {
+            // There are some open databases, lets check if they are for this origin.
+            DatabaseNameMap* nameMap = m_openDatabaseMap->get(origin);
+            if (nameMap && nameMap->size()) {
+                // There are some open databases for this origin, lets check
+                // if they are this database by name.
+                DatabaseSet* databaseSet = nameMap->get(name);
+                if (databaseSet && databaseSet->size()) {
+                    // We have some database open with this name. Mark them as deleted.
+                    DatabaseSet::const_iterator end = databaseSet->end();
+                    for (DatabaseSet::const_iterator it = databaseSet->begin(); it != end; ++it)
+                        (*it)->markAsDeleted();
+                }
+            }
+        }
+    }
+
     return deleteFile(fullPath);
 }
 
index dbecee7..6202701 100644 (file)
@@ -38,6 +38,7 @@
 
 namespace WebCore {
 
+class Database;
 class DatabaseTrackerClient;
 class Document;
 class OriginQuotaManager;
@@ -59,7 +60,10 @@ public:
     bool databaseNamesForOrigin(SecurityOrigin*, Vector<String>& result);
 
     DatabaseDetails detailsForNameAndOrigin(const String&, SecurityOrigin*);
-    
+
+    void addOpenDatabase(Database*);
+    void removeOpenDatabase(Database*);
+
     unsigned long long usageForDatabase(const String&, SecurityOrigin*);
     unsigned long long usageForOrigin(SecurityOrigin*);
     unsigned long long quotaForOrigin(SecurityOrigin*);
@@ -99,7 +103,14 @@ private:
     typedef HashMap<RefPtr<SecurityOrigin>, unsigned long long, SecurityOriginHash, SecurityOriginTraits> QuotaMap;
     Mutex m_quotaMapGuard;
     mutable OwnPtr<QuotaMap> m_quotaMap;
-    
+
+    typedef HashSet<Database*> DatabaseSet;
+    typedef HashMap<String, DatabaseSet*> DatabaseNameMap;
+    typedef HashMap<RefPtr<SecurityOrigin>, DatabaseNameMap*, SecurityOriginHash, SecurityOriginTraits> DatabaseOriginMap;
+
+    Mutex m_openDatabaseMapGuard;
+    mutable OwnPtr<DatabaseOriginMap> m_openDatabaseMap;
+
     OwnPtr<OriginQuotaManager> m_quotaManager;
 
     String m_databaseDirectoryPath;
index 102d8a2..ce5f894 100644 (file)
@@ -139,6 +139,12 @@ bool SQLStatement::execute(Database* db)
     return true;
 }
 
+void SQLStatement::setDatabaseDeletedError()
+{
+    ASSERT(!m_error && !m_resultSet);
+    m_error = new SQLError(0, "unable to execute statement, because the user deleted the database");
+}
+
 void SQLStatement::setVersionMismatchedError()
 {
     ASSERT(!m_error && !m_resultSet);
index 400e0aa..538f645 100644 (file)
@@ -56,6 +56,8 @@ public:
     
     bool hasStatementCallback() const { return m_statementCallback; }
     bool hasStatementErrorCallback() const { return m_statementErrorCallback; }
+
+    void setDatabaseDeletedError();
     void setVersionMismatchedError();
 
     bool performCallback(SQLTransaction*);
index 4cc1336..7e67cb8 100644 (file)
@@ -78,7 +78,10 @@ void SQLTransaction::executeSQL(const String& sqlStatement, const Vector<SQLValu
     }
     
     RefPtr<SQLStatement> statement = new SQLStatement(sqlStatement.copy(), arguments, callback, callbackError);
-    
+
+    if (m_database->deleted())
+        statement->setDatabaseDeletedError();
+
     if (!m_database->versionMatchesExpected())
         statement->setVersionMismatchedError();
         
@@ -119,7 +122,14 @@ void SQLTransaction::openTransactionAndPreflight()
     ASSERT(!m_database->m_sqliteDatabase.transactionInProgress());
 
     LOG(StorageAPI, "Opening and preflighting transaction %p", this);
-    
+
+    // If the database was deleted, jump to the error callback
+    if (m_database->deleted()) {
+        m_transactionError = new SQLError(0, "unable to open a transaction, because the user deleted the database");
+        handleTransactionError(false);
+        return;
+    }
+
     // Set the maximum usage for this transaction
     m_database->m_sqliteDatabase.setMaximumSize(m_database->maximumSize());