Title: [89130] trunk/Source/WebCore
Revision
89130
Author
h...@chromium.org
Date
2011-06-17 02:43:59 -0700 (Fri, 17 Jun 2011)

Log Message

2011-06-09  Hans Wennborg  <h...@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:

Modified Paths

Diff

Modified: trunk/Source/WebCore/ChangeLog (89129 => 89130)


--- trunk/Source/WebCore/ChangeLog	2011-06-17 09:01:05 UTC (rev 89129)
+++ trunk/Source/WebCore/ChangeLog	2011-06-17 09:43:59 UTC (rev 89130)
@@ -1,3 +1,28 @@
+2011-06-09  Hans Wennborg  <h...@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  <aand...@google.com>
 
         Reviewed by Pavel Feldman.

Modified: trunk/Source/WebCore/storage/IDBFactoryBackendImpl.cpp (89129 => 89130)


--- trunk/Source/WebCore/storage/IDBFactoryBackendImpl.cpp	2011-06-17 09:01:05 UTC (rev 89129)
+++ trunk/Source/WebCore/storage/IDBFactoryBackendImpl.cpp	2011-06-17 09:43:59 UTC (rev 89130)
@@ -58,21 +58,26 @@
     m_databaseBackendMap.remove(uniqueIdentifier);
 }
 
-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()) {
         callbacks->onSuccess(it->second);
@@ -82,30 +87,27 @@
     // FIXME: Everything from now on should be done on another thread.
 
     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) {
             callbacks->onError(IDBDatabaseError::create(IDBDatabaseException::UNKNOWN_ERR, "Internal error."));
@@ -118,9 +120,9 @@
     m_databaseBackendMap.set(uniqueIdentifier, databaseBackend.get());
 }
 
-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.
 }
 
 } // namespace WebCore

Modified: trunk/Source/WebCore/storage/IDBFactoryBackendImpl.h (89129 => 89130)


--- trunk/Source/WebCore/storage/IDBFactoryBackendImpl.h	2011-06-17 09:01:05 UTC (rev 89129)
+++ trunk/Source/WebCore/storage/IDBFactoryBackendImpl.h	2011-06-17 09:43:59 UTC (rev 89130)
@@ -52,14 +52,14 @@
 
     // 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);
 
 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;
     IDBDatabaseBackendMap m_databaseBackendMap;
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
http://lists.webkit.org/mailman/listinfo.cgi/webkit-changes

Reply via email to