Index: third_party/WebKit/Source/modules/indexeddb/IDBObjectStore.cpp |
diff --git a/third_party/WebKit/Source/modules/indexeddb/IDBObjectStore.cpp b/third_party/WebKit/Source/modules/indexeddb/IDBObjectStore.cpp |
index c5dbcc858911bd88419b1c1c5ec0f5247487b25e..c0f6524e7860de3cf8e57fcc49d7803e83b733c7 100644 |
--- a/third_party/WebKit/Source/modules/indexeddb/IDBObjectStore.cpp |
+++ b/third_party/WebKit/Source/modules/indexeddb/IDBObjectStore.cpp |
@@ -55,26 +55,26 @@ using blink::WebIDBCursor; |
using blink::WebIDBDatabase; |
using blink::WebVector; |
+ |
jsbell
2016/09/26 22:23:40
Nit: remove extra blank line
pwnall
2016/09/27 00:12:19
Done.
|
namespace blink { |
namespace { |
using IndexKeys = HeapVector<Member<IDBKey>>; |
- |
} |
-IDBObjectStore::IDBObjectStore(const IDBObjectStoreMetadata& metadata, IDBTransaction* transaction) |
- : m_metadata(metadata) |
+IDBObjectStore::IDBObjectStore(RefPtr<IDBObjectStoreMetadata> metadata, IDBTransaction* transaction) |
+ : m_metadata(std::move(metadata)) |
, m_transaction(transaction) |
{ |
DCHECK(m_transaction); |
+ DCHECK(m_metadata.get()); |
} |
DEFINE_TRACE(IDBObjectStore) |
{ |
visitor->trace(m_transaction); |
visitor->trace(m_indexMap); |
- visitor->trace(m_createdIndexes); |
} |
void IDBObjectStore::setName(const String& name, ExceptionState& exceptionState) |
@@ -111,13 +111,7 @@ void IDBObjectStore::setName(const String& name, ExceptionState& exceptionState) |
return; |
} |
- backendDB()->renameObjectStore(m_transaction->id(), id(), name); |
- m_transaction->objectStoreRenamed(m_metadata.name, name); |
- m_metadata.name = name; |
- |
- // The name inside the database's version of the object store metadata is used by IDBDatabase.objectStoreNames(). |
- // If the transaction is aborted, this name will be reverted when the metadata is overwritten with the previousMetadata in IDBTransaction. |
- m_transaction->db()->objectStoreRenamed(id(), name); |
+ m_transaction->db()->renameObjectStore(id(), name); |
} |
ScriptValue IDBObjectStore::keyPath(ScriptState* scriptState) const |
@@ -130,7 +124,7 @@ DOMStringList* IDBObjectStore::indexNames() const |
IDB_TRACE("IDBObjectStore::indexNames"); |
DOMStringList* indexNames = DOMStringList::create(DOMStringList::IndexedDB); |
for (const auto& it : metadata().indexes) |
- indexNames->append(it.value.name); |
+ indexNames->append(it.value->name); |
indexNames->sort(); |
return indexNames; |
} |
@@ -412,7 +406,7 @@ IDBRequest* IDBObjectStore::put(ScriptState* scriptState, WebIDBPutMode putMode, |
if (clone.isEmpty()) |
clone = deserializeScriptValue(scriptState, serializedValue.get(), &blobInfo); |
IndexKeys keys; |
- generateIndexKeysForValue(scriptState->isolate(), it.value, clone, &keys); |
+ generateIndexKeysForValue(scriptState->isolate(), *it.value, clone, &keys); |
indexIds.append(it.key); |
indexKeys.append(keys); |
} |
@@ -500,9 +494,9 @@ namespace { |
// cursor success handlers are kept alive. |
class IndexPopulator final : public EventListener { |
public: |
- static IndexPopulator* create(ScriptState* scriptState, IDBDatabase* database, int64_t transactionId, int64_t objectStoreId, const IDBIndexMetadata& indexMetadata) |
+ static IndexPopulator* create(ScriptState* scriptState, IDBDatabase* database, int64_t transactionId, int64_t objectStoreId, RefPtr<const IDBIndexMetadata> indexMetadata) |
{ |
- return new IndexPopulator(scriptState, database, transactionId, objectStoreId, indexMetadata); |
+ return new IndexPopulator(scriptState, database, transactionId, objectStoreId, std::move(indexMetadata)); |
} |
bool operator==(const EventListener& other) const override |
@@ -517,17 +511,18 @@ public: |
} |
private: |
- IndexPopulator(ScriptState* scriptState, IDBDatabase* database, int64_t transactionId, int64_t objectStoreId, const IDBIndexMetadata& indexMetadata) |
+ IndexPopulator(ScriptState* scriptState, IDBDatabase* database, int64_t transactionId, int64_t objectStoreId, RefPtr<const IDBIndexMetadata> indexMetadata) |
: EventListener(CPPEventListenerType) |
, m_scriptState(scriptState) |
, m_database(database) |
, m_transactionId(transactionId) |
, m_objectStoreId(objectStoreId) |
- , m_indexMetadata(indexMetadata) |
+ , m_indexMetadata(std::move(indexMetadata)) |
{ |
+ DCHECK(m_indexMetadata.get()); |
} |
- const IDBIndexMetadata& indexMetadata() const { return m_indexMetadata; } |
+ const IDBIndexMetadata& indexMetadata() const { return *m_indexMetadata; } |
void handleEvent(ExecutionContext* executionContext, Event* event) override |
{ |
@@ -572,7 +567,7 @@ private: |
Member<IDBDatabase> m_database; |
const int64_t m_transactionId; |
const int64_t m_objectStoreId; |
- const IDBIndexMetadata m_indexMetadata; |
+ RefPtr<const IDBIndexMetadata> m_indexMetadata; |
}; |
} // namespace |
@@ -612,18 +607,17 @@ IDBIndex* IDBObjectStore::createIndex(ScriptState* scriptState, const String& na |
return nullptr; |
} |
- int64_t indexId = m_metadata.maxIndexId + 1; |
+ int64_t indexId = m_metadata->maxIndexId + 1; |
DCHECK_NE(indexId, IDBIndexMetadata::InvalidId); |
backendDB()->createIndex(m_transaction->id(), id(), indexId, name, keyPath, options.unique(), options.multiEntry()); |
- ++m_metadata.maxIndexId; |
+ ++m_metadata->maxIndexId; |
- IDBIndexMetadata indexMetadata(name, indexId, keyPath, options.unique(), options.multiEntry()); |
+ RefPtr<IDBIndexMetadata> indexMetadata = adoptRef(new IDBIndexMetadata( |
+ name, indexId, keyPath, options.unique(), options.multiEntry())); |
IDBIndex* index = IDBIndex::create(indexMetadata, this, m_transaction.get()); |
m_indexMap.set(name, index); |
- m_createdIndexes.add(index); |
- m_metadata.indexes.set(indexId, indexMetadata); |
- m_transaction->db()->indexCreated(id(), indexMetadata); |
+ m_metadata->indexes.set(indexId, indexMetadata); |
DCHECK(!exceptionState.hadException()); |
if (exceptionState.hadException()) |
@@ -633,7 +627,7 @@ IDBIndex* IDBObjectStore::createIndex(ScriptState* scriptState, const String& na |
indexRequest->preventPropagation(); |
// This is kept alive by being the success handler of the request, which is in turn kept alive by the owning transaction. |
- IndexPopulator* indexPopulator = IndexPopulator::create(scriptState, transaction()->db(), m_transaction->id(), id(), indexMetadata); |
+ IndexPopulator* indexPopulator = IndexPopulator::create(scriptState, transaction()->db(), m_transaction->id(), id(), std::move(indexMetadata)); |
indexRequest->setOnsuccess(indexPopulator); |
return index; |
} |
@@ -661,8 +655,9 @@ IDBIndex* IDBObjectStore::index(const String& name, ExceptionState& exceptionSta |
} |
DCHECK(metadata().indexes.contains(indexId)); |
- const IDBIndexMetadata& indexMetadata = metadata().indexes.get(indexId); |
- IDBIndex* index = IDBIndex::create(indexMetadata, this, m_transaction.get()); |
+ RefPtr<IDBIndexMetadata> indexMetadata = metadata().indexes.get(indexId); |
+ DCHECK(indexMetadata.get()); |
+ IDBIndex* index = IDBIndex::create(std::move(indexMetadata), this, m_transaction.get()); |
m_indexMap.set(name, index); |
return index; |
} |
@@ -698,10 +693,10 @@ void IDBObjectStore::deleteIndex(const String& name, ExceptionState& exceptionSt |
backendDB()->deleteIndex(m_transaction->id(), id(), indexId); |
- m_metadata.indexes.remove(indexId); |
- m_transaction->db()->indexDeleted(id(), indexId); |
+ m_metadata->indexes.remove(indexId); |
IDBIndexMap::iterator it = m_indexMap.find(name); |
if (it != m_indexMap.end()) { |
+ m_transaction->indexDeleted(it->value); |
it->value->markDeleted(); |
m_indexMap.remove(name); |
} |
@@ -811,45 +806,104 @@ IDBRequest* IDBObjectStore::count(ScriptState* scriptState, const ScriptValue& r |
void IDBObjectStore::markDeleted() |
{ |
DCHECK(m_transaction->isVersionChange()) << "An object store got deleted outside a versionchange transaction."; |
+ |
m_deleted = true; |
+ m_metadata->indexes.clear(); |
+ |
+ for (auto& it : m_indexMap) { |
+ IDBIndex* index = it.value; |
+ index->markDeleted(); |
+ } |
} |
-void IDBObjectStore::abort() |
+void IDBObjectStore::clearIndexCache() |
{ |
- for (auto& index : m_createdIndexes) |
- index->markDeleted(); |
+ DCHECK(!m_transaction->isActive() || (isDeleted() && id() > m_transaction->oldMaxObjectStoreId())); |
+ |
+ // There is no harm in having clearIndexCache() happen multiple times for |
+ // the same object. We assert that it is called once to uncover potential |
+ // object store accounting bugs. |
+#if DCHECK_IS_ON() |
+ DCHECK(!m_clearIndexCacheCalled); |
+ m_clearIndexCacheCalled = true; |
+#endif // DCHECK_IS_ON() |
+ |
+ m_indexMap.clear(); |
} |
-void IDBObjectStore::transactionFinished() |
+void IDBObjectStore::revertMetadata(RefPtr<IDBObjectStoreMetadata> oldMetadata) |
{ |
+ DCHECK(m_transaction->isVersionChange()); |
DCHECK(!m_transaction->isActive()); |
+ DCHECK(oldMetadata.get()); |
+ DCHECK(id() == oldMetadata->id); |
+ |
+ // Index IDs are allocated sequentially, so we can tell if an index was |
+ // created in this transaction by comparing its ID with the object store's |
+ // maximum index ID at the time when the transaction was started. |
+ const int64_t oldMaxIndexId = oldMetadata->maxIndexId; |
+ for (auto& it : m_indexMap) { |
+ IDBIndex* index = it.value; |
+ const int64_t indexId = index->id(); |
+ |
+ if (indexId > oldMaxIndexId) { |
+ // The index was created by this transaction. According to the spec, |
+ // its metadata will remain as-is. We just need to look up the |
jsbell
2016/09/26 22:23:40
You can drop the second sentence here ("We just ne
pwnall
2016/09/27 00:12:19
Done.
Thanks, that's a very good point!
|
+ // index's IDBIndex instance, and mark it as deleted. |
+ DCHECK(!oldMetadata->indexes.contains(indexId)); |
+ index->markDeleted(); |
+ continue; |
+ } |
- // Break reference cycles. |
- // TODO(jsbell): This can be removed c/o Oilpan. |
- m_indexMap.clear(); |
- m_createdIndexes.clear(); |
+ // The index was created in a previous transaction. We need to revert |
+ // its metadata. The index might have been deleted, so we |
+ // unconditionally reset the deletion marker. We could find out if the |
jsbell
2016/09/26 22:23:40
I think you can drop the alternative here ("We cou
pwnall
2016/09/27 00:12:19
Done.
Sure, as long as one of us remembers why I w
|
+ // index was deleted by looking up the index ID in the store's metadata, |
+ // but that would be more work than the unconditional reset. |
+ DCHECK(oldMetadata->indexes.contains(indexId)); |
+ RefPtr<IDBIndexMetadata> oldIndexMetadata = oldMetadata->indexes.get(indexId); |
+ index->revertMetadata(std::move(oldIndexMetadata)); |
+ } |
+ m_metadata = std::move(oldMetadata); |
+ |
+ // An object store's metadata will only get reverted if the index was in the |
+ // database when the versionchange transaction started. |
+ m_deleted = false; |
+} |
+ |
+void IDBObjectStore::revertDeletedIndexMetadata(IDBIndex& deletedIndex) |
+{ |
+ DCHECK(m_transaction->isVersionChange()); |
+ DCHECK(!m_transaction->isActive()); |
+ DCHECK(deletedIndex.objectStore() == this); |
+ DCHECK(deletedIndex.isDeleted()); |
+ |
+ const int64_t indexId = deletedIndex.id(); |
+ DCHECK(m_metadata->indexes.contains(indexId)) << "The object store's metadata was not correctly reverted"; |
+ RefPtr<IDBIndexMetadata> oldIndexMetadata = m_metadata->indexes.get(indexId); |
+ deletedIndex.revertMetadata(std::move(oldIndexMetadata)); |
} |
-void IDBObjectStore::indexRenamed(int64_t indexId, const String& newName) |
+void IDBObjectStore::renameIndex(int64_t indexId, const String& newName) |
{ |
DCHECK(m_transaction->isVersionChange()); |
DCHECK(m_transaction->isActive()); |
- auto metadataIterator = m_metadata.indexes.find(indexId); |
- DCHECK_NE(metadataIterator, m_metadata.indexes.end()) << "Invalid indexId"; |
- const String& oldName = metadataIterator->value.name; |
+ auto metadataIterator = m_metadata->indexes.find(indexId); |
+ DCHECK_NE(metadataIterator, m_metadata->indexes.end()) << "Invalid indexId"; |
+ const String& oldName = metadataIterator->value->name; |
DCHECK(m_indexMap.contains(oldName)) << "The index had to be accessed in order to be renamed."; |
DCHECK(!m_indexMap.contains(newName)); |
m_indexMap.set(newName, m_indexMap.take(oldName)); |
- metadataIterator->value.name = newName; |
+ metadataIterator->value->name = newName; |
} |
int64_t IDBObjectStore::findIndexId(const String& name) const |
{ |
for (const auto& it : metadata().indexes) { |
- if (it.value.name == name) { |
+ if (it.value->name == name) { |
DCHECK_NE(it.key, IDBIndexMetadata::InvalidId); |
return it.key; |
} |