Changeset 135927 in webkit

Timestamp:

Nov 27, 2012, 3:08:17 PM (12 years ago)

Author:

jsbell@chromium.org

Message:

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:

Location:

trunk/Source

Files:

• WebCore/ChangeLog (modified) (1 diff)
• WebCore/Modules/indexeddb/IDBCursorBackendImpl.cpp (modified) (2 diffs)
• WebCore/Modules/indexeddb/IDBDatabaseBackendImpl.cpp (modified) (2 diffs)
• WebCore/Modules/indexeddb/IDBObjectStoreBackendImpl.cpp (modified) (3 diffs)
• WebCore/Modules/indexeddb/IDBRequest.cpp (modified) (3 diffs)
• WebCore/Modules/indexeddb/IDBTransactionBackendImpl.cpp (modified) (8 diffs)
• WebCore/Modules/indexeddb/IDBTransactionBackendImpl.h (modified) (5 diffs)
• WebCore/Modules/indexeddb/IDBTransactionBackendInterface.h (modified) (1 diff)
• WebKit/chromium/ChangeLog (modified) (1 diff)
• WebKit/chromium/src/IDBTransactionBackendProxy.cpp (modified) (1 diff)
• WebKit/chromium/src/IDBTransactionBackendProxy.h (modified) (1 diff)
• WebKit/chromium/src/WebIDBTransactionImpl.cpp (modified) (1 diff)
• WebKit/chromium/src/WebIDBTransactionImpl.h (modified) (1 diff)

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+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>
 

trunk/Source/WebCore/Modules/indexeddb/IDBCursorBackendImpl.cpp

@@ -174 +174 @@
     }
 
-    cursor->m_transaction->addPendingEvents(foundKeys.size() - 1);
     callbacks->onSuccessWithPrefetch(foundKeys, foundPrimaryKeys, foundValues);
 }
@@ -181 +180 @@
 {
     IDB_TRACE("IDBCursorBackendImpl::prefetchReset");
-    m_transaction->addPendingEvents(-unusedPrefetches);
     m_cursor = m_savedCursor;
     m_savedCursor = 0;

trunk/Source/WebCore/Modules/indexeddb/IDBDatabaseBackendImpl.cpp

@@ -181 +181 @@
         return;
     }
-
-    transaction->didCompleteTaskEvents();
 }
 
@@ -211 +209 @@
 {
     database->m_backingStore->deleteObjectStore(transaction->backingStoreTransaction(), database->id(), objectStore->id());
-    transaction->didCompleteTaskEvents();
 }
 

trunk/Source/WebCore/Modules/indexeddb/IDBObjectStoreBackendImpl.cpp

@@ -269 +269 @@
     for (size_t i = 0; i < indexIds->size(); ++i)
         transaction->didCompletePreemptiveEvent();
-    transaction->didCompleteTaskEvents();
 }
 
@@ -408 +407 @@
         return;
     }
-
-    transaction->didCompleteTaskEvents();
 }
 
@@ -440 +437 @@
 {
     objectStore->backingStore()->deleteIndex(transaction->backingStoreTransaction(), objectStore->databaseId(), objectStore->id(), index->id());
-    transaction->didCompleteTaskEvents();
 }
 

trunk/Source/WebCore/Modules/indexeddb/IDBRequest.cpp

@@ -79 +79 @@
     , 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);
-    }
 }
 
@@ -477 +478 @@
     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);
@@ -497 +490 @@
         }
 
-        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;

trunk/Source/WebCore/Modules/indexeddb/IDBTransactionBackendImpl.cpp

@@ -50 +50 @@
     , 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);
@@ -93 +92 @@
     if (m_state == Unused)
         start();
+    else if (m_state == Running && !m_taskTimer.isActive())
+        m_taskTimer.startOneShot(0);
 
     return true;
@@ -117 +118 @@
     m_state = Finished;
     m_taskTimer.stop();
-    m_taskEventTimer.stop();
 
     if (wasRunning)
@@ -155 +155 @@
 bool IDBTransactionBackendImpl::hasPendingTasks() const
 {
-    return m_pendingEvents || m_pendingPreemptiveEvents || !isTaskQueueEmpty();
+    return m_pendingPreemptiveEvents || !isTaskQueueEmpty();
 }
 
@@ -166 +166 @@
 {
     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);
 }
 
@@ -213 +192 @@
 
     ASSERT(m_state == Unused || m_state == Running);
+    m_commitPending = true;
 
     // Front-end has requested a commit, but there may be tasks like createIndex which
@@ -271 +251 @@
         ASSERT(m_state == Running);
         OwnPtr<ScriptExecutionContext::Task> task(taskQueue->takeFirst());
-        m_pendingEvents++;
         task->performTask(0);
 
@@ -277 +256 @@
         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);
 }
 

trunk/Source/WebCore/Modules/indexeddb/IDBTransactionBackendImpl.h

@@ -54 +54 @@
     // IDBTransactionBackendInterface
     virtual PassRefPtr<IDBObjectStoreBackendInterface> objectStore(int64_t, ExceptionCode&);
-    virtual void didCompleteTaskEvents();
     virtual void abort();
     virtual void setCallbacks(IDBTransactionCallbacks* callbacks) { m_callbacks = callbacks; }
@@ -65 +64 @@
     void registerOpenCursor(IDBCursorBackendImpl*);
     void unregisterOpenCursor(IDBCursorBackendImpl*);
-    void addPendingEvents(int);
     void addPreemptiveEvent() { m_pendingPreemptiveEvents++; }
     void didCompletePreemptiveEvent() { m_pendingPreemptiveEvents--; ASSERT(m_pendingPreemptiveEvents >= 0); }
@@ -88 +86 @@
 
     void taskTimerFired(Timer<IDBTransactionBackendImpl>*);
-    void taskEventTimerFired(Timer<IDBTransactionBackendImpl>*);
     void closeOpenCursors();
 
@@ -96 +93 @@
 
     State m_state;
+    bool m_commitPending;
     RefPtr<IDBTransactionCallbacks> m_callbacks;
     RefPtr<IDBDatabaseBackendImpl> m_database;
@@ -108 +106 @@
     // 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;

trunk/Source/WebCore/Modules/indexeddb/IDBTransactionBackendInterface.h

@@ -55 +55 @@
 
     virtual PassRefPtr<IDBObjectStoreBackendInterface> objectStore(int64_t, ExceptionCode&) = 0;
-    virtual void didCompleteTaskEvents() = 0;
     virtual void commit() = 0;
     virtual void abort() = 0;

trunk/Source/WebKit/chromium/ChangeLog

@@ +1 @@
+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>
 

trunk/Source/WebKit/chromium/src/IDBTransactionBackendProxy.cpp

@@ -73 +73 @@
 }
 
-void IDBTransactionBackendProxy::didCompleteTaskEvents()
-{
-    m_webIDBTransaction->didCompleteTaskEvents();
-}
-
 void IDBTransactionBackendProxy::setCallbacks(IDBTransactionCallbacks* callbacks)
 {

trunk/Source/WebKit/chromium/src/IDBTransactionBackendProxy.h

@@ -45 +45 @@
     virtual void commit();
     virtual void abort();
-    virtual void didCompleteTaskEvents();
     virtual void setCallbacks(WebCore::IDBTransactionCallbacks*);
 

trunk/Source/WebKit/chromium/src/WebIDBTransactionImpl.cpp

@@ -66 +66 @@
 }
 
-void WebIDBTransactionImpl::didCompleteTaskEvents()
-{
-    m_backend->didCompleteTaskEvents();
-}
-
 void WebIDBTransactionImpl::setCallbacks(WebIDBTransactionCallbacks* callbacks)
 {

trunk/Source/WebKit/chromium/src/WebIDBTransactionImpl.h

@@ -45 +45 @@
     virtual void commit();
     virtual void abort();
-    virtual void didCompleteTaskEvents();
     virtual void setCallbacks(WebIDBTransactionCallbacks*);