[webkit-reviews] review canceled: [Bug 100903] IndexedDB: Move control of transaction completion to front end : [Attachment 175029] Patch

[bugzilla-daemon at webkit.org]

Joshua Bell <jsbell at chromium.org> has canceled Joshua Bell
<jsbell at chromium.org>'s request for review:
Bug 100903: IndexedDB: Move control of transaction completion to front end
https://bugs.webkit.org/show_bug.cgi?id=100903

Attachment 175029: Patch
https://bugs.webkit.org/attachment.cgi?id=175029&action=review

------- Additional Comments from Joshua Bell <jsbell at chromium.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=175029&action=review


>> Source/WebCore/Modules/indexeddb/IDBTransactionBackendImpl.cpp:51
>> +	, m_commitPending(false)
> 
> How does m_commitPending work with m_state? I'm concerned (more from a code
maintenance perspective than a correctness perspective) that we're now doubling
the possible "states" - to the combinations of m_state and m_commitPending
(even if most of the "new" ones are invalid) - i.e. is m_state Finished &&
m_commitPending = false possible? etc...

In theory orthogonal - can enter commitPending in parallel with Unused or
Running. Logically can't enter commitPending when in StartPending because the
front-end has queued tasks that haven't completed. This is covered by the
ASSERT(m_state == Unused || m_state == Running) in ::commit().

Given that a commit() of an Unused transaction should be a no-op, I'll see if I
can collapse it to an extra state "after" Running.

>> Source/WebCore/Modules/indexeddb/IDBTransactionBackendImpl.cpp:127
>> +	    m_abortTaskQueue.removeFirst();
> 
> why this change?

Good catch. This is a rebase glitch. I'll undo.