Permalink
Browse files

IndexedDB: Simplify transaction timers and event tracking

https://bugs.webkit.org/show_bug.cgi?id=102984

Reviewed by Tony Chang.

Source/WebCore:

Now that the transaction "commit" decision is made on the front-end, the back end no-longer
needs to be aware of when individual IDBRequests have dispatched to script or track pending
events (except for preemptive ones like createIndex). This also lets two timers be collapsed
into one which significantly simplifies the code flow in IDBTransactionBackendImpl.

No new tests - just simplification. Exercised by storage/indexeddb/transaction-*.html etc.

* Modules/indexeddb/IDBCursorBackendImpl.cpp:
(WebCore::IDBCursorBackendImpl::prefetchContinueInternal): No more tracking.
(WebCore::IDBCursorBackendImpl::prefetchReset): No more tracking.
* Modules/indexeddb/IDBDatabaseBackendImpl.cpp:
(WebCore::IDBDatabaseBackendImpl::createObjectStoreInternal): No more tracking.
(WebCore::IDBDatabaseBackendImpl::deleteObjectStoreInternal): No more tracking.
* Modules/indexeddb/IDBObjectStoreBackendImpl.cpp:
(WebCore::IDBObjectStoreBackendImpl::setIndexesReadyInternal): No more tracking.
(WebCore::IDBObjectStoreBackendImpl::createIndexInternal): No more tracking.
(WebCore::IDBObjectStoreBackendImpl::deleteIndexInternal): No more tracking.
* Modules/indexeddb/IDBRequest.cpp:
(WebCore::IDBRequest::dispatchEvent): Order must be:
1. request is unregistered from transaction (so request list may be empty)
2. abort transaction if event being dispatched was an error
3. deactivate transaction (which may commit if #1 left it empty and #2 didn't abort)
* Modules/indexeddb/IDBTransactionBackendImpl.cpp:
(WebCore::IDBTransactionBackendImpl::IDBTransactionBackendImpl): Need to track if commit
was requested; previously the front end would have triggered an event timer which, on
completion, would be the signal that the front end was finished.
(WebCore::IDBTransactionBackendImpl::scheduleTask): Schedule a timer to service the new
task, if necessary.
(WebCore::IDBTransactionBackendImpl::abort):
(WebCore::IDBTransactionBackendImpl::hasPendingTasks):
(WebCore::IDBTransactionBackendImpl::commit):
(WebCore::IDBTransactionBackendImpl::taskTimerFired): Picks up "commit" responsibilities
from the now deleted taskEventTimerFired, if everything is truly complete done.
* Modules/indexeddb/IDBTransactionBackendImpl.h:
(IDBTransactionBackendImpl):
* Modules/indexeddb/IDBTransactionBackendInterface.h:
(WebCore::IDBTransactionBackendInterface::didCompleteTaskEvents): Removed from interface.

Source/WebKit/chromium:

Remove now-unused didCompleteTaskEvents() method.

* src/IDBTransactionBackendProxy.cpp:
* src/IDBTransactionBackendProxy.h:
(IDBTransactionBackendProxy):
* src/WebIDBTransactionImpl.cpp:
* src/WebIDBTransactionImpl.h:


git-svn-id: http://svn.webkit.org/repository/webkit/trunk@135927 268f45cc-cd09-0410-ab3c-d52691b4dbfc
1 parent 07734b6 commit 9fbdd3543cc2bdcb0af1285c0f7078a8a3506cf8 jsbell@chromium.org committed Nov 27, 2012
View
@@ -1,3 +1,48 @@
+2012-11-27 Joshua Bell <jsbell@chromium.org>
+
+ IndexedDB: Simplify transaction timers and event tracking
+ https://bugs.webkit.org/show_bug.cgi?id=102984
+
+ Reviewed by Tony Chang.
+
+ Now that the transaction "commit" decision is made on the front-end, the back end no-longer
+ needs to be aware of when individual IDBRequests have dispatched to script or track pending
+ events (except for preemptive ones like createIndex). This also lets two timers be collapsed
+ into one which significantly simplifies the code flow in IDBTransactionBackendImpl.
+
+ No new tests - just simplification. Exercised by storage/indexeddb/transaction-*.html etc.
+
+ * Modules/indexeddb/IDBCursorBackendImpl.cpp:
+ (WebCore::IDBCursorBackendImpl::prefetchContinueInternal): No more tracking.
+ (WebCore::IDBCursorBackendImpl::prefetchReset): No more tracking.
+ * Modules/indexeddb/IDBDatabaseBackendImpl.cpp:
+ (WebCore::IDBDatabaseBackendImpl::createObjectStoreInternal): No more tracking.
+ (WebCore::IDBDatabaseBackendImpl::deleteObjectStoreInternal): No more tracking.
+ * Modules/indexeddb/IDBObjectStoreBackendImpl.cpp:
+ (WebCore::IDBObjectStoreBackendImpl::setIndexesReadyInternal): No more tracking.
+ (WebCore::IDBObjectStoreBackendImpl::createIndexInternal): No more tracking.
+ (WebCore::IDBObjectStoreBackendImpl::deleteIndexInternal): No more tracking.
+ * Modules/indexeddb/IDBRequest.cpp:
+ (WebCore::IDBRequest::dispatchEvent): Order must be:
+ 1. request is unregistered from transaction (so request list may be empty)
+ 2. abort transaction if event being dispatched was an error
+ 3. deactivate transaction (which may commit if #1 left it empty and #2 didn't abort)
+ * Modules/indexeddb/IDBTransactionBackendImpl.cpp:
+ (WebCore::IDBTransactionBackendImpl::IDBTransactionBackendImpl): Need to track if commit
+ was requested; previously the front end would have triggered an event timer which, on
+ completion, would be the signal that the front end was finished.
+ (WebCore::IDBTransactionBackendImpl::scheduleTask): Schedule a timer to service the new
+ task, if necessary.
+ (WebCore::IDBTransactionBackendImpl::abort):
+ (WebCore::IDBTransactionBackendImpl::hasPendingTasks):
+ (WebCore::IDBTransactionBackendImpl::commit):
+ (WebCore::IDBTransactionBackendImpl::taskTimerFired): Picks up "commit" responsibilities
+ from the now deleted taskEventTimerFired, if everything is truly complete done.
+ * Modules/indexeddb/IDBTransactionBackendImpl.h:
+ (IDBTransactionBackendImpl):
+ * Modules/indexeddb/IDBTransactionBackendInterface.h:
+ (WebCore::IDBTransactionBackendInterface::didCompleteTaskEvents): Removed from interface.
+
2012-11-27 Kentaro Hara <haraken@chromium.org>
[V8] Replace toWebCoreString()/toWebCoreAtomicString() in CodeGeneratorV8.pm with V8StringResource
@@ -173,14 +173,12 @@ void IDBCursorBackendImpl::prefetchContinueInternal(ScriptExecutionContext*, Pas
return;
}
- cursor->m_transaction->addPendingEvents(foundKeys.size() - 1);
callbacks->onSuccessWithPrefetch(foundKeys, foundPrimaryKeys, foundValues);
}
void IDBCursorBackendImpl::prefetchReset(int usedPrefetches, int unusedPrefetches)
{
IDB_TRACE("IDBCursorBackendImpl::prefetchReset");
- m_transaction->addPendingEvents(-unusedPrefetches);
m_cursor = m_savedCursor;
m_savedCursor = 0;
@@ -180,8 +180,6 @@ void IDBDatabaseBackendImpl::createObjectStoreInternal(ScriptExecutionContext*,
transaction->abort();
return;
}
-
- transaction->didCompleteTaskEvents();
}
PassRefPtr<IDBObjectStoreBackendImpl> IDBDatabaseBackendImpl::objectStore(int64_t id)
@@ -210,7 +208,6 @@ void IDBDatabaseBackendImpl::deleteObjectStore(int64_t id, IDBTransactionBackend
void IDBDatabaseBackendImpl::deleteObjectStoreInternal(ScriptExecutionContext*, PassRefPtr<IDBDatabaseBackendImpl> database, PassRefPtr<IDBObjectStoreBackendImpl> objectStore, PassRefPtr<IDBTransactionBackendImpl> transaction)
{
database->m_backingStore->deleteObjectStore(transaction->backingStoreTransaction(), database->id(), objectStore->id());
- transaction->didCompleteTaskEvents();
}
void IDBDatabaseBackendImpl::setIntVersionInternal(ScriptExecutionContext*, PassRefPtr<IDBDatabaseBackendImpl> database, int64_t version, PassRefPtr<IDBCallbacks> prpCallbacks, PassRefPtr<IDBDatabaseCallbacks> databaseCallbacks, PassRefPtr<IDBTransactionBackendImpl> transaction)
@@ -268,7 +268,6 @@ void IDBObjectStoreBackendImpl::setIndexesReadyInternal(ScriptExecutionContext*,
OwnPtr<Vector<int64_t> > indexIds = popIndexIds;
for (size_t i = 0; i < indexIds->size(); ++i)
transaction->didCompletePreemptiveEvent();
- transaction->didCompleteTaskEvents();
}
void IDBObjectStoreBackendImpl::putInternal(ScriptExecutionContext*, PassRefPtr<IDBObjectStoreBackendImpl> objectStore, PassRefPtr<SerializedScriptValue> prpValue, PassRefPtr<IDBKey> prpKey, PutMode putMode, PassRefPtr<IDBCallbacks> callbacks, PassRefPtr<IDBTransactionBackendImpl> prpTransaction, PassOwnPtr<Vector<int64_t> > popIndexIds, PassOwnPtr<Vector<IndexKeys> > popIndexKeys)
@@ -407,8 +406,6 @@ void IDBObjectStoreBackendImpl::createIndexInternal(ScriptExecutionContext*, Pas
transaction->abort();
return;
}
-
- transaction->didCompleteTaskEvents();
}
PassRefPtr<IDBIndexBackendInterface> IDBObjectStoreBackendImpl::index(int64_t indexId)
@@ -439,7 +436,6 @@ void IDBObjectStoreBackendImpl::deleteIndex(int64_t indexId, IDBTransactionBacke
void IDBObjectStoreBackendImpl::deleteIndexInternal(ScriptExecutionContext*, PassRefPtr<IDBObjectStoreBackendImpl> objectStore, PassRefPtr<IDBIndexBackendImpl> index, PassRefPtr<IDBTransactionBackendImpl> transaction)
{
objectStore->backingStore()->deleteIndex(transaction->backingStoreTransaction(), objectStore->databaseId(), objectStore->id(), index->id());
- transaction->didCompleteTaskEvents();
}
void IDBObjectStoreBackendImpl::openCursor(PassRefPtr<IDBKeyRange> prpRange, IDBCursor::Direction direction, PassRefPtr<IDBCallbacks> prpCallbacks, IDBTransactionBackendInterface::TaskType taskType, IDBTransactionBackendInterface* transactionPtr, ExceptionCode&)
@@ -78,9 +78,10 @@ IDBRequest::IDBRequest(ScriptExecutionContext* context, PassRefPtr<IDBAny> sourc
, m_preventPropagation(false)
, m_requestState(context)
{
- if (m_transaction) {
+ // Requests associated with IDBFactory (open/deleteDatabase/getDatabaseNames) are not
+ // associated with transactions.
+ if (m_transaction)
m_transaction->registerRequest(this);
- }
}
IDBRequest::~IDBRequest()
@@ -476,30 +477,29 @@ bool IDBRequest::dispatchEvent(PassRefPtr<Event> event)
bool dontPreventDefault = IDBEventDispatcher::dispatch(event.get(), targets);
- if (m_transaction && m_readyState == DONE)
- m_transaction->unregisterRequest(this);
-
- // If this was the last request in the transaction's list, it may commit here.
- if (setTransactionActive)
- m_transaction->setActive(false);
-
- if (cursorToNotify)
- cursorToNotify->postSuccessHandlerCallback();
-
- if (m_readyState == DONE && (!cursorToNotify || m_cursorFinished) && event->type() != eventNames().upgradeneededEvent)
- m_hasPendingActivity = false;
-
if (m_transaction) {
+ if (m_readyState == DONE)
+ m_transaction->unregisterRequest(this);
+
+ // Possibly abort the transaction. This must occur after unregistering (so this request
+ // doesn't receive a second error) and before deactivating (which might trigger commit).
if (event->type() == eventNames().errorEvent && dontPreventDefault && !m_requestAborted) {
m_transaction->setError(m_error);
ExceptionCode unused;
m_transaction->abort(unused);
}
- if (event->type() != eventNames().blockedEvent)
- m_transaction->backend()->didCompleteTaskEvents();
+ // If this was the last request in the transaction's list, it may commit here.
+ if (setTransactionActive)
+ m_transaction->setActive(false);
}
+ if (cursorToNotify)
+ cursorToNotify->postSuccessHandlerCallback();
+
+ if (m_readyState == DONE && (!cursorToNotify || m_cursorFinished) && event->type() != eventNames().upgradeneededEvent)
+ m_hasPendingActivity = false;
+
return dontPreventDefault;
}
@@ -49,12 +49,11 @@ IDBTransactionBackendImpl::IDBTransactionBackendImpl(int64_t id, const Vector<in
, m_objectStoreIds(objectStoreIds)
, m_mode(mode)
, m_state(Unused)
+ , m_commitPending(false)
, m_database(database)
, m_transaction(database->backingStore().get())
, m_taskTimer(this, &IDBTransactionBackendImpl::taskTimerFired)
- , m_taskEventTimer(this, &IDBTransactionBackendImpl::taskEventTimerFired)
, m_pendingPreemptiveEvents(0)
- , m_pendingEvents(0)
{
m_database->transactionCoordinator()->didCreateTransaction(this);
}
@@ -92,6 +91,8 @@ bool IDBTransactionBackendImpl::scheduleTask(TaskType type, PassOwnPtr<ScriptExe
if (m_state == Unused)
start();
+ else if (m_state == Running && !m_taskTimer.isActive())
+ m_taskTimer.startOneShot(0);
return true;
}
@@ -116,7 +117,6 @@ void IDBTransactionBackendImpl::abort(PassRefPtr<IDBDatabaseError> error)
m_state = Finished;
m_taskTimer.stop();
- m_taskEventTimer.stop();
if (wasRunning)
m_transaction.rollback();
@@ -154,7 +154,7 @@ bool IDBTransactionBackendImpl::isTaskQueueEmpty() const
bool IDBTransactionBackendImpl::hasPendingTasks() const
{
- return m_pendingEvents || m_pendingPreemptiveEvents || !isTaskQueueEmpty();
+ return m_pendingPreemptiveEvents || !isTaskQueueEmpty();
}
void IDBTransactionBackendImpl::registerOpenCursor(IDBCursorBackendImpl* cursor)
@@ -167,27 +167,6 @@ void IDBTransactionBackendImpl::unregisterOpenCursor(IDBCursorBackendImpl* curso
m_openCursors.remove(cursor);
}
-void IDBTransactionBackendImpl::addPendingEvents(int n)
-{
- m_pendingEvents += n;
- ASSERT(m_pendingEvents >= 0);
-}
-
-void IDBTransactionBackendImpl::didCompleteTaskEvents()
-{
- if (m_state == Finished)
- return;
-
- ASSERT(m_state == Running);
- ASSERT(m_pendingEvents);
- m_pendingEvents--;
-
- // A single task has completed and error/success events fired. Schedule
- // timer to process another.
- if (!m_taskEventTimer.isActive())
- m_taskEventTimer.startOneShot(0);
-}
-
void IDBTransactionBackendImpl::run()
{
// TransactionCoordinator has started this transaction. Schedule a timer
@@ -212,6 +191,7 @@ void IDBTransactionBackendImpl::commit()
IDB_TRACE("IDBTransactionBackendImpl::commit");
ASSERT(m_state == Unused || m_state == Running);
+ m_commitPending = true;
// Front-end has requested a commit, but there may be tasks like createIndex which
// are considered synchronous by the front-end but are processed asynchronously.
@@ -270,31 +250,16 @@ void IDBTransactionBackendImpl::taskTimerFired(Timer<IDBTransactionBackendImpl>*
while (!taskQueue->isEmpty() && m_state != Finished) {
ASSERT(m_state == Running);
OwnPtr<ScriptExecutionContext::Task> task(taskQueue->takeFirst());
- m_pendingEvents++;
task->performTask(0);
// Event itself may change which queue should be processed next.
taskQueue = m_pendingPreemptiveEvents ? &m_preemptiveTaskQueue : &m_taskQueue;
}
-}
-
-void IDBTransactionBackendImpl::taskEventTimerFired(Timer<IDBTransactionBackendImpl>*)
-{
- IDB_TRACE("IDBTransactionBackendImpl::taskEventTimerFired");
- ASSERT(m_state == Running);
- if (!hasPendingTasks()) {
- // The last task event has completed and the task
- // queue is empty. Commit the transaction.
+ // If there are no pending tasks, we haven't already committed/aborted,
+ // and the front-end requested a commit, it is now safe to do so.
+ if (!hasPendingTasks() && m_state != Finished && m_commitPending)
commit();
- return;
- }
-
- // We are still waiting for other events to complete. However,
- // the task queue is non-empty and the timer is inactive.
- // We can therfore schedule the timer again.
- if (!isTaskQueueEmpty() && !m_taskTimer.isActive())
- m_taskTimer.startOneShot(0);
}
void IDBTransactionBackendImpl::closeOpenCursors()
@@ -53,7 +53,6 @@ class IDBTransactionBackendImpl : public IDBTransactionBackendInterface {
// IDBTransactionBackendInterface
virtual PassRefPtr<IDBObjectStoreBackendInterface> objectStore(int64_t, ExceptionCode&);
- virtual void didCompleteTaskEvents();
virtual void abort();
virtual void setCallbacks(IDBTransactionCallbacks* callbacks) { m_callbacks = callbacks; }
@@ -64,7 +63,6 @@ class IDBTransactionBackendImpl : public IDBTransactionBackendInterface {
bool scheduleTask(TaskType, PassOwnPtr<ScriptExecutionContext::Task>, PassOwnPtr<ScriptExecutionContext::Task> abortTask = nullptr);
void registerOpenCursor(IDBCursorBackendImpl*);
void unregisterOpenCursor(IDBCursorBackendImpl*);
- void addPendingEvents(int);
void addPreemptiveEvent() { m_pendingPreemptiveEvents++; }
void didCompletePreemptiveEvent() { m_pendingPreemptiveEvents--; ASSERT(m_pendingPreemptiveEvents >= 0); }
IDBBackingStore::Transaction* backingStoreTransaction() { return &m_transaction; }
@@ -87,14 +85,14 @@ class IDBTransactionBackendImpl : public IDBTransactionBackendInterface {
bool hasPendingTasks() const;
void taskTimerFired(Timer<IDBTransactionBackendImpl>*);
- void taskEventTimerFired(Timer<IDBTransactionBackendImpl>*);
void closeOpenCursors();
const int64_t m_id;
const Vector<int64_t> m_objectStoreIds;
const unsigned short m_mode;
State m_state;
+ bool m_commitPending;
RefPtr<IDBTransactionCallbacks> m_callbacks;
RefPtr<IDBDatabaseBackendImpl> m_database;
@@ -107,9 +105,7 @@ class IDBTransactionBackendImpl : public IDBTransactionBackendInterface {
// FIXME: delete the timer once we have threads instead.
Timer<IDBTransactionBackendImpl> m_taskTimer;
- Timer<IDBTransactionBackendImpl> m_taskEventTimer;
int m_pendingPreemptiveEvents;
- int m_pendingEvents;
HashSet<IDBCursorBackendImpl*> m_openCursors;
};
@@ -54,7 +54,6 @@ class IDBTransactionBackendInterface : public ThreadSafeRefCounted<IDBTransactio
};
virtual PassRefPtr<IDBObjectStoreBackendInterface> objectStore(int64_t, ExceptionCode&) = 0;
- virtual void didCompleteTaskEvents() = 0;
virtual void commit() = 0;
virtual void abort() = 0;
virtual void setCallbacks(IDBTransactionCallbacks*) = 0;
@@ -1,3 +1,18 @@
+2012-11-27 Joshua Bell <jsbell@chromium.org>
+
+ IndexedDB: Simplify transaction timers and event tracking
+ https://bugs.webkit.org/show_bug.cgi?id=102984
+
+ Reviewed by Tony Chang.
+
+ Remove now-unused didCompleteTaskEvents() method.
+
+ * src/IDBTransactionBackendProxy.cpp:
+ * src/IDBTransactionBackendProxy.h:
+ (IDBTransactionBackendProxy):
+ * src/WebIDBTransactionImpl.cpp:
+ * src/WebIDBTransactionImpl.h:
+
2012-11-27 Alpha Lam <hclam@chromium.org>
[chromium] Implement full-featured image cache
@@ -72,11 +72,6 @@ void IDBTransactionBackendProxy::abort()
m_webIDBTransaction->abort();
}
-void IDBTransactionBackendProxy::didCompleteTaskEvents()
-{
- m_webIDBTransaction->didCompleteTaskEvents();
-}
-
void IDBTransactionBackendProxy::setCallbacks(IDBTransactionCallbacks* callbacks)
{
m_webIDBTransaction->setCallbacks(new WebIDBTransactionCallbacksImpl(callbacks));
@@ -44,7 +44,6 @@ class IDBTransactionBackendProxy : public WebCore::IDBTransactionBackendInterfac
virtual PassRefPtr<WebCore::IDBObjectStoreBackendInterface> objectStore(int64_t, WebCore::ExceptionCode&);
virtual void commit();
virtual void abort();
- virtual void didCompleteTaskEvents();
virtual void setCallbacks(WebCore::IDBTransactionCallbacks*);
WebIDBTransaction* getWebIDBTransaction() const { return m_webIDBTransaction.get(); }
@@ -65,11 +65,6 @@ void WebIDBTransactionImpl::abort()
m_backend->abort();
}
-void WebIDBTransactionImpl::didCompleteTaskEvents()
-{
- m_backend->didCompleteTaskEvents();
-}
-
void WebIDBTransactionImpl::setCallbacks(WebIDBTransactionCallbacks* callbacks)
{
RefPtr<IDBTransactionCallbacks> idbCallbacks = IDBTransactionCallbacksProxy::create(adoptPtr(callbacks));
@@ -44,7 +44,6 @@ class WebIDBTransactionImpl: public WebIDBTransaction {
virtual WebIDBObjectStore* objectStore(long long indexId, WebExceptionCode&);
virtual void commit();
virtual void abort();
- virtual void didCompleteTaskEvents();
virtual void setCallbacks(WebIDBTransactionCallbacks*);
virtual WebCore::IDBTransactionBackendInterface* getIDBTransactionBackendInterface() const;

0 comments on commit 9fbdd35

Please sign in to comment.