Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(2339)

Unified Diff: third_party/WebKit/Source/modules/indexeddb/IDBTransaction.h

Issue 2314933005: Align IndexedDB metadata rollback on transaction abort to spec. (Closed)
Patch Set: Rebased. Created 4 years, 3 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
Index: third_party/WebKit/Source/modules/indexeddb/IDBTransaction.h
diff --git a/third_party/WebKit/Source/modules/indexeddb/IDBTransaction.h b/third_party/WebKit/Source/modules/indexeddb/IDBTransaction.h
index 154120dbc681958c92aad62d807b81d6f05aad53..ac008a43f04a2adf9929881322b48eeb690f662f 100644
--- a/third_party/WebKit/Source/modules/indexeddb/IDBTransaction.h
+++ b/third_party/WebKit/Source/modules/indexeddb/IDBTransaction.h
@@ -40,15 +40,16 @@
#include "public/platform/modules/indexeddb/WebIDBDatabase.h"
#include "public/platform/modules/indexeddb/WebIDBTypes.h"
#include "wtf/HashSet.h"
+#include "wtf/Vector.h"
namespace blink {
class DOMException;
class ExceptionState;
class IDBDatabase;
+class IDBIndex;
class IDBObjectStore;
class IDBOpenDBRequest;
-struct IDBObjectStoreMetadata;
class MODULES_EXPORT IDBTransaction final
: public EventTargetWithInlineData
@@ -57,8 +58,8 @@ class MODULES_EXPORT IDBTransaction final
USING_GARBAGE_COLLECTED_MIXIN(IDBTransaction);
DEFINE_WRAPPERTYPEINFO();
public:
- static IDBTransaction* create(ScriptState*, int64_t, const HashSet<String>& objectStoreNames, WebIDBTransactionMode, IDBDatabase*);
- static IDBTransaction* create(ScriptState*, int64_t, IDBDatabase*, IDBOpenDBRequest*, const IDBDatabaseMetadata& previousMetadata);
+ static IDBTransaction* createNonVersionChange(ScriptState*, int64_t, const HashSet<String>& objectStoreNames, WebIDBTransactionMode, IDBDatabase*);
jsbell 2016/09/16 18:17:29 Why these renames? If it's just to avoid the over
pwnall 2016/09/17 01:34:22 The renames document when each create version gets
+ static IDBTransaction* createVersionChange(ScriptState*, int64_t, IDBDatabase*, IDBOpenDBRequest*, const IDBDatabaseOwnMetadata& oldMetadata);
~IDBTransaction() override;
DECLARE_VIRTUAL_TRACE();
@@ -84,9 +85,14 @@ public:
void registerRequest(IDBRequest*);
void unregisterRequest(IDBRequest*);
- void objectStoreCreated(const String&, IDBObjectStore*);
- void objectStoreDeleted(const String&);
+
+ // The methods below are called before the changes are applied to the
+ // database's metadata.
jsbell 2016/09/16 18:17:29 Can you expand this comment to answer "why?"
pwnall 2016/09/17 01:34:22 Done. Can you please see if my revision makes sen
+ void objectStoreCreated(const String& name, IDBObjectStore*);
+ void objectStoreDeleted(const int64_t objectStoreId, const String& name);
void objectStoreRenamed(const String& oldName, const String& newName);
+ void indexDeleted(IDBIndex*); // only called when the index's IDBIndex had been created
+
void setActive(bool);
void setError(DOMException*);
@@ -112,10 +118,17 @@ protected:
DispatchEventResult dispatchEventInternal(Event*) override;
private:
- IDBTransaction(ScriptState*, int64_t, const HashSet<String>&, WebIDBTransactionMode, IDBDatabase*, IDBOpenDBRequest*, const IDBDatabaseMetadata&);
+ IDBTransaction(ScriptState*, int64_t, const HashSet<String>&, WebIDBTransactionMode, IDBDatabase*, IDBOpenDBRequest*, const IDBDatabaseOwnMetadata&);
void enqueueEvent(Event*);
+ // Called when a transaction is aborted.
+ void abortOutstandingRequests();
+ void revertDatabaseMetadata();
+
+ // Called when a transaction is completed (committed or aborted).
+ void finish();
+
enum State {
Inactive, // Created or started, but not in an event callback
Active, // Created or started, in creation scope or an event callback
@@ -125,7 +138,11 @@ private:
const int64_t m_id;
Member<IDBDatabase> m_database;
- const HashSet<String> m_objectStoreNames;
+ // The name of the object stores that the transaction may operate on.
+ //
+ // The scope is empty for versionchange transactions, which can operate on
+ // the entire database.
+ const HashSet<String> m_scope;
Member<IDBOpenDBRequest> m_openDBRequest;
const WebIDBTransactionMode m_mode;
State m_state = Active;
@@ -135,21 +152,42 @@ private:
HeapListHashSet<Member<IDBRequest>> m_requestList;
- typedef HeapHashMap<String, Member<IDBObjectStore>> IDBObjectStoreMap;
+ // Caches the IDBObjectStore instances returned by the objectStore() method.
+ //
+ // The spec requires that a transaction's objectStore() returns the same
+ // IDBObjectStore instance for a specific store, so this cache is necessary
+ // for correctness.
+ //
+ // The cache may be in an inconsistent state after a transaction is aborted.
jsbell 2016/09/16 18:17:29 I think this can be simplified to e.g.: // object
pwnall 2016/09/17 01:34:22 Done. I also revised the comment on IDBIndex.
+ // objectStore() throws for inactive transactions, which makes the cache not
+ // observable, so reverting the cache would be a waste of resources.
+ using IDBObjectStoreMap = HeapHashMap<String, Member<IDBObjectStore>>;
IDBObjectStoreMap m_objectStoreMap;
- // Used to mark stores created in an aborted upgrade transaction as
- // deleted.
- HeapHashSet<Member<IDBObjectStore>> m_createdObjectStores;
-
- // Used to notify object stores (which are no longer in m_objectStoreMap)
- // when the transaction is finished.
- HeapHashSet<Member<IDBObjectStore>> m_deletedObjectStores;
-
- // Holds stores created, deleted, or used during upgrade transactions to
- // reset metadata in case of abort.
- HeapHashMap<Member<IDBObjectStore>, IDBObjectStoreMetadata> m_objectStoreCleanupMap;
- IDBDatabaseMetadata m_previousMetadata;
+ // The metadata of object stores when they are opened by this transaction.
+ //
+ // Only valid for versionchange transactions.
+ HeapHashMap<Member<IDBObjectStore>, RefPtr<IDBObjectStoreMetadata>> m_oldStoreMetadata;
+
+ // The metadata of deleted object stores without IDBObjectStore instaces.
jsbell 2016/09/16 18:17:29 typo: instances
pwnall 2016/09/17 01:34:22 Done.
+ //
+ // Only valid for versionchange transactions.
+ Vector<RefPtr<IDBObjectStoreMetadata>> m_deletedObjectStores;
+
+ // Tracks the indexes deleted by this transaction.
+ //
+ // This set only includes indexes that were created before this transaction,
+ // and were deleted during this transaction. Once marked for deletion, these
+ // indexes are removed from their object stores' index maps, so we need to
+ // stash them somewhere else in case the transaction gets aborted.
+ //
+ // This set does not include indexes created and deleted during this
+ // transaction, because we don't need to change their metadata when the
+ // transaction aborts, as they will still be marked for deletion.
+ HeapVector<Member<IDBIndex>> m_deletedIndexes;
+
+ // Only valid for versionchange transactions.
+ IDBDatabaseOwnMetadata m_oldDatabaseMetadata;
};
} // namespace blink

Powered by Google App Engine
This is Rietveld 408576698