Diff
Modified: trunk/Source/WebCore/ChangeLog (147232 => 147233)
--- trunk/Source/WebCore/ChangeLog 2013-03-29 18:54:24 UTC (rev 147232)
+++ trunk/Source/WebCore/ChangeLog 2013-03-29 19:12:04 UTC (rev 147233)
@@ -1,3 +1,28 @@
+2013-03-29 Joshua Bell <jsb...@chromium.org>
+
+ IndexedDB: Bind lifetime of in-memory backing stores to IDBFactory backend
+ https://bugs.webkit.org/show_bug.cgi?id=110820
+
+ Reviewed by Tony Chang.
+
+ Backing stores are dropped as soon as all connections are closed. That makes sense for
+ disk-backed stores to free up memory (although there's a performance trade-off...). But
+ for memory-backed stores, the expected lifetime should match the lifetime of the factory
+ so that an open/write/close/re-open/read yields the written data.
+
+ Test: Chromium's webkit_unit_tests, IDBFactoryBackendTest.MemoryBackingStoreLifetime
+
+ * Modules/indexeddb/IDBBackingStore.cpp:
+ (WebCore::IDBBackingStore::IDBBackingStore): Pass comparator into constructor since it was always
+ assigned immediately afterwards anyway.
+ (WebCore::IDBBackingStore::open): Split into three parts - open() which is disk-backed...
+ (WebCore::IDBBackingStore::openInMemory): ...explit in-memory creation (previously: specified by empty path)
+ (WebCore::IDBBackingStore::create): ... and the common logic which calls the constructor.
+ * Modules/indexeddb/IDBBackingStore.h: Headers for the above.
+ * Modules/indexeddb/IDBFactoryBackendImpl.cpp:
+ (WebCore::IDBFactoryBackendImpl::openBackingStore): Add in-memory backing stores to dependent set.
+ * Modules/indexeddb/IDBFactoryBackendImpl.h: Add member to track dependent backing stores.
+
2013-03-29 Nate Chapin <jap...@chromium.org>
ASSERT d->m_defersLoading != defers on detik.com and drive.google.com
Modified: trunk/Source/WebCore/Modules/indexeddb/IDBBackingStore.cpp (147232 => 147233)
--- trunk/Source/WebCore/Modules/indexeddb/IDBBackingStore.cpp 2013-03-29 18:54:24 UTC (rev 147232)
+++ trunk/Source/WebCore/Modules/indexeddb/IDBBackingStore.cpp 2013-03-29 19:12:04 UTC (rev 147233)
@@ -43,6 +43,9 @@
#include "LevelDBTransaction.h"
#include "SecurityOrigin.h"
#include "SharedBuffer.h"
+#if PLATFORM(CHROMIUM)
+#include <public/Platform.h>
+#endif
#include <wtf/Assertions.h>
namespace WebCore {
@@ -335,9 +338,10 @@
}
};
-IDBBackingStore::IDBBackingStore(const String& identifier, PassOwnPtr<LevelDBDatabase> db)
+IDBBackingStore::IDBBackingStore(const String& identifier, PassOwnPtr<LevelDBDatabase> db, PassOwnPtr<LevelDBComparator> comparator)
: m_identifier(identifier)
, m_db(db)
+ , m_comparator(comparator)
, m_weakFactory(this)
{
}
@@ -345,6 +349,10 @@
IDBBackingStore::IDBBackingStore()
: m_weakFactory(this)
{
+ // This constructor should only be used in unit tests.
+#if PLATFORM(CHROMIUM)
+ ASSERT(WebKit::Platform::current()->unitTestSupport());
+#endif
}
IDBBackingStore::~IDBBackingStore()
@@ -378,65 +386,56 @@
PassRefPtr<IDBBackingStore> IDBBackingStore::open(SecurityOrigin* securityOrigin, const String& pathBaseArg, const String& fileIdentifier, LevelDBFactory* levelDBFactory)
{
IDB_TRACE("IDBBackingStore::open");
+ ASSERT(!pathBaseArg.isEmpty());
String pathBase = pathBaseArg;
OwnPtr<LevelDBComparator> comparator = adoptPtr(new Comparator());
OwnPtr<LevelDBDatabase> db;
- if (pathBase.isEmpty()) {
- db = LevelDBDatabase::openInMemory(comparator.get());
- if (!db) {
- LOG_ERROR("LevelDBDatabase::openInMemory failed.");
- HistogramSupport::histogramEnumeration("WebCore.IndexedDB.BackingStore.OpenStatus", IDBLevelDBBackingStoreOpenMemoryFailed, IDBLevelDBBackingStoreOpenMax);
- return PassRefPtr<IDBBackingStore>();
- }
- HistogramSupport::histogramEnumeration("WebCore.IndexedDB.BackingStore.OpenStatus", IDBLevelDBBackingStoreOpenMemorySuccess, IDBLevelDBBackingStoreOpenMax);
- } else {
- if (!pathBase.containsOnlyASCII())
+ if (!pathBase.containsOnlyASCII())
HistogramSupport::histogramEnumeration("WebCore.IndexedDB.BackingStore.OpenStatus", IDBLevelDBBackingStoreOpenAttemptNonASCII, IDBLevelDBBackingStoreOpenMax);
- if (!makeAllDirectories(pathBase)) {
- LOG_ERROR("Unable to create IndexedDB database path %s", pathBase.utf8().data());
- HistogramSupport::histogramEnumeration("WebCore.IndexedDB.BackingStore.OpenStatus", IDBLevelDBBackingStoreOpenFailedDirectory, IDBLevelDBBackingStoreOpenMax);
- return PassRefPtr<IDBBackingStore>();
- }
+ if (!makeAllDirectories(pathBase)) {
+ LOG_ERROR("Unable to create IndexedDB database path %s", pathBase.utf8().data());
+ HistogramSupport::histogramEnumeration("WebCore.IndexedDB.BackingStore.OpenStatus", IDBLevelDBBackingStoreOpenFailedDirectory, IDBLevelDBBackingStoreOpenMax);
+ return PassRefPtr<IDBBackingStore>();
+ }
- String path = pathByAppendingComponent(pathBase, securityOrigin->databaseIdentifier() + ".indexeddb.leveldb");
+ String path = pathByAppendingComponent(pathBase, securityOrigin->databaseIdentifier() + ".indexeddb.leveldb");
- db = levelDBFactory->openLevelDB(path, comparator.get());
- if (db) {
- bool known = false;
- bool ok = isSchemaKnown(db.get(), known);
- if (!ok) {
- LOG_ERROR("IndexedDB had IO error checking schema, treating it as failure to open");
- HistogramSupport::histogramEnumeration("WebCore.IndexedDB.BackingStore.OpenStatus", IDBLevelDBBackingStoreOpenFailedIOErrCheckingSchema, IDBLevelDBBackingStoreOpenMax);
- db.clear();
- } else if (!known) {
- LOG_ERROR("IndexedDB backing store had unknown schema, treating it as failure to open");
- HistogramSupport::histogramEnumeration("WebCore.IndexedDB.BackingStore.OpenStatus", IDBLevelDBBackingStoreOpenFailedUnknownSchema, IDBLevelDBBackingStoreOpenMax);
- db.clear();
- }
+ db = levelDBFactory->openLevelDB(path, comparator.get());
+ if (db) {
+ bool known = false;
+ bool ok = isSchemaKnown(db.get(), known);
+ if (!ok) {
+ LOG_ERROR("IndexedDB had IO error checking schema, treating it as failure to open");
+ HistogramSupport::histogramEnumeration("WebCore.IndexedDB.BackingStore.OpenStatus", IDBLevelDBBackingStoreOpenFailedIOErrCheckingSchema, IDBLevelDBBackingStoreOpenMax);
+ db.clear();
+ } else if (!known) {
+ LOG_ERROR("IndexedDB backing store had unknown schema, treating it as failure to open");
+ HistogramSupport::histogramEnumeration("WebCore.IndexedDB.BackingStore.OpenStatus", IDBLevelDBBackingStoreOpenFailedUnknownSchema, IDBLevelDBBackingStoreOpenMax);
+ db.clear();
}
+ }
- if (db)
- HistogramSupport::histogramEnumeration("WebCore.IndexedDB.BackingStore.OpenStatus", IDBLevelDBBackingStoreOpenSuccess, IDBLevelDBBackingStoreOpenMax);
- else {
- LOG_ERROR("IndexedDB backing store open failed, attempting cleanup");
- bool success = levelDBFactory->destroyLevelDB(path);
- if (!success) {
- LOG_ERROR("IndexedDB backing store cleanup failed");
- HistogramSupport::histogramEnumeration("WebCore.IndexedDB.BackingStore.OpenStatus", IDBLevelDBBackingStoreOpenCleanupDestroyFailed, IDBLevelDBBackingStoreOpenMax);
- return PassRefPtr<IDBBackingStore>();
- }
+ if (db)
+ HistogramSupport::histogramEnumeration("WebCore.IndexedDB.BackingStore.OpenStatus", IDBLevelDBBackingStoreOpenSuccess, IDBLevelDBBackingStoreOpenMax);
+ else {
+ LOG_ERROR("IndexedDB backing store open failed, attempting cleanup");
+ bool success = levelDBFactory->destroyLevelDB(path);
+ if (!success) {
+ LOG_ERROR("IndexedDB backing store cleanup failed");
+ HistogramSupport::histogramEnumeration("WebCore.IndexedDB.BackingStore.OpenStatus", IDBLevelDBBackingStoreOpenCleanupDestroyFailed, IDBLevelDBBackingStoreOpenMax);
+ return PassRefPtr<IDBBackingStore>();
+ }
- LOG_ERROR("IndexedDB backing store cleanup succeeded, reopening");
- db = levelDBFactory->openLevelDB(path, comparator.get());
- if (!db) {
- LOG_ERROR("IndexedDB backing store reopen after recovery failed");
- HistogramSupport::histogramEnumeration("WebCore.IndexedDB.BackingStore.OpenStatus", IDBLevelDBBackingStoreOpenCleanupReopenFailed, IDBLevelDBBackingStoreOpenMax);
- return PassRefPtr<IDBBackingStore>();
- }
- HistogramSupport::histogramEnumeration("WebCore.IndexedDB.BackingStore.OpenStatus", IDBLevelDBBackingStoreOpenCleanupReopenSuccess, IDBLevelDBBackingStoreOpenMax);
+ LOG_ERROR("IndexedDB backing store cleanup succeeded, reopening");
+ db = levelDBFactory->openLevelDB(path, comparator.get());
+ if (!db) {
+ LOG_ERROR("IndexedDB backing store reopen after recovery failed");
+ HistogramSupport::histogramEnumeration("WebCore.IndexedDB.BackingStore.OpenStatus", IDBLevelDBBackingStoreOpenCleanupReopenFailed, IDBLevelDBBackingStoreOpenMax);
+ return PassRefPtr<IDBBackingStore>();
}
+ HistogramSupport::histogramEnumeration("WebCore.IndexedDB.BackingStore.OpenStatus", IDBLevelDBBackingStoreOpenCleanupReopenSuccess, IDBLevelDBBackingStoreOpenMax);
}
if (!db) {
@@ -445,14 +444,39 @@
return PassRefPtr<IDBBackingStore>();
}
- // FIXME: Handle comparator name changes.
+ return create(fileIdentifier, db.release(), comparator.release());
+}
- RefPtr<IDBBackingStore> backingStore(adoptRef(new IDBBackingStore(fileIdentifier, db.release())));
- backingStore->m_comparator = comparator.release();
+PassRefPtr<IDBBackingStore> IDBBackingStore::openInMemory(SecurityOrigin* securityOrigin, const String& identifier)
+{
+ DefaultLevelDBFactory levelDBFactory;
+ return IDBBackingStore::openInMemory(securityOrigin, identifier, &levelDBFactory);
+}
- if (!setUpMetadata(backingStore->m_db.get(), fileIdentifier))
+PassRefPtr<IDBBackingStore> IDBBackingStore::openInMemory(SecurityOrigin* securityOrigin, const String& identifier, LevelDBFactory* levelDBFactory)
+{
+ IDB_TRACE("IDBBackingStore::openInMemory");
+
+ OwnPtr<LevelDBComparator> comparator = adoptPtr(new Comparator());
+ OwnPtr<LevelDBDatabase> db = LevelDBDatabase::openInMemory(comparator.get());
+ if (!db) {
+ LOG_ERROR("LevelDBDatabase::openInMemory failed.");
+ HistogramSupport::histogramEnumeration("WebCore.IndexedDB.BackingStore.OpenStatus", IDBLevelDBBackingStoreOpenMemoryFailed, IDBLevelDBBackingStoreOpenMax);
return PassRefPtr<IDBBackingStore>();
+ }
+ HistogramSupport::histogramEnumeration("WebCore.IndexedDB.BackingStore.OpenStatus", IDBLevelDBBackingStoreOpenMemorySuccess, IDBLevelDBBackingStoreOpenMax);
+ return create(identifier, db.release(), comparator.release());
+}
+
+PassRefPtr<IDBBackingStore> IDBBackingStore::create(const String& identifier, PassOwnPtr<LevelDBDatabase> db, PassOwnPtr<LevelDBComparator> comparator)
+{
+ // FIXME: Handle comparator name changes.
+ RefPtr<IDBBackingStore> backingStore(adoptRef(new IDBBackingStore(identifier, db, comparator)));
+
+ if (!setUpMetadata(backingStore->m_db.get(), identifier))
+ return PassRefPtr<IDBBackingStore>();
+
return backingStore.release();
}
Modified: trunk/Source/WebCore/Modules/indexeddb/IDBBackingStore.h (147232 => 147233)
--- trunk/Source/WebCore/Modules/indexeddb/IDBBackingStore.h 2013-03-29 18:54:24 UTC (rev 147232)
+++ trunk/Source/WebCore/Modules/indexeddb/IDBBackingStore.h 2013-03-29 19:12:04 UTC (rev 147233)
@@ -59,6 +59,8 @@
virtual ~IDBBackingStore();
static PassRefPtr<IDBBackingStore> open(SecurityOrigin*, const String& pathBase, const String& fileIdentifier);
static PassRefPtr<IDBBackingStore> open(SecurityOrigin*, const String& pathBase, const String& fileIdentifier, LevelDBFactory*);
+ static PassRefPtr<IDBBackingStore> openInMemory(SecurityOrigin*, const String& identifier);
+ static PassRefPtr<IDBBackingStore> openInMemory(SecurityOrigin*, const String& identifier, LevelDBFactory*);
WeakPtr<IDBBackingStore> createWeakPtr() { return m_weakFactory.createWeakPtr(); }
virtual Vector<String> getDatabaseNames();
@@ -176,12 +178,14 @@
};
protected:
- IDBBackingStore(const String& identifier, PassOwnPtr<LevelDBDatabase>);
+ IDBBackingStore(const String& identifier, PassOwnPtr<LevelDBDatabase>, PassOwnPtr<LevelDBComparator>);
// Should only used for mocking.
IDBBackingStore();
private:
+ static PassRefPtr<IDBBackingStore> create(const String& identifier, PassOwnPtr<LevelDBDatabase>, PassOwnPtr<LevelDBComparator>);
+
bool findKeyInIndex(IDBBackingStore::Transaction*, int64_t databaseId, int64_t objectStoreId, int64_t indexId, const IDBKey&, Vector<char>& foundEncodedPrimaryKey, bool& found);
bool getIndexes(int64_t databaseId, int64_t objectStoreId, IDBObjectStoreMetadata::IndexMap*) WARN_UNUSED_RETURN;
Modified: trunk/Source/WebCore/Modules/indexeddb/IDBFactoryBackendImpl.cpp (147232 => 147233)
--- trunk/Source/WebCore/Modules/indexeddb/IDBFactoryBackendImpl.cpp 2013-03-29 18:54:24 UTC (rev 147232)
+++ trunk/Source/WebCore/Modules/indexeddb/IDBFactoryBackendImpl.cpp 2013-03-29 19:12:04 UTC (rev 147233)
@@ -131,17 +131,28 @@
PassRefPtr<IDBBackingStore> IDBFactoryBackendImpl::openBackingStore(PassRefPtr<SecurityOrigin> securityOrigin, const String& dataDirectory)
{
const String fileIdentifier = computeFileIdentifier(securityOrigin.get());
+ const bool openInMemory = dataDirectory.isEmpty();
IDBBackingStoreMap::iterator it2 = m_backingStoreMap.find(fileIdentifier);
- if (it2 != m_backingStoreMap.end()) {
- if (it2->value.get())
- return it2->value.get();
- }
+ if (it2 != m_backingStoreMap.end() && it2->value.get())
+ return it2->value.get();
- RefPtr<IDBBackingStore> backingStore = IDBBackingStore::open(securityOrigin.get(), dataDirectory, fileIdentifier);
+ RefPtr<IDBBackingStore> backingStore;
+ if (openInMemory)
+ backingStore = IDBBackingStore::openInMemory(securityOrigin.get(), fileIdentifier);
+ else
+ backingStore = IDBBackingStore::open(securityOrigin.get(), dataDirectory, fileIdentifier);
+
if (backingStore) {
cleanWeakMap(m_backingStoreMap);
m_backingStoreMap.set(fileIdentifier, backingStore->createWeakPtr());
+ // If an in-memory database, bind lifetime to this factory instance.
+ if (openInMemory)
+ m_sessionOnlyBackingStores.add(backingStore);
+
+ // All backing stores associated with this factory should be of the same type.
+ ASSERT(m_sessionOnlyBackingStores.isEmpty() || openInMemory);
+
return backingStore.release();
}
Modified: trunk/Source/WebCore/Modules/indexeddb/IDBFactoryBackendImpl.h (147232 => 147233)
--- trunk/Source/WebCore/Modules/indexeddb/IDBFactoryBackendImpl.h 2013-03-29 18:54:24 UTC (rev 147232)
+++ trunk/Source/WebCore/Modules/indexeddb/IDBFactoryBackendImpl.h 2013-03-29 19:12:04 UTC (rev 147233)
@@ -33,6 +33,7 @@
#include "IDBFactoryBackendInterface.h"
#include "SecurityOrigin.h"
#include <wtf/HashMap.h>
+#include <wtf/HashSet.h>
#include <wtf/RefCounted.h>
#include <wtf/WeakPtr.h>
#include <wtf/text/StringHash.h>
@@ -73,6 +74,8 @@
typedef HashMap<String, WeakPtr<IDBBackingStore> > IDBBackingStoreMap;
IDBBackingStoreMap m_backingStoreMap;
+ HashSet<RefPtr<IDBBackingStore> > m_sessionOnlyBackingStores;
+
// Only one instance of the factory should exist at any given time.
static IDBFactoryBackendImpl* idbFactoryBackendImpl;
};
Modified: trunk/Source/WebKit/chromium/ChangeLog (147232 => 147233)
--- trunk/Source/WebKit/chromium/ChangeLog 2013-03-29 18:54:24 UTC (rev 147232)
+++ trunk/Source/WebKit/chromium/ChangeLog 2013-03-29 19:12:04 UTC (rev 147233)
@@ -1,3 +1,14 @@
+2013-03-29 Joshua Bell <jsb...@chromium.org>
+
+ IndexedDB: Bind lifetime of in-memory backing stores to IDBFactory backend
+ https://bugs.webkit.org/show_bug.cgi?id=110820
+
+ Reviewed by Tony Chang.
+
+ * tests/IDBBackingStoreTest.cpp:
+ (WebCore::IDBBackingStoreTest::SetUp): Use openInMemory rather than empty path.
+ (WebCore::TEST): Added IDBFactoryBackendTest.MemoryBackingStoreLifetime to verify refs.
+
2013-03-28 James Robinson <jam...@chromium.org>
[chromium] Remove unused WebRegularExpression API
Modified: trunk/Source/WebKit/chromium/tests/IDBBackingStoreTest.cpp (147232 => 147233)
--- trunk/Source/WebKit/chromium/tests/IDBBackingStoreTest.cpp 2013-03-29 18:54:24 UTC (rev 147232)
+++ trunk/Source/WebKit/chromium/tests/IDBBackingStoreTest.cpp 2013-03-29 19:12:04 UTC (rev 147233)
@@ -29,6 +29,7 @@
#include "IDBFactoryBackendImpl.h"
#include "IDBLevelDBCoding.h"
+#include "SecurityOrigin.h"
#include "SharedBuffer.h"
#include <gtest/gtest.h>
@@ -47,10 +48,8 @@
void SetUp()
{
SecurityOrigin* securityOrigin = 0;
- // Empty pathBase means an in-memory database.
- String pathBase;
String fileIdentifier;
- m_backingStore = IDBBackingStore::open(securityOrigin, pathBase, fileIdentifier);
+ m_backingStore = IDBBackingStore::openInMemory(securityOrigin, fileIdentifier);
// useful keys and values during tests
const char rawValue1[] = "value1";
@@ -290,14 +289,35 @@
EXPECT_TRUE(diskStore1->hasOneRef());
RefPtr<IDBBackingStore> diskStore2 = factory->testOpenBackingStore(origin1, path);
- EXPECT_EQ(diskStore2.get(), diskStore1.get());
- EXPECT_EQ(diskStore2->refCount(), 2);
+ EXPECT_EQ(diskStore1.get(), diskStore2.get());
+ EXPECT_EQ(2, diskStore2->refCount());
RefPtr<IDBBackingStore> diskStore3 = factory->testOpenBackingStore(origin2, path);
EXPECT_TRUE(diskStore3->hasOneRef());
- EXPECT_EQ(diskStore1->refCount(), 2);
+ EXPECT_EQ(2, diskStore1->refCount());
}
+TEST(IDBFactoryBackendTest, MemoryBackingStoreLifetime)
+{
+ RefPtr<SecurityOrigin> origin1 = SecurityOrigin::create("http", "localhost", 81);
+ RefPtr<SecurityOrigin> origin2 = SecurityOrigin::create("http", "localhost", 82);
+
+ RefPtr<MockIDBFactoryBackend> factory = MockIDBFactoryBackend::create();
+ RefPtr<IDBBackingStore> memStore1 = factory->testOpenBackingStore(origin1, String());
+ EXPECT_EQ(2, memStore1->refCount());
+ RefPtr<IDBBackingStore> memStore2 = factory->testOpenBackingStore(origin1, String());
+ EXPECT_EQ(memStore1.get(), memStore2.get());
+ EXPECT_EQ(3, memStore2->refCount());
+
+ RefPtr<IDBBackingStore> memStore3 = factory->testOpenBackingStore(origin2, String());
+ EXPECT_EQ(2, memStore3->refCount());
+ EXPECT_EQ(3, memStore1->refCount());
+
+ factory.clear();
+ EXPECT_EQ(2, memStore1->refCount());
+ EXPECT_EQ(1, memStore3->refCount());
+}
+
} // namespace
#endif // ENABLE(INDEXED_DATABASE)
