Changeset 89130 in webkit

Timestamp:

Jun 17, 2011 2:43:59 AM (13 years ago)

Author:

hans@chromium.org

Message:

2011-06-09 Hans Wennborg <​hans@chromium.org>

Reviewed by Tony Gentilcore.

IndexedDB: backingStoreMap is per backing store, not per database
​https://bugs.webkit.org/show_bug.cgi?id=62382

IDBFactoryBackendImpl::m_backingStoreMap should contain an entry per
backing store, not per database. Otherwise, we might accidentally open
the same backing store more than once, which is dangerous.

Also tweak the code that chooses backing store type. It should be
simple: we default to SQLite; if LevelDB is specifically requested, we
use that. If LevelDB is requested and there is a SQLite database, we
migrate.

No new tests, just cleaning up the code.

• storage/IDBFactoryBackendImpl.cpp: (WebCore::IDBFactoryBackendImpl::addIDBBackingStore): (WebCore::IDBFactoryBackendImpl::removeIDBBackingStore): (WebCore::IDBFactoryBackendImpl::open): (WebCore::IDBFactoryBackendImpl::migrateFromSQLiteToLevelDB):
• storage/IDBFactoryBackendImpl.h:

Location:

trunk/Source/WebCore

Files:

• ChangeLog (modified) (1 diff)
• storage/IDBFactoryBackendImpl.cpp (modified) (3 diffs)
• storage/IDBFactoryBackendImpl.h (modified) (2 diffs)

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+2011-06-09  Hans Wennborg  <hans@chromium.org>
+
+        Reviewed by Tony Gentilcore.
+
+        IndexedDB: backingStoreMap is per backing store, not per database
+        https://bugs.webkit.org/show_bug.cgi?id=62382
+
+        IDBFactoryBackendImpl::m_backingStoreMap should contain an entry per
+        backing store, not per database. Otherwise, we might accidentally open
+        the same backing store more than once, which is dangerous.
+
+        Also tweak the code that chooses backing store type. It should be
+        simple: we default to SQLite; if LevelDB is specifically requested, we
+        use that. If LevelDB is requested and there is a SQLite database, we
+        migrate.
+
+        No new tests, just cleaning up the code.
+
+        * storage/IDBFactoryBackendImpl.cpp:
+        (WebCore::IDBFactoryBackendImpl::addIDBBackingStore):
+        (WebCore::IDBFactoryBackendImpl::removeIDBBackingStore):
+        (WebCore::IDBFactoryBackendImpl::open):
+        (WebCore::IDBFactoryBackendImpl::migrateFromSQLiteToLevelDB):
+        * storage/IDBFactoryBackendImpl.h:
+
 2011-06-17  Andrey Adaikin  <aandrey@google.com>
 

trunk/Source/WebCore/storage/IDBFactoryBackendImpl.cpp

@@ -59 +59 @@
 }
 
-void IDBFactoryBackendImpl::addIDBBackingStore(const String& uniqueIdentifier, IDBBackingStore* backingStore)
+void IDBFactoryBackendImpl::addIDBBackingStore(const String& fileIdentifier, IDBBackingStore* backingStore)
 {
-    ASSERT(!m_backingStoreMap.contains(uniqueIdentifier));
-    m_backingStoreMap.set(uniqueIdentifier, backingStore);
+    ASSERT(!m_backingStoreMap.contains(fileIdentifier));
+    m_backingStoreMap.set(fileIdentifier, backingStore);
 }
 
-void IDBFactoryBackendImpl::removeIDBBackingStore(const String& uniqueIdentifier)
+void IDBFactoryBackendImpl::removeIDBBackingStore(const String& fileIdentifier)
 {
-    ASSERT(m_backingStoreMap.contains(uniqueIdentifier));
-    m_backingStoreMap.remove(uniqueIdentifier);
+    ASSERT(m_backingStoreMap.contains(fileIdentifier));
+    m_backingStoreMap.remove(fileIdentifier);
 }
 
 void IDBFactoryBackendImpl::open(const String& name, PassRefPtr<IDBCallbacks> callbacks, PassRefPtr<SecurityOrigin> securityOrigin, Frame*, const String& dataDir, int64_t maximumSize, BackingStoreType backingStoreType)
 {
-    String uniqueIdentifier = securityOrigin->databaseIdentifier() + "@" + name + String::format("@%d", (int)backingStoreType);
+    if (backingStoreType == DefaultBackingStore)
+        backingStoreType = SQLiteBackingStore; // FIXME: DefaultBackingStore is confusing; get rid of it.
+
+    const String fileIdentifier = securityOrigin->databaseIdentifier() + String::format("@%d", static_cast<int>(backingStoreType));
+    const String uniqueIdentifier = fileIdentifier + "@" + name;
+
     IDBDatabaseBackendMap::iterator it = m_databaseBackendMap.find(uniqueIdentifier);
     if (it != m_databaseBackendMap.end()) {
@@ -83 +88 @@
 
     RefPtr<IDBBackingStore> backingStore;
-    IDBBackingStoreMap::iterator it2 = m_backingStoreMap.find(uniqueIdentifier);
+    IDBBackingStoreMap::iterator it2 = m_backingStoreMap.find(fileIdentifier);
     if (it2 != m_backingStoreMap.end() && (backingStoreType == it2->second->backingStoreType()))
         backingStore = it2->second;
     else {
+
 #if ENABLE(LEVELDB)
-        // Should we migrate this backing store?
-        bool hasSQLBackingStore = IDBSQLiteBackingStore::backingStoreExists(securityOrigin.get(), dataDir);
-        bool hasLevelDBBackingStore = IDBLevelDBBackingStore::backingStoreExists(securityOrigin.get(), dataDir);
+        if (backingStoreType == LevelDBBackingStore) {
+            const bool hasSQLBackingStore = IDBSQLiteBackingStore::backingStoreExists(securityOrigin.get(), dataDir);
 
-        if (hasSQLBackingStore && hasLevelDBBackingStore)
-            backingStoreType = LevelDBBackingStore;
-
-        // Migration: if the database exists and is SQLite we want to migrate it to LevelDB.
-        if (hasSQLBackingStore && !hasLevelDBBackingStore) {
-            if (migrate(name, securityOrigin.get(), dataDir, maximumSize))
-                backingStoreType = LevelDBBackingStore;
+            if (hasSQLBackingStore) {
+                bool migrationSucceeded = migrateFromSQLiteToLevelDB(name, securityOrigin.get(), dataDir, maximumSize);
+                (void)migrationSucceeded; // FIXME: When migration is actually implemented, we need error handling here.
+            }
         }
 #endif
 
-        if (backingStoreType == DefaultBackingStore || backingStoreType == SQLiteBackingStore)
-            backingStore = IDBSQLiteBackingStore::open(securityOrigin.get(), dataDir, maximumSize, uniqueIdentifier, this);
+        if (backingStoreType == SQLiteBackingStore)
+            backingStore = IDBSQLiteBackingStore::open(securityOrigin.get(), dataDir, maximumSize, fileIdentifier, this);
 #if ENABLE(LEVELDB)
         else if (backingStoreType == LevelDBBackingStore)
-            backingStore = IDBLevelDBBackingStore::open(securityOrigin.get(), dataDir, maximumSize, uniqueIdentifier, this);
+            backingStore = IDBLevelDBBackingStore::open(securityOrigin.get(), dataDir, maximumSize, fileIdentifier, this);
 #endif
         if (!backingStore) {
@@ -119 +121 @@
 }
 
-bool IDBFactoryBackendImpl::migrate(const String& name, SecurityOrigin* securityOrigin, const String& dataDir, int64_t maximumSize)
+bool IDBFactoryBackendImpl::migrateFromSQLiteToLevelDB(const String& name, SecurityOrigin* securityOrigin, const String& dataDir, int64_t maximumSize)
 {
-    return false;
+    return false; // FIXME: To be implemented.
 }
 

trunk/Source/WebCore/storage/IDBFactoryBackendImpl.h

@@ -53 +53 @@
     // Notifications from weak pointers.
     void removeIDBDatabaseBackend(const String& uniqueIdentifier);
-    void addIDBBackingStore(const String& uniqueIdentifier, IDBBackingStore*);
-    void removeIDBBackingStore(const String& uniqueIdentifier);
+    void addIDBBackingStore(const String& fileIdentifier, IDBBackingStore*);
+    void removeIDBBackingStore(const String& fileIdentifier);
 
     virtual void open(const String& name, PassRefPtr<IDBCallbacks>, PassRefPtr<SecurityOrigin>, Frame*, const String& dataDir, int64_t maximumSize, BackingStoreType);
@@ -60 +60 @@
 private:
     IDBFactoryBackendImpl();
-    bool migrate(const String& name, SecurityOrigin*, const String& dataDir, int64_t maximumSize);
+    bool migrateFromSQLiteToLevelDB(const String& name, SecurityOrigin*, const String& dataDir, int64_t maximumSize);
 
     typedef HashMap<String, IDBDatabaseBackendImpl*> IDBDatabaseBackendMap;